Skip to content

Use shared executors in App Distribution SDK #4338

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions firebase-appdistribution/firebase-appdistribution.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ android {
dependencies {
implementation 'org.jetbrains:annotations:15.0'
implementation project(':firebase-appdistribution-api')
implementation project(':firebase-annotations')
implementation project(':firebase-components')
implementation project(':firebase-installations-interop')
implementation project(':firebase-common')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import static com.google.firebase.appdistribution.impl.TaskUtils.runAsyncInTask;
import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskException;

import android.annotation.SuppressLint;
import android.app.Activity;
import android.content.Intent;
import android.net.Uri;
Expand All @@ -31,7 +30,6 @@
import com.google.firebase.appdistribution.UpdateStatus;
import java.io.IOException;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import javax.net.ssl.HttpsURLConnection;

/** Class that handles updateApp functionality for AABs in {@link FirebaseAppDistribution}. */
Expand All @@ -40,7 +38,7 @@ class AabUpdater {

private final FirebaseAppDistributionLifecycleNotifier lifecycleNotifier;
private final HttpsUrlConnectionFactory httpsUrlConnectionFactory;
private final Executor executor;
private final Executor blockingExecutor;

private final Object updateAabLock = new Object();

Expand All @@ -53,23 +51,21 @@ class AabUpdater {
@GuardedBy("updateAabLock")
private boolean hasBeenSentToPlayForCurrentTask = false;

// TODO(b/258264924): Migrate to go/firebase-android-executors
@SuppressLint("ThreadPoolCreation")
AabUpdater() {
AabUpdater(@NonNull Executor blockingExecutor) {
this(
FirebaseAppDistributionLifecycleNotifier.getInstance(),
new HttpsUrlConnectionFactory(),
Executors.newSingleThreadExecutor());
blockingExecutor);
}

AabUpdater(
@NonNull FirebaseAppDistributionLifecycleNotifier lifecycleNotifier,
@NonNull HttpsUrlConnectionFactory httpsUrlConnectionFactory,
@NonNull Executor executor) {
@NonNull Executor blockingExecutor) {
this.lifecycleNotifier = lifecycleNotifier;
this.httpsUrlConnectionFactory = httpsUrlConnectionFactory;
lifecycleNotifier.addOnActivityStartedListener(this::onActivityStarted);
this.executor = executor;
this.blockingExecutor = blockingExecutor;
}

@VisibleForTesting
Expand Down Expand Up @@ -97,13 +93,13 @@ UpdateTaskImpl updateAab(@NonNull AppDistributionReleaseInternal newRelease) {
hasBeenSentToPlayForCurrentTask = false;

// On a background thread, fetch the redirect URL and open it in the Play app
runAsyncInTask(executor, () -> fetchDownloadRedirectUrl(newRelease.getDownloadUrl()))
runAsyncInTask(blockingExecutor, () -> fetchDownloadRedirectUrl(newRelease.getDownloadUrl()))
.onSuccessTask(
executor,
blockingExecutor,
redirectUrl ->
lifecycleNotifier.consumeForegroundActivity(
activity -> openRedirectUrlInPlay(redirectUrl, activity)))
.addOnFailureListener(executor, this::setUpdateTaskCompletionError);
.addOnFailureListener(blockingExecutor, this::setUpdateTaskCompletionError);

return cachedUpdateTask;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskException;
import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskResult;

import android.annotation.SuppressLint;
import android.content.Context;
import android.os.Build.VERSION;
import android.os.Build.VERSION_CODES;
Expand All @@ -38,7 +37,6 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import java.util.jar.JarFile;
import javax.net.ssl.HttpsURLConnection;

Expand All @@ -51,7 +49,7 @@ class ApkUpdater {
private static final String DEFAULT_APK_FILE_NAME = "downloaded_release.apk";

private TaskCompletionSource<File> downloadTaskCompletionSource;
private final Executor taskExecutor; // Executor to run task listeners on a background thread
private final Executor blockingExecutor; // Executor to run task listeners on a background thread
private final Context context;
private final ApkInstaller apkInstaller;
private final FirebaseAppDistributionNotificationsManager appDistributionNotificationsManager;
Expand All @@ -63,11 +61,12 @@ class ApkUpdater {

private final Object updateTaskLock = new Object();

// TODO(b/258264924): Migrate to go/firebase-android-executors
@SuppressLint("ThreadPoolCreation")
public ApkUpdater(@NonNull FirebaseApp firebaseApp, @NonNull ApkInstaller apkInstaller) {
public ApkUpdater(
@NonNull FirebaseApp firebaseApp,
@NonNull ApkInstaller apkInstaller,
@NonNull Executor blockingExecutor) {
this(
Executors.newSingleThreadExecutor(),
blockingExecutor,
firebaseApp.getApplicationContext(),
apkInstaller,
new FirebaseAppDistributionNotificationsManager(firebaseApp.getApplicationContext()),
Expand All @@ -77,13 +76,13 @@ public ApkUpdater(@NonNull FirebaseApp firebaseApp, @NonNull ApkInstaller apkIns

@VisibleForTesting
public ApkUpdater(
@NonNull Executor taskExecutor,
@NonNull Executor blockingExecutor,
@NonNull Context context,
@NonNull ApkInstaller apkInstaller,
@NonNull FirebaseAppDistributionNotificationsManager appDistributionNotificationsManager,
@NonNull HttpsUrlConnectionFactory httpsUrlConnectionFactory,
@NonNull FirebaseAppDistributionLifecycleNotifier lifeCycleNotifier) {
this.taskExecutor = taskExecutor;
this.blockingExecutor = blockingExecutor;
this.context = context;
this.apkInstaller = apkInstaller;
this.appDistributionNotificationsManager = appDistributionNotificationsManager;
Expand All @@ -102,9 +101,9 @@ UpdateTaskImpl updateApk(
}

downloadApk(newRelease, showNotification)
.addOnSuccessListener(taskExecutor, file -> installApk(file, showNotification))
.addOnSuccessListener(blockingExecutor, file -> installApk(file, showNotification))
.addOnFailureListener(
taskExecutor,
blockingExecutor,
e -> {
setUpdateTaskCompletionErrorWithDefault(
e, "Failed to download APK", Status.DOWNLOAD_FAILURE);
Expand All @@ -120,14 +119,14 @@ private void installApk(File file, boolean showDownloadNotificationManager) {
.applyToForegroundActivityTask(
activity -> apkInstaller.installApk(file.getPath(), activity))
.addOnSuccessListener(
taskExecutor,
blockingExecutor,
unused -> {
synchronized (updateTaskLock) {
safeSetTaskResult(cachedUpdateTask);
}
})
.addOnFailureListener(
taskExecutor,
blockingExecutor,
e -> {
postUpdateProgress(
file.length(),
Expand All @@ -151,7 +150,7 @@ Task<File> downloadApk(

downloadTaskCompletionSource = new TaskCompletionSource<>();

taskExecutor.execute(
blockingExecutor.execute(
() -> {
try {
makeApkDownloadRequest(newRelease, showNotification);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,19 @@
import androidx.annotation.Keep;
import androidx.annotation.NonNull;
import com.google.firebase.FirebaseApp;
import com.google.firebase.annotations.concurrent.Blocking;
import com.google.firebase.appdistribution.FirebaseAppDistribution;
import com.google.firebase.components.Component;
import com.google.firebase.components.ComponentContainer;
import com.google.firebase.components.ComponentRegistrar;
import com.google.firebase.components.Dependency;
import com.google.firebase.components.Qualified;
import com.google.firebase.inject.Provider;
import com.google.firebase.installations.FirebaseInstallationsApi;
import com.google.firebase.platforminfo.LibraryVersionComponent;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.Executor;

/**
* Registers FirebaseAppDistribution.
Expand All @@ -39,31 +42,37 @@
public class FirebaseAppDistributionRegistrar implements ComponentRegistrar {
private static final String LIBRARY_NAME = "fire-appdistribution";

private static String TAG = "Registrar:";
private static String TAG = "FirebaseAppDistributionRegistrar";

@Override
public @NonNull List<Component<?>> getComponents() {
Qualified<Executor> blockingExecutor = Qualified.qualified(Blocking.class, Executor.class);
return Arrays.asList(
Component.builder(FirebaseAppDistribution.class)
.name(LIBRARY_NAME)
.add(Dependency.required(FirebaseApp.class))
.add(Dependency.requiredProvider(FirebaseInstallationsApi.class))
.factory(this::buildFirebaseAppDistribution)
.add(Dependency.required(blockingExecutor))
.factory(c -> buildFirebaseAppDistribution(c, c.get(blockingExecutor)))
// construct FirebaseAppDistribution instance on startup so we can register for
// activity lifecycle callbacks before the API is called
.alwaysEager()
.build(),
LibraryVersionComponent.create(LIBRARY_NAME, BuildConfig.VERSION_NAME));
}

private FirebaseAppDistribution buildFirebaseAppDistribution(ComponentContainer container) {
private FirebaseAppDistribution buildFirebaseAppDistribution(
ComponentContainer container, Executor blockingExecutor) {
FirebaseApp firebaseApp = container.get(FirebaseApp.class);
Context context = firebaseApp.getApplicationContext();
Provider<FirebaseInstallationsApi> firebaseInstallationsApiProvider =
container.getProvider(FirebaseInstallationsApi.class);
FirebaseAppDistributionTesterApiClient testerApiClient =
new FirebaseAppDistributionTesterApiClient(
firebaseApp, firebaseInstallationsApiProvider, new TesterApiHttpClient(firebaseApp));
firebaseApp,
firebaseInstallationsApiProvider,
new TesterApiHttpClient(firebaseApp),
blockingExecutor);
SignInStorage signInStorage = new SignInStorage(context);
FirebaseAppDistributionLifecycleNotifier lifecycleNotifier =
FirebaseAppDistributionLifecycleNotifier.getInstance();
Expand All @@ -74,8 +83,8 @@ private FirebaseAppDistribution buildFirebaseAppDistribution(ComponentContainer
new TesterSignInManager(firebaseApp, firebaseInstallationsApiProvider, signInStorage),
new NewReleaseFetcher(
firebaseApp.getApplicationContext(), testerApiClient, releaseIdentifier),
new ApkUpdater(firebaseApp, new ApkInstaller()),
new AabUpdater(),
new ApkUpdater(firebaseApp, new ApkInstaller(), blockingExecutor),
new AabUpdater(blockingExecutor),
signInStorage,
lifecycleNotifier);

Expand All @@ -85,11 +94,11 @@ private FirebaseAppDistribution buildFirebaseAppDistribution(ComponentContainer
} else {
LogWrapper.getInstance()
.e(
TAG
+ "Context "
+ context
+ " was not an Application, can't register for lifecycle callbacks. SDK might not"
+ " function correctly.");
TAG,
String.format(
"Context %s was not an Application, can't register for lifecycle callbacks. SDK"
+ " might not function correctly.",
context));
}

return appDistribution;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import static com.google.firebase.appdistribution.impl.TaskUtils.runAsyncInTask;

import android.annotation.SuppressLint;
import androidx.annotation.NonNull;
import com.google.android.gms.tasks.Task;
import com.google.android.gms.tasks.Tasks;
Expand All @@ -29,7 +28,6 @@
import com.google.firebase.installations.InstallationTokenResult;
import java.io.File;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
Expand Down Expand Up @@ -64,18 +62,17 @@ private interface FidDependentJob<TResult> {
private final FirebaseApp firebaseApp;
private final Provider<FirebaseInstallationsApi> firebaseInstallationsApiProvider;
private final TesterApiHttpClient testerApiHttpClient;

// TODO(b/258264924): Migrate to go/firebase-android-executors
@SuppressLint("ThreadPoolCreation")
private final Executor taskExecutor = Executors.newSingleThreadExecutor();
private final Executor blockingExecutor;

FirebaseAppDistributionTesterApiClient(
@NonNull FirebaseApp firebaseApp,
@NonNull Provider<FirebaseInstallationsApi> firebaseInstallationsApiProvider,
@NonNull TesterApiHttpClient testerApiHttpClient) {
@NonNull TesterApiHttpClient testerApiHttpClient,
@NonNull Executor blockingExecutor) {
this.firebaseApp = firebaseApp;
this.firebaseInstallationsApiProvider = firebaseInstallationsApiProvider;
this.testerApiHttpClient = testerApiHttpClient;
this.blockingExecutor = blockingExecutor;
}

/**
Expand Down Expand Up @@ -265,9 +262,9 @@ private <TResult> Task<TResult> runWithFidAndToken(FidDependentJob<TResult> job)
firebaseInstallationsApiProvider.get().getToken(/* forceRefresh= */ false);

return Tasks.whenAllSuccess(installationIdTask, installationAuthTokenTask)
.continueWithTask(taskExecutor, TaskUtils::handleTaskFailure)
.continueWithTask(blockingExecutor, TaskUtils::handleTaskFailure)
.onSuccessTask(
taskExecutor,
blockingExecutor,
resultsList -> {
// Tasks.whenAllSuccess combines task results into a list
if (resultsList.size() != 2) {
Expand All @@ -276,7 +273,7 @@ private <TResult> Task<TResult> runWithFidAndToken(FidDependentJob<TResult> job)
}
String fid = (String) resultsList.get(0);
InstallationTokenResult tokenResult = (InstallationTokenResult) resultsList.get(1);
return runAsyncInTask(taskExecutor, () -> job.run(fid, tokenResult.getToken()));
return runAsyncInTask(blockingExecutor, () -> job.run(fid, tokenResult.getToken()));
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ void e(@NonNull String msg, @NonNull Throwable tr) {
Log.e(LOG_TAG, msg, tr);
}

void e(@NonNull String additionalTag, @NonNull String msg) {
Log.e(LOG_TAG, prependTag(additionalTag, msg));
}

void e(@NonNull String additionalTag, @NonNull String msg, @NonNull Throwable tr) {
Log.e(LOG_TAG, prependTag(additionalTag, msg), tr);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.google.firebase.installations.FirebaseInstallationsApi;
import com.google.firebase.installations.InstallationTokenResult;
import java.io.File;
import java.util.concurrent.Executors;
import org.json.JSONException;
import org.json.JSONObject;
import org.junit.Before;
Expand Down Expand Up @@ -102,7 +103,10 @@ public void setup() {

firebaseAppDistributionTesterApiClient =
new FirebaseAppDistributionTesterApiClient(
firebaseApp, mockFirebaseInstallationsProvider, mockTesterApiHttpClient);
firebaseApp,
mockFirebaseInstallationsProvider,
mockTesterApiHttpClient,
Executors.newSingleThreadExecutor());
}

@Test
Expand Down