Clean up task management in NewReleaseFetcher

Author: lfkellogg

firebase-app-distribution/src/main/java/com/google/firebase/app/distribution/NewReleaseFetcher.java

@@ -15,16 +15,21 @@
 package com.google.firebase.app.distribution;
 
 import static com.google.firebase.app.distribution.ReleaseIdentificationUtils.calculateApkHash;
+import static com.google.firebase.app.distribution.ReleaseIdentificationUtils.getPackageInfo;
+import static com.google.firebase.app.distribution.ReleaseIdentificationUtils.getPackageInfoWithMetadata;
+import static com.google.firebase.app.distribution.TaskUtils.runAsyncInTask;
 
 import android.content.Context;
 import android.content.pm.PackageInfo;
-import android.content.pm.PackageManager;
 import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 import androidx.core.content.pm.PackageInfoCompat;
 import com.google.android.gms.tasks.Task;
 import com.google.android.gms.tasks.Tasks;
 import com.google.firebase.FirebaseApp;
+import com.google.firebase.app.distribution.Constants.ErrorMessages;
+import com.google.firebase.app.distribution.FirebaseAppDistributionException.Status;
 import com.google.firebase.app.distribution.internal.LogWrapper;
 import com.google.firebase.inject.Provider;
 import com.google.firebase.installations.FirebaseInstallationsApi;
@@ -87,82 +92,72 @@ public synchronized Task<AppDistributionReleaseInternal> checkForNewRelease() {
 
     this.cachedCheckForNewRelease =
         Tasks.whenAllSuccess(installationIdTask, installationAuthTokenTask)
+            .continueWithTask(TaskUtils::handleTaskFailure)
             .onSuccessTask(
-                taskExecutor,
-                tasks -> {
-                  String fid = installationIdTask.getResult();
-                  InstallationTokenResult installationTokenResult =
-                      installationAuthTokenTask.getResult();
-                  try {
-                    AppDistributionReleaseInternal newRelease =
-                        getNewReleaseFromClient(
-                            fid,
-                            firebaseApp.getOptions().getApplicationId(),
-                            firebaseApp.getOptions().getApiKey(),
-                            installationTokenResult.getToken());
-                    return Tasks.forResult(newRelease);
-                  } catch (FirebaseAppDistributionException ex) {
-                    return Tasks.forException(ex);
-                  }
-                })
-            .continueWithTask(
-                taskExecutor,
-                task ->
-                    TaskUtils.handleTaskFailure(
-                        task,
-                        Constants.ErrorMessages.NETWORK_ERROR,
-                        FirebaseAppDistributionException.Status.NETWORK_FAILURE));
+                unused ->
+                    getNewReleaseFromClientTask(
+                        installationIdTask.getResult(),
+                        firebaseApp.getOptions().getApplicationId(),
+                        firebaseApp.getOptions().getApiKey(),
+                        installationAuthTokenTask.getResult().getToken()));
 
     return cachedCheckForNewRelease;
   }
 
+  private Task<AppDistributionReleaseInternal> getNewReleaseFromClientTask(
+      String fid, String appId, String apiKey, String authToken) {
+    return runAsyncInTask(
+        taskExecutor, () -> getNewReleaseFromClient(fid, appId, apiKey, authToken));
+  }
+
+  @Nullable
   @VisibleForTesting
   AppDistributionReleaseInternal getNewReleaseFromClient(
       String fid, String appId, String apiKey, String authToken)
       throws FirebaseAppDistributionException {
+    AppDistributionReleaseInternal retrievedNewRelease =
+        firebaseAppDistributionTesterApiClient.fetchNewRelease(
+            fid, appId, apiKey, authToken, firebaseApp.getApplicationContext());
+
+    if (retrievedNewRelease == null) {
+      LogWrapper.getInstance().v(TAG + "Tester does not have access to any releases");
+      return null;
+    }
+
+    long newReleaseBuildVersion = parseBuildVersion(retrievedNewRelease.getBuildVersion());
+
+    if (isOlderBuildVersion(newReleaseBuildVersion)) {
+      LogWrapper.getInstance().v(TAG + "New release has lower version code than current release");
+      return null;
+    }
+
+    if (isNewerBuildVersion(newReleaseBuildVersion)
+        || !isSameAsInstalledRelease(retrievedNewRelease)
+        || hasDifferentAppVersionName(retrievedNewRelease)) {
+      return retrievedNewRelease;
+    } else {
+      LogWrapper.getInstance().v(TAG + "New release is older or is currently installed");
+      return null;
+    }
+  }
+
+  private long parseBuildVersion(String buildVersion) throws FirebaseAppDistributionException {
     try {
-      AppDistributionReleaseInternal retrievedNewRelease =
-          firebaseAppDistributionTesterApiClient.fetchNewRelease(
-              fid, appId, apiKey, authToken, firebaseApp.getApplicationContext());
-
-      if (retrievedNewRelease == null) {
-        LogWrapper.getInstance().v(TAG + "Tester does not have access to any releases");
-        return null;
-      }
-
-      if (!canInstall(retrievedNewRelease)) {
-        LogWrapper.getInstance().v(TAG + "New release has lower version code than current release");
-        return null;
-      }
-
-      if (isNewerBuildVersion(retrievedNewRelease)
-          || !isSameAsInstalledRelease(retrievedNewRelease)
-          || hasDifferentAppVersionName(retrievedNewRelease)) {
-        return retrievedNewRelease;
-      } else {
-        // Return null if retrieved new release is older or currently installed
-        LogWrapper.getInstance().v(TAG + "New release is older or is currently installed");
-        return null;
-      }
+      return Long.parseLong(buildVersion);
     } catch (NumberFormatException e) {
-      LogWrapper.getInstance().e(TAG + "Error parsing buildVersion.", e);
       throw new FirebaseAppDistributionException(
-          Constants.ErrorMessages.NETWORK_ERROR,
-          FirebaseAppDistributionException.Status.NETWORK_FAILURE,
-          e);
+          "Could not parse build version of new release: " + buildVersion, Status.UNKNOWN, e);
     }
   }
 
-  private boolean canInstall(AppDistributionReleaseInternal newRelease)
+  private boolean isOlderBuildVersion(long newReleaseBuildVersion)
       throws FirebaseAppDistributionException {
-    return Long.parseLong(newRelease.getBuildVersion())
-        >= getInstalledAppVersionCode(firebaseApp.getApplicationContext());
+    return newReleaseBuildVersion < getInstalledAppVersionCode(firebaseApp.getApplicationContext());
   }
 
-  private boolean isNewerBuildVersion(AppDistributionReleaseInternal newRelease)
+  private boolean isNewerBuildVersion(long newReleaseBuildVersion)
       throws FirebaseAppDistributionException {
-    return Long.parseLong(newRelease.getBuildVersion())
-        > getInstalledAppVersionCode(firebaseApp.getApplicationContext());
+    return newReleaseBuildVersion > getInstalledAppVersionCode(firebaseApp.getApplicationContext());
   }
 
   private boolean hasDifferentAppVersionName(AppDistributionReleaseInternal newRelease)
@@ -179,17 +174,26 @@ boolean isSameAsInstalledRelease(AppDistributionReleaseInternal newRelease)
       return hasSameHashAsInstalledRelease(newRelease);
     }
 
-    // TODO(lkellogg): getIasArtifactId() will likely never be null since it's set to the empty
-    //  string if not present in the response
-    if (newRelease.getIasArtifactId() == null) {
+    if (newRelease.getIasArtifactId() == null || newRelease.getIasArtifactId().isEmpty()) {
+      LogWrapper.getInstance()
+          .w(TAG + "AAB release missing IAS Artifact ID. Assuming new release is different.");
       return false;
     }
-    // AAB BinaryType
-    return newRelease
-        .getIasArtifactId()
-        .equals(
-            ReleaseIdentificationUtils.extractInternalAppSharingArtifactId(
-                firebaseApp.getApplicationContext()));
+
+    String installedIasArtifactId;
+    try {
+      installedIasArtifactId =
+          ReleaseIdentificationUtils.extractInternalAppSharingArtifactId(
+              firebaseApp.getApplicationContext());
+    } catch (FirebaseAppDistributionException e) {
+      LogWrapper.getInstance()
+          .w(
+              TAG + "Could not get installed IAS artifact ID. Assuming new release is different.",
+              e);
+      return false;
+    }
+
+    return newRelease.getIasArtifactId().equals(installedIasArtifactId);
   }
 
   private long getInstalledAppVersionCode(Context context) throws FirebaseAppDistributionException {
@@ -201,19 +205,6 @@ private String getInstalledAppVersionName(Context context)
     return getPackageInfo(context).versionName;
   }
 
-  private PackageInfo getPackageInfo(Context context) throws FirebaseAppDistributionException {
-    try {
-      return context.getPackageManager().getPackageInfo(context.getPackageName(), 0);
-    } catch (PackageManager.NameNotFoundException e) {
-      LogWrapper.getInstance()
-          .e(TAG + "Unable to find package with name " + context.getPackageName(), e);
-      throw new FirebaseAppDistributionException(
-          Constants.ErrorMessages.UNKNOWN_ERROR,
-          FirebaseAppDistributionException.Status.UNKNOWN,
-          e);
-    }
-  }
-
   @VisibleForTesting
   String extractApkHash(PackageInfo packageInfo) {
     File sourceFile = new File(packageInfo.applicationInfo.sourceDir);
@@ -229,25 +220,19 @@ String extractApkHash(PackageInfo packageInfo) {
 
   private boolean hasSameHashAsInstalledRelease(AppDistributionReleaseInternal newRelease)
       throws FirebaseAppDistributionException {
-    try {
-      Context context = firebaseApp.getApplicationContext();
-      PackageInfo metadataPackageInfo =
-          context
-              .getPackageManager()
-              .getPackageInfo(context.getPackageName(), PackageManager.GET_META_DATA);
-      String installedReleaseApkHash = extractApkHash(metadataPackageInfo);
-
-      if (installedReleaseApkHash.isEmpty() || newRelease.getApkHash().isEmpty()) {
-        LogWrapper.getInstance().e(TAG + "Missing APK hash.");
-        throw new FirebaseAppDistributionException(
-            Constants.ErrorMessages.UNKNOWN_ERROR, FirebaseAppDistributionException.Status.UNKNOWN);
-      }
-      // If the hash of the zipped APK for the retrieved newRelease is equal to the stored hash
-      // of the installed release, then they are the same release.
-      return installedReleaseApkHash.equals(newRelease.getApkHash());
-    } catch (PackageManager.NameNotFoundException e) {
-      LogWrapper.getInstance().e(TAG + "Unable to locate App.", e);
-      return false;
+    Context context = firebaseApp.getApplicationContext();
+    PackageInfo metadataPackageInfo = getPackageInfoWithMetadata(context);
+    String installedReleaseApkHash = extractApkHash(metadataPackageInfo);
+
+    if (installedReleaseApkHash == null || installedReleaseApkHash.isEmpty()) {
+      throw new FirebaseAppDistributionException(
+          "Could not calculate hash of installed APK", Status.UNKNOWN);
+    } else if (newRelease.getApkHash().isEmpty()) {
+      throw new FirebaseAppDistributionException(
+          "Missing APK hash from new release", Status.UNKNOWN);
     }
+    // If the hash of the zipped APK for the retrieved newRelease is equal to the stored hash
+    // of the installed release, then they are the same release.
+    return installedReleaseApkHash.equals(newRelease.getApkHash());
   }
 }

firebase-app-distribution/src/main/java/com/google/firebase/app/distribution/ReleaseIdentificationUtils.java

@@ -19,6 +19,7 @@
 import android.content.pm.PackageManager;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import com.google.firebase.app.distribution.FirebaseAppDistributionException.Status;
 import com.google.firebase.app.distribution.internal.LogWrapper;
 import java.io.File;
 import java.io.IOException;
@@ -33,22 +34,51 @@
 final class ReleaseIdentificationUtils {
   private static final String TAG = "ReleaseIdentification";
   private static final int BYTES_IN_LONG = 8;
+  private static final int NO_FLAGS = 0;
 
-  @Nullable
-  static String extractInternalAppSharingArtifactId(@NonNull Context appContext) {
+  /**
+   * Get the package info for the currently installed app
+   *
+   * @throws FirebaseAppDistributionException if the package name can't be found
+   */
+  static PackageInfo getPackageInfo(Context context) throws FirebaseAppDistributionException {
+    return getPackageInfoWithFlags(context, NO_FLAGS);
+  }
+
+  /**
+   * Get the package info for the currently installed app, with the PackageManager.GET_META_DATA
+   * flag set.
+   *
+   * @throws FirebaseAppDistributionException if the package name can't be found
+   */
+  static PackageInfo getPackageInfoWithMetadata(Context context)
+      throws FirebaseAppDistributionException {
+    return getPackageInfoWithFlags(context, PackageManager.GET_META_DATA);
+  }
+
+  private static PackageInfo getPackageInfoWithFlags(Context context, int flags)
+      throws FirebaseAppDistributionException {
     try {
-      PackageInfo packageInfo =
-          appContext
-              .getPackageManager()
-              .getPackageInfo(appContext.getPackageName(), PackageManager.GET_META_DATA);
-      if (packageInfo.applicationInfo.metaData == null) {
-        return null;
-      }
-      return packageInfo.applicationInfo.metaData.getString("com.android.vending.internal.apk.id");
+      return context.getPackageManager().getPackageInfo(context.getPackageName(), flags);
     } catch (PackageManager.NameNotFoundException e) {
-      LogWrapper.getInstance().w(TAG + "Could not extract internal app sharing artifact ID");
-      return null;
+      throw new FirebaseAppDistributionException(
+          "Unable to find package with name " + context.getPackageName(), Status.UNKNOWN, e);
+    }
+  }
+
+  static String extractInternalAppSharingArtifactId(@NonNull Context appContext)
+      throws FirebaseAppDistributionException {
+    PackageInfo packageInfo = getPackageInfoWithMetadata(appContext);
+    if (packageInfo.applicationInfo.metaData == null) {
+      throw new FirebaseAppDistributionException("Missing package info metadata", Status.UNKNOWN);
+    }
+    String id =
+        packageInfo.applicationInfo.metaData.getString("com.android.vending.internal.apk.id");
+    if (id == null) {
+      throw new FirebaseAppDistributionException(
+          "IAS artifact ID missing from package info metadata ", Status.UNKNOWN);
     }
+    return id;
   }
 
   @Nullable

firebase-app-distribution/src/main/java/com/google/firebase/app/distribution/TaskUtils.java

@@ -17,23 +17,44 @@
 import com.google.android.gms.tasks.Task;
 import com.google.android.gms.tasks.TaskCompletionSource;
 import com.google.android.gms.tasks.Tasks;
+import com.google.firebase.app.distribution.Constants.ErrorMessages;
+import com.google.firebase.app.distribution.FirebaseAppDistributionException.Status;
 import com.google.firebase.app.distribution.internal.LogWrapper;
+import java.util.concurrent.Executor;
 
 class TaskUtils {
   private static final String TAG = "TaskUtils:";
 
-  static <TResult> Task<TResult> handleTaskFailure(
-      Task<TResult> task,
-      String defaultErrorMessage,
-      FirebaseAppDistributionException.Status defaultErrorStatus) {
+  interface Operation<TResult> {
+    TResult run() throws FirebaseAppDistributionException;
+  }
+
+  static <TResult> Task<TResult> runAsyncInTask(Executor executor, Operation<TResult> operation) {
+    TaskCompletionSource<TResult> taskCompletionSource = new TaskCompletionSource<>();
+    executor.execute(
+        () -> {
+          try {
+            taskCompletionSource.setResult(operation.run());
+          } catch (FirebaseAppDistributionException e) {
+            taskCompletionSource.setException(e);
+          } catch (Throwable t) {
+            taskCompletionSource.setException(
+                new FirebaseAppDistributionException(
+                    "Unknown error: " + t.getMessage(), Status.UNKNOWN, t));
+          }
+        });
+    return taskCompletionSource.getTask();
+  }
+
+  static <TResult> Task<TResult> handleTaskFailure(Task<TResult> task) {
     if (task.isComplete() && !task.isSuccessful()) {
       Exception e = task.getException();
       LogWrapper.getInstance().e(TAG + "Task failed to complete due to " + e.getMessage(), e);
       if (e instanceof FirebaseAppDistributionException) {
         return task;
       }
       return Tasks.forException(
-          new FirebaseAppDistributionException(defaultErrorMessage, defaultErrorStatus, e));
+          new FirebaseAppDistributionException(ErrorMessages.UNKNOWN_ERROR, Status.UNKNOWN, e));
     }
     return task;
   }

firebase-app-distribution/src/main/java/com/google/firebase/app/distribution/internal/LogWrapper.java

@@ -48,6 +48,10 @@ public void w(@NonNull String msg) {
     Log.w(LOG_TAG, msg);
   }
 
+  public void w(@NonNull String msg, @NonNull Throwable tr) {
+    Log.w(LOG_TAG, msg, tr);
+  }
+
   public void e(@NonNull String msg) {
     Log.e(LOG_TAG, msg);
   }