Updating based on reviewer comments.

Author: annzimmer

firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/CustomModel.java

@@ -42,11 +42,7 @@ public class CustomModel {
    */
   public CustomModel(
       @NonNull String name, long downloadId, long fileSize, @NonNull String modelHash) {
-    this.modelHash = modelHash;
-    this.name = name;
-    this.fileSize = fileSize;
-    this.downloadId = downloadId;
-    this.localFilePath = "";
+    this(name, downloadId, fileSize, modelHash, "");
   }
 
   /**
@@ -137,6 +133,11 @@ public boolean equals(Object o) {
         && Objects.equal(downloadId, other.downloadId);
   }
 
+  @Override
+  public int hashCode() {
+    return Objects.hashCode(name, modelHash, fileSize, localFilePath, downloadId);
+  }
+
   /**
    * @return the model file path
    * @hide

firebase-ml-modeldownloader/src/main/java/com/google/firebase/ml/modeldownloader/internal/SharedPreferencesUtil.java

@@ -16,6 +16,7 @@
 
 import android.content.Context;
 import android.content.SharedPreferences;
+import android.content.SharedPreferences.Editor;
 import android.os.SystemClock;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
@@ -51,8 +52,8 @@ public SharedPreferencesUtil(FirebaseApp firebaseApp) {
    * Returns the Custom Model details currently associated with this model. If a fully downloaded
    * model is present - this returns the details of that model, including local file path. If an
    * update of an existing model is in progress, the local model plus the download id for the new
-   * upload is returned. To get only details related to the downloading model use
-   * getDownloadingCustomModelDetails. If this is the initial download of a local file - the
+   * upload is returned. To get only details related to the downloading model use {@link
+   * #getDownloadingCustomModelDetails}. If this is the initial download of a local file - the
    * downloading model details are returned.
    *
    * @param modelName - name of the model
@@ -138,7 +139,7 @@ public synchronized void setDownloadingCustomModelDetails(@NonNull CustomModel c
         .putLong(
             String.format(DOWNLOAD_BEGIN_TIME_MS_PATTERN, persistenceKey, modelName),
             SystemClock.elapsedRealtime())
-        .apply();
+        .commit();
   }
 
   /**
@@ -154,35 +155,19 @@ public synchronized void setUploadedCustomModelDetails(@NonNull CustomModel cust
     if (!id.equals(0L)) {
       throw new IllegalArgumentException("Only call when Custom model has completed download.");
     }
+    Editor editor = getSharedPreferences().edit();
+    clearDownloadingModelDetails(editor, customModel.getName());
 
     String modelName = customModel.getName();
     String hash = customModel.getModelHash();
     long size = customModel.getSize();
     String filePath = customModel.getLocalFilePath();
-    getSharedPreferences()
-        .edit()
+    editor
         .putString(String.format(LOCAL_MODEL_HASH_PATTERN, persistenceKey, modelName), hash)
         .putLong(String.format(LOCAL_MODEL_FILE_SIZE_PATTERN, persistenceKey, modelName), size)
         .putString(
             String.format(LOCAL_MODEL_FILE_PATH_PATTERN, persistenceKey, modelName), filePath)
-        .apply();
-
-    clearDownloadingModelDetails(customModel.getName());
-  }
-
-  /**
-   * Clears all stored data related to a custom model download.
-   *
-   * @param modelName - name of model
-   */
-  public synchronized void clearDownloadingModelDetails(@NonNull String modelName) {
-    getSharedPreferences()
-        .edit()
-        .remove(String.format(DOWNLOADING_MODEL_ID_PATTERN, persistenceKey, modelName))
-        .remove(String.format(DOWNLOADING_MODEL_HASH_PATTERN, persistenceKey, modelName))
-        .remove(String.format(DOWNLOADING_MODEL_SIZE_PATTERN, persistenceKey, modelName))
-        .remove(String.format(DOWNLOAD_BEGIN_TIME_MS_PATTERN, persistenceKey, modelName))
-        .apply();
+        .commit();
   }
 
   /**
@@ -194,18 +179,34 @@ public synchronized void clearModelDetails(@NonNull String modelName, boolean cl
     if (cleanUpModelFile) {
       // TODO(annz) - add code to remove model files from device
     }
+    Editor editor = getSharedPreferences().edit();
 
-    clearDownloadingModelDetails(modelName);
+    clearDownloadingModelDetails(editor, modelName);
 
-    getSharedPreferences()
-        .edit()
+    editor
         .remove(String.format(LOCAL_MODEL_FILE_PATH_PATTERN, persistenceKey, modelName))
         .remove(String.format(LOCAL_MODEL_FILE_SIZE_PATTERN, persistenceKey, modelName))
         .remove(String.format(LOCAL_MODEL_HASH_PATTERN, persistenceKey, modelName))
+        .commit();
+  }
+
+  /**
+   * Clears all stored data related to a custom model download.
+   *
+   * @param modelName - name of model
+   */
+  @VisibleForTesting
+  synchronized void clearDownloadingModelDetails(Editor editor, @NonNull String modelName) {
+    editor
+        .remove(String.format(DOWNLOADING_MODEL_ID_PATTERN, persistenceKey, modelName))
+        .remove(String.format(DOWNLOADING_MODEL_HASH_PATTERN, persistenceKey, modelName))
+        .remove(String.format(DOWNLOADING_MODEL_SIZE_PATTERN, persistenceKey, modelName))
+        .remove(String.format(DOWNLOAD_BEGIN_TIME_MS_PATTERN, persistenceKey, modelName))
         .apply();
   }
 
-  private SharedPreferences getSharedPreferences() {
+  @VisibleForTesting
+  SharedPreferences getSharedPreferences() {
     return firebaseApp
         .getApplicationContext()
         .getSharedPreferences(PREFERENCES_PACKAGE_NAME, Context.MODE_PRIVATE);

firebase-ml-modeldownloader/src/test/java/com/google/firebase/ml/modeldownloader/CustomModelTest.java

@@ -15,7 +15,10 @@
 package com.google.firebase.ml.modeldownloader;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -52,4 +55,21 @@ public void customModel_getDownloadId() {
   public void customModel_getFile_downloadIncomplete() {
     assertNull(CUSTOM_MODEL.getFile());
   }
+
+  @Test
+  public void customModel_equals() {
+    assertTrue(CUSTOM_MODEL.equals(new CustomModel(MODEL_NAME, 0, 100, MODEL_HASH)));
+    assertFalse(CUSTOM_MODEL.equals(new CustomModel(MODEL_NAME, 0, 101, MODEL_HASH)));
+    assertFalse(CUSTOM_MODEL.equals(new CustomModel(MODEL_NAME, 101, 100, MODEL_HASH)));
+  }
+
+  @Test
+  public void customModel_hashCode() {
+    assertEquals(
+        CUSTOM_MODEL.hashCode(), new CustomModel(MODEL_NAME, 0, 100, MODEL_HASH).hashCode());
+    assertNotEquals(
+        CUSTOM_MODEL.hashCode(), new CustomModel(MODEL_NAME, 0, 101, MODEL_HASH).hashCode());
+    assertNotEquals(
+        CUSTOM_MODEL.hashCode(), new CustomModel(MODEL_NAME, 101, 100, MODEL_HASH).hashCode());
+  }
 }

firebase-ml-modeldownloader/src/test/java/com/google/firebase/ml/modeldownloader/internal/SharedPreferencesUtilTest.java

@@ -44,13 +44,6 @@ public class SharedPreferencesUtilTest {
   private static final CustomModel CUSTOM_MODEL_DOWNLOADING =
       new CustomModel(MODEL_NAME, 986, 100, MODEL_HASH);
 
-  //  private static final FirebaseApp FIREBASE_APP = FirebaseApp.initializeApp(
-  //      ApplicationProvider.getApplicationContext(),
-  //      new FirebaseOptions.Builder()
-  //          .setApplicationId("1:123456789:android:abcdef")
-  //          .setProjectId(TEST_PROJECT_ID)
-  //          .build());
-
   private SharedPreferencesUtil sharedPreferencesUtil;
 
   @Before
@@ -119,7 +112,9 @@ public void clearDownloadingModelDetails_keepsLocalModel() throws IllegalArgumen
     sharedPreferencesUtil.setDownloadingCustomModelDetails(CUSTOM_MODEL_DOWNLOADING);
     CustomModel retrievedModel = sharedPreferencesUtil.getCustomModelDetails(MODEL_NAME);
     assertEquals(retrievedModel, CUSTOM_MODEL_UPDATE_IN_BACKGROUND);
-    sharedPreferencesUtil.clearDownloadingModelDetails(CUSTOM_MODEL_DOWNLOAD_COMPLETE.getName());
+    sharedPreferencesUtil.clearDownloadingModelDetails(
+        sharedPreferencesUtil.getSharedPreferences().edit(),
+        CUSTOM_MODEL_DOWNLOAD_COMPLETE.getName());
     retrievedModel = sharedPreferencesUtil.getCustomModelDetails(MODEL_NAME);
     assertEquals(retrievedModel, CUSTOM_MODEL_DOWNLOAD_COMPLETE);
   }