Review

Author: schmidt-sebastian

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

@@ -846,8 +846,8 @@ public QueryResult executeQuery(Query query, boolean usePreviousResults) {
     ImmutableSortedMap<DocumentKey, Document> documents =
         queryEngine.getDocumentsMatchingQuery(
             query,
-            remoteKeys,
-            usePreviousResults ? lastLimboFreeSnapshotVersion : SnapshotVersion.NONE);
+            usePreviousResults ? lastLimboFreeSnapshotVersion : SnapshotVersion.NONE,
+            remoteKeys);
     return new QueryResult(documents, remoteKeys);
   }
 

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

@@ -81,8 +81,8 @@ public void initialize(LocalDocumentsView localDocumentsView, IndexManager index
 
   public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
       Query query,
-      ImmutableSortedSet<DocumentKey> remoteKeys,
-      SnapshotVersion lastLimboFreeSnapshotVersion) {
+      SnapshotVersion lastLimboFreeSnapshotVersion,
+      ImmutableSortedSet<DocumentKey> remoteKeys) {
     hardAssert(initialized, "initialize() not called");
 
     ImmutableSortedMap<DocumentKey, Document> result =
@@ -118,7 +118,8 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
     Set<DocumentKey> keys = indexManager.getDocumentsMatchingTarget(fieldIndex, target);
     ImmutableSortedMap<DocumentKey, Document> indexedDocuments =
         localDocumentsView.getDocuments(keys);
-    return mergeResults(values(indexedDocuments), query, fieldIndex.getIndexState().getOffset());
+    return appendRemainingResults(
+        values(indexedDocuments), query, fieldIndex.getIndexState().getOffset());
   }
 
   /**
@@ -158,7 +159,8 @@ && needsRefill(
           query.toString());
     }
 
-    return mergeResults(previousResults, query, IndexOffset.create(lastLimboFreeSnapshotVersion));
+    return appendRemainingResults(
+        previousResults, query, IndexOffset.create(lastLimboFreeSnapshotVersion));
   }
 
   /** Applies the query filter and sorting to the provided documents. */
@@ -223,15 +225,18 @@ private ImmutableSortedMap<DocumentKey, Document> executeFullCollectionScan(Quer
     return localDocumentsView.getDocumentsMatchingQuery(query, IndexOffset.NONE);
   }
 
-  private ImmutableSortedMap<DocumentKey, Document> mergeResults(
-      Iterable<Document> before, Query query, IndexOffset offset) {
-    // Retrieve all results for documents that were updated since the last limbo-document free
-    // remote snapshot.
-    ImmutableSortedMap<DocumentKey, Document> after =
+  /**
+   * Combines the results from an indexed execution with the remaining documents that have not yet
+   * been indexed.
+   */
+  private ImmutableSortedMap<DocumentKey, Document> appendRemainingResults(
+      Iterable<Document> indexedResults, Query query, IndexOffset offset) {
+    // Retrieve all results for documents that were updated since the offset.
+    ImmutableSortedMap<DocumentKey, Document> remainingResults =
         localDocumentsView.getDocumentsMatchingQuery(query, offset);
-    for (Document entry : before) {
-      after = after.insert(entry.getKey(), entry);
+    for (Document entry : indexedResults) {
+      remainingResults = remainingResults.insert(entry.getKey(), entry);
     }
-    return after;
+    return remainingResults;
   }
 }

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

@@ -68,9 +68,9 @@ public void initialize(LocalDocumentsView localDocuments, IndexManager indexMana
   @Override
   public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
       Query query,
-      ImmutableSortedSet<DocumentKey> remoteKeys,
-      SnapshotVersion lastLimboFreeSnapshotVersion) {
-    return queryEngine.getDocumentsMatchingQuery(query, remoteKeys, lastLimboFreeSnapshotVersion);
+      SnapshotVersion lastLimboFreeSnapshotVersion,
+      ImmutableSortedSet<DocumentKey> remoteKeys) {
+    return queryEngine.getDocumentsMatchingQuery(query, lastLimboFreeSnapshotVersion, remoteKeys);
   }
 
   /** Returns the query engine that is used as the backing implementation. */

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

@@ -90,7 +90,7 @@ public void combinesIndexedWithNonIndexedResults() {
     Query queryWithFilter = query("coll").filter(filter("foo", "==", true));
     ImmutableSortedMap<DocumentKey, Document> results =
         queryEngine.getDocumentsMatchingQuery(
-            queryWithFilter, DocumentKey.emptyKeySet(), SnapshotVersion.NONE);
+            queryWithFilter, SnapshotVersion.NONE, DocumentKey.emptyKeySet());
 
     assertTrue(results.containsKey(doc1.getKey()));
     assertTrue(results.containsKey(doc2.getKey()));

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

@@ -185,8 +185,8 @@ private DocumentSet runQuery(Query query, SnapshotVersion lastLimboFreeSnapshotV
     ImmutableSortedMap<DocumentKey, Document> docs =
         queryEngine.getDocumentsMatchingQuery(
             query,
-            targetCache.getMatchingKeysForTargetId(TEST_TARGET_ID),
-            lastLimboFreeSnapshotVersion);
+            lastLimboFreeSnapshotVersion,
+            targetCache.getMatchingKeysForTargetId(TEST_TARGET_ID));
     View view =
         new View(query, new ImmutableSortedSet<>(Collections.emptyList(), DocumentKey::compareTo));
     View.DocumentChanges viewDocChanges = view.computeDocChanges(docs);
@@ -405,8 +405,8 @@ public void doesNotIncludeDocumentsDeletedByMutation() throws Exception {
             () ->
                 queryEngine.getDocumentsMatchingQuery(
                     query,
-                    targetCache.getMatchingKeysForTargetId(TEST_TARGET_ID),
-                    LAST_LIMBO_FREE_SNAPSHOT));
+                    LAST_LIMBO_FREE_SNAPSHOT,
+                    targetCache.getMatchingKeysForTargetId(TEST_TARGET_ID)));
     assertEquals(emptyMutableDocumentMap().insert(MATCHING_DOC_A.getKey(), MATCHING_DOC_A), docs);
   }
 }