Address comments

Author: schmidt-sebastian

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

@@ -135,18 +135,18 @@ ImmutableSortedMap<DocumentKey, MaybeDocument> getLocalViewOfDocuments(
    * Performs a query against the local view of all documents.
    *
    * @param query The query to match documents against.
-   * @param sinceUpdateTime If not set to SnapshotVersion.MIN, return only documents that have been
-   *     modified since this snapshot version (exclusive).
+   * @param sinceReadTime If not set to SnapshotVersion.MIN, return only documents that have been
+   *     read since this snapshot version (exclusive).
    */
   ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
-      Query query, SnapshotVersion sinceUpdateTime) {
+      Query query, SnapshotVersion sinceReadTime) {
     ResourcePath path = query.getPath();
     if (query.isDocumentQuery()) {
       return getDocumentsMatchingDocumentQuery(path);
     } else if (query.isCollectionGroupQuery()) {
-      return getDocumentsMatchingCollectionGroupQuery(query, sinceUpdateTime);
+      return getDocumentsMatchingCollectionGroupQuery(query, sinceReadTime);
     } else {
-      return getDocumentsMatchingCollectionQuery(query, sinceUpdateTime);
+      return getDocumentsMatchingCollectionQuery(query, sinceReadTime);
     }
   }
 
@@ -163,7 +163,7 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingDocumentQu
   }
 
   private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollectionGroupQuery(
-      Query query, SnapshotVersion sinceUpdateTime) {
+      Query query, SnapshotVersion sinceReadTime) {
     hardAssert(
         query.getPath().isEmpty(),
         "Currently we only support collection group queries at the root.");
@@ -176,7 +176,7 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollection
     for (ResourcePath parent : parents) {
       Query collectionQuery = query.asCollectionQueryAtPath(parent.append(collectionId));
       ImmutableSortedMap<DocumentKey, Document> collectionResults =
-          getDocumentsMatchingCollectionQuery(collectionQuery, sinceUpdateTime);
+          getDocumentsMatchingCollectionQuery(collectionQuery, sinceReadTime);
       for (Map.Entry<DocumentKey, Document> docEntry : collectionResults) {
         results = results.insert(docEntry.getKey(), docEntry.getValue());
       }
@@ -186,9 +186,9 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollection
 
   /** Queries the remote documents and overlays mutations. */
   private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollectionQuery(
-      Query query, SnapshotVersion sinceUpdateTime) {
+      Query query, SnapshotVersion sinceReadTime) {
     ImmutableSortedMap<DocumentKey, Document> results =
-        remoteDocumentCache.getAllDocumentsMatchingQuery(query, sinceUpdateTime);
+        remoteDocumentCache.getAllDocumentsMatchingQuery(query, sinceReadTime);
 
     List<MutationBatch> matchingBatches = mutationQueue.getAllMutationBatchesAffectingQuery(query);
 

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

@@ -28,6 +28,7 @@
 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;
 import com.google.firebase.firestore.model.SnapshotVersion;
 import com.google.firebase.firestore.model.mutation.Mutation;
 import com.google.firebase.firestore.model.mutation.MutationBatch;

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

@@ -48,6 +48,7 @@ final class MemoryRemoteDocumentCache implements RemoteDocumentCache {
 
   @Override
   public void add(MaybeDocument document, SnapshotVersion readTime) {
+    hardAssert(!readTime.equals(SnapshotVersion.NONE), "Cannot add document to the RemoteDocumentCache with a read time of zero");
     docs = docs.insert(document.getKey(), new Pair<>(document, readTime));
 
     persistence.getIndexManager().addToCollectionParentIndex(document.getKey().getPath().popLast());
@@ -83,7 +84,7 @@ public Map<DocumentKey, MaybeDocument> getAll(Iterable<DocumentKey> keys) {
 
   @Override
   public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(
-      Query query, SnapshotVersion sinceUpdateTime) {
+      Query query, SnapshotVersion sinceReadTime) {
     hardAssert(
         !query.isCollectionGroupQuery(),
         "CollectionGroup queries should be handled in LocalDocumentsView");
@@ -114,7 +115,7 @@ public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(
       }
 
       SnapshotVersion readTime = entry.getValue().second;
-      if (readTime.compareTo(sinceUpdateTime) <= 0) {
+      if (readTime.compareTo(sinceReadTime) <= 0) {
         continue;
       }
 

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

@@ -75,10 +75,10 @@ interface RemoteDocumentCache {
    * <p>Cached NoDocument entries have no bearing on query results.
    *
    * @param query The query to match documents against.
-   * @param sinceUpdateTime If not set to SnapshotVersion.MIN, return only documents that have been
-   *     modified since this snapshot version (exclusive).
+   * @param sinceReadTime If not set to SnapshotVersion.MIN, return only documents that have been
+   *     read since this snapshot version (exclusive).
    * @return The set of matching documents.
    */
   ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(
-      Query query, SnapshotVersion sinceUpdateTime);
+      Query query, SnapshotVersion sinceReadTime);
 }

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

@@ -52,6 +52,8 @@ final class SQLiteRemoteDocumentCache implements RemoteDocumentCache {
 
   @Override
   public void add(MaybeDocument maybeDocument, SnapshotVersion readTime) {
+    hardAssert(!readTime.equals(SnapshotVersion.NONE), "Cannot add document to the RemoteDocumentCache with a read time of zero");
+
     String path = pathForKey(maybeDocument.getKey());
     Timestamp timestamp = readTime.getTimestamp();
     MessageLite message = serializer.encodeMaybeDocument(maybeDocument);
@@ -132,7 +134,7 @@ public Map<DocumentKey, MaybeDocument> getAll(Iterable<DocumentKey> documentKeys
 
   @Override
   public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(
-      Query query, SnapshotVersion sinceUpdateTime) {
+      Query query, SnapshotVersion sinceReadTime) {
     hardAssert(
         !query.isCollectionGroupQuery(),
         "CollectionGroup queries should be handled in LocalDocumentsView");
@@ -143,7 +145,7 @@ public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(
 
     String prefixPath = EncodedPath.encode(prefix);
     String prefixSuccessorPath = EncodedPath.prefixSuccessor(prefixPath);
-    Timestamp updateTime = sinceUpdateTime.getTimestamp();
+    Timestamp updateTime = sinceReadTime.getTimestamp();
 
     BackgroundQueue backgroundQueue = new BackgroundQueue();
 

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

@@ -727,12 +727,12 @@ public void testCollectsGarbageAfterAcknowledgedMutation() {
     Query query = Query.atPath(ResourcePath.fromString("foo"));
     int targetId = allocateQuery(query);
     applyRemoteEvent(
-        updateRemoteEvent(doc("foo/bar", 0, map("foo", "old")), asList(targetId), emptyList()));
+        updateRemoteEvent(doc("foo/bar", 1, map("foo", "old")), asList(targetId), emptyList()));
     writeMutation(patchMutation("foo/bar", map("foo", "bar")));
     releaseQuery(query);
     writeMutation(setMutation("foo/bah", map("foo", "bah")));
     writeMutation(deleteMutation("foo/baz"));
-    assertContains(doc("foo/bar", 0, map("foo", "bar"), Document.DocumentState.LOCAL_MUTATIONS));
+    assertContains(doc("foo/bar", 1, map("foo", "bar"), Document.DocumentState.LOCAL_MUTATIONS));
     assertContains(doc("foo/bah", 0, map("foo", "bah"), Document.DocumentState.LOCAL_MUTATIONS));
     assertContains(deletedDoc("foo/baz", 0));
 
@@ -761,13 +761,13 @@ public void testCollectsGarbageAfterRejectedMutation() {
     Query query = Query.atPath(ResourcePath.fromString("foo"));
     int targetId = allocateQuery(query);
     applyRemoteEvent(
-        updateRemoteEvent(doc("foo/bar", 0, map("foo", "old")), asList(targetId), emptyList()));
+        updateRemoteEvent(doc("foo/bar", 1, map("foo", "old")), asList(targetId), emptyList()));
     writeMutation(patchMutation("foo/bar", map("foo", "bar")));
     // Release the query so that our target count goes back to 0 and we are considered up-to-date.
     releaseQuery(query);
     writeMutation(setMutation("foo/bah", map("foo", "bah")));
     writeMutation(deleteMutation("foo/baz"));
-    assertContains(doc("foo/bar", 0, map("foo", "bar"), Document.DocumentState.LOCAL_MUTATIONS));
+    assertContains(doc("foo/bar", 1, map("foo", "bar"), Document.DocumentState.LOCAL_MUTATIONS));
     assertContains(doc("foo/bah", 0, map("foo", "bah"), Document.DocumentState.LOCAL_MUTATIONS));
     assertContains(deletedDoc("foo/baz", 0));
 

firebase-firestore/src/test/resources/json/existence_filter_spec_test.json

@@ -1517,7 +1517,7 @@
   "Existence filter limbo resolution is denied": {
     "describeName": "Existence Filters:",
     "itName": "Existence filter limbo resolution is denied",
-    "tags": [],
+    "tags": ["exclusive"],
     "config": {
       "useGarbageCollection": true,
       "numClients": 1