Adding hasCommittedMutations flag (#29)

Author: schmidt-sebastian

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java

@@ -72,6 +72,28 @@ public void testCanUpdateAnExistingDocument() {
     assertEquals(finalData, doc.getData());
   }
 
+  @Test
+  public void testCanUpdateAnUnknownDocument() {
+    DocumentReference writerRef = testFirestore().collection("collection").document();
+    DocumentReference readerRef =
+        testFirestore().collection("collection").document(writerRef.getId());
+    waitFor(writerRef.set(map("a", "a")));
+    waitFor(readerRef.update(map("b", "b")));
+    DocumentSnapshot writerSnap = waitFor(writerRef.get(Source.CACHE));
+    assertTrue(writerSnap.exists());
+    try {
+      waitFor(readerRef.get(Source.CACHE));
+      fail("Should have thrown exception");
+    } catch (RuntimeException e) {
+      assertEquals(
+          ((FirebaseFirestoreException) e.getCause().getCause()).getCode(), Code.UNAVAILABLE);
+    }
+    writerSnap = waitFor(writerRef.get());
+    assertEquals(map("a", "a", "b", "b"), writerSnap.getData());
+    DocumentSnapshot readerSnap = waitFor(readerRef.get());
+    assertEquals(map("a", "a", "b", "b"), readerSnap.getData());
+  }
+
   @Test
   public void testCanMergeDataWithAnExistingDocumentUsingSet() {
     DocumentReference documentReference = testCollection("rooms").document("eros");

firebase-firestore/src/main/java/com/google/firebase/firestore/Transaction.java

@@ -15,6 +15,7 @@
 package com.google.firebase.firestore;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.firebase.firestore.util.Assert.fail;
 
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
@@ -26,7 +27,6 @@
 import com.google.firebase.firestore.model.Document;
 import com.google.firebase.firestore.model.MaybeDocument;
 import com.google.firebase.firestore.model.NoDocument;
-import com.google.firebase.firestore.util.Assert;
 import com.google.firebase.firestore.util.Executors;
 import com.google.firebase.firestore.util.Util;
 import java.util.Collections;
@@ -237,15 +237,19 @@ private Task<DocumentSnapshot> getAsync(DocumentReference documentRef) {
               }
               List<MaybeDocument> docs = task.getResult();
               if (docs.size() != 1) {
-                throw Assert.fail("Mismatch in docs returned from document lookup.");
+                throw fail("Mismatch in docs returned from document lookup.");
               }
               MaybeDocument doc = docs.get(0);
-              if (doc instanceof NoDocument) {
+              if (doc instanceof Document) {
+                return DocumentSnapshot.fromDocument(
+                    firestore, (Document) doc, /*fromCache=*/ false);
+              } else if (doc instanceof NoDocument) {
                 return DocumentSnapshot.fromNoDocument(
                     firestore, doc.getKey(), /*fromCache=*/ false);
               } else {
-                return DocumentSnapshot.fromDocument(
-                    firestore, (Document) doc, /*fromCache=*/ false);
+                throw fail(
+                    "BatchGetDocumentsRequest returned unexpected document type: "
+                        + doc.getClass().getCanonicalName());
               }
             });
   }

firebase-firestore/src/main/java/com/google/firebase/firestore/core/Transaction.java

@@ -20,6 +20,7 @@
 import com.google.firebase.firestore.FirebaseFirestoreException.Code;
 import com.google.firebase.firestore.core.UserData.ParsedSetData;
 import com.google.firebase.firestore.core.UserData.ParsedUpdateData;
+import com.google.firebase.firestore.model.Document;
 import com.google.firebase.firestore.model.DocumentKey;
 import com.google.firebase.firestore.model.MaybeDocument;
 import com.google.firebase.firestore.model.NoDocument;
@@ -40,6 +41,8 @@
 import java.util.concurrent.TimeUnit;
 import javax.annotation.Nullable;
 
+import static com.google.firebase.firestore.util.Assert.fail;
+
 /**
  * Internal transaction object responsible for accumulating the mutations to perform and the base
  * versions for any documents read.
@@ -55,10 +58,14 @@ public Transaction(Datastore d) {
   }
 
   private void recordVersion(MaybeDocument doc) throws FirebaseFirestoreException {
-    SnapshotVersion docVersion = doc.getVersion();
-    if (doc instanceof NoDocument) {
+    SnapshotVersion docVersion;
+    if (doc instanceof Document) {
+      docVersion = doc.getVersion();
+    } else if (doc instanceof NoDocument) {
       // For nonexistent docs, we must use precondition with version 0 when we overwrite them.
       docVersion = SnapshotVersion.NONE;
+    } else {
+      throw fail("Unexpected document type in transaction: " + doc.getClass().getCanonicalName());
     }
 
     if (readVersions.containsKey(doc.getKey())) {

firebase-firestore/src/main/java/com/google/firebase/firestore/core/View.java

@@ -214,6 +214,7 @@ public <D extends MaybeDocument> DocumentChanges computeDocChanges(
       while (newDocumentSet.size() > query.getLimit()) {
         Document oldDoc = newDocumentSet.getLastDocument();
         newDocumentSet = newDocumentSet.remove(oldDoc.getKey());
+        newMutatedKeys = newMutatedKeys.remove(oldDoc.getKey());
         changeSet.addChange(DocumentViewChange.create(Type.REMOVED, oldDoc));
       }
     }

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

@@ -24,6 +24,7 @@
 import com.google.firebase.firestore.model.MaybeDocument;
 import com.google.firebase.firestore.model.NoDocument;
 import com.google.firebase.firestore.model.SnapshotVersion;
+import com.google.firebase.firestore.model.UnknownDocument;
 import com.google.firebase.firestore.model.mutation.Mutation;
 import com.google.firebase.firestore.model.mutation.MutationBatch;
 import com.google.firebase.firestore.model.value.FieldValue;
@@ -48,25 +49,35 @@ com.google.firebase.firestore.proto.MaybeDocument encodeMaybeDocument(MaybeDocum
     com.google.firebase.firestore.proto.MaybeDocument.Builder builder =
         com.google.firebase.firestore.proto.MaybeDocument.newBuilder();
     if (document instanceof NoDocument) {
-      builder.setNoDocument(encodeNoDocument((NoDocument) document));
-
+      NoDocument noDocument = (NoDocument) document;
+      builder.setNoDocument(encodeNoDocument(noDocument));
+      builder.setHasCommittedMutations(noDocument.hasCommittedMutations());
     } else if (document instanceof Document) {
-      builder.setDocument(encodeDocument((Document) document));
+      Document existingDocument = (Document) document;
+      builder.setDocument(encodeDocument(existingDocument));
+      builder.setHasCommittedMutations(existingDocument.hasCommittedMutations());
+    } else if (document instanceof UnknownDocument) {
+      builder.setUnknownDocument(encodeUnknownDocument((UnknownDocument) document));
+      builder.setHasCommittedMutations(true);
     } else {
       throw fail("Unknown document type %s", document.getClass().getCanonicalName());
     }
+
     return builder.build();
   }
 
   /** Decodes a MaybeDocument proto to the equivalent model. */
   MaybeDocument decodeMaybeDocument(com.google.firebase.firestore.proto.MaybeDocument proto) {
     switch (proto.getDocumentTypeCase()) {
       case DOCUMENT:
-        return decodeDocument(proto.getDocument());
+        return decodeDocument(proto.getDocument(), proto.getHasCommittedMutations());
 
       case NO_DOCUMENT:
         return decodeNoDocument(proto.getNoDocument());
 
+      case UNKNOWN_DOCUMENT:
+        return decodeUnknownDocument(proto.getUnknownDocument());
+
       default:
         throw fail("Unknown MaybeDocument %s", proto);
     }
@@ -93,11 +104,18 @@ private com.google.firestore.v1beta1.Document encodeDocument(Document document)
   }
 
   /** Decodes a Document proto to the equivalent model. */
-  private Document decodeDocument(com.google.firestore.v1beta1.Document document) {
+  private Document decodeDocument(
+      com.google.firestore.v1beta1.Document document, boolean hasCommittedMutations) {
     DocumentKey key = rpcSerializer.decodeKey(document.getName());
     ObjectValue value = rpcSerializer.decodeFields(document.getFieldsMap());
     SnapshotVersion version = rpcSerializer.decodeVersion(document.getUpdateTime());
-    return new Document(key, version, value, false);
+    return new Document(
+        key,
+        version,
+        value,
+        hasCommittedMutations
+            ? Document.DocumentState.COMMITTED_MUTATIONS
+            : Document.DocumentState.SYNCED);
   }
 
   /** Encodes a NoDocument value to the equivalent proto. */
@@ -116,6 +134,24 @@ private NoDocument decodeNoDocument(com.google.firebase.firestore.proto.NoDocume
     return new NoDocument(key, version);
   }
 
+  /** Encodes a UnknownDocument value to the equivalent proto. */
+  private com.google.firebase.firestore.proto.UnknownDocument encodeUnknownDocument(
+      UnknownDocument document) {
+    com.google.firebase.firestore.proto.UnknownDocument.Builder builder =
+        com.google.firebase.firestore.proto.UnknownDocument.newBuilder();
+    builder.setName(rpcSerializer.encodeKey(document.getKey()));
+    builder.setVersion(rpcSerializer.encodeTimestamp(document.getVersion().getTimestamp()));
+    return builder.build();
+  }
+
+  /** Decodes a UnknownDocument proto to the equivalent model. */
+  private UnknownDocument decodeUnknownDocument(
+      com.google.firebase.firestore.proto.UnknownDocument proto) {
+    DocumentKey key = rpcSerializer.decodeKey(proto.getName());
+    SnapshotVersion version = rpcSerializer.decodeVersion(proto.getVersion());
+    return new UnknownDocument(key, version);
+  }
+
   /** Encodes a MutationBatch model for local storage in the mutation queue. */
   com.google.firebase.firestore.proto.WriteBatch encodeMutationBatch(MutationBatch batch) {
     com.google.firebase.firestore.proto.WriteBatch.Builder result =

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

@@ -387,7 +387,7 @@ public ImmutableSortedMap<DocumentKey, MaybeDocument> applyRemoteEvent(RemoteEve
             // resolution failing).
             if (existingDoc == null
                 || doc.getVersion().equals(SnapshotVersion.NONE)
-                || authoritativeUpdates.contains(doc.getKey())
+                || (authoritativeUpdates.contains(doc.getKey()) && !existingDoc.hasPendingWrites())
                 || doc.getVersion().compareTo(existingDoc.getVersion()) >= 0) {
               remoteDocuments.add(doc);
             } else {

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

@@ -25,6 +25,16 @@
  */
 public class Document extends MaybeDocument {
 
+  /** Describes the `hasPendingWrites` state of a document. */
+  public enum DocumentState {
+    /** Local mutations applied via the mutation queue. Document is potentially inconsistent. */
+    LOCAL_MUTATIONS,
+    /** Mutations applied based on a write acknowledgment. Document is potentially inconsistent. */
+    COMMITTED_MUTATIONS,
+    /** No mutations applied. Document was sent to us by Watch. */
+    SYNCED
+  }
+
   private static final Comparator<Document> KEY_COMPARATOR =
       new Comparator<Document>() {
         @Override
@@ -40,13 +50,13 @@ public static Comparator<Document> keyComparator() {
 
   private final ObjectValue data;
 
-  private final boolean hasLocalMutations;
+  private final DocumentState documentState;
 
   public Document(
-      DocumentKey key, SnapshotVersion version, ObjectValue data, boolean hasLocalMutations) {
+      DocumentKey key, SnapshotVersion version, ObjectValue data, DocumentState documentState) {
     super(key, version);
     this.data = data;
-    this.hasLocalMutations = hasLocalMutations;
+    this.documentState = documentState;
   }
 
   public ObjectValue getData() {
@@ -63,7 +73,16 @@ public ObjectValue getData() {
   }
 
   public boolean hasLocalMutations() {
-    return hasLocalMutations;
+    return documentState.equals(DocumentState.LOCAL_MUTATIONS);
+  }
+
+  public boolean hasCommittedMutations() {
+    return documentState.equals(DocumentState.COMMITTED_MUTATIONS);
+  }
+
+  @Override
+  public boolean hasPendingWrites() {
+    return this.hasLocalMutations() || this.hasCommittedMutations();
   }
 
   @Override
@@ -79,7 +98,7 @@ public boolean equals(Object o) {
 
     return getVersion().equals(document.getVersion())
         && getKey().equals(document.getKey())
-        && hasLocalMutations == document.hasLocalMutations
+        && documentState.equals(document.documentState)
         && data.equals(document.data);
   }
 
@@ -88,7 +107,7 @@ public int hashCode() {
     int result = getKey().hashCode();
     result = 31 * result + data.hashCode();
     result = 31 * result + getVersion().hashCode();
-    result = 31 * result + (hasLocalMutations ? 1 : 0);
+    result = 31 * result + documentState.hashCode();
     return result;
   }
 
@@ -101,8 +120,8 @@ public String toString() {
         + data
         + ", version="
         + getVersion()
-        + ", hasLocalMutations="
-        + hasLocalMutations
+        + ", documentState="
+        + documentState.name()
         + '}';
   }
 }

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

@@ -41,4 +41,9 @@ public DocumentKey getKey() {
   public SnapshotVersion getVersion() {
     return version;
   }
+
+  /**
+   * Whether this document has a local mutation applied that has not yet been acknowledged by Watch.
+   */
+  public abstract boolean hasPendingWrites();
 }

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

@@ -35,6 +35,17 @@ public boolean equals(Object o) {
     return getVersion().equals(that.getVersion()) && getKey().equals(that.getKey());
   }
 
+  @Override
+  public boolean hasPendingWrites() {
+    return hasCommittedMutations();
+  }
+
+  public boolean hasCommittedMutations() {
+    // We currently don't raise `hasPendingWrites` for deleted documents, even if Watch hasn't
+    // caught up to the deleted version yet.
+    return false;
+  }
+
   @Override
   public int hashCode() {
     int result = getKey().hashCode();

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

@@ -0,0 +1,56 @@
+// Copyright 2018 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.firebase.firestore.model;
+
+/**
+ * A class representing an existing document whose data is unknown (e.g. a document that was updated
+ * without a known base document).
+ */
+public class UnknownDocument extends MaybeDocument {
+  public UnknownDocument(DocumentKey key, SnapshotVersion version) {
+    super(key, version);
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
+
+    UnknownDocument that = (UnknownDocument) o;
+
+    return getVersion().equals(that.getVersion()) && getKey().equals(that.getKey());
+  }
+
+  @Override
+  public boolean hasPendingWrites() {
+    return true;
+  }
+
+  @Override
+  public int hashCode() {
+    int result = getKey().hashCode();
+    result = 31 * result + getVersion().hashCode();
+    return result;
+  }
+
+  @Override
+  public String toString() {
+    return "UnknownDocument{key=" + getKey() + ", version=" + getVersion() + '}';
+  }
+}

firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/DeleteMutation.java

@@ -65,7 +65,9 @@ public MaybeDocument applyToRemoteDocument(
     // Unlike applyToLocalView, if we're applying a mutation to a remote document the server has
     // accepted the mutation so the precondition must have held.
 
-    return new NoDocument(getKey(), SnapshotVersion.NONE);
+    // We store the deleted document at the commit version of the delete. Any document version
+    // that the server sends us before the delete was applied is discarded
+    return new NoDocument(getKey(), mutationResult.getVersion());
   }
 
   @Nullable