Actually skips overlay migration when run a second time. (#3704)

* Feedback.

* Fix change log

Author: wu-hui

firebase-firestore/CHANGELOG.md

@@ -3,7 +3,9 @@ by opting into a release at
 [go/firebase-android-release](http:go/firebase-android-release) (Googlers only).
 
 # Unreleased
-- [changed] Added `TransactionOptions` to control how many times a transaction
+
+# 24.1.2
+- [feature] Added `TransactionOptions` to control how many times a transaction
   will retry commits before failing.
 - [fixed] Fixed an issue where patching multiple fields shadows each other (#3528).
 

firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteOverlayMigrationManager.java

@@ -46,6 +46,10 @@ private void buildOverlays() {
     db.runTransaction(
         "build overlays",
         () -> {
+          if (!hasPendingOverlayMigration()) {
+            return;
+          }
+
           Set<String> userIds = getAllUserIds();
           RemoteDocumentCache remoteDocumentCache = db.getRemoteDocumentCache();
           for (String uid : userIds) {

firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteOverlayMigrationManagerTest.java

@@ -25,6 +25,7 @@
 import static java.util.Arrays.asList;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 
 import com.google.firebase.firestore.FieldValue;
 import com.google.firebase.firestore.auth.User;
@@ -110,6 +111,46 @@ public void testCreateOverlayFromSet() {
     assertFalse(migrationManager.hasPendingOverlayMigration());
   }
 
+  @Test
+  public void testSkipsIfAlreadyMigrated() {
+    writeRemoteDocument(doc("foo/bar", 2, map("it", "original")));
+    writeMutation(setMutation("foo/bar", map("foo", "bar")));
+
+    // Switch to new persistence and run migrations
+    this.persistence.shutdown();
+    persistence = PersistenceTestHelpers.createSQLitePersistence("test-data-migration");
+    IndexBackfiller indexBackfiller = new IndexBackfiller(persistence, new AsyncQueue());
+    localStore =
+        new LocalStore(persistence, indexBackfiller, new QueryEngine(), User.UNAUTHENTICATED);
+    localStore.start();
+
+    assertEquals(
+        setMutation("foo/bar", map("foo", "bar")),
+        persistence
+            .getDocumentOverlayCache(User.UNAUTHENTICATED)
+            .getOverlay(key("foo/bar"))
+            .getMutation());
+
+    // Delete the overlay to verify migration is skipped the second time.
+    persistence.getDocumentOverlayCache(User.UNAUTHENTICATED).removeOverlaysForBatchId(1);
+
+    // Switch to new persistence and run migrations which should be a no-op.
+    this.persistence.shutdown();
+    persistence = PersistenceTestHelpers.createSQLitePersistence("test-data-migration");
+    indexBackfiller = new IndexBackfiller(persistence, new AsyncQueue());
+    localStore =
+        new LocalStore(persistence, indexBackfiller, new QueryEngine(), User.UNAUTHENTICATED);
+    localStore.start();
+
+    SQLiteOverlayMigrationManager migrationManager =
+        (SQLiteOverlayMigrationManager) persistence.getOverlayMigrationManager();
+    assertFalse(migrationManager.hasPendingOverlayMigration());
+
+    // We deleted the overlay earlier and the migration is not run again, so we get a null.
+    assertNull(
+        persistence.getDocumentOverlayCache(User.UNAUTHENTICATED).getOverlay(key("foo/bar")));
+  }
+
   @Test
   public void testCreateOverlayFromDelete() {
     writeRemoteDocument(doc("foo/bar", 2, map("it", "original")));