Skip to content

Commit d50472a

Browse files
author
Hui-Wu
authored
Fix latency-compensated query missing patch mutations. (#621)
* Fix latency-compensated query missing patch mutations. * Move comments to impl method * Fixing nits * fix another naming
1 parent a673b99 commit d50472a

File tree

5 files changed

+1237
-0
lines changed

5 files changed

+1237
-0
lines changed

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import com.google.firebase.firestore.model.SnapshotVersion;
2929
import com.google.firebase.firestore.model.mutation.Mutation;
3030
import com.google.firebase.firestore.model.mutation.MutationBatch;
31+
import com.google.firebase.firestore.model.mutation.PatchMutation;
32+
import java.util.HashSet;
3133
import java.util.List;
3234
import java.util.Map;
3335
import javax.annotation.Nullable;
@@ -182,6 +184,9 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollection
182184
remoteDocumentCache.getAllDocumentsMatchingQuery(query);
183185

184186
List<MutationBatch> matchingBatches = mutationQueue.getAllMutationBatchesAffectingQuery(query);
187+
188+
results = addMissingBaseDocuments(matchingBatches, results);
189+
185190
for (MutationBatch batch : matchingBatches) {
186191
for (Mutation mutation : batch.getMutations()) {
187192
// Only process documents belonging to the collection.
@@ -210,4 +215,34 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollection
210215

211216
return results;
212217
}
218+
219+
/**
220+
* It is possible that a {@code PatchMutation} can make a document match a query, even if the
221+
* version in the {@code RemoteDocumentCache} is not a match yet (waiting for server to ack). To
222+
* handle this, we find all document keys affected by the {@code PatchMutation}s that are not in
223+
* {@code existingDocs} yet, and back fill them via {@code remoteDocumentCache.getAll}, otherwise
224+
* those {@code PatchMutation}s will be ignored because no base document can be found, and lead to
225+
* missing results for the query.
226+
*/
227+
private ImmutableSortedMap<DocumentKey, Document> addMissingBaseDocuments(
228+
List<MutationBatch> matchingBatches, ImmutableSortedMap<DocumentKey, Document> existingDocs) {
229+
HashSet<DocumentKey> missingDocKeys = new HashSet<>();
230+
for (MutationBatch batch : matchingBatches) {
231+
for (Mutation mutation : batch.getMutations()) {
232+
if (mutation instanceof PatchMutation && !existingDocs.containsKey(mutation.getKey())) {
233+
missingDocKeys.add(mutation.getKey());
234+
}
235+
}
236+
}
237+
238+
ImmutableSortedMap<DocumentKey, Document> mergedDocs = existingDocs;
239+
Map<DocumentKey, MaybeDocument> missingDocs = remoteDocumentCache.getAll(missingDocKeys);
240+
for (Map.Entry<DocumentKey, MaybeDocument> entry : missingDocs.entrySet()) {
241+
if (entry.getValue() != null && (entry.getValue() instanceof Document)) {
242+
mergedDocs = mergedDocs.insert(entry.getKey(), (Document) entry.getValue());
243+
}
244+
}
245+
246+
return mergedDocs;
247+
}
213248
}

0 commit comments

Comments
 (0)