Skip to content

Commit 71f2411

Browse files
committed
Fix missing results for latency-compensatoed update/patch
1 parent ca8d829 commit 71f2411

File tree

2 files changed

+59
-2
lines changed

2 files changed

+59
-2
lines changed

packages/firestore/src/local/local_documents_view.ts

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ import {
2424
MaybeDocumentMap,
2525
maybeDocumentMap,
2626
NullableMaybeDocumentMap,
27-
nullableMaybeDocumentMap
27+
nullableMaybeDocumentMap,
28+
documentKeySet,
2829
} from '../model/collections';
2930
import { Document, MaybeDocument, NoDocument } from '../model/document';
3031
import { DocumentKey } from '../model/document_key';
@@ -37,6 +38,7 @@ import { MutationQueue } from './mutation_queue';
3738
import { PersistenceTransaction } from './persistence';
3839
import { PersistencePromise } from './persistence_promise';
3940
import { RemoteDocumentCache } from './remote_document_cache';
41+
import { PatchMutation } from '../model/mutation';
4042

4143
/**
4244
* A readonly view of the local state of all documents we're tracking (i.e. we
@@ -49,7 +51,7 @@ export class LocalDocumentsView {
4951
private remoteDocumentCache: RemoteDocumentCache,
5052
private mutationQueue: MutationQueue,
5153
private indexManager: IndexManager
52-
) {}
54+
) { }
5355

5456
/**
5557
* Get the local view of the document identified by `key`.
@@ -208,6 +210,7 @@ export class LocalDocumentsView {
208210
): PersistencePromise<DocumentMap> {
209211
// Query the remote documents and overlay mutations.
210212
let results: DocumentMap;
213+
let mutationBatches: MutationBatch[];
211214
return this.remoteDocumentCache
212215
.getDocumentsMatchingQuery(transaction, query)
213216
.next(queryResults => {
@@ -218,7 +221,35 @@ export class LocalDocumentsView {
218221
);
219222
})
220223
.next(matchingMutationBatches => {
224+
mutationBatches = matchingMutationBatches;
225+
// It is possible that a PatchMutation can make a document a match to the query,
226+
// but the version in `remoteDocumentCache` is not a match yet (waiting for server
227+
// to ack). To handle this, we find all document keys affected by some`PatchMutation`s
228+
// that are not in `result` yet, and back fill them via `remoteDocumentCache.getEntries`,
229+
// otherwise those `PatchMutations` will be ignored because no base document can be found,
230+
// and lead to missing result for the query.
231+
let missingBaseDocEntriesForPatching = documentKeySet();
221232
for (const batch of matchingMutationBatches) {
233+
for (const mutation of batch.mutations) {
234+
const key = mutation.key;
235+
// Only process documents belonging to the collection.
236+
if (!query.path.isImmediateParentOf(key.path)) {
237+
continue;
238+
}
239+
if (mutation instanceof PatchMutation && results.get(mutation.key) === null) {
240+
missingBaseDocEntriesForPatching = missingBaseDocEntriesForPatching.add(mutation.key);
241+
}
242+
}
243+
}
244+
return this.remoteDocumentCache.getEntries(transaction, missingBaseDocEntriesForPatching);
245+
})
246+
.next(baseDocsForPatching => {
247+
baseDocsForPatching.forEach((key, doc) => {
248+
if (doc !== null && doc instanceof Document) {
249+
results = results.insert(key, doc);
250+
}
251+
});
252+
for (const batch of mutationBatches) {
222253
for (const mutation of batch.mutations) {
223254
const key = mutation.key;
224255
// Only process documents belonging to the collection.

packages/firestore/test/unit/specs/query_spec.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,30 @@ describeSpec('Queries:', [], () => {
9898
hasPendingWrites: true
9999
});
100100
});
101+
102+
specTest(
103+
'Ensure correct query results for latency-compensated updates',
104+
[],
105+
() => {
106+
const fullQuery = Query.atPath(ResourcePath.fromString('collection'));
107+
const filteredQuery = fullQuery.addFilter(filter("match", "==", true));
108+
const docA = doc('collection/a', 1000, { match: false });
109+
const docAv2Local = doc('collection/a', 1000, { match: true }, { hasLocalMutations: true });
110+
111+
return (
112+
spec()
113+
.userListens(fullQuery)
114+
.watchAcksFull(fullQuery, 1000, docA)
115+
.expectEvents(fullQuery, { added: [docA] })
116+
117+
// patch docA so that it will now match the filtered query.
118+
.userPatches("collection/a", {match: true })
119+
.expectEvents(fullQuery, { modified: [docAv2Local], hasPendingWrites: true })
120+
// Make sure docA shows up in filtered query.
121+
.userListens(filteredQuery)
122+
.expectEvents(filteredQuery, { added: [docAv2Local], fromCache: true,
123+
hasPendingWrites: true })
124+
);
125+
}
126+
);
101127
});

0 commit comments

Comments
 (0)