diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index 701d6752077..fb073fc6153 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -46,7 +46,12 @@ import { PersistenceTransaction } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { RemoteDocumentCache } from './remote_document_cache'; import { RemoteDocumentChangeBuffer } from './remote_document_change_buffer'; -import { SimpleDb, SimpleDbStore, SimpleDbTransaction } from './simple_db'; +import { + IterateOptions, + SimpleDb, + SimpleDbStore, + SimpleDbTransaction +} from './simple_db'; import { ObjectMap } from '../util/obj_map'; export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { @@ -257,7 +262,8 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { getDocumentsMatchingQuery( transaction: PersistenceTransaction, - query: Query + query: Query, + sinceReadTime: SnapshotVersion ): PersistencePromise { assert( !query.isCollectionGroupQuery(), @@ -267,12 +273,27 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { const immediateChildrenPathLength = query.path.length + 1; - // 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 range = IDBKeyRange.lowerBound(startKey); + 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(); + 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 readTimeKey = this.serializer.toDbTimestampKey(sinceReadTime); + iterationOptions.range = IDBKeyRange.lowerBound( + [collectionKey, readTimeKey], + /* open= */ true + ); + iterationOptions.index = DbRemoteDocument.collectionReadTimeIndex; + } + return remoteDocumentsStore(transaction) - .iterate({ range }, (key, dbRemoteDoc, control) => { + .iterate(iterationOptions, (key, dbRemoteDoc, control) => { // The query is actually returning any path that starts with the query // path prefix which may include documents in subcollections. For // example, a query on 'rooms' will return rooms/abc/messages/xyx but we @@ -440,6 +461,10 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { `Cannot modify a document that wasn't read (for ${key})` ); if (maybeDocument) { + assert( + !this.readTime.isEqual(SnapshotVersion.MIN), + 'Cannot add a document with a read time of zero' + ); const doc = this.documentCache.serializer.toDbRemoteDocument( maybeDocument, this.readTime diff --git a/packages/firestore/src/local/local_documents_view.ts b/packages/firestore/src/local/local_documents_view.ts index 5ad8365ccc9..78056dd78e3 100644 --- a/packages/firestore/src/local/local_documents_view.ts +++ b/packages/firestore/src/local/local_documents_view.ts @@ -143,17 +143,33 @@ export class LocalDocumentsView { }); } - /** Performs a query against the local view of all documents. */ + /** + * Performs a query against the local view of all documents. + * + * @param transaction The persistence transaction. + * @param query The query to match documents against. + * @param sinceReadTime If not set to SnapshotVersion.MIN, return only + * documents that have been read since this snapshot version (exclusive). + */ getDocumentsMatchingQuery( transaction: PersistenceTransaction, - query: Query + query: Query, + sinceReadTime: SnapshotVersion ): PersistencePromise { if (query.isDocumentQuery()) { return this.getDocumentsMatchingDocumentQuery(transaction, query.path); } else if (query.isCollectionGroupQuery()) { - return this.getDocumentsMatchingCollectionGroupQuery(transaction, query); + return this.getDocumentsMatchingCollectionGroupQuery( + transaction, + query, + sinceReadTime + ); } else { - return this.getDocumentsMatchingCollectionQuery(transaction, query); + return this.getDocumentsMatchingCollectionQuery( + transaction, + query, + sinceReadTime + ); } } @@ -175,7 +191,8 @@ export class LocalDocumentsView { private getDocumentsMatchingCollectionGroupQuery( transaction: PersistenceTransaction, - query: Query + query: Query, + sinceReadTime: SnapshotVersion ): PersistencePromise { assert( query.path.isEmpty(), @@ -194,7 +211,8 @@ export class LocalDocumentsView { ); return this.getDocumentsMatchingCollectionQuery( transaction, - collectionQuery + collectionQuery, + sinceReadTime ).next(r => { r.forEach((key, doc) => { results = results.insert(key, doc); @@ -206,13 +224,14 @@ export class LocalDocumentsView { private getDocumentsMatchingCollectionQuery( transaction: PersistenceTransaction, - query: Query + query: Query, + sinceReadTime: SnapshotVersion ): PersistencePromise { // Query the remote documents and overlay mutations. let results: DocumentMap; let mutationBatches: MutationBatch[]; return this.remoteDocumentCache - .getDocumentsMatchingQuery(transaction, query) + .getDocumentsMatchingQuery(transaction, query, sinceReadTime) .next(queryResults => { results = queryResults; return this.mutationQueue.getAllMutationBatchesAffectingQuery( diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index 7c3ee1d6832..4d09f9b8ecc 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -819,7 +819,11 @@ export class LocalStore { */ executeQuery(query: Query): Promise { return this.persistence.runTransaction('Execute query', 'readonly', txn => { - return this.localDocuments.getDocumentsMatchingQuery(txn, query); + return this.localDocuments.getDocumentsMatchingQuery( + txn, + query, + SnapshotVersion.MIN + ); }); } diff --git a/packages/firestore/src/local/memory_remote_document_cache.ts b/packages/firestore/src/local/memory_remote_document_cache.ts index bc3ebfcbe03..ad4ede63083 100644 --- a/packages/firestore/src/local/memory_remote_document_cache.ts +++ b/packages/firestore/src/local/memory_remote_document_cache.ts @@ -83,6 +83,11 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache { doc: MaybeDocument, readTime: SnapshotVersion ): PersistencePromise { + assert( + !readTime.isEqual(SnapshotVersion.MIN), + 'Cannot add a document with a read time of zero' + ); + const key = doc.key; const entry = this.docs.get(key); const previousSize = entry ? entry.size : 0; @@ -140,7 +145,8 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache { getDocumentsMatchingQuery( transaction: PersistenceTransaction, - query: Query + query: Query, + sinceReadTime: SnapshotVersion ): PersistencePromise { assert( !query.isCollectionGroupQuery(), @@ -155,11 +161,14 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache { while (iterator.hasNext()) { const { key, - value: { maybeDocument } + value: { maybeDocument, readTime } } = iterator.getNext(); if (!query.path.isPrefixOf(key.path)) { break; } + if (readTime.compareTo(sinceReadTime) <= 0) { + continue; + } if (maybeDocument instanceof Document && query.matches(maybeDocument)) { results = results.insert(maybeDocument.key, maybeDocument); } diff --git a/packages/firestore/src/local/remote_document_cache.ts b/packages/firestore/src/local/remote_document_cache.ts index 831cd248f38..3a5e2fdbb6f 100644 --- a/packages/firestore/src/local/remote_document_cache.ts +++ b/packages/firestore/src/local/remote_document_cache.ts @@ -28,6 +28,7 @@ import { DocumentKey } from '../model/document_key'; import { PersistenceTransaction } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { RemoteDocumentChangeBuffer } from './remote_document_change_buffer'; +import { SnapshotVersion } from '../core/snapshot_version'; /** * Represents cached documents received from the remote backend. @@ -71,11 +72,14 @@ export interface RemoteDocumentCache { * Cached NoDocument entries have no bearing on query results. * * @param query The query to match documents against. + * @param sinceReadTime If not set to SnapshotVersion.MIN, return only + * documents that have been read since this snapshot version (exclusive). * @return The set of matching documents. */ getDocumentsMatchingQuery( transaction: PersistenceTransaction, - query: Query + query: Query, + sinceReadTime: SnapshotVersion ): PersistencePromise; /** diff --git a/packages/firestore/test/unit/local/local_store.test.ts b/packages/firestore/test/unit/local/local_store.test.ts index 57cafe9719e..813a307a965 100644 --- a/packages/firestore/test/unit/local/local_store.test.ts +++ b/packages/firestore/test/unit/local/local_store.test.ts @@ -759,7 +759,7 @@ function genericLocalStoreTests( expectLocalStore() .afterAllocatingQuery(query) .toReturnTargetId(2) - .after(docAddedRemoteEvent(doc('foo/bar', 0, { foo: 'old' }), [2])) + .after(docAddedRemoteEvent(doc('foo/bar', 1, { foo: 'old' }), [2])) .after(patchMutation('foo/bar', { foo: 'bar' })) // Release the query so that our target count goes back to 0 and we are considered // up-to-date. @@ -767,7 +767,7 @@ function genericLocalStoreTests( .after(setMutation('foo/bah', { foo: 'bah' })) .after(deleteMutation('foo/baz')) .toContain( - doc('foo/bar', 0, { foo: 'bar' }, { hasLocalMutations: true }) + doc('foo/bar', 1, { foo: 'bar' }, { hasLocalMutations: true }) ) .toContain( doc('foo/bah', 0, { foo: 'bah' }, { hasLocalMutations: true }) @@ -800,7 +800,7 @@ function genericLocalStoreTests( expectLocalStore() .afterAllocatingQuery(query) .toReturnTargetId(2) - .after(docAddedRemoteEvent(doc('foo/bar', 0, { foo: 'old' }), [2])) + .after(docAddedRemoteEvent(doc('foo/bar', 1, { foo: 'old' }), [2])) .after(patchMutation('foo/bar', { foo: 'bar' })) // Release the query so that our target count goes back to 0 and we are considered // up-to-date. @@ -808,7 +808,7 @@ function genericLocalStoreTests( .after(setMutation('foo/bah', { foo: 'bah' })) .after(deleteMutation('foo/baz')) .toContain( - doc('foo/bar', 0, { foo: 'bar' }, { hasLocalMutations: true }) + doc('foo/bar', 1, { foo: 'bar' }, { hasLocalMutations: true }) ) .toContain( doc('foo/bah', 0, { foo: 'bah' }, { hasLocalMutations: true }) diff --git a/packages/firestore/test/unit/local/remote_document_cache.test.ts b/packages/firestore/test/unit/local/remote_document_cache.test.ts index 7f034a59dce..8b24e508eed 100644 --- a/packages/firestore/test/unit/local/remote_document_cache.test.ts +++ b/packages/firestore/test/unit/local/remote_document_cache.test.ts @@ -17,6 +17,7 @@ import { expect } from 'chai'; import { Query } from '../../../src/core/query'; +import { SnapshotVersion } from '../../../src/core/snapshot_version'; import { IndexedDbPersistence } from '../../../src/local/indexeddb_persistence'; import { MaybeDocument } from '../../../src/model/document'; import { @@ -312,7 +313,10 @@ function genericRemoteDocumentCacheTests( ); const query = new Query(path('b')); - const matchingDocs = await cache.getDocumentsMatchingQuery(query); + const matchingDocs = await cache.getDocumentsMatchingQuery( + query, + SnapshotVersion.MIN + ); assertMatches( [doc('b/1', VERSION, DOC_DATA), doc('b/2', VERSION, DOC_DATA)], @@ -320,6 +324,46 @@ function genericRemoteDocumentCacheTests( ); }); + it('can get documents matching query by read time', async () => { + await cache.addEntries( + [doc('b/old', 1, DOC_DATA)], + /* readTime= */ version(11) + ); + await cache.addEntries( + [doc('b/current', 2, DOC_DATA)], + /* readTime= */ version(12) + ); + await cache.addEntries( + [doc('b/new', 3, DOC_DATA)], + /* readTime= */ version(13) + ); + + const query = new Query(path('b')); + const matchingDocs = await cache.getDocumentsMatchingQuery( + query, + /* sinceReadTime= */ version(12) + ); + assertMatches([doc('b/new', 3, DOC_DATA)], matchingDocs); + }); + + it('query matching uses read time rather than update time', async () => { + await cache.addEntries( + [doc('b/old', 1, DOC_DATA)], + /* readTime= */ version(2) + ); + await cache.addEntries( + [doc('b/new', 2, DOC_DATA)], + /* readTime= */ version(1) + ); + + const query = new Query(path('b')); + const matchingDocs = await cache.getDocumentsMatchingQuery( + query, + /* sinceReadTime= */ version(1) + ); + assertMatches([doc('b/old', 1, DOC_DATA)], matchingDocs); + }); + it('can get changes', async () => { await cache.addEntries( [ diff --git a/packages/firestore/test/unit/local/test_remote_document_cache.ts b/packages/firestore/test/unit/local/test_remote_document_cache.ts index 53b0c5ac8e6..24b879e14fc 100644 --- a/packages/firestore/test/unit/local/test_remote_document_cache.ts +++ b/packages/firestore/test/unit/local/test_remote_document_cache.ts @@ -107,12 +107,15 @@ export class TestRemoteDocumentCache { }); } - getDocumentsMatchingQuery(query: Query): Promise { + getDocumentsMatchingQuery( + query: Query, + sinceReadTime: SnapshotVersion + ): Promise { return this.persistence.runTransaction( 'getDocumentsMatchingQuery', 'readonly', txn => { - return this.cache.getDocumentsMatchingQuery(txn, query); + return this.cache.getDocumentsMatchingQuery(txn, query, sinceReadTime); } ); }