remove the ReleaseIdentifierStorage, rely on apk hash (#3188)

* remove the ReleaseIdentifierStorage, rely on apk hash

* Fix failing test'

Author: rachaprince

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

@@ -40,8 +40,8 @@ class CheckForNewReleaseClient {
   private final FirebaseApp firebaseApp;
   private final FirebaseAppDistributionTesterApiClient firebaseAppDistributionTesterApiClient;
   private final FirebaseInstallationsApi firebaseInstallationsApi;
+  // Maintain an in-memory mapping from source file to APK hash to avoid re-calculating the hash
   private static final ConcurrentMap<String, String> cachedApkHashes = new ConcurrentHashMap<>();
-  private final ReleaseIdentifierStorage releaseIdentifierStorage;
 
   Task<AppDistributionReleaseInternal> cachedCheckForNewRelease = null;
   private final Executor checkForNewReleaseExecutor;
@@ -54,20 +54,16 @@ class CheckForNewReleaseClient {
     this.firebaseAppDistributionTesterApiClient = firebaseAppDistributionTesterApiClient;
     this.firebaseInstallationsApi = firebaseInstallationsApi;
     this.checkForNewReleaseExecutor = Executors.newSingleThreadExecutor();
-    this.releaseIdentifierStorage =
-        new ReleaseIdentifierStorage(firebaseApp.getApplicationContext());
   }
 
   CheckForNewReleaseClient(
       @NonNull FirebaseApp firebaseApp,
       @NonNull FirebaseAppDistributionTesterApiClient firebaseAppDistributionTesterApiClient,
       @NonNull FirebaseInstallationsApi firebaseInstallationsApi,
-      @NonNull ReleaseIdentifierStorage releaseIdentifierStorage,
       @NonNull Executor executor) {
     this.firebaseApp = firebaseApp;
     this.firebaseAppDistributionTesterApiClient = firebaseAppDistributionTesterApiClient;
     this.firebaseInstallationsApi = firebaseInstallationsApi;
-    this.releaseIdentifierStorage = releaseIdentifierStorage;
     this.checkForNewReleaseExecutor = executor;
   }
 
@@ -158,7 +154,8 @@ private boolean isNewerBuildVersion(AppDistributionReleaseInternal newRelease)
   }
 
   @VisibleForTesting
-  boolean isSameAsInstalledRelease(AppDistributionReleaseInternal newRelease) {
+  boolean isSameAsInstalledRelease(AppDistributionReleaseInternal newRelease)
+      throws FirebaseAppDistributionException {
     if (newRelease.getBinaryType().equals(BinaryType.APK)) {
       return hasSameHashAsInstalledRelease(newRelease);
     }
@@ -201,7 +198,8 @@ String extractApkHash(PackageInfo packageInfo) {
     return cachedApkHashes.get(key);
   }
 
-  private boolean hasSameHashAsInstalledRelease(AppDistributionReleaseInternal newRelease) {
+  private boolean hasSameHashAsInstalledRelease(AppDistributionReleaseInternal newRelease)
+      throws FirebaseAppDistributionException {
     try {
       Context context = firebaseApp.getApplicationContext();
       PackageInfo metadataPackageInfo =
@@ -211,12 +209,9 @@ private boolean hasSameHashAsInstalledRelease(AppDistributionReleaseInternal new
       String installedReleaseApkHash = extractApkHash(metadataPackageInfo);
 
       if (installedReleaseApkHash.isEmpty() || newRelease.getApkHash().isEmpty()) {
-        // We don't have enough information about the APK hashes. Fallback to the external codehash.
-        // TODO: Consider removing this when all returned releases have the efficient ApkHash
-        String externalCodeHash =
-            releaseIdentifierStorage.getExternalCodeHash(installedReleaseApkHash);
-        LogWrapper.getInstance().v(TAG + "Defaulting to external codehash " + externalCodeHash);
-        return externalCodeHash != null && externalCodeHash.equals(newRelease.getCodeHash());
+        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.

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

@@ -15,7 +15,6 @@
 package com.google.firebase.app.distribution;
 
 import static com.google.firebase.app.distribution.FirebaseAppDistributionException.Status.NETWORK_FAILURE;
-import static com.google.firebase.app.distribution.ReleaseIdentificationUtils.calculateApkHash;
 import static com.google.firebase.app.distribution.TaskUtils.safeSetTaskException;
 import static com.google.firebase.app.distribution.TaskUtils.safeSetTaskResult;
 
@@ -51,8 +50,6 @@ class UpdateApkClient {
   @GuardedBy("updateTaskLock")
   private UpdateTaskImpl cachedUpdateTask;
 
-  private final ReleaseIdentifierStorage releaseIdentifierStorage;
-
   private final Object updateTaskLock = new Object();
 
   public UpdateApkClient(
@@ -64,8 +61,6 @@ public UpdateApkClient(
       @NonNull Executor downloadExecutor,
       @NonNull FirebaseApp firebaseApp,
       @NonNull InstallApkClient installApkClient) {
-    this.releaseIdentifierStorage =
-        new ReleaseIdentifierStorage(firebaseApp.getApplicationContext());
     this.appDistributionNotificationsManager =
         new FirebaseAppDistributionNotificationsManager(firebaseApp);
     this.downloadExecutor = downloadExecutor;
@@ -250,14 +245,6 @@ private void downloadToDisk(
               FirebaseAppDistributionException.Status.DOWNLOAD_FAILURE));
     }
 
-    File downloadedFile = new File(firebaseApp.getApplicationContext().getFilesDir(), fileName);
-
-    String internalCodeHash = calculateApkHash(downloadedFile);
-
-    if (internalCodeHash != null) {
-      releaseIdentifierStorage.setCodeHashMap(internalCodeHash, newRelease);
-    }
-
     // completion
     postUpdateProgress(
         totalSize, totalSize, UpdateStatus.DOWNLOADED, showDownloadNotificationManager);

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

@@ -77,7 +77,6 @@ public class CheckForNewReleaseClientTest {
   @Mock private FirebaseInstallationsApi mockFirebaseInstallations;
   @Mock private FirebaseAppDistributionTesterApiClient mockFirebaseAppDistributionTesterApiClient;
   @Mock private InstallationTokenResult mockInstallationTokenResult;
-  @Mock private ReleaseIdentifierStorage mockReleaseIdentifierStorage;
 
   Executor testExecutor = Executors.newSingleThreadExecutor();
 
@@ -127,7 +126,6 @@ public void setup() {
                 firebaseApp,
                 mockFirebaseAppDistributionTesterApiClient,
                 mockFirebaseInstallations,
-                mockReleaseIdentifierStorage,
                 testExecutor));
   }
 
@@ -307,27 +305,29 @@ public void handleNewReleaseFromClient_whenNewReleaseIsSameAsInstalledAab_return
   }
 
   @Test
-  public void iisSameAsInstalledRelease_whenApkHashesEqual_returnsTrue() {
+  public void iisSameAsInstalledRelease_whenApkHashesEqual_returnsTrue()
+      throws FirebaseAppDistributionException {
     doReturn(CURRENT_APK_HASH).when(checkForNewReleaseClient).extractApkHash(any());
     assertTrue(
         checkForNewReleaseClient.isSameAsInstalledRelease(getTestInstalledRelease().build()));
   }
 
   @Test
-  public void isSameAsInstalledRelease_whenApkHashesNotEqual_returnsFalse() {
+  public void isSameAsInstalledRelease_whenApkHashesNotEqual_returnsFalse()
+      throws FirebaseAppDistributionException {
     doReturn(CURRENT_APK_HASH).when(checkForNewReleaseClient).extractApkHash(any());
     assertFalse(checkForNewReleaseClient.isSameAsInstalledRelease(getTestNewRelease().build()));
   }
 
   @Test
-  public void isSameAsInstalledRelease_ifApkHashNotPresent_fallsBackToExternalCodeHash() {
+  public void isSameAsInstalledRelease_ifApkHashNotPresent_throwsError() {
     doReturn(CURRENT_APK_HASH).when(checkForNewReleaseClient).extractApkHash(any());
-    when(mockReleaseIdentifierStorage.getExternalCodeHash(any())).thenReturn(CURRENT_CODEHASH);
 
-    assertFalse(
-        checkForNewReleaseClient.isSameAsInstalledRelease(
-            getTestNewRelease().setApkHash("").build()));
-    verify(mockReleaseIdentifierStorage).getExternalCodeHash(CURRENT_APK_HASH);
+    assertThrows(
+        FirebaseAppDistributionException.class,
+        () ->
+            checkForNewReleaseClient.isSameAsInstalledRelease(
+                getTestNewRelease().setApkHash("").build()));
   }
 
   @Test

firebase-app-distribution/src/test/java/com/google/firebase/app/distribution/CheckForNewReleaseClientTest.java

@@ -76,7 +76,7 @@ public class UpdateApkClientTest {
   Executor testExecutor = Executors.newSingleThreadExecutor();
 
   @Before
-  public void setup() {
+  public void setup() throws FirebaseAppDistributionException {
 
     MockitoAnnotations.initMocks(this);
 
@@ -96,12 +96,11 @@ public void setup() {
 
     this.updateApkClient =
         Mockito.spy(new UpdateApkClient(testExecutor, firebaseApp, mockInstallApkClient));
+    doReturn(mockHttpsUrlConnection).when(updateApkClient).openHttpsUrlConnection(TEST_URL);
   }
 
   @Test
   public void updateApk_whenDownloadFails_setsNetworkError() throws Exception {
-
-    doReturn(mockHttpsUrlConnection).when(updateApkClient).openHttpsUrlConnection(TEST_URL);
     // null inputStream causes download failure
     when(mockHttpsUrlConnection.getInputStream()).thenReturn(null);
     UpdateTaskImpl updateTask = updateApkClient.updateApk(TEST_RELEASE, false);
@@ -211,6 +210,8 @@ public void postProgressUpdate_whenErrorStatus_updatesNotificationsManagerWithEr
 
   @Test
   public void updateApp_whenCalledMultipleTimesWithApk_returnsSameUpdateTask() {
+    doReturn(Tasks.forResult(mockFile)).when(updateApkClient).downloadApk(TEST_RELEASE, false);
+
     UpdateTask updateTask1 = updateApkClient.updateApk(TEST_RELEASE, false);
     UpdateTask updateTask2 = updateApkClient.updateApk(TEST_RELEASE, false);