Skip to content

Use RemoteDocumentCache.getAll() #5986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 10, 2022
Merged

Use RemoteDocumentCache.getAll() #5986

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c29af64
Use RemoteDocumentCache.getAll()
schmidt-sebastian Feb 9, 2022
ffb8f6f
Merge
schmidt-sebastian Feb 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 8 additions & 13 deletions packages/firestore/src/local/indexeddb_remote_document_cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* limitations under the License.
*/

import { isCollectionGroupQuery, Query, queryMatches } from '../core/query';
import { SnapshotVersion } from '../core/snapshot_version';
import {
DocumentKeySet,
Expand Down Expand Up @@ -246,30 +245,26 @@ class IndexedDbRemoteDocumentCacheImpl implements IndexedDbRemoteDocumentCache {
});
}

getDocumentsMatchingQuery(
getAll(
transaction: PersistenceTransaction,
query: Query,
collection: ResourcePath,
sinceReadTime: SnapshotVersion
): PersistencePromise<MutableDocumentMap> {
debugAssert(
!isCollectionGroupQuery(query),
'CollectionGroup queries should be handled in LocalDocumentsView'
);
let results = mutableDocumentMap();

const immediateChildrenPathLength = query.path.length + 1;
const immediateChildrenPathLength = collection.length + 1;

const iterationOptions: IterateOptions = {};
if (sinceReadTime.isEqual(SnapshotVersion.min())) {
// Documents are ordered by key, so we can use a prefix scan to narrow
// down the documents we need to match the query against.
const startKey = query.path.toArray();
const startKey = collection.toArray();
iterationOptions.range = IDBKeyRange.lowerBound(startKey);
} else {
// Execute an index-free query and filter by read time. This is safe
// since all document changes to queries that have a
// lastLimboFreeSnapshotVersion (`sinceReadTime`) have a read time set.
const collectionKey = query.path.toArray();
const collectionKey = collection.toArray();
const readTimeKey = toDbTimestampKey(sinceReadTime);
iterationOptions.range = IDBKeyRange.lowerBound(
[collectionKey, readTimeKey],
Expand All @@ -293,10 +288,10 @@ class IndexedDbRemoteDocumentCacheImpl implements IndexedDbRemoteDocumentCache {
DocumentKey.fromSegments(key),
dbRemoteDoc
);
if (!query.path.isPrefixOf(document.key.path)) {
control.done();
} else if (queryMatches(query, document)) {
if (collection.isPrefixOf(document.key.path)) {
results = results.insert(document.key, document);
} else {
control.done();
}
})
.next(() => results);
Expand Down
84 changes: 16 additions & 68 deletions packages/firestore/src/local/local_documents_view.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,13 @@ import {
import { SnapshotVersion } from '../core/snapshot_version';
import {
DocumentKeySet,
documentKeySet,
DocumentMap,
documentMap,
MutableDocumentMap
} from '../model/collections';
import { Document, MutableDocument } from '../model/document';
import { DocumentKey } from '../model/document_key';
import { mutationApplyToLocalView, PatchMutation } from '../model/mutation';
import { mutationApplyToLocalView } from '../model/mutation';
import { MutationBatch } from '../model/mutation_batch';
import { ResourcePath } from '../model/path';
import { debugAssert } from '../util/assert';
Expand Down Expand Up @@ -217,51 +216,31 @@ export class LocalDocumentsView {
): PersistencePromise<DocumentMap> {
// Query the remote documents and overlay mutations.
let results: MutableDocumentMap;
let mutationBatches: MutationBatch[];
return this.remoteDocumentCache
.getDocumentsMatchingQuery(transaction, query, sinceReadTime)
.getAll(transaction, query.path, sinceReadTime)
.next(queryResults => {
results = queryResults;
return this.mutationQueue.getAllMutationBatchesAffectingQuery(
transaction,
query
);
})
.next(matchingMutationBatches => {
mutationBatches = matchingMutationBatches;
// It is possible that a PatchMutation can make a document match a query, even if
// the version in the RemoteDocumentCache is not a match yet (waiting for server
// to ack). To handle this, we find all document keys affected by the PatchMutations
// that are not in `result` yet, and back fill them via `remoteDocumentCache.getEntries`,
// otherwise those `PatchMutations` will be ignored because no base document can be found,
// and lead to missing result for the query.
return this.addMissingBaseDocuments(
transaction,
mutationBatches,
results
).next(mergedDocuments => {
results = mergedDocuments;

for (const batch of mutationBatches) {
for (const mutation of batch.mutations) {
const key = mutation.key;
let document = results.get(key);
if (document == null) {
// Create invalid document to apply mutations on top of
document = MutableDocument.newInvalidDocument(key);
results = results.insert(key, document);
}
mutationApplyToLocalView(
mutation,
document,
batch.localWriteTime
);
if (!document.isFoundDocument()) {
results = results.remove(key);
}
.next(mutationBatches => {
for (const batch of mutationBatches) {
for (const mutation of batch.mutations) {
const key = mutation.key;
let document = results.get(key);
if (document == null) {
// Create invalid document to apply mutations on top of
document = MutableDocument.newInvalidDocument(key);
results = results.insert(key, document);
}
mutationApplyToLocalView(mutation, document, batch.localWriteTime);
if (!document.isFoundDocument()) {
results = results.remove(key);
}
}
});
}
})
.next(() => {
// Finally, filter out any documents that don't actually match
Expand All @@ -275,35 +254,4 @@ export class LocalDocumentsView {
return results as DocumentMap;
});
}

private addMissingBaseDocuments(
transaction: PersistenceTransaction,
matchingMutationBatches: MutationBatch[],
existingDocuments: MutableDocumentMap
): PersistencePromise<MutableDocumentMap> {
let missingBaseDocEntriesForPatching = documentKeySet();
for (const batch of matchingMutationBatches) {
for (const mutation of batch.mutations) {
if (
mutation instanceof PatchMutation &&
existingDocuments.get(mutation.key) === null
) {
missingBaseDocEntriesForPatching =
missingBaseDocEntriesForPatching.add(mutation.key);
}
}
}

let mergedDocuments = existingDocuments;
return this.remoteDocumentCache
.getEntries(transaction, missingBaseDocEntriesForPatching)
.next(missingBaseDocs => {
missingBaseDocs.forEach((key, doc) => {
if (doc.isFoundDocument()) {
mergedDocuments = mergedDocuments.insert(key, doc);
}
});
return mergedDocuments;
});
}
}
19 changes: 8 additions & 11 deletions packages/firestore/src/local/memory_remote_document_cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* limitations under the License.
*/

import { isCollectionGroupQuery, Query, queryMatches } from '../core/query';
import { SnapshotVersion } from '../core/snapshot_version';
import {
DocumentKeySet,
Expand All @@ -24,6 +23,7 @@ import {
} from '../model/collections';
import { Document, MutableDocument } from '../model/document';
import { DocumentKey } from '../model/document_key';
import { ResourcePath } from '../model/path';
import { debugAssert } from '../util/assert';
import { SortedMap } from '../util/sorted_map';

Expand Down Expand Up @@ -152,33 +152,30 @@ class MemoryRemoteDocumentCacheImpl implements MemoryRemoteDocumentCache {
return PersistencePromise.resolve(results);
}

getDocumentsMatchingQuery(
getAll(
transaction: PersistenceTransaction,
query: Query,
collectionPath: ResourcePath,
sinceReadTime: SnapshotVersion
): PersistencePromise<MutableDocumentMap> {
debugAssert(
!isCollectionGroupQuery(query),
'CollectionGroup queries should be handled in LocalDocumentsView'
);
let results = mutableDocumentMap();

// Documents are ordered by key, so we can use a prefix scan to narrow down
// the documents we need to match the query against.
const prefix = new DocumentKey(query.path.child(''));
const prefix = new DocumentKey(collectionPath.child(''));
const iterator = this.docs.getIteratorFrom(prefix);
while (iterator.hasNext()) {
const {
key,
value: { document }
} = iterator.getNext();
if (!query.path.isPrefixOf(key.path)) {
if (!collectionPath.isPrefixOf(key.path)) {
break;
}
if (document.readTime.compareTo(sinceReadTime) <= 0) {
if (key.path.length > collectionPath.length + 1) {
// Exclude entries from subcollections.
continue;
}
if (!queryMatches(query, document)) {
if (document.readTime.compareTo(sinceReadTime) <= 0) {
continue;
}
results = results.insert(document.key, document.mutableCopy());
Expand Down
15 changes: 5 additions & 10 deletions packages/firestore/src/local/remote_document_cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
* limitations under the License.
*/

import { Query } from '../core/query';
import { SnapshotVersion } from '../core/snapshot_version';
import { DocumentKeySet, MutableDocumentMap } from '../model/collections';
import { MutableDocument } from '../model/document';
import { DocumentKey } from '../model/document_key';
import { ResourcePath } from '../model/path';

import { PersistencePromise } from './persistence_promise';
import { PersistenceTransaction } from './persistence_transaction';
Expand Down Expand Up @@ -58,21 +58,16 @@ export interface RemoteDocumentCache {
): PersistencePromise<MutableDocumentMap>;

/**
* Executes a query against the cached Document entries.
* Returns the documents from the provided collection.
*
* Implementations may return extra documents if convenient. The results
* should be re-filtered by the consumer before presenting them to the user.
*
* Cached NoDocument entries have no bearing on query results.
*
* @param query - The query to match documents against.
* @param collection - The collection to read.
* @param sinceReadTime - If not set to SnapshotVersion.min(), return only
* documents that have been read since this snapshot version (exclusive).
* @returns The set of matching documents.
*/
getDocumentsMatchingQuery(
getAll(
transaction: PersistenceTransaction,
query: Query,
collection: ResourcePath,
sinceReadTime: SnapshotVersion
): PersistencePromise<MutableDocumentMap>;

Expand Down
18 changes: 9 additions & 9 deletions packages/firestore/test/unit/local/counting_query_engine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export class CountingQueryEngine extends QueryEngine {
* `getAllMutationBatchesAffectingQuery()` API (since the last call to
* `resetCounts()`)
*/
mutationsReadByQuery = 0;
mutationsReadByCollection = 0;

/**
* The number of mutations returned by the MutationQueue's
Expand All @@ -47,9 +47,9 @@ export class CountingQueryEngine extends QueryEngine {

/**
* The number of documents returned by the RemoteDocumentCache's
* `getDocumentsMatchingQuery()` API (since the last call to `resetCounts()`)
* `getAll()` API (since the last call to `resetCounts()`)
*/
documentsReadByQuery = 0;
documentsReadByCollection = 0;

/**
* The number of documents returned by the RemoteDocumentCache's `getEntry()`
Expand All @@ -58,9 +58,9 @@ export class CountingQueryEngine extends QueryEngine {
documentsReadByKey = 0;

resetCounts(): void {
this.mutationsReadByQuery = 0;
this.mutationsReadByCollection = 0;
this.mutationsReadByKey = 0;
this.documentsReadByQuery = 0;
this.documentsReadByCollection = 0;
this.documentsReadByKey = 0;
}

Expand Down Expand Up @@ -92,11 +92,11 @@ export class CountingQueryEngine extends QueryEngine {
subject: RemoteDocumentCache
): RemoteDocumentCache {
return {
getDocumentsMatchingQuery: (transaction, query, sinceReadTime) => {
getAll: (transaction, collectionGroup, sinceReadTime) => {
return subject
.getDocumentsMatchingQuery(transaction, query, sinceReadTime)
.getAll(transaction, collectionGroup, sinceReadTime)
.next(result => {
this.documentsReadByQuery += result.size;
this.documentsReadByCollection += result.size;
return result;
});
},
Expand Down Expand Up @@ -150,7 +150,7 @@ export class CountingQueryEngine extends QueryEngine {
return subject
.getAllMutationBatchesAffectingQuery(transaction, query)
.next(result => {
this.mutationsReadByQuery += result.length;
this.mutationsReadByCollection += result.length;
return result;
});
},
Expand Down
Loading