Skip to content

Commit 1e6792f

Browse files
schmidt-sebastianschmidt-sebastian
committed
Address comments
1 parent 1820b54 commit 1e6792f

File tree

5 files changed

+27
-20
lines changed

5 files changed

+27
-20
lines changed

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -135,18 +135,18 @@ ImmutableSortedMap<DocumentKey, MaybeDocument> getLocalViewOfDocuments(
135135
* Performs a query against the local view of all documents.
136136
*
137137
* @param query The query to match documents against.
138-
* @param sinceUpdateTime If not set to SnapshotVersion.MIN, return only documents that have been
139-
* modified since this snapshot version (exclusive).
138+
* @param sinceReadTime If not set to SnapshotVersion.MIN, return only documents that have been
139+
* read since this snapshot version (exclusive).
140140
*/
141141
ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
142-
Query query, SnapshotVersion sinceUpdateTime) {
142+
Query query, SnapshotVersion sinceReadTime) {
143143
ResourcePath path = query.getPath();
144144
if (query.isDocumentQuery()) {
145145
return getDocumentsMatchingDocumentQuery(path);
146146
} else if (query.isCollectionGroupQuery()) {
147-
return getDocumentsMatchingCollectionGroupQuery(query, sinceUpdateTime);
147+
return getDocumentsMatchingCollectionGroupQuery(query, sinceReadTime);
148148
} else {
149-
return getDocumentsMatchingCollectionQuery(query, sinceUpdateTime);
149+
return getDocumentsMatchingCollectionQuery(query, sinceReadTime);
150150
}
151151
}
152152

@@ -163,7 +163,7 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingDocumentQu
163163
}
164164

165165
private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollectionGroupQuery(
166-
Query query, SnapshotVersion sinceUpdateTime) {
166+
Query query, SnapshotVersion sinceReadTime) {
167167
hardAssert(
168168
query.getPath().isEmpty(),
169169
"Currently we only support collection group queries at the root.");
@@ -176,7 +176,7 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollection
176176
for (ResourcePath parent : parents) {
177177
Query collectionQuery = query.asCollectionQueryAtPath(parent.append(collectionId));
178178
ImmutableSortedMap<DocumentKey, Document> collectionResults =
179-
getDocumentsMatchingCollectionQuery(collectionQuery, sinceUpdateTime);
179+
getDocumentsMatchingCollectionQuery(collectionQuery, sinceReadTime);
180180
for (Map.Entry<DocumentKey, Document> docEntry : collectionResults) {
181181
results = results.insert(docEntry.getKey(), docEntry.getValue());
182182
}
@@ -186,9 +186,9 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollection
186186

187187
/** Queries the remote documents and overlays mutations. */
188188
private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollectionQuery(
189-
Query query, SnapshotVersion sinceUpdateTime) {
189+
Query query, SnapshotVersion sinceReadTime) {
190190
ImmutableSortedMap<DocumentKey, Document> results =
191-
remoteDocumentCache.getAllDocumentsMatchingQuery(query, sinceUpdateTime);
191+
remoteDocumentCache.getAllDocumentsMatchingQuery(query, sinceReadTime);
192192

193193
List<MutationBatch> matchingBatches = mutationQueue.getAllMutationBatchesAffectingQuery(query);
194194

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ final class MemoryRemoteDocumentCache implements RemoteDocumentCache {
4848

4949
@Override
5050
public void add(MaybeDocument document, SnapshotVersion readTime) {
51+
hardAssert(
52+
!readTime.equals(SnapshotVersion.NONE),
53+
"Cannot add document to the RemoteDocumentCache with a read time of zero");
5154
docs = docs.insert(document.getKey(), new Pair<>(document, readTime));
5255

5356
persistence.getIndexManager().addToCollectionParentIndex(document.getKey().getPath().popLast());
@@ -83,7 +86,7 @@ public Map<DocumentKey, MaybeDocument> getAll(Iterable<DocumentKey> keys) {
8386

8487
@Override
8588
public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(
86-
Query query, SnapshotVersion sinceUpdateTime) {
89+
Query query, SnapshotVersion sinceReadTime) {
8790
hardAssert(
8891
!query.isCollectionGroupQuery(),
8992
"CollectionGroup queries should be handled in LocalDocumentsView");
@@ -114,7 +117,7 @@ public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(
114117
}
115118

116119
SnapshotVersion readTime = entry.getValue().second;
117-
if (readTime.compareTo(sinceUpdateTime) <= 0) {
120+
if (readTime.compareTo(sinceReadTime) <= 0) {
118121
continue;
119122
}
120123

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,10 @@ interface RemoteDocumentCache {
7575
* <p>Cached NoDocument entries have no bearing on query results.
7676
*
7777
* @param query The query to match documents against.
78-
* @param sinceUpdateTime If not set to SnapshotVersion.MIN, return only documents that have been
79-
* modified since this snapshot version (exclusive).
78+
* @param sinceReadTime If not set to SnapshotVersion.MIN, return only documents that have been
79+
* read since this snapshot version (exclusive).
8080
* @return The set of matching documents.
8181
*/
8282
ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(
83-
Query query, SnapshotVersion sinceUpdateTime);
83+
Query query, SnapshotVersion sinceReadTime);
8484
}

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ final class SQLiteRemoteDocumentCache implements RemoteDocumentCache {
5252

5353
@Override
5454
public void add(MaybeDocument maybeDocument, SnapshotVersion readTime) {
55+
hardAssert(
56+
!readTime.equals(SnapshotVersion.NONE),
57+
"Cannot add document to the RemoteDocumentCache with a read time of zero");
58+
5559
String path = pathForKey(maybeDocument.getKey());
5660
Timestamp timestamp = readTime.getTimestamp();
5761
MessageLite message = serializer.encodeMaybeDocument(maybeDocument);
@@ -132,7 +136,7 @@ public Map<DocumentKey, MaybeDocument> getAll(Iterable<DocumentKey> documentKeys
132136

133137
@Override
134138
public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(
135-
Query query, SnapshotVersion sinceUpdateTime) {
139+
Query query, SnapshotVersion sinceReadTime) {
136140
hardAssert(
137141
!query.isCollectionGroupQuery(),
138142
"CollectionGroup queries should be handled in LocalDocumentsView");
@@ -143,7 +147,7 @@ public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(
143147

144148
String prefixPath = EncodedPath.encode(prefix);
145149
String prefixSuccessorPath = EncodedPath.prefixSuccessor(prefixPath);
146-
Timestamp updateTime = sinceUpdateTime.getTimestamp();
150+
Timestamp updateTime = sinceReadTime.getTimestamp();
147151

148152
BackgroundQueue backgroundQueue = new BackgroundQueue();
149153

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -727,12 +727,12 @@ public void testCollectsGarbageAfterAcknowledgedMutation() {
727727
Query query = Query.atPath(ResourcePath.fromString("foo"));
728728
int targetId = allocateQuery(query);
729729
applyRemoteEvent(
730-
updateRemoteEvent(doc("foo/bar", 0, map("foo", "old")), asList(targetId), emptyList()));
730+
updateRemoteEvent(doc("foo/bar", 1, map("foo", "old")), asList(targetId), emptyList()));
731731
writeMutation(patchMutation("foo/bar", map("foo", "bar")));
732732
releaseQuery(query);
733733
writeMutation(setMutation("foo/bah", map("foo", "bah")));
734734
writeMutation(deleteMutation("foo/baz"));
735-
assertContains(doc("foo/bar", 0, map("foo", "bar"), Document.DocumentState.LOCAL_MUTATIONS));
735+
assertContains(doc("foo/bar", 1, map("foo", "bar"), Document.DocumentState.LOCAL_MUTATIONS));
736736
assertContains(doc("foo/bah", 0, map("foo", "bah"), Document.DocumentState.LOCAL_MUTATIONS));
737737
assertContains(deletedDoc("foo/baz", 0));
738738

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

0 commit comments

Comments
 (0)