Use immutable Documents in MemoryRemoteDocumentCache

Author: schmidt-sebastian

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

@@ -21,6 +21,7 @@
 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;
@@ -48,7 +49,7 @@ public LocalSerializer(RemoteSerializer rpcSerializer) {
   }
 
   /** Encodes a MaybeDocument model to the equivalent protocol buffer for local storage. */
-  com.google.firebase.firestore.proto.MaybeDocument encodeMaybeDocument(MutableDocument document) {
+  com.google.firebase.firestore.proto.MaybeDocument encodeMaybeDocument(Document document) {
     com.google.firebase.firestore.proto.MaybeDocument.Builder builder =
         com.google.firebase.firestore.proto.MaybeDocument.newBuilder();
 
@@ -88,7 +89,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(MutableDocument document) {
+  private com.google.firestore.v1.Document encodeDocument(Document document) {
     com.google.firestore.v1.Document.Builder builder =
         com.google.firestore.v1.Document.newBuilder();
     builder.setName(rpcSerializer.encodeKey(document.getKey()));
@@ -110,8 +111,7 @@ private MutableDocument decodeDocument(
   }
 
   /** Encodes a NoDocument value to the equivalent proto. */
-  private com.google.firebase.firestore.proto.NoDocument encodeNoDocument(
-      MutableDocument document) {
+  private com.google.firebase.firestore.proto.NoDocument encodeNoDocument(Document 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(
-      MutableDocument document) {
+      Document 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/MemoryLruReferenceDelegate.java

@@ -18,6 +18,7 @@
 
 import android.util.SparseArray;
 import com.google.firebase.firestore.core.ListenSequence;
+import com.google.firebase.firestore.model.Document;
 import com.google.firebase.firestore.model.DocumentKey;
 import com.google.firebase.firestore.model.MutableDocument;
 import com.google.firebase.firestore.util.Consumer;
@@ -117,7 +118,7 @@ public int removeTargets(long upperBound, SparseArray<?> activeTargetIds) {
   public int removeOrphanedDocuments(long upperBound) {
     int count = 0;
     MemoryRemoteDocumentCache cache = persistence.getRemoteDocumentCache();
-    for (MutableDocument doc : cache.getDocuments()) {
+    for (Document doc : cache.getDocuments()) {
       DocumentKey key = doc.getKey();
       if (!isPinned(key, upperBound)) {
         cache.remove(key);

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

@@ -14,10 +14,12 @@
 
 package com.google.firebase.firestore.local;
 
+import static com.google.firebase.firestore.model.DocumentCollections.emptyDocumentMap;
 import static com.google.firebase.firestore.util.Assert.hardAssert;
 
 import androidx.annotation.NonNull;
 import com.google.firebase.database.collection.ImmutableSortedMap;
+import com.google.firebase.firestore.model.Document;
 import com.google.firebase.firestore.model.DocumentKey;
 import com.google.firebase.firestore.model.FieldIndex.IndexOffset;
 import com.google.firebase.firestore.model.MutableDocument;
@@ -31,14 +33,14 @@
 final class MemoryRemoteDocumentCache implements RemoteDocumentCache {
 
   /** Underlying cache of documents and their read times. */
-  private ImmutableSortedMap<DocumentKey, MutableDocument> docs;
+  private ImmutableSortedMap<DocumentKey, Document> docs;
   /** Manages the collection group index. */
   private IndexManager indexManager;
   /** The latest read time of any document in the cache. */
   private SnapshotVersion latestReadTime;
 
   MemoryRemoteDocumentCache() {
-    docs = ImmutableSortedMap.Builder.emptyMap(DocumentKey.comparator());
+    docs = emptyDocumentMap();
     latestReadTime = SnapshotVersion.NONE;
   }
 
@@ -66,7 +68,7 @@ public void remove(DocumentKey key) {
 
   @Override
   public MutableDocument get(DocumentKey key) {
-    MutableDocument doc = docs.get(key);
+    Document doc = docs.get(key);
     return doc != null ? doc.clone() : MutableDocument.newInvalidDocument(key);
   }
 
@@ -93,11 +95,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, MutableDocument>> iterator = docs.iteratorFrom(prefix);
+    Iterator<Map.Entry<DocumentKey, Document>> iterator = docs.iteratorFrom(prefix);
 
     while (iterator.hasNext()) {
-      Map.Entry<DocumentKey, MutableDocument> entry = iterator.next();
-      MutableDocument doc = entry.getValue();
+      Map.Entry<DocumentKey, Document> entry = iterator.next();
+      Document doc = entry.getValue();
 
       DocumentKey key = entry.getKey();
       if (!collection.isPrefixOf(key.getPath())) {
@@ -126,13 +128,13 @@ public SnapshotVersion getLatestReadTime() {
     return latestReadTime;
   }
 
-  Iterable<MutableDocument> getDocuments() {
+  Iterable<Document> getDocuments() {
     return new DocumentIterable();
   }
 
   long getByteSize(LocalSerializer serializer) {
     long count = 0;
-    for (MutableDocument doc : new DocumentIterable()) {
+    for (Document doc : new DocumentIterable()) {
       count += serializer.encodeMaybeDocument(doc).getSerializedSize();
     }
     return count;
@@ -141,20 +143,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<MutableDocument> {
+  private class DocumentIterable implements Iterable<Document> {
     @NonNull
     @Override
-    public Iterator<MutableDocument> iterator() {
-      Iterator<Map.Entry<DocumentKey, MutableDocument>> iterator =
+    public Iterator<Document> iterator() {
+      Iterator<Map.Entry<DocumentKey, Document>> iterator =
           MemoryRemoteDocumentCache.this.docs.iterator();
-      return new Iterator<MutableDocument>() {
+      return new Iterator<Document>() {
         @Override
         public boolean hasNext() {
           return iterator.hasNext();
         }
 
         @Override
-        public MutableDocument next() {
+        public Document next() {
           return iterator.next().getValue();
         }
       };

firebase-firestore/src/main/java/com/google/firebase/firestore/model/Document.java

@@ -14,6 +14,7 @@
 
 package com.google.firebase.firestore.model;
 
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import com.google.firestore.v1.Value;
 import java.util.Comparator;
@@ -22,7 +23,7 @@
  * Represents a document in Firestore with a key, version, data and whether the data has local
  * mutations applied to it.
  */
-public interface Document {
+public interface Document extends Cloneable {
   /** A document comparator that returns document by key and key only. */
   Comparator<Document> KEY_COMPARATOR = (left, right) -> left.getKey().compareTo(right.getKey());
 
@@ -73,4 +74,7 @@ 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/main/java/com/google/firebase/firestore/model/MutableDocument.java

@@ -26,7 +26,7 @@
  * one of these states even after all mutations have been applied, {@link #isValidDocument} returns
  * false and the document should be removed from all views.
  */
-public final class MutableDocument implements Document, Cloneable {
+public final class MutableDocument implements Document {
 
   private enum DocumentType {
     /**