Skip to content

Commit 264d78a

Browse files
milaGGLmilaGGL
authored
Merge 0bd5820 into d939e63 
2 parents d939e63 + 0bd5820 commit 264d78a

File tree

13 files changed

+4410
-2600
lines changed

13 files changed

+4410
-2600
lines changed

‎firebase-firestore/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
# Unreleased
2+
- [fixed] Fixed Firestore failing to raise initial snapshot from empty local cache result.
23

34
# 24.4.0
45
* [unchanged] Updated to accommodate the release of the updated

‎firebase-firestore/ktx/src/test/java/com/google/firebase/firestore/TestUtil.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ public static QuerySnapshot querySnapshot(
105105
isFromCache,
106106
mutatedKeys,
107107
/* didSyncStateChange= */ true,
108-
/* excludesMetadataChanges= */ false);
108+
/* excludesMetadataChanges= */ false,
109+
/* hasCachedResults= */ false);
109110
return new QuerySnapshot(query(path), viewSnapshot, FIRESTORE);
110111
}
111112
}

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,50 @@ public void testQueriesFireFromCacheWhenOffline() {
550550
listener.remove();
551551
}
552552

553+
@Test
554+
public void testQueriesCanRaiseInitialSnapshotFromCachedEmptyResults() {
555+
CollectionReference collectionReference = testCollection();
556+
557+
// Populate the cache with empty query result.
558+
QuerySnapshot querySnapshotA = waitFor(collectionReference.get());
559+
assertFalse(querySnapshotA.getMetadata().isFromCache());
560+
assertEquals(asList(), querySnapshotToValues(querySnapshotA));
561+
562+
// Add a snapshot listener whose first event should be raised from cache.
563+
EventAccumulator<QuerySnapshot> accum = new EventAccumulator<>();
564+
ListenerRegistration listenerRegistration =
565+
collectionReference.addSnapshotListener(accum.listener());
566+
QuerySnapshot querySnapshotB = accum.await();
567+
assertTrue(querySnapshotB.getMetadata().isFromCache());
568+
assertEquals(asList(), querySnapshotToValues(querySnapshotB));
569+
570+
listenerRegistration.remove();
571+
}
572+
573+
@Test
574+
public void testQueriesCanRaiseInitialSnapshotFromEmptyDueToDeleteCachedResults() {
575+
Map<String, Map<String, Object>> testDocs = map("a", map("foo", 1L));
576+
CollectionReference collectionReference = testCollectionWithDocs(testDocs);
577+
// Populate the cache with single document.
578+
QuerySnapshot querySnapshotA = waitFor(collectionReference.get());
579+
assertFalse(querySnapshotA.getMetadata().isFromCache());
580+
assertEquals(asList(testDocs.get("a")), querySnapshotToValues(querySnapshotA));
581+
582+
// delete the document, make cached result empty.
583+
DocumentReference docRef = collectionReference.document("a");
584+
waitFor(docRef.delete());
585+
586+
// Add a snapshot listener whose first event should be raised from cache.
587+
EventAccumulator<QuerySnapshot> accum = new EventAccumulator<>();
588+
ListenerRegistration listenerRegistration =
589+
collectionReference.addSnapshotListener(accum.listener());
590+
QuerySnapshot querySnapshotB = accum.await();
591+
assertTrue(querySnapshotB.getMetadata().isFromCache());
592+
assertEquals(asList(), querySnapshotToValues(querySnapshotB));
593+
594+
listenerRegistration.remove();
595+
}
596+
553597
@Test
554598
public void testQueriesCanUseNotEqualFilters() {
555599
// These documents are ordered by value in "zip" since the notEquals filter is an inequality,

‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/QueryListener.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ public boolean onViewSnapshot(ViewSnapshot newSnapshot) {
8787
newSnapshot.isFromCache(),
8888
newSnapshot.getMutatedKeys(),
8989
newSnapshot.didSyncStateChange(),
90-
/* excludesMetadataChanges= */ true);
90+
/* excludesMetadataChanges= */ true,
91+
newSnapshot.hasCachedResults());
9192
}
9293

9394
if (!raisedInitialEvent) {
@@ -139,8 +140,11 @@ private boolean shouldRaiseInitialEvent(ViewSnapshot snapshot, OnlineState onlin
139140
return false;
140141
}
141142

142-
// Raise data from cache if we have any documents or we are offline
143-
return !snapshot.getDocuments().isEmpty() || onlineState.equals(OnlineState.OFFLINE);
143+
// Raise data from cache if we have any documents, have cached results before,
144+
// or we are offline.
145+
return (!snapshot.getDocuments().isEmpty()
146+
|| snapshot.hasCachedResults()
147+
|| onlineState.equals(OnlineState.OFFLINE));
144148
}
145149

146150
private boolean shouldRaiseEvent(ViewSnapshot snapshot) {
@@ -171,7 +175,8 @@ private void raiseInitialEvent(ViewSnapshot snapshot) {
171175
snapshot.getDocuments(),
172176
snapshot.getMutatedKeys(),
173177
snapshot.isFromCache(),
174-
snapshot.excludesMetadataChanges());
178+
snapshot.excludesMetadataChanges(),
179+
snapshot.hasCachedResults());
175180
raisedInitialEvent = true;
176181
listener.onEvent(snapshot, null);
177182
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/SyncEngine.java

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import com.google.firebase.firestore.util.Function;
5555
import com.google.firebase.firestore.util.Logger;
5656
import com.google.firebase.firestore.util.Util;
57+
import com.google.protobuf.ByteString;
5758
import io.grpc.Status;
5859
import java.io.IOException;
5960
import java.util.ArrayList;
@@ -204,13 +205,16 @@ public int listen(Query query) {
204205
TargetData targetData = localStore.allocateTarget(query.toTarget());
205206
remoteStore.listen(targetData);
206207

207-
ViewSnapshot viewSnapshot = initializeViewAndComputeSnapshot(query, targetData.getTargetId());
208+
ViewSnapshot viewSnapshot =
209+
initializeViewAndComputeSnapshot(
210+
query, targetData.getTargetId(), targetData.getResumeToken());
208211
syncEngineListener.onViewSnapshots(Collections.singletonList(viewSnapshot));
209212

210213
return targetData.getTargetId();
211214
}
212215

213-
private ViewSnapshot initializeViewAndComputeSnapshot(Query query, int targetId) {
216+
private ViewSnapshot initializeViewAndComputeSnapshot(
217+
Query query, int targetId, ByteString resumeToken) {
214218
QueryResult queryResult = localStore.executeQuery(query, /* usePreviousResults= */ true);
215219

216220
SyncState currentTargetSyncState = SyncState.NONE;
@@ -221,10 +225,10 @@ private ViewSnapshot initializeViewAndComputeSnapshot(Query query, int targetId)
221225
if (this.queriesByTarget.get(targetId) != null) {
222226
Query mirrorQuery = this.queriesByTarget.get(targetId).get(0);
223227
currentTargetSyncState = this.queryViewsByQuery.get(mirrorQuery).getView().getSyncState();
224-
synthesizedCurrentChange =
225-
TargetChange.createSynthesizedTargetChangeForCurrentChange(
226-
currentTargetSyncState == SyncState.SYNCED);
227228
}
229+
synthesizedCurrentChange =
230+
TargetChange.createSynthesizedTargetChangeForCurrentChange(
231+
currentTargetSyncState == SyncState.SYNCED, resumeToken);
228232

229233
// TODO(wuandy): Investigate if we can extract the logic of view change computation and
230234
// update tracked limbo in one place, and have both emitNewSnapsAndNotifyLocalStore

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,8 +300,11 @@ public ViewChange applyChanges(DocumentChanges docChanges, TargetChange targetCh
300300
boolean syncStatedChanged = newSyncState != syncState;
301301
syncState = newSyncState;
302302
ViewSnapshot snapshot = null;
303+
303304
if (viewChanges.size() != 0 || syncStatedChanged) {
304305
boolean fromCache = newSyncState == SyncState.LOCAL;
306+
boolean hasCachedResults =
307+
targetChange == null ? false : !targetChange.getResumeToken().isEmpty();
305308
snapshot =
306309
new ViewSnapshot(
307310
query,
@@ -311,7 +314,8 @@ public ViewChange applyChanges(DocumentChanges docChanges, TargetChange targetCh
311314
fromCache,
312315
docChanges.mutatedKeys,
313316
syncStatedChanged,
314-
/* excludesMetadataChanges= */ false);
317+
/* excludesMetadataChanges= */ false,
318+
hasCachedResults);
315319
}
316320
return new ViewChange(snapshot, limboDocumentChanges);
317321
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/core/ViewSnapshot.java

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public enum SyncState {
3939
private final ImmutableSortedSet<DocumentKey> mutatedKeys;
4040
private final boolean didSyncStateChange;
4141
private boolean excludesMetadataChanges;
42+
private boolean hasCachedResults;
4243

4344
public ViewSnapshot(
4445
Query query,
@@ -48,7 +49,8 @@ public ViewSnapshot(
4849
boolean isFromCache,
4950
ImmutableSortedSet<DocumentKey> mutatedKeys,
5051
boolean didSyncStateChange,
51-
boolean excludesMetadataChanges) {
52+
boolean excludesMetadataChanges,
53+
boolean hasCachedResults) {
5254
this.query = query;
5355
this.documents = documents;
5456
this.oldDocuments = oldDocuments;
@@ -57,6 +59,7 @@ public ViewSnapshot(
5759
this.mutatedKeys = mutatedKeys;
5860
this.didSyncStateChange = didSyncStateChange;
5961
this.excludesMetadataChanges = excludesMetadataChanges;
62+
this.hasCachedResults = hasCachedResults;
6063
}
6164

6265
/** Returns a view snapshot as if all documents in the snapshot were added. */
@@ -65,7 +68,8 @@ public static ViewSnapshot fromInitialDocuments(
6568
DocumentSet documents,
6669
ImmutableSortedSet<DocumentKey> mutatedKeys,
6770
boolean fromCache,
68-
boolean excludesMetadataChanges) {
71+
boolean excludesMetadataChanges,
72+
boolean hasCachedResults) {
6973
List<DocumentViewChange> viewChanges = new ArrayList<>();
7074
for (Document doc : documents) {
7175
viewChanges.add(DocumentViewChange.create(DocumentViewChange.Type.ADDED, doc));
@@ -78,7 +82,8 @@ public static ViewSnapshot fromInitialDocuments(
7882
fromCache,
7983
mutatedKeys,
8084
/* didSyncStateChange= */ true,
81-
excludesMetadataChanges);
85+
excludesMetadataChanges,
86+
hasCachedResults);
8287
}
8388

8489
public Query getQuery() {
@@ -117,6 +122,10 @@ public boolean excludesMetadataChanges() {
117122
return excludesMetadataChanges;
118123
}
119124

125+
public boolean hasCachedResults() {
126+
return hasCachedResults;
127+
}
128+
120129
@Override
121130
public final boolean equals(Object o) {
122131
if (this == o) {
@@ -149,6 +158,9 @@ public final boolean equals(Object o) {
149158
if (!oldDocuments.equals(that.oldDocuments)) {
150159
return false;
151160
}
161+
if (hasCachedResults != that.hasCachedResults) {
162+
return false;
163+
}
152164
return changes.equals(that.changes);
153165
}
154166

@@ -162,6 +174,7 @@ public int hashCode() {
162174
result = 31 * result + (isFromCache ? 1 : 0);
163175
result = 31 * result + (didSyncStateChange ? 1 : 0);
164176
result = 31 * result + (excludesMetadataChanges ? 1 : 0);
177+
result = 31 * result + (hasCachedResults ? 1 : 0);
165178
return result;
166179
}
167180

@@ -183,6 +196,8 @@ public String toString() {
183196
+ didSyncStateChange
184197
+ ", excludesMetadataChanges="
185198
+ excludesMetadataChanges
199+
+ ", hasCachedResults="
200+
+ hasCachedResults
186201
+ ")";
187202
}
188203
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/remote/TargetChange.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,10 @@ public final class TargetChange {
3131
private final ImmutableSortedSet<DocumentKey> modifiedDocuments;
3232
private final ImmutableSortedSet<DocumentKey> removedDocuments;
3333

34-
public static TargetChange createSynthesizedTargetChangeForCurrentChange(boolean isCurrent) {
34+
public static TargetChange createSynthesizedTargetChangeForCurrentChange(
35+
boolean isCurrent, ByteString resumeToken) {
3536
return new TargetChange(
36-
ByteString.EMPTY,
37+
resumeToken,
3738
isCurrent,
3839
DocumentKey.emptyKeySet(),
3940
DocumentKey.emptyKeySet(),

‎firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/TestUtil.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ public static QuerySnapshot querySnapshot(
8383
Map<String, ObjectValue> oldDocs,
8484
Map<String, ObjectValue> docsToAdd,
8585
boolean hasPendingWrites,
86-
boolean isFromCache) {
86+
boolean isFromCache,
87+
boolean hasCachedResults) {
8788
DocumentSet oldDocuments = docSet(Document.KEY_COMPARATOR);
8889
ImmutableSortedSet<DocumentKey> mutatedKeys = DocumentKey.emptyKeySet();
8990
for (Map.Entry<String, ObjectValue> pair : oldDocs.entrySet()) {
@@ -116,7 +117,8 @@ public static QuerySnapshot querySnapshot(
116117
isFromCache,
117118
mutatedKeys,
118119
/* didSyncStateChange= */ true,
119-
/* excludesMetadataChanges= */ false);
120+
/* excludesMetadataChanges= */ false,
121+
hasCachedResults);
120122
return new QuerySnapshot(query(path), viewSnapshot, FIRESTORE);
121123
}
122124

‎firebase-firestore/src/test/java/com/google/firebase/firestore/QuerySnapshotTest.java

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,21 +56,26 @@ public void testEquals() {
5656
ObjectValue firstValue = wrapObject("a", 1);
5757
ObjectValue secondValue = wrapObject("b", 1);
5858

59-
QuerySnapshot foo = TestUtil.querySnapshot("foo", map(), map("a", firstValue), true, false);
60-
QuerySnapshot fooDup = TestUtil.querySnapshot("foo", map(), map("a", firstValue), true, false);
59+
QuerySnapshot foo =
60+
TestUtil.querySnapshot("foo", map(), map("a", firstValue), true, false, false);
61+
QuerySnapshot fooDup =
62+
TestUtil.querySnapshot("foo", map(), map("a", firstValue), true, false, false);
6163
QuerySnapshot differentPath =
62-
TestUtil.querySnapshot("bar", map(), map("a", firstValue), true, false);
64+
TestUtil.querySnapshot("bar", map(), map("a", firstValue), true, false, false);
6365
QuerySnapshot differentDoc =
64-
TestUtil.querySnapshot("foo", map(), map("a", secondValue), true, false);
66+
TestUtil.querySnapshot("foo", map(), map("a", secondValue), true, false, false);
6567
QuerySnapshot noPendingWrites =
66-
TestUtil.querySnapshot("foo", map(), map("a", firstValue), false, false);
68+
TestUtil.querySnapshot("foo", map(), map("a", firstValue), false, false, false);
6769
QuerySnapshot fromCache =
68-
TestUtil.querySnapshot("foo", map(), map("a", firstValue), true, true);
70+
TestUtil.querySnapshot("foo", map(), map("a", firstValue), true, true, false);
71+
QuerySnapshot hasCachedResults =
72+
TestUtil.querySnapshot("foo", map(), map("a", firstValue), true, false, true);
6973
assertEquals(foo, fooDup);
7074
assertNotEquals(foo, differentPath);
7175
assertNotEquals(foo, differentDoc);
7276
assertNotEquals(foo, noPendingWrites);
7377
assertNotEquals(foo, fromCache);
78+
assertNotEquals(foo, hasCachedResults);
7479

7580
// Note: `foo` and `differentDoc` have the same hash code since we no longer take document
7681
// contents into account.
@@ -88,7 +93,8 @@ public void testToObjects() {
8893

8994
ObjectValue objectData =
9095
ObjectValue.fromMap(map("timestamp", ServerTimestamps.valueOf(Timestamp.now(), null)));
91-
QuerySnapshot foo = TestUtil.querySnapshot("foo", map(), map("a", objectData), true, false);
96+
QuerySnapshot foo =
97+
TestUtil.querySnapshot("foo", map(), map("a", objectData), true, false, false);
9298

9399
List<POJO> docs = foo.toObjects(POJO.class);
94100
assertEquals(1, docs.size());
@@ -126,7 +132,8 @@ public void testIncludeMetadataChanges() {
126132
/*isFromCache=*/ false,
127133
/*mutatedKeys=*/ keySet(),
128134
/*didSyncStateChange=*/ true,
129-
/* excludesMetadataChanges= */ false);
135+
/* excludesMetadataChanges= */ false,
136+
/* hasCachedResults= */ false);
130137

131138
QuerySnapshot snapshot =
132139
new QuerySnapshot(new Query(fooQuery, firestore), viewSnapshot, firestore);

0 commit comments

Comments
 (0)