Skip to content

Commit 3fe2c4a

Browse files
schmidt-sebastianschmidt-sebastian
authored
Use RemoteDocumentCache.getAll() (#5986)
1 parent 434d87e commit 3fe2c4a

8 files changed

+76
-153
lines changed

‎packages/firestore/src/local/indexeddb_remote_document_cache.ts

+8-13
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
* limitations under the License.
1616
*/
1717

18-
import { isCollectionGroupQuery, Query, queryMatches } from '../core/query';
1918
import { SnapshotVersion } from '../core/snapshot_version';
2019
import {
2120
DocumentKeySet,
@@ -245,30 +244,26 @@ class IndexedDbRemoteDocumentCacheImpl implements IndexedDbRemoteDocumentCache {
245244
});
246245
}
247246

248-
getDocumentsMatchingQuery(
247+
getAll(
249248
transaction: PersistenceTransaction,
250-
query: Query,
249+
collection: ResourcePath,
251250
sinceReadTime: SnapshotVersion
252251
): PersistencePromise<MutableDocumentMap> {
253-
debugAssert(
254-
!isCollectionGroupQuery(query),
255-
'CollectionGroup queries should be handled in LocalDocumentsView'
256-
);
257252
let results = mutableDocumentMap();
258253

259-
const immediateChildrenPathLength = query.path.length + 1;
254+
const immediateChildrenPathLength = collection.length + 1;
260255

261256
const iterationOptions: IterateOptions = {};
262257
if (sinceReadTime.isEqual(SnapshotVersion.min())) {
263258
// Documents are ordered by key, so we can use a prefix scan to narrow
264259
// down the documents we need to match the query against.
265-
const startKey = query.path.toArray();
260+
const startKey = collection.toArray();
266261
iterationOptions.range = IDBKeyRange.lowerBound(startKey);
267262
} else {
268263
// Execute an index-free query and filter by read time. This is safe
269264
// since all document changes to queries that have a
270265
// lastLimboFreeSnapshotVersion (`sinceReadTime`) have a read time set.
271-
const collectionKey = query.path.toArray();
266+
const collectionKey = collection.toArray();
272267
const readTimeKey = toDbTimestampKey(sinceReadTime);
273268
iterationOptions.range = IDBKeyRange.lowerBound(
274269
[collectionKey, readTimeKey],
@@ -292,10 +287,10 @@ class IndexedDbRemoteDocumentCacheImpl implements IndexedDbRemoteDocumentCache {
292287
DocumentKey.fromSegments(key),
293288
dbRemoteDoc
294289
);
295-
if (!query.path.isPrefixOf(document.key.path)) {
296-
control.done();
297-
} else if (queryMatches(query, document)) {
290+
if (collection.isPrefixOf(document.key.path)) {
298291
results = results.insert(document.key, document);
292+
} else {
293+
control.done();
299294
}
300295
})
301296
.next(() => results);

‎packages/firestore/src/local/local_documents_view.ts

+16-68
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,13 @@ import {
2525
import { SnapshotVersion } from '../core/snapshot_version';
2626
import {
2727
DocumentKeySet,
28-
documentKeySet,
2928
DocumentMap,
3029
documentMap,
3130
MutableDocumentMap
3231
} from '../model/collections';
3332
import { Document, MutableDocument } from '../model/document';
3433
import { DocumentKey } from '../model/document_key';
35-
import { mutationApplyToLocalView, PatchMutation } from '../model/mutation';
34+
import { mutationApplyToLocalView } from '../model/mutation';
3635
import { MutationBatch } from '../model/mutation_batch';
3736
import { ResourcePath } from '../model/path';
3837
import { debugAssert } from '../util/assert';
@@ -217,51 +216,31 @@ export class LocalDocumentsView {
217216
): PersistencePromise<DocumentMap> {
218217
// Query the remote documents and overlay mutations.
219218
let results: MutableDocumentMap;
220-
let mutationBatches: MutationBatch[];
221219
return this.remoteDocumentCache
222-
.getDocumentsMatchingQuery(transaction, query, sinceReadTime)
220+
.getAll(transaction, query.path, sinceReadTime)
223221
.next(queryResults => {
224222
results = queryResults;
225223
return this.mutationQueue.getAllMutationBatchesAffectingQuery(
226224
transaction,
227225
query
228226
);
229227
})
230-
.next(matchingMutationBatches => {
231-
mutationBatches = matchingMutationBatches;
232-
// It is possible that a PatchMutation can make a document match a query, even if
233-
// the version in the RemoteDocumentCache is not a match yet (waiting for server
234-
// to ack). To handle this, we find all document keys affected by the PatchMutations
235-
// that are not in `result` yet, and back fill them via `remoteDocumentCache.getEntries`,
236-
// otherwise those `PatchMutations` will be ignored because no base document can be found,
237-
// and lead to missing result for the query.
238-
return this.addMissingBaseDocuments(
239-
transaction,
240-
mutationBatches,
241-
results
242-
).next(mergedDocuments => {
243-
results = mergedDocuments;
244-
245-
for (const batch of mutationBatches) {
246-
for (const mutation of batch.mutations) {
247-
const key = mutation.key;
248-
let document = results.get(key);
249-
if (document == null) {
250-
// Create invalid document to apply mutations on top of
251-
document = MutableDocument.newInvalidDocument(key);
252-
results = results.insert(key, document);
253-
}
254-
mutationApplyToLocalView(
255-
mutation,
256-
document,
257-
batch.localWriteTime
258-
);
259-
if (!document.isFoundDocument()) {
260-
results = results.remove(key);
261-
}
228+
.next(mutationBatches => {
229+
for (const batch of mutationBatches) {
230+
for (const mutation of batch.mutations) {
231+
const key = mutation.key;
232+
let document = results.get(key);
233+
if (document == null) {
234+
// Create invalid document to apply mutations on top of
235+
document = MutableDocument.newInvalidDocument(key);
236+
results = results.insert(key, document);
237+
}
238+
mutationApplyToLocalView(mutation, document, batch.localWriteTime);
239+
if (!document.isFoundDocument()) {
240+
results = results.remove(key);
262241
}
263242
}
264-
});
243+
}
265244
})
266245
.next(() => {
267246
// Finally, filter out any documents that don't actually match
@@ -275,35 +254,4 @@ export class LocalDocumentsView {
275254
return results as DocumentMap;
276255
});
277256
}
278-
279-
private addMissingBaseDocuments(
280-
transaction: PersistenceTransaction,
281-
matchingMutationBatches: MutationBatch[],
282-
existingDocuments: MutableDocumentMap
283-
): PersistencePromise<MutableDocumentMap> {
284-
let missingBaseDocEntriesForPatching = documentKeySet();
285-
for (const batch of matchingMutationBatches) {
286-
for (const mutation of batch.mutations) {
287-
if (
288-
mutation instanceof PatchMutation &&
289-
existingDocuments.get(mutation.key) === null
290-
) {
291-
missingBaseDocEntriesForPatching =
292-
missingBaseDocEntriesForPatching.add(mutation.key);
293-
}
294-
}
295-
}
296-
297-
let mergedDocuments = existingDocuments;
298-
return this.remoteDocumentCache
299-
.getEntries(transaction, missingBaseDocEntriesForPatching)
300-
.next(missingBaseDocs => {
301-
missingBaseDocs.forEach((key, doc) => {
302-
if (doc.isFoundDocument()) {
303-
mergedDocuments = mergedDocuments.insert(key, doc);
304-
}
305-
});
306-
return mergedDocuments;
307-
});
308-
}
309257
}

‎packages/firestore/src/local/memory_remote_document_cache.ts

+8-11
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
* limitations under the License.
1616
*/
1717

18-
import { isCollectionGroupQuery, Query, queryMatches } from '../core/query';
1918
import { SnapshotVersion } from '../core/snapshot_version';
2019
import {
2120
DocumentKeySet,
@@ -24,6 +23,7 @@ import {
2423
} from '../model/collections';
2524
import { Document, MutableDocument } from '../model/document';
2625
import { DocumentKey } from '../model/document_key';
26+
import { ResourcePath } from '../model/path';
2727
import { debugAssert } from '../util/assert';
2828
import { SortedMap } from '../util/sorted_map';
2929

@@ -154,33 +154,30 @@ class MemoryRemoteDocumentCacheImpl implements MemoryRemoteDocumentCache {
154154
return PersistencePromise.resolve(results);
155155
}
156156

157-
getDocumentsMatchingQuery(
157+
getAll(
158158
transaction: PersistenceTransaction,
159-
query: Query,
159+
collectionPath: ResourcePath,
160160
sinceReadTime: SnapshotVersion
161161
): PersistencePromise<MutableDocumentMap> {
162-
debugAssert(
163-
!isCollectionGroupQuery(query),
164-
'CollectionGroup queries should be handled in LocalDocumentsView'
165-
);
166162
let results = mutableDocumentMap();
167163

168164
// Documents are ordered by key, so we can use a prefix scan to narrow down
169165
// the documents we need to match the query against.
170-
const prefix = new DocumentKey(query.path.child(''));
166+
const prefix = new DocumentKey(collectionPath.child(''));
171167
const iterator = this.docs.getIteratorFrom(prefix);
172168
while (iterator.hasNext()) {
173169
const {
174170
key,
175171
value: { document }
176172
} = iterator.getNext();
177-
if (!query.path.isPrefixOf(key.path)) {
173+
if (!collectionPath.isPrefixOf(key.path)) {
178174
break;
179175
}
180-
if (document.readTime.compareTo(sinceReadTime) <= 0) {
176+
if (key.path.length > collectionPath.length + 1) {
177+
// Exclude entries from subcollections.
181178
continue;
182179
}
183-
if (!queryMatches(query, document)) {
180+
if (document.readTime.compareTo(sinceReadTime) <= 0) {
184181
continue;
185182
}
186183
results = results.insert(document.key, document.mutableCopy());

‎packages/firestore/src/local/remote_document_cache.ts

+5-10
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@
1515
* limitations under the License.
1616
*/
1717

18-
import { Query } from '../core/query';
1918
import { SnapshotVersion } from '../core/snapshot_version';
2019
import { DocumentKeySet, MutableDocumentMap } from '../model/collections';
2120
import { MutableDocument } from '../model/document';
2221
import { DocumentKey } from '../model/document_key';
22+
import { ResourcePath } from '../model/path';
2323

2424
import { IndexManager } from './index_manager';
2525
import { PersistencePromise } from './persistence_promise';
@@ -62,21 +62,16 @@ export interface RemoteDocumentCache {
6262
): PersistencePromise<MutableDocumentMap>;
6363

6464
/**
65-
* Executes a query against the cached Document entries.
65+
* Returns the documents from the provided collection.
6666
*
67-
* Implementations may return extra documents if convenient. The results
68-
* should be re-filtered by the consumer before presenting them to the user.
69-
*
70-
* Cached NoDocument entries have no bearing on query results.
71-
*
72-
* @param query - The query to match documents against.
67+
* @param collection - The collection to read.
7368
* @param sinceReadTime - If not set to SnapshotVersion.min(), return only
7469
* documents that have been read since this snapshot version (exclusive).
7570
* @returns The set of matching documents.
7671
*/
77-
getDocumentsMatchingQuery(
72+
getAll(
7873
transaction: PersistenceTransaction,
79-
query: Query,
74+
collection: ResourcePath,
8075
sinceReadTime: SnapshotVersion
8176
): PersistencePromise<MutableDocumentMap>;
8277

‎packages/firestore/test/unit/local/counting_query_engine.ts

+9-9
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export class CountingQueryEngine extends QueryEngine {
3636
* `getAllMutationBatchesAffectingQuery()` API (since the last call to
3737
* `resetCounts()`)
3838
*/
39-
mutationsReadByQuery = 0;
39+
mutationsReadByCollection = 0;
4040

4141
/**
4242
* The number of mutations returned by the MutationQueue's
@@ -48,9 +48,9 @@ export class CountingQueryEngine extends QueryEngine {
4848

4949
/**
5050
* The number of documents returned by the RemoteDocumentCache's
51-
* `getDocumentsMatchingQuery()` API (since the last call to `resetCounts()`)
51+
* `getAll()` API (since the last call to `resetCounts()`)
5252
*/
53-
documentsReadByQuery = 0;
53+
documentsReadByCollection = 0;
5454

5555
/**
5656
* The number of documents returned by the RemoteDocumentCache's `getEntry()`
@@ -59,9 +59,9 @@ export class CountingQueryEngine extends QueryEngine {
5959
documentsReadByKey = 0;
6060

6161
resetCounts(): void {
62-
this.mutationsReadByQuery = 0;
62+
this.mutationsReadByCollection = 0;
6363
this.mutationsReadByKey = 0;
64-
this.documentsReadByQuery = 0;
64+
this.documentsReadByCollection = 0;
6565
this.documentsReadByKey = 0;
6666
}
6767

@@ -96,11 +96,11 @@ export class CountingQueryEngine extends QueryEngine {
9696
setIndexManager: (indexManager: IndexManager) => {
9797
subject.setIndexManager(indexManager);
9898
},
99-
getDocumentsMatchingQuery: (transaction, query, sinceReadTime) => {
99+
getAll: (transaction, collectionGroup, sinceReadTime) => {
100100
return subject
101-
.getDocumentsMatchingQuery(transaction, query, sinceReadTime)
101+
.getAll(transaction, collectionGroup, sinceReadTime)
102102
.next(result => {
103-
this.documentsReadByQuery += result.size;
103+
this.documentsReadByCollection += result.size;
104104
return result;
105105
});
106106
},
@@ -154,7 +154,7 @@ export class CountingQueryEngine extends QueryEngine {
154154
return subject
155155
.getAllMutationBatchesAffectingQuery(transaction, query)
156156
.next(result => {
157-
this.mutationsReadByQuery += result.length;
157+
this.mutationsReadByCollection += result.length;
158158
return result;
159159
});
160160
},

0 commit comments

Comments
 (0)