Clear the lastLimboFreeSnapshot version on existence filter mismatch

firebase-firestore/CHANGELOG.md

@@ -7,6 +7,8 @@ by opting into a release at
   updates that have not yet been synced with the backend.
 - [changed] Improved performance for queries against collections that contain
   subcollections.
+- [fixed] Fixed an issue that can result in incomplete Query snapshots when an
+  app is backgrounded during query execution.
 
 # 24.0.0
 - [feature] Added support for Firebase AppCheck.

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

@@ -417,20 +417,24 @@ public ImmutableSortedMap<DocumentKey, Document> applyRemoteEvent(RemoteEvent re
             targetCache.removeMatchingKeys(change.getRemovedDocuments(), targetId);
             targetCache.addMatchingKeys(change.getAddedDocuments(), targetId);
 
-            ByteString resumeToken = change.getResumeToken();
-            // Update the resume token if the change includes one.
-            if (!resumeToken.isEmpty()) {
-              TargetData newTargetData =
-                  oldTargetData
-                      .withResumeToken(resumeToken, remoteEvent.getSnapshotVersion())
-                      .withSequenceNumber(sequenceNumber);
-              queryDataByTarget.put(targetId, newTargetData);
-
-              // Update the query data if there are target changes (or if sufficient time has
-              // passed since the last update).
-              if (shouldPersistTargetData(oldTargetData, newTargetData, change)) {
-                targetCache.updateTargetData(newTargetData);
-              }
+            TargetData newTargetData = oldTargetData.withSequenceNumber(sequenceNumber);
+            if (remoteEvent.getTargetMismatches().contains(targetId)) {
+              newTargetData =
+                  newTargetData
+                      .withResumeToken(ByteString.EMPTY, SnapshotVersion.NONE)
+                      .withLastLimboFreeSnapshotVersion(SnapshotVersion.NONE);
+            } else if (!change.getResumeToken().isEmpty()) {
+              newTargetData =
+                  newTargetData.withResumeToken(
+                      change.getResumeToken(), remoteEvent.getSnapshotVersion());
+            }
+
+            queryDataByTarget.put(targetId, newTargetData);
+
+            // Update the query data if there are target changes (or if sufficient time has passed
+            // since the last update).
+            if (shouldPersistTargetData(oldTargetData, newTargetData, change)) {
+              targetCache.updateTargetData(newTargetData);
             }
           }
 
@@ -554,10 +558,6 @@ private DocumentChangeResult populateDocumentChanges(
    */
   private static boolean shouldPersistTargetData(
       TargetData oldTargetData, TargetData newTargetData, TargetChange change) {
-    hardAssert(
-        !newTargetData.getResumeToken().isEmpty(),
-        "Attempted to persist query data with empty resume token");
-
     // Always persist query data if we don't already have a resume token.
     if (oldTargetData.getResumeToken().isEmpty()) return true;
 

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteStore.java

@@ -534,7 +534,7 @@ private void raiseWatchSnapshot(SnapshotVersion snapshotVersion) {
       }
     }
 
-    // Re-establish listens for the targets that have been invalidated by  existence filter
+    // Re-establish listens for the targets that have been invalidated by existence filter
     // mismatches.
     for (int targetId : remoteEvent.getTargetMismatches()) {
       TargetData targetData = this.listenTargets.get(targetId);

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

@@ -21,6 +21,7 @@
 import static com.google.firebase.firestore.testutil.TestUtil.deletedDoc;
 import static com.google.firebase.firestore.testutil.TestUtil.doc;
 import static com.google.firebase.firestore.testutil.TestUtil.docMap;
+import static com.google.firebase.firestore.testutil.TestUtil.existenceFilterEvent;
 import static com.google.firebase.firestore.testutil.TestUtil.filter;
 import static com.google.firebase.firestore.testutil.TestUtil.key;
 import static com.google.firebase.firestore.testutil.TestUtil.keySet;
@@ -75,6 +76,7 @@
 import com.google.firebase.firestore.remote.WriteStream;
 import com.google.firebase.firestore.testutil.TestUtil;
 import com.google.firebase.firestore.util.AsyncQueue;
+import com.google.protobuf.ByteString;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -1123,6 +1125,38 @@ public void testUsesTargetMappingToExecuteQueries() {
     assertQueryReturned("foo/a", "foo/b");
   }
 
+  @Test
+  public void testIgnoresTargetMappingAfterExistenceFilterMismatch() {
+    assumeFalse(garbageCollectorIsEager());
+
+    Query query = query("foo").filter(filter("matches", "==", true));
+    int targetId = allocateQuery(query);
+
+    executeQuery(query);
+
+    // Persist a mapping with a single document
+    applyRemoteEvent(
+        addedRemoteEvent(
+            asList(doc("foo/a", 10, map("matches", true))), asList(targetId), emptyList()));
+    applyRemoteEvent(noChangeEvent(targetId, 10));
+    updateViews(targetId, /* fromCache= */ false);
+
+    TargetData cachedTargetData = localStore.getTargetData(query.toTarget());
+    Assert.assertEquals(version(10), cachedTargetData.getLastLimboFreeSnapshotVersion());
+
+    // Create an existence filter mismatch and verify that the last limbo free snapshot version
+    // is deleted
+    applyRemoteEvent(existenceFilterEvent(targetId, 2, 20));
+    cachedTargetData = localStore.getTargetData(query.toTarget());
+    Assert.assertEquals(version(0), cachedTargetData.getLastLimboFreeSnapshotVersion());
+    Assert.assertEquals(ByteString.EMPTY, cachedTargetData.getResumeToken());
+
+    // Re-run the query as a collection scan
+    executeQuery(query);
+    assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 1);
+    assertQueryReturned("foo/a");
+  }
+
   @Test
   public void testLastLimboFreeSnapshotIsAdvancedDuringViewProcessing() {
     // This test verifies that the `lastLimboFreeSnapshot` version for TargetData is advanced when

firebase-firestore/src/test/resources/json/existence_filter_spec_test.json

@@ -1072,6 +1072,242 @@
       }
     ]
   },
+  "Existence filter mismatch invalidates index-free query": {
+    "describeName": "Existence Filters:",
+    "itName": "Existence filter clears resume token",
+    "tags": [
+      "durable-persistence"
+    ],
+    "config": {
+      "numClients": 1,
+      "useGarbageCollection": true
+    },
+    "steps": [
+      {
+        "userListen": {
+          "query": {
+            "filters": [
+            ],
+            "orderBys": [
+            ],
+            "path": "collection"
+          },
+          "targetId": 2
+        },
+        "expectedState": {
+          "activeTargets": {
+            "2": {
+              "queries": [
+                {
+                  "filters": [
+                  ],
+                  "orderBys": [
+                  ],
+                  "path": "collection"
+                }
+              ],
+              "resumeToken": ""
+            }
+          }
+        }
+      },
+      {
+        "watchAck": [
+          2
+        ]
+      },
+      {
+        "watchEntity": {
+          "docs": [
+            {
+              "key": "collection/1",
+              "options": {
+                "hasCommittedMutations": false,
+                "hasLocalMutations": false
+              },
+              "value": {
+                "v": 1
+              },
+              "version": 1000
+            },
+            {
+              "key": "collection/2",
+              "options": {
+                "hasCommittedMutations": false,
+                "hasLocalMutations": false
+              },
+              "value": {
+                "v": 2
+              },
+              "version": 1000
+            }
+          ],
+          "targets": [
+            2
+          ]
+        }
+      },
+      {
+        "watchCurrent": [
+          [
+            2
+          ],
+          "resume-token-1000"
+        ]
+      },
+      {
+        "watchSnapshot": {
+          "targetIds": [
+          ],
+          "version": 1000
+        },
+        "expectedSnapshotEvents": [
+          {
+            "added": [
+              {
+                "key": "collection/1",
+                "options": {
+                  "hasCommittedMutations": false,
+                  "hasLocalMutations": false
+                },
+                "value": {
+                  "v": 1
+                },
+                "version": 1000
+              },
+              {
+                "key": "collection/2",
+                "options": {
+                  "hasCommittedMutations": false,
+                  "hasLocalMutations": false
+                },
+                "value": {
+                  "v": 2
+                },
+                "version": 1000
+              }
+            ],
+            "errorCode": 0,
+            "fromCache": false,
+            "hasPendingWrites": false,
+            "query": {
+              "filters": [
+              ],
+              "orderBys": [
+              ],
+              "path": "collection"
+            }
+          }
+        ]
+      },
+      {
+        "watchFilter": [
+          [
+            2
+          ],
+          "collection/1"
+        ]
+      },
+      {
+        "watchSnapshot": {
+          "targetIds": [
+          ],
+          "version": 2000
+        },
+        "expectedSnapshotEvents": [
+          {
+            "errorCode": 0,
+            "fromCache": true,
+            "hasPendingWrites": false,
+            "query": {
+              "filters": [
+              ],
+              "orderBys": [
+              ],
+              "path": "collection"
+            }
+          }
+        ]
+      },
+      {
+        "restart": true,
+        "expectedState": {
+          "activeLimboDocs": [
+          ],
+          "activeTargets": {
+          },
+          "enqueuedLimboDocs": [
+          ]
+        }
+      },
+      {
+        "userListen": {
+          "query": {
+            "filters": [
+            ],
+            "orderBys": [
+            ],
+            "path": "collection"
+          },
+          "targetId": 2
+        },
+        "expectedSnapshotEvents": [
+          {
+            "added": [
+              {
+                "key": "collection/1",
+                "options": {
+                  "hasCommittedMutations": false,
+                  "hasLocalMutations": false
+                },
+                "value": {
+                  "v": 1
+                },
+                "version": 1000
+              },
+              {
+                "key": "collection/2",
+                "options": {
+                  "hasCommittedMutations": false,
+                  "hasLocalMutations": false
+                },
+                "value": {
+                  "v": 2
+                },
+                "version": 1000
+              }
+            ],
+            "errorCode": 0,
+            "fromCache": true,
+            "hasPendingWrites": false,
+            "query": {
+              "filters": [
+              ],
+              "orderBys": [
+              ],
+              "path": "collection"
+            }
+          }
+        ],
+        "expectedState": {
+          "activeTargets": {
+            "2": {
+              "queries": [
+                {
+                  "filters": [
+                  ],
+                  "orderBys": [
+                  ],
+                  "path": "collection"
+                }
+              ],
+              "resumeToken": ""
+            }
+          }
+        }
+      }
+    ]
+  },
   "Existence filter mismatch triggers re-run of query": {
     "describeName": "Existence Filters:",
     "itName": "Existence filter mismatch triggers re-run of query",

firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java

@@ -71,6 +71,7 @@
 import com.google.firebase.firestore.model.mutation.Precondition;
 import com.google.firebase.firestore.model.mutation.SetMutation;
 import com.google.firebase.firestore.model.mutation.VerifyMutation;
+import com.google.firebase.firestore.remote.ExistenceFilter;
 import com.google.firebase.firestore.remote.RemoteEvent;
 import com.google.firebase.firestore.remote.TargetChange;
 import com.google.firebase.firestore.remote.WatchChange;
@@ -426,6 +427,20 @@ public static RemoteEvent addedRemoteEvent(
     return addedRemoteEvent(singletonList(doc), updatedInTargets, removedFromTargets);
   }
 
+  public static RemoteEvent existenceFilterEvent(int targetId, int count, int version) {
+    TargetData targetData = TestUtil.targetData(targetId, QueryPurpose.LISTEN, "foo");
+    TestTargetMetadataProvider testTargetMetadataProvider = new TestTargetMetadataProvider();
+    testTargetMetadataProvider.setSyncedKeys(targetData, DocumentKey.emptyKeySet());
+
+    ExistenceFilter existenceFilter = new ExistenceFilter(count);
+    WatchChangeAggregator aggregator = new WatchChangeAggregator(testTargetMetadataProvider);
+
+    WatchChange.ExistenceFilterWatchChange existenceFilterWatchChange =
+        new WatchChange.ExistenceFilterWatchChange(targetId, existenceFilter);
+    aggregator.handleExistenceFilter(existenceFilterWatchChange);
+    return aggregator.createRemoteEvent(version(version));
+  }
+
   public static RemoteEvent addedRemoteEvent(
       List<MutableDocument> docs,
       List<Integer> updatedInTargets,