Revert "Simplify IndexFree tests (#790)"

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

@@ -1073,31 +1073,49 @@ public void testUsesTargetMappingToExecuteQueries() {
 
   @Test
   public void testLastLimboFreeSnapshotIsAdvancedDuringViewProcessing() {
+    assumeFalse(garbageCollectorIsEager());
+    assumeTrue(queryEngine instanceof IndexFreeQueryEngine);
+
     // This test verifies that the `lastLimboFreeSnapshot` version for QueryData is advanced when
     // we compute a limbo-free free view and that the mapping is persisted when we release a query.
 
-    Query query = Query.atPath(ResourcePath.fromString("foo"));
+    writeMutation(setMutation("foo/ignored", map("matches", false)));
+    acknowledgeMutation(10);
+
+    Query query =
+        Query.atPath(ResourcePath.fromString("foo")).filter(filter("matches", "==", true));
     int targetId = allocateQuery(query);
 
-    // Advance the query snapshot.
+    // Mark the query as current.
+    applyRemoteEvent(
+        addedRemoteEvent(
+            asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 10, map("matches", true))),
+            asList(targetId),
+            emptyList()));
     applyRemoteEvent(noChangeEvent(targetId, 10));
 
     // At this point, we have not yet confirmed that the query is limbo free.
-    QueryData cachedQueryData = localStore.getQueryData(query);
-    Assert.assertEquals(SnapshotVersion.NONE, cachedQueryData.getLastLimboFreeSnapshotVersion());
+    Assert.assertEquals(
+        SnapshotVersion.NONE, localStore.getQueryData(query).getLastLimboFreeSnapshotVersion());
 
-    // Mark the view synced, which updates the last limbo free snapshot version.
+    // Update the view, but don't mark the view synced.
+    Assert.assertEquals(
+        SnapshotVersion.NONE, localStore.getQueryData(query).getLastLimboFreeSnapshotVersion());
     udpateViews(targetId, /* fromCache=*/ false);
-    cachedQueryData = localStore.getQueryData(query);
-    Assert.assertEquals(version(10), cachedQueryData.getLastLimboFreeSnapshotVersion());
+
+    // The query is marked limbo-free only when we mark the view synced.
+    udpateViews(targetId, /* fromCache=*/ false);
+    Assert.assertNotEquals(
+        SnapshotVersion.NONE, localStore.getQueryData(query).getLastLimboFreeSnapshotVersion());
 
     // The last limbo free snapshot version is persisted even if we release the query.
     releaseQuery(query);
+    allocateQuery(query);
 
-    if (!garbageCollectorIsEager()) {
-      cachedQueryData = localStore.getQueryData(query);
-      Assert.assertEquals(version(10), cachedQueryData.getLastLimboFreeSnapshotVersion());
-    }
+    // Verify that we only read the two documents that match the query.
+    executeQuery(query);
+    assertRemoteDocumentsRead(2);
+    assertQueryReturned("foo/a", "foo/b");
   }
 
   @Test
@@ -1112,26 +1130,28 @@ public void testQueriesIncludeLocallyModifiedDocuments() {
 
     applyRemoteEvent(
         addedRemoteEvent(
-            asList(doc("foo/a", 10, map("matches", true))), asList(targetId), emptyList()));
+            asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 10, map("matches", true))),
+            asList(targetId),
+            emptyList()));
     applyRemoteEvent(noChangeEvent(targetId, 10));
     udpateViews(targetId, /* fromCache= */ false);
 
     // Execute the query based on the RemoteEvent.
     executeQuery(query);
-    assertQueryReturned("foo/a");
+    assertQueryReturned("foo/a", "foo/b");
 
     // Write a document.
-    writeMutation(setMutation("foo/b", map("matches", true)));
+    writeMutation(setMutation("foo/c", map("matches", true)));
 
     // Execute the query and make sure that the pending mutation is included in the result.
     executeQuery(query);
-    assertQueryReturned("foo/a", "foo/b");
+    assertQueryReturned("foo/a", "foo/b", "foo/c");
 
     acknowledgeMutation(11);
 
     // Execute the query and make sure that the acknowledged mutation is included in the result.
     executeQuery(query);
-    assertQueryReturned("foo/a", "foo/b");
+    assertQueryReturned("foo/a", "foo/b", "foo/c");
   }
 
   @Test
@@ -1147,17 +1167,23 @@ public void testQueriesIncludeDocumentsFromOtherQueries() {
 
     applyRemoteEvent(
         addedRemoteEvent(
-            asList(doc("foo/a", 10, map("matches", true))), asList(targetId), emptyList()));
+            asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 10, map("matches", true))),
+            asList(targetId),
+            emptyList()));
     applyRemoteEvent(noChangeEvent(targetId, 10));
     udpateViews(targetId, /* fromCache=*/ false);
     releaseQuery(filteredQuery);
 
     // Start another query and add more matching documents to the collection.
-    Query fullQuery = Query.atPath(ResourcePath.fromString("foo"));
-    targetId = allocateQuery(fullQuery);
+    Query fullQuery =
+        Query.atPath(ResourcePath.fromString("foo")).filter(filter("matches", "==", true));
+    targetId = allocateQuery(filteredQuery);
     applyRemoteEvent(
         addedRemoteEvent(
-            asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 20, map("matches", true))),
+            asList(
+                doc("foo/a", 10, map("matches", true)),
+                doc("foo/b", 10, map("matches", true)),
+                doc("foo/c", 20, map("matches", true))),
             asList(targetId),
             emptyList()));
     releaseQuery(fullQuery);
@@ -1166,7 +1192,7 @@ public void testQueriesIncludeDocumentsFromOtherQueries() {
     // matches are included in the result set.
     allocateQuery(filteredQuery);
     executeQuery(filteredQuery);
-    assertQueryReturned("foo/a", "foo/b");
+    assertQueryReturned("foo/a", "foo/b", "foo/c");
   }
 
   @Test
@@ -1191,8 +1217,9 @@ public void testQueriesFilterDocumentsThatNoLongerMatch() {
     releaseQuery(filteredQuery);
 
     // Modify one of the documents to no longer match while the filtered query is inactive.
-    Query fullQuery = Query.atPath(ResourcePath.fromString("foo"));
-    targetId = allocateQuery(fullQuery);
+    Query fullQuery =
+        Query.atPath(ResourcePath.fromString("foo")).filter(filter("matches", "==", true));
+    targetId = allocateQuery(filteredQuery);
     applyRemoteEvent(
         addedRemoteEvent(
             asList(doc("foo/a", 10, map("matches", true)), doc("foo/b", 20, map("matches", false))),

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

@@ -418,15 +418,13 @@ public QueryData getQueryDataForTarget(int targetId) {
               }
             });
 
-    SnapshotVersion version = SnapshotVersion.NONE;
-
     for (MaybeDocument doc : docs) {
       DocumentChange change =
           new DocumentChange(updatedInTargets, removedFromTargets, doc.getKey(), doc);
       aggregator.handleDocumentChange(change);
-      version = doc.getVersion().compareTo(version) > 0 ? doc.getVersion() : version;
     }
 
+    SnapshotVersion version = docs.get(0).getVersion();
     return aggregator.createRemoteEvent(version);
   }