Reduce diff

Author: schmidt-sebastian

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

@@ -21,7 +21,6 @@
 import com.google.firebase.firestore.bundle.BundledQuery;
 import com.google.firebase.firestore.core.Query.LimitType;
 import com.google.firebase.firestore.core.Target;
-import com.google.firebase.firestore.model.Document;
 import com.google.firebase.firestore.model.DocumentKey;
 import com.google.firebase.firestore.model.FieldIndex;
 import com.google.firebase.firestore.model.FieldPath;
@@ -49,7 +48,7 @@ public LocalSerializer(RemoteSerializer rpcSerializer) {
   }
 
   /** Encodes a MaybeDocument model to the equivalent protocol buffer for local storage. */
-  com.google.firebase.firestore.proto.MaybeDocument encodeMaybeDocument(Document document) {
+  com.google.firebase.firestore.proto.MaybeDocument encodeMaybeDocument(MutableDocument document) {
     com.google.firebase.firestore.proto.MaybeDocument.Builder builder =
         com.google.firebase.firestore.proto.MaybeDocument.newBuilder();
 
@@ -89,7 +88,7 @@ MutableDocument decodeMaybeDocument(com.google.firebase.firestore.proto.MaybeDoc
    * Encodes a Document for local storage. This differs from the v1 RPC serializer for Documents in
    * that it preserves the updateTime, which is considered an output only value by the server.
    */
-  private com.google.firestore.v1.Document encodeDocument(Document document) {
+  private com.google.firestore.v1.Document encodeDocument(MutableDocument document) {
     com.google.firestore.v1.Document.Builder builder =
         com.google.firestore.v1.Document.newBuilder();
     builder.setName(rpcSerializer.encodeKey(document.getKey()));
@@ -111,7 +110,8 @@ private MutableDocument decodeDocument(
   }
 
   /** Encodes a NoDocument value to the equivalent proto. */
-  private com.google.firebase.firestore.proto.NoDocument encodeNoDocument(Document document) {
+  private com.google.firebase.firestore.proto.NoDocument encodeNoDocument(
+      MutableDocument document) {
     com.google.firebase.firestore.proto.NoDocument.Builder builder =
         com.google.firebase.firestore.proto.NoDocument.newBuilder();
     builder.setName(rpcSerializer.encodeKey(document.getKey()));
@@ -130,7 +130,7 @@ private MutableDocument decodeNoDocument(
 
   /** Encodes a UnknownDocument value to the equivalent proto. */
   private com.google.firebase.firestore.proto.UnknownDocument encodeUnknownDocument(
-      Document document) {
+      MutableDocument document) {
     com.google.firebase.firestore.proto.UnknownDocument.Builder builder =
         com.google.firebase.firestore.proto.UnknownDocument.newBuilder();
     builder.setName(rpcSerializer.encodeKey(document.getKey()));

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

@@ -848,8 +848,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/MemoryRemoteDocumentCache.java

@@ -34,14 +34,14 @@
 final class MemoryRemoteDocumentCache implements RemoteDocumentCache {
 
   /** Underlying cache of documents and their read times. */
-  private ImmutableSortedMap<DocumentKey, Document> docs;
+  private ImmutableSortedMap<DocumentKey, MutableDocument> docs;
   /** Manages the collection group index. */
   private IndexManager indexManager;
   /** The latest read time of any document in the cache. */
   private SnapshotVersion latestReadTime;
 
   MemoryRemoteDocumentCache() {
-    docs = emptyDocumentMap();
+    docs = ImmutableSortedMap.Builder.emptyMap(DocumentKey.comparator());
     latestReadTime = SnapshotVersion.NONE;
   }
 
@@ -77,7 +77,7 @@ public void removeAll(Collection<DocumentKey> keys) {
 
   @Override
   public MutableDocument get(DocumentKey key) {
-    Document doc = docs.get(key);
+    MutableDocument doc = docs.get(key);
     return doc != null ? doc.clone() : MutableDocument.newInvalidDocument(key);
   }
 
@@ -104,11 +104,11 @@ public Map<DocumentKey, MutableDocument> getAll(ResourcePath collection, IndexOf
     // Documents are ordered by key, so we can use a prefix scan to narrow down the documents
     // we need to match the query against.
     DocumentKey prefix = DocumentKey.fromPath(collection.append(""));
-    Iterator<Map.Entry<DocumentKey, Document>> iterator = docs.iteratorFrom(prefix);
+    Iterator<Map.Entry<DocumentKey, MutableDocument>> iterator = docs.iteratorFrom(prefix);
 
     while (iterator.hasNext()) {
-      Map.Entry<DocumentKey, Document> entry = iterator.next();
-      Document doc = entry.getValue();
+      Map.Entry<DocumentKey, MutableDocument> entry = iterator.next();
+      MutableDocument doc = entry.getValue();
 
       DocumentKey key = entry.getKey();
       if (!collection.isPrefixOf(key.getPath())) {
@@ -137,13 +137,13 @@ public SnapshotVersion getLatestReadTime() {
     return latestReadTime;
   }
 
-  Iterable<Document> getDocuments() {
+  Iterable<MutableDocument> getDocuments() {
     return new DocumentIterable();
   }
 
   long getByteSize(LocalSerializer serializer) {
     long count = 0;
-    for (Document doc : new DocumentIterable()) {
+    for (MutableDocument doc : new DocumentIterable()) {
       count += serializer.encodeMaybeDocument(doc).getSerializedSize();
     }
     return count;
@@ -152,20 +152,20 @@ long getByteSize(LocalSerializer serializer) {
   /**
    * A proxy that exposes an iterator over the current set of documents in the RemoteDocumentCache.
    */
-  private class DocumentIterable implements Iterable<Document> {
+  private class DocumentIterable implements Iterable<MutableDocument> {
     @NonNull
     @Override
-    public Iterator<Document> iterator() {
-      Iterator<Map.Entry<DocumentKey, Document>> iterator =
+    public Iterator<MutableDocument> iterator() {
+      Iterator<Map.Entry<DocumentKey, MutableDocument>> iterator =
           MemoryRemoteDocumentCache.this.docs.iterator();
-      return new Iterator<Document>() {
+      return new Iterator<MutableDocument>() {
         @Override
         public boolean hasNext() {
           return iterator.hasNext();
         }
 
         @Override
-        public Document next() {
+        public MutableDocument next() {
           return iterator.next().getValue();
         }
       };

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/main/java/com/google/firebase/firestore/model/Document.java

@@ -14,7 +14,6 @@
 
 package com.google.firebase.firestore.model;
 
-import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import com.google.firestore.v1.Value;
 import java.util.Comparator;
@@ -74,7 +73,4 @@ public interface Document {
    * Whether this document has a local mutation applied that has not yet been acknowledged by Watch.
    */
   boolean hasPendingWrites();
-
-  @NonNull
-  MutableDocument clone();
 }

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

@@ -69,9 +69,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);
   }
 }