From 3da020e7090d02122de22d641a0c23b71787b0ba Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Thu, 13 Dec 2018 16:54:47 -0800 Subject: [PATCH 01/29] 1 --- .../local/indexeddb_remote_document_cache.ts | 31 ++++++++++++++++++- packages/firestore/src/local/local_store.ts | 10 +++--- .../src/local/memory_remote_document_cache.ts | 15 +++++++++ .../src/local/remote_document_cache.ts | 18 ++++++++++- 4 files changed, 67 insertions(+), 7 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index e70996c7d6d..03563439c7c 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -16,6 +16,7 @@ import { Query } from '../core/query'; import { + DocumentKeySet, documentKeySet, DocumentMap, documentMap, @@ -178,6 +179,34 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { }); } + getEntries( + transaction : PersistenceTransaction, + documentKeys : DocumentKeySet, + ): PersistencePromise { + let results = maybeDocumentMap(); + if (documentKeys.isEmpty()) { + return PersistencePromise.resolve(results); + } + + const range = IDBKeyRange.bound(documentKeys.first(), documentKeys.last()); + let key = documentKeys.first(); + + return remoteDocumentsStore(transaction) + .iterate({ range }, (potentialKey, dbRemoteDoc, control) => { + if (DocumentKey.fromSegments(potentialKey) === key!) { + results = results.insert(key!, this.serializer.fromDbRemoteDocument(dbRemoteDoc)); + } + + key = documentKeys.firstAfterOrEqual(key!); + if (!key) { + control.done(); + return; + } + control.skip(key.path.toArray()); + }) + .next(() => results); + } + getDocumentsMatchingQuery( transaction: PersistenceTransaction, query: Query @@ -358,7 +387,7 @@ class IndexedDbRemoteDocumentChangeBuffer extends RemoteDocumentChangeBuffer { const toApply: Array<{ doc: DbRemoteDocument; key: DocumentKey }> = []; changes.forEach((key, maybeDocument) => { const doc = this.documentCache.serializer.toDbRemoteDocument( - maybeDocument + maybeDocument! ); const previousSize = this.documentSizes.get(key); // NOTE: if we ever decide we need to support doing writes without diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index 10511aa5a53..e704b3d8c98 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -478,12 +478,12 @@ export class LocalStore { // resolution failing). if ( existingDoc == null || - doc.version.isEqual(SnapshotVersion.MIN) || - (authoritativeUpdates.has(doc.key) && + doc!.version.isEqual(SnapshotVersion.MIN) || + (authoritativeUpdates.has(doc!.key) && !existingDoc.hasPendingWrites) || - doc.version.compareTo(existingDoc.version) >= 0 + doc!.version.compareTo(existingDoc.version) >= 0 ) { - documentBuffer.addEntry(doc); + documentBuffer.addEntry(doc!); } else { log.debug( LOG_TAG, @@ -492,7 +492,7 @@ export class LocalStore { '. Current version:', existingDoc.version, ' Watch version:', - doc.version + doc!.version ); } }) diff --git a/packages/firestore/src/local/memory_remote_document_cache.ts b/packages/firestore/src/local/memory_remote_document_cache.ts index 42c548aeb3b..0ee05324fb7 100644 --- a/packages/firestore/src/local/memory_remote_document_cache.ts +++ b/packages/firestore/src/local/memory_remote_document_cache.ts @@ -16,6 +16,7 @@ import { Query } from '../core/query'; import { + DocumentKeySet, documentKeySet, DocumentMap, documentMap, @@ -107,6 +108,20 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache { return PersistencePromise.resolve(this.docs.get(documentKey)); } + getEntries( + transaction : PersistenceTransaction, + documentKeys : DocumentKeySet, + ): PersistencePromise { + let results = maybeDocumentMap(); + documentKeys.forEach(documentKey => { + const entry = this.docs.get(documentKey); + if (entry) { + results = results.insert(documentKey, entry.maybeDocument); + } + }); + return PersistencePromise.resolve(results); + } + getDocumentsMatchingQuery( transaction: PersistenceTransaction, query: Query diff --git a/packages/firestore/src/local/remote_document_cache.ts b/packages/firestore/src/local/remote_document_cache.ts index 7a37355cd17..1b7ab020e41 100644 --- a/packages/firestore/src/local/remote_document_cache.ts +++ b/packages/firestore/src/local/remote_document_cache.ts @@ -15,7 +15,11 @@ */ import { Query } from '../core/query'; -import { DocumentMap, MaybeDocumentMap } from '../model/collections'; +import { + DocumentKeySet, + DocumentMap + MaybeDocumentMap +} from '../model/collections'; import { MaybeDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; @@ -44,6 +48,18 @@ export interface RemoteDocumentCache { documentKey: DocumentKey ): PersistencePromise; + /** + * Looks up a set of entries in the cache. + * + * @param documentKeys The keys of the entries to look up. + * @return The cached Document or NoDocument entries indexed by key. If an entry is not cached, + * the corresponding key will be mapped to a null value. + */ + getEntries( + transaction : PersistenceTransaction, + documentKeys : DocumentKeySet, + ): PersistencePromise; + /** * Executes a query against the cached Document entries. * From 9717fef15a51c80eae2dccadd0a02312d4180c6e Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 14 Dec 2018 13:55:28 -0800 Subject: [PATCH 02/29] Compiles --- .../local/indexeddb_remote_document_cache.ts | 8 +- .../src/local/local_documents_view.ts | 77 ++++++++++++++----- .../src/local/memory_remote_document_cache.ts | 8 +- .../src/local/remote_document_cache.ts | 5 +- packages/firestore/src/model/collections.ts | 6 ++ 5 files changed, 77 insertions(+), 27 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index 03563439c7c..30b07f11552 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -21,6 +21,8 @@ import { DocumentMap, documentMap, DocumentSizeEntry, + nullableMaybeDocumentMap, + NullableMaybeDocumentMap, MaybeDocumentMap, maybeDocumentMap } from '../model/collections'; @@ -182,8 +184,8 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { getEntries( transaction : PersistenceTransaction, documentKeys : DocumentKeySet, - ): PersistencePromise { - let results = maybeDocumentMap(); + ): PersistencePromise { + let results = nullableMaybeDocumentMap(); if (documentKeys.isEmpty()) { return PersistencePromise.resolve(results); } @@ -195,6 +197,8 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { .iterate({ range }, (potentialKey, dbRemoteDoc, control) => { if (DocumentKey.fromSegments(potentialKey) === key!) { results = results.insert(key!, this.serializer.fromDbRemoteDocument(dbRemoteDoc)); + } else { + results = results.insert(key!, null); } key = documentKeys.firstAfterOrEqual(key!); diff --git a/packages/firestore/src/local/local_documents_view.ts b/packages/firestore/src/local/local_documents_view.ts index 7edae45dd68..0648241fa9d 100644 --- a/packages/firestore/src/local/local_documents_view.ts +++ b/packages/firestore/src/local/local_documents_view.ts @@ -17,16 +17,20 @@ import { Query } from '../core/query'; import { SnapshotVersion } from '../core/snapshot_version'; import { + documentKeySet, DocumentKeySet, DocumentMap, documentMap, MaybeDocumentMap, - maybeDocumentMap + maybeDocumentMap, + NullableMaybeDocumentMap, + nullableMaybeDocumentMap } from '../model/collections'; import { Document, MaybeDocument, NoDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; import { MutationBatch } from '../model/mutation_batch'; import { ResourcePath } from '../model/path'; +import { SortedMap } from '../util/sorted_map'; import { MutationQueue } from './mutation_queue'; import { PersistenceTransaction } from './persistence'; @@ -74,6 +78,24 @@ export class LocalDocumentsView { }); } + // Returns the view of the given `docs` as they would appear after applying + // all mutations in the given `batches`. + private applyLocalMutationsToDocuments( + transaction: PersistenceTransaction, + docs: NullableMaybeDocumentMap, + batches: MutationBatch[] + ): PersistencePromise { + let results = nullableMaybeDocumentMap(); + return new PersistencePromise(() => { + docs.forEach((key, localView : MaybeDocument | null) => { + for (const batch of batches) { + localView = batch.applyToLocalView(key, localView); + } + results = results.insert(key, localView); + }); + }); + } + /** * Gets the local view of the documents identified by `keys`. * @@ -84,29 +106,42 @@ export class LocalDocumentsView { transaction: PersistenceTransaction, keys: DocumentKeySet ): PersistencePromise { + return this.remoteDocumentCache.getEntries(transaction, keys).next(docs => { + return this.getLocalViewOfDocuments(transaction, docs); + }); + } + +/** + * Similar to `getDocuments`, but creates the local view from the given + * `baseDocs` without retrieving documents from the local store. + */ + getLocalViewOfDocuments( + transaction: PersistenceTransaction, + baseDocs: NullableMaybeDocumentMap + ): PersistencePromise { + let allKeys = documentKeySet(); + baseDocs.forEach((key) => { + allKeys = allKeys.add(key); + }); + return this.mutationQueue - .getAllMutationBatchesAffectingDocumentKeys(transaction, keys) - .next(batches => { - const promises = [] as Array>; + .getAllMutationBatchesAffectingDocumentKeys(transaction, allKeys) + .next(batches => this.applyLocalMutationsToDocuments(transaction, baseDocs, batches)) + .next(docs => { let results = maybeDocumentMap(); - keys.forEach(key => { - promises.push( - this.getDocumentInternal(transaction, key, batches).next( - maybeDoc => { - // TODO(http://b/32275378): Don't conflate missing / deleted. - if (!maybeDoc) { - maybeDoc = new NoDocument( - key, - SnapshotVersion.forDeletedDoc() - ); - } - results = results.insert(key, maybeDoc); - } - ) - ); + docs.forEach((key, maybeDoc) => { + // TODO(http://b/32275378): Don't conflate missing / deleted. + if (!maybeDoc) { + maybeDoc = new NoDocument( + key, + SnapshotVersion.forDeletedDoc() + ); + results = results.insert(key, maybeDoc); + } + }); + + return results; }); - return PersistencePromise.waitFor(promises).next(() => results); - }); } /** Performs a query against the local view of all documents. */ diff --git a/packages/firestore/src/local/memory_remote_document_cache.ts b/packages/firestore/src/local/memory_remote_document_cache.ts index 0ee05324fb7..7f0599522f8 100644 --- a/packages/firestore/src/local/memory_remote_document_cache.ts +++ b/packages/firestore/src/local/memory_remote_document_cache.ts @@ -22,7 +22,9 @@ import { documentMap, DocumentSizeEntry, MaybeDocumentMap, - maybeDocumentMap + maybeDocumentMap, + NullableMaybeDocumentMap, + nullableMaybeDocumentMap } from '../model/collections'; import { Document, MaybeDocument, NoDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; @@ -111,8 +113,8 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache { getEntries( transaction : PersistenceTransaction, documentKeys : DocumentKeySet, - ): PersistencePromise { - let results = maybeDocumentMap(); + ): PersistencePromise { + let results = nullableMaybeDocumentMap(); documentKeys.forEach(documentKey => { const entry = this.docs.get(documentKey); if (entry) { diff --git a/packages/firestore/src/local/remote_document_cache.ts b/packages/firestore/src/local/remote_document_cache.ts index 1b7ab020e41..7e81f873930 100644 --- a/packages/firestore/src/local/remote_document_cache.ts +++ b/packages/firestore/src/local/remote_document_cache.ts @@ -18,6 +18,7 @@ import { Query } from '../core/query'; import { DocumentKeySet, DocumentMap + NullableMaybeDocumentMap MaybeDocumentMap } from '../model/collections'; import { MaybeDocument } from '../model/document'; @@ -27,6 +28,8 @@ import { PersistenceTransaction } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { RemoteDocumentChangeBuffer } from './remote_document_change_buffer'; +import { SortedMap } from '../util/sorted_map'; + /** * Represents cached documents received from the remote backend. * @@ -58,7 +61,7 @@ export interface RemoteDocumentCache { getEntries( transaction : PersistenceTransaction, documentKeys : DocumentKeySet, - ): PersistencePromise; + ): PersistencePromise; /** * Executes a query against the cached Document entries. diff --git a/packages/firestore/src/model/collections.ts b/packages/firestore/src/model/collections.ts index cb19919a563..3cab6ed76af 100644 --- a/packages/firestore/src/model/collections.ts +++ b/packages/firestore/src/model/collections.ts @@ -37,6 +37,12 @@ export function maybeDocumentMap(): MaybeDocumentMap { return EMPTY_MAYBE_DOCUMENT_MAP; } +export type NullableMaybeDocumentMap = SortedMap; + +export function nullableMaybeDocumentMap(): NullableMaybeDocumentMap { + return maybeDocumentMap(); +} + export type DocumentMap = SortedMap; const EMPTY_DOCUMENT_MAP = new SortedMap( DocumentKey.comparator From 204443e315ac257e57fbc6da8d89081a49904fc8 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 14 Dec 2018 14:03:52 -0800 Subject: [PATCH 03/29] Compiles, pt 2 --- .../local/indexeddb_remote_document_cache.ts | 9 ++-- .../src/local/local_documents_view.ts | 49 +++++++++---------- .../src/local/memory_remote_document_cache.ts | 8 +-- .../src/local/remote_document_cache.ts | 2 +- packages/firestore/src/model/collections.ts | 5 +- 5 files changed, 39 insertions(+), 34 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index 30b07f11552..c15b2160004 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -182,8 +182,8 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { } getEntries( - transaction : PersistenceTransaction, - documentKeys : DocumentKeySet, + transaction: PersistenceTransaction, + documentKeys: DocumentKeySet ): PersistencePromise { let results = nullableMaybeDocumentMap(); if (documentKeys.isEmpty()) { @@ -196,7 +196,10 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { return remoteDocumentsStore(transaction) .iterate({ range }, (potentialKey, dbRemoteDoc, control) => { if (DocumentKey.fromSegments(potentialKey) === key!) { - results = results.insert(key!, this.serializer.fromDbRemoteDocument(dbRemoteDoc)); + results = results.insert( + key!, + this.serializer.fromDbRemoteDocument(dbRemoteDoc) + ); } else { results = results.insert(key!, null); } diff --git a/packages/firestore/src/local/local_documents_view.ts b/packages/firestore/src/local/local_documents_view.ts index 0648241fa9d..37cd39ef097 100644 --- a/packages/firestore/src/local/local_documents_view.ts +++ b/packages/firestore/src/local/local_documents_view.ts @@ -85,13 +85,13 @@ export class LocalDocumentsView { docs: NullableMaybeDocumentMap, batches: MutationBatch[] ): PersistencePromise { - let results = nullableMaybeDocumentMap(); - return new PersistencePromise(() => { - docs.forEach((key, localView : MaybeDocument | null) => { - for (const batch of batches) { - localView = batch.applyToLocalView(key, localView); - } - results = results.insert(key, localView); + let results = nullableMaybeDocumentMap(); + return new PersistencePromise(() => { + docs.forEach((key, localView: MaybeDocument | null) => { + for (const batch of batches) { + localView = batch.applyToLocalView(key, localView); + } + results = results.insert(key, localView); }); }); } @@ -111,37 +111,36 @@ export class LocalDocumentsView { }); } -/** - * Similar to `getDocuments`, but creates the local view from the given - * `baseDocs` without retrieving documents from the local store. - */ + /** + * Similar to `getDocuments`, but creates the local view from the given + * `baseDocs` without retrieving documents from the local store. + */ getLocalViewOfDocuments( transaction: PersistenceTransaction, baseDocs: NullableMaybeDocumentMap ): PersistencePromise { let allKeys = documentKeySet(); - baseDocs.forEach((key) => { + baseDocs.forEach(key => { allKeys = allKeys.add(key); }); return this.mutationQueue .getAllMutationBatchesAffectingDocumentKeys(transaction, allKeys) - .next(batches => this.applyLocalMutationsToDocuments(transaction, baseDocs, batches)) - .next(docs => { + .next(batches => + this.applyLocalMutationsToDocuments(transaction, baseDocs, batches) + ) + .next(docs => { let results = maybeDocumentMap(); docs.forEach((key, maybeDoc) => { - // TODO(http://b/32275378): Don't conflate missing / deleted. - if (!maybeDoc) { - maybeDoc = new NoDocument( - key, - SnapshotVersion.forDeletedDoc() - ); - results = results.insert(key, maybeDoc); - } - }); - - return results; + // TODO(http://b/32275378): Don't conflate missing / deleted. + if (!maybeDoc) { + maybeDoc = new NoDocument(key, SnapshotVersion.forDeletedDoc()); + results = results.insert(key, maybeDoc); + } }); + + return results; + }); } /** Performs a query against the local view of all documents. */ diff --git a/packages/firestore/src/local/memory_remote_document_cache.ts b/packages/firestore/src/local/memory_remote_document_cache.ts index 7f0599522f8..0c50a3c4930 100644 --- a/packages/firestore/src/local/memory_remote_document_cache.ts +++ b/packages/firestore/src/local/memory_remote_document_cache.ts @@ -111,16 +111,16 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache { } getEntries( - transaction : PersistenceTransaction, - documentKeys : DocumentKeySet, + transaction: PersistenceTransaction, + documentKeys: DocumentKeySet ): PersistencePromise { let results = nullableMaybeDocumentMap(); documentKeys.forEach(documentKey => { - const entry = this.docs.get(documentKey); + const entry = this.docs.get(documentKey); if (entry) { results = results.insert(documentKey, entry.maybeDocument); } - }); + }); return PersistencePromise.resolve(results); } diff --git a/packages/firestore/src/local/remote_document_cache.ts b/packages/firestore/src/local/remote_document_cache.ts index 7e81f873930..9cd045413c9 100644 --- a/packages/firestore/src/local/remote_document_cache.ts +++ b/packages/firestore/src/local/remote_document_cache.ts @@ -18,7 +18,7 @@ import { Query } from '../core/query'; import { DocumentKeySet, DocumentMap - NullableMaybeDocumentMap + NullableMaybeDocumentMap, MaybeDocumentMap } from '../model/collections'; import { MaybeDocument } from '../model/document'; diff --git a/packages/firestore/src/model/collections.ts b/packages/firestore/src/model/collections.ts index 3cab6ed76af..64a2e162edf 100644 --- a/packages/firestore/src/model/collections.ts +++ b/packages/firestore/src/model/collections.ts @@ -37,7 +37,10 @@ export function maybeDocumentMap(): MaybeDocumentMap { return EMPTY_MAYBE_DOCUMENT_MAP; } -export type NullableMaybeDocumentMap = SortedMap; +export type NullableMaybeDocumentMap = SortedMap< + DocumentKey, + MaybeDocument | null +>; export function nullableMaybeDocumentMap(): NullableMaybeDocumentMap { return maybeDocumentMap(); From c56820b030868af56b4a500fedd2203208aa1fe2 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 14 Dec 2018 14:05:40 -0800 Subject: [PATCH 04/29] Compiles, pt 3m --- packages/firestore/src/local/remote_document_cache.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/src/local/remote_document_cache.ts b/packages/firestore/src/local/remote_document_cache.ts index 9cd045413c9..88bbe54d7d7 100644 --- a/packages/firestore/src/local/remote_document_cache.ts +++ b/packages/firestore/src/local/remote_document_cache.ts @@ -17,7 +17,7 @@ import { Query } from '../core/query'; import { DocumentKeySet, - DocumentMap + DocumentMap, NullableMaybeDocumentMap, MaybeDocumentMap } from '../model/collections'; From f3174ca0a235d65de2252510f07950776b614e39 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 14 Dec 2018 14:06:24 -0800 Subject: [PATCH 05/29] [AUTOMATED]: Prettier Code Styling --- .../firestore/src/local/remote_document_cache.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/firestore/src/local/remote_document_cache.ts b/packages/firestore/src/local/remote_document_cache.ts index 88bbe54d7d7..8001894118f 100644 --- a/packages/firestore/src/local/remote_document_cache.ts +++ b/packages/firestore/src/local/remote_document_cache.ts @@ -52,15 +52,15 @@ export interface RemoteDocumentCache { ): PersistencePromise; /** - * Looks up a set of entries in the cache. - * - * @param documentKeys The keys of the entries to look up. - * @return The cached Document or NoDocument entries indexed by key. If an entry is not cached, - * the corresponding key will be mapped to a null value. - */ + * Looks up a set of entries in the cache. + * + * @param documentKeys The keys of the entries to look up. + * @return The cached Document or NoDocument entries indexed by key. If an entry is not cached, + * the corresponding key will be mapped to a null value. + */ getEntries( - transaction : PersistenceTransaction, - documentKeys : DocumentKeySet, + transaction: PersistenceTransaction, + documentKeys: DocumentKeySet ): PersistencePromise; /** From 36c53d3f334faea52270b67390abca99a4d50aa8 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 14 Dec 2018 14:21:30 -0800 Subject: [PATCH 06/29] Some fixes --- .../src/local/indexeddb_remote_document_cache.ts | 2 +- packages/firestore/src/local/local_documents_view.ts | 6 +++--- packages/firestore/src/local/local_store.ts | 10 +++++----- .../src/local/memory_remote_document_cache.ts | 4 +--- packages/firestore/src/local/remote_document_cache.ts | 2 -- 5 files changed, 10 insertions(+), 14 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index c15b2160004..60f6e89df1a 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -394,7 +394,7 @@ class IndexedDbRemoteDocumentChangeBuffer extends RemoteDocumentChangeBuffer { const toApply: Array<{ doc: DbRemoteDocument; key: DocumentKey }> = []; changes.forEach((key, maybeDocument) => { const doc = this.documentCache.serializer.toDbRemoteDocument( - maybeDocument! + maybeDocument ); const previousSize = this.documentSizes.get(key); // NOTE: if we ever decide we need to support doing writes without diff --git a/packages/firestore/src/local/local_documents_view.ts b/packages/firestore/src/local/local_documents_view.ts index 37cd39ef097..9ad586d1d05 100644 --- a/packages/firestore/src/local/local_documents_view.ts +++ b/packages/firestore/src/local/local_documents_view.ts @@ -30,7 +30,6 @@ import { Document, MaybeDocument, NoDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; import { MutationBatch } from '../model/mutation_batch'; import { ResourcePath } from '../model/path'; -import { SortedMap } from '../util/sorted_map'; import { MutationQueue } from './mutation_queue'; import { PersistenceTransaction } from './persistence'; @@ -87,12 +86,13 @@ export class LocalDocumentsView { ): PersistencePromise { let results = nullableMaybeDocumentMap(); return new PersistencePromise(() => { - docs.forEach((key, localView: MaybeDocument | null) => { + docs.forEach((key, localView) => { for (const batch of batches) { localView = batch.applyToLocalView(key, localView); } results = results.insert(key, localView); }); + return results; }); } @@ -135,8 +135,8 @@ export class LocalDocumentsView { // TODO(http://b/32275378): Don't conflate missing / deleted. if (!maybeDoc) { maybeDoc = new NoDocument(key, SnapshotVersion.forDeletedDoc()); - results = results.insert(key, maybeDoc); } + results = results.insert(key, maybeDoc); }); return results; diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index e704b3d8c98..10511aa5a53 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -478,12 +478,12 @@ export class LocalStore { // resolution failing). if ( existingDoc == null || - doc!.version.isEqual(SnapshotVersion.MIN) || - (authoritativeUpdates.has(doc!.key) && + doc.version.isEqual(SnapshotVersion.MIN) || + (authoritativeUpdates.has(doc.key) && !existingDoc.hasPendingWrites) || - doc!.version.compareTo(existingDoc.version) >= 0 + doc.version.compareTo(existingDoc.version) >= 0 ) { - documentBuffer.addEntry(doc!); + documentBuffer.addEntry(doc); } else { log.debug( LOG_TAG, @@ -492,7 +492,7 @@ export class LocalStore { '. Current version:', existingDoc.version, ' Watch version:', - doc!.version + doc.version ); } }) diff --git a/packages/firestore/src/local/memory_remote_document_cache.ts b/packages/firestore/src/local/memory_remote_document_cache.ts index 0c50a3c4930..f1cfa3d2ef1 100644 --- a/packages/firestore/src/local/memory_remote_document_cache.ts +++ b/packages/firestore/src/local/memory_remote_document_cache.ts @@ -117,9 +117,7 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache { let results = nullableMaybeDocumentMap(); documentKeys.forEach(documentKey => { const entry = this.docs.get(documentKey); - if (entry) { - results = results.insert(documentKey, entry.maybeDocument); - } + results = results.insert(documentKey, entry ? entry.maybeDocument : null); }); return PersistencePromise.resolve(results); } diff --git a/packages/firestore/src/local/remote_document_cache.ts b/packages/firestore/src/local/remote_document_cache.ts index 8001894118f..8cd41f4bbf3 100644 --- a/packages/firestore/src/local/remote_document_cache.ts +++ b/packages/firestore/src/local/remote_document_cache.ts @@ -28,8 +28,6 @@ import { PersistenceTransaction } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { RemoteDocumentChangeBuffer } from './remote_document_change_buffer'; -import { SortedMap } from '../util/sorted_map'; - /** * Represents cached documents received from the remote backend. * From 942b0facf8d5c35f8710b1fd04d7fb91569cfa7c Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 14 Dec 2018 17:27:23 -0800 Subject: [PATCH 07/29] Very hacky version works --- .../local/indexeddb_remote_document_cache.ts | 44 ++++++++++++------- .../src/local/local_documents_view.ts | 10 ++--- packages/firestore/src/util/sorted_set.ts | 13 ++++++ .../unit/local/indexeddb_persistence.test.ts | 10 ++--- 4 files changed, 50 insertions(+), 27 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index 60f6e89df1a..5c09b318277 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -190,28 +190,38 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { return PersistencePromise.resolve(results); } - const range = IDBKeyRange.bound(documentKeys.first(), documentKeys.last()); + const range = IDBKeyRange.bound(documentKeys.first().path.toArray(), + documentKeys.last().path.toArray()); let key = documentKeys.first(); return remoteDocumentsStore(transaction) - .iterate({ range }, (potentialKey, dbRemoteDoc, control) => { - if (DocumentKey.fromSegments(potentialKey) === key!) { - results = results.insert( - key!, - this.serializer.fromDbRemoteDocument(dbRemoteDoc) - ); - } else { - results = results.insert(key!, null); - } + .iterate( {range}, (potentialKeyRaw, dbRemoteDoc, control) => { + const potentialKey = DocumentKey.fromSegments(potentialKeyRaw); + while (DocumentKey.comparator(key, potentialKey) != 1) { + if (key.isEqual(potentialKey)) { + results = results.insert( + key!, + this.serializer.fromDbRemoteDocument(dbRemoteDoc) + ); + } else { + results = results.insert(key!, null); + } - key = documentKeys.firstAfterOrEqual(key!); - if (!key) { - control.done(); - return; + key = documentKeys.firstAfter(key!); + if (!key) { + control.done(); + return; + } } - control.skip(key.path.toArray()); }) - .next(() => results); + .next(() => { + documentKeys.forEach(key => { + if (results.get(key) === null) { + results = results.insert(key, null); + } + }); + return results; + }); } getDocumentsMatchingQuery( @@ -253,7 +263,7 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { const changesStore = documentChangesStore(transaction); return changesStore - .iterate({ range }, (_, documentChange) => { + .iterate({ }, (_, documentChange) => { if (firstIteration) { firstIteration = false; diff --git a/packages/firestore/src/local/local_documents_view.ts b/packages/firestore/src/local/local_documents_view.ts index 9ad586d1d05..76677412b8f 100644 --- a/packages/firestore/src/local/local_documents_view.ts +++ b/packages/firestore/src/local/local_documents_view.ts @@ -85,14 +85,14 @@ export class LocalDocumentsView { batches: MutationBatch[] ): PersistencePromise { let results = nullableMaybeDocumentMap(); - return new PersistencePromise(() => { + return new PersistencePromise((resolve) => { docs.forEach((key, localView) => { for (const batch of batches) { localView = batch.applyToLocalView(key, localView); } results = results.insert(key, localView); }); - return results; + resolve(results); }); } @@ -106,9 +106,9 @@ export class LocalDocumentsView { transaction: PersistenceTransaction, keys: DocumentKeySet ): PersistencePromise { - return this.remoteDocumentCache.getEntries(transaction, keys).next(docs => { - return this.getLocalViewOfDocuments(transaction, docs); - }); + return this.remoteDocumentCache.getEntries(transaction, keys).next(docs => + this.getLocalViewOfDocuments(transaction, docs) + ); } /** diff --git a/packages/firestore/src/util/sorted_set.ts b/packages/firestore/src/util/sorted_set.ts index 1c7da829016..234119bd8cc 100644 --- a/packages/firestore/src/util/sorted_set.ts +++ b/packages/firestore/src/util/sorted_set.ts @@ -97,6 +97,19 @@ export class SortedSet { } } + /** Finds the least element greater than or equal to `elem`. */ + firstAfter(elem: T): T | null { + const iter = this.data.getIteratorFrom(elem); + if (!iter.hasNext()) { + return null; + } + if (this.comparator(iter.peek().key, elem) == 0) { + // Skip + iter.getNext(); + } + return iter.hasNext() ? iter.getNext().key : null; + } + /** Finds the least element greater than or equal to `elem`. */ firstAfterOrEqual(elem: T): T | null { const iter = this.data.getIteratorFrom(elem); diff --git a/packages/firestore/test/unit/local/indexeddb_persistence.test.ts b/packages/firestore/test/unit/local/indexeddb_persistence.test.ts index c6d1eb33b9c..a06ddd5fafc 100644 --- a/packages/firestore/test/unit/local/indexeddb_persistence.test.ts +++ b/packages/firestore/test/unit/local/indexeddb_persistence.test.ts @@ -749,9 +749,9 @@ describe('IndexedDb: allowTabSynchronization', () => { }); }); - it('grants access when synchronization is enabled', async () => { - return withMultiClientPersistence('clientA', async db1 => { - await withMultiClientPersistence('clientB', async db2 => {}); - }); - }); + // it('grants access when synchronization is enabled', async () => { + // return withMultiClientPersistence('clientA', async db1 => { + // await withMultiClientPersistence('clientB', async db2 => {}); + // }); + // }); }); From 7767b608a83bf23cf4e0da902289c2cce7de5269 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Sun, 16 Dec 2018 21:07:43 -0500 Subject: [PATCH 08/29] Most unit tests pass --- .../local/indexeddb_remote_document_cache.ts | 17 ++++++++--------- packages/firestore/src/util/sorted_set.ts | 4 ++-- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index 5c09b318277..9717865b09a 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -190,15 +190,15 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { return PersistencePromise.resolve(results); } - const range = IDBKeyRange.bound(documentKeys.first().path.toArray(), - documentKeys.last().path.toArray()); + const range = IDBKeyRange.bound(documentKeys.first()!.path.toArray(), + documentKeys.last()!.path.toArray()); let key = documentKeys.first(); return remoteDocumentsStore(transaction) .iterate( {range}, (potentialKeyRaw, dbRemoteDoc, control) => { const potentialKey = DocumentKey.fromSegments(potentialKeyRaw); - while (DocumentKey.comparator(key, potentialKey) != 1) { - if (key.isEqual(potentialKey)) { + while (DocumentKey.comparator(key!, potentialKey) != 1) { + if (key!.isEqual(potentialKey)) { results = results.insert( key!, this.serializer.fromDbRemoteDocument(dbRemoteDoc) @@ -215,11 +215,10 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { } }) .next(() => { - documentKeys.forEach(key => { - if (results.get(key) === null) { - results = results.insert(key, null); - } - }); + while (key) { + results = results.insert(key, null); + key = documentKeys.firstAfter(key!); + } return results; }); } diff --git a/packages/firestore/src/util/sorted_set.ts b/packages/firestore/src/util/sorted_set.ts index 234119bd8cc..ea86626ece3 100644 --- a/packages/firestore/src/util/sorted_set.ts +++ b/packages/firestore/src/util/sorted_set.ts @@ -97,13 +97,13 @@ export class SortedSet { } } - /** Finds the least element greater than or equal to `elem`. */ + /** Finds the least element greater than `elem`. */ firstAfter(elem: T): T | null { const iter = this.data.getIteratorFrom(elem); if (!iter.hasNext()) { return null; } - if (this.comparator(iter.peek().key, elem) == 0) { + if (this.comparator(iter.peek()!.key, elem) == 0) { // Skip iter.getNext(); } From a1ad6d248cec63c77c7b956de406807ea41db6a2 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 17 Dec 2018 15:30:31 -0500 Subject: [PATCH 09/29] [AUTOMATED]: Prettier Code Styling --- packages/firestore/src/local/local_documents_view.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/firestore/src/local/local_documents_view.ts b/packages/firestore/src/local/local_documents_view.ts index 76677412b8f..6e3c6a9ef3c 100644 --- a/packages/firestore/src/local/local_documents_view.ts +++ b/packages/firestore/src/local/local_documents_view.ts @@ -85,7 +85,7 @@ export class LocalDocumentsView { batches: MutationBatch[] ): PersistencePromise { let results = nullableMaybeDocumentMap(); - return new PersistencePromise((resolve) => { + return new PersistencePromise(resolve => { docs.forEach((key, localView) => { for (const batch of batches) { localView = batch.applyToLocalView(key, localView); @@ -106,9 +106,9 @@ export class LocalDocumentsView { transaction: PersistenceTransaction, keys: DocumentKeySet ): PersistencePromise { - return this.remoteDocumentCache.getEntries(transaction, keys).next(docs => - this.getLocalViewOfDocuments(transaction, docs) - ); + return this.remoteDocumentCache + .getEntries(transaction, keys) + .next(docs => this.getLocalViewOfDocuments(transaction, docs)); } /** From cc29731d10d596258d68c78060edf87ab0d02b10 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 17 Dec 2018 15:43:56 -0500 Subject: [PATCH 10/29] Undo temp/accidental changes --- .../src/local/indexeddb_remote_document_cache.ts | 2 +- .../test/unit/local/indexeddb_persistence.test.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index 9717865b09a..8e941776a10 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -262,7 +262,7 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { const changesStore = documentChangesStore(transaction); return changesStore - .iterate({ }, (_, documentChange) => { + .iterate({ range }, (_, documentChange) => { if (firstIteration) { firstIteration = false; diff --git a/packages/firestore/test/unit/local/indexeddb_persistence.test.ts b/packages/firestore/test/unit/local/indexeddb_persistence.test.ts index a06ddd5fafc..c6d1eb33b9c 100644 --- a/packages/firestore/test/unit/local/indexeddb_persistence.test.ts +++ b/packages/firestore/test/unit/local/indexeddb_persistence.test.ts @@ -749,9 +749,9 @@ describe('IndexedDb: allowTabSynchronization', () => { }); }); - // it('grants access when synchronization is enabled', async () => { - // return withMultiClientPersistence('clientA', async db1 => { - // await withMultiClientPersistence('clientB', async db2 => {}); - // }); - // }); + it('grants access when synchronization is enabled', async () => { + return withMultiClientPersistence('clientA', async db1 => { + await withMultiClientPersistence('clientB', async db2 => {}); + }); + }); }); From 307e0b34463206f0f97793d1dddfd70a4b72f56e Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 17 Dec 2018 20:35:16 -0500 Subject: [PATCH 11/29] applyRemoteEvent, sized entries --- .../local/indexeddb_remote_document_cache.ts | 24 +++++++++++-- packages/firestore/src/local/local_store.ts | 23 ++++++++---- .../src/local/memory_remote_document_cache.ts | 28 +++++++++++++++ .../local/remote_document_change_buffer.ts | 36 +++++++++++++++++++ packages/firestore/src/model/collections.ts | 5 +++ 5 files changed, 107 insertions(+), 9 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index 8e941776a10..962f340ad35 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -21,12 +21,14 @@ import { DocumentMap, documentMap, DocumentSizeEntry, + DocumentSizeEntries, nullableMaybeDocumentMap, NullableMaybeDocumentMap, MaybeDocumentMap, maybeDocumentMap } from '../model/collections'; import { Document, MaybeDocument, NoDocument } from '../model/document'; +import { SortedMap } from '../util/sorted_map'; import { DocumentKey } from '../model/document_key'; import { SnapshotVersion } from '../core/snapshot_version'; @@ -185,9 +187,17 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { transaction: PersistenceTransaction, documentKeys: DocumentKeySet ): PersistencePromise { + return this.getSizedEntries(transaction, documentKeys).next(result => result.maybeDocuments); + } + + getSizedEntries( + transaction: PersistenceTransaction, + documentKeys: DocumentKeySet + ): PersistencePromise { let results = nullableMaybeDocumentMap(); + let sizeMap = new SortedMap(DocumentKey.comparator); if (documentKeys.isEmpty()) { - return PersistencePromise.resolve(results); + return PersistencePromise.resolve({maybeDocuments: results, sizeMap}); } const range = IDBKeyRange.bound(documentKeys.first()!.path.toArray(), @@ -203,8 +213,10 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { key!, this.serializer.fromDbRemoteDocument(dbRemoteDoc) ); + sizeMap = sizeMap.insert(key!, dbDocumentSize(dbRemoteDoc)); } else { results = results.insert(key!, null); + sizeMap = sizeMap.insert(key!, 0); } key = documentKeys.firstAfter(key!); @@ -217,9 +229,10 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { .next(() => { while (key) { results = results.insert(key, null); + sizeMap = sizeMap.insert(key, 0); key = documentKeys.firstAfter(key!); } - return results; + return { maybeDocuments: results, sizeMap }; }); } @@ -426,6 +439,13 @@ class IndexedDbRemoteDocumentChangeBuffer extends RemoteDocumentChangeBuffer { ): PersistencePromise { return this.documentCache.getSizedEntry(transaction, documentKey); } + + protected getAllFromCache( + transaction: PersistenceTransaction, + documentKeys: DocumentKeySet + ): PersistencePromise { + return this.documentCache.getSizedEntries(transaction, documentKeys); + } } export function isDocumentChangeMissingError(err: FirestoreError): boolean { diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index 10511aa5a53..c236a0654ea 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -23,6 +23,7 @@ import { DocumentKeySet, documentKeySet, DocumentMap, + maybeDocumentMap, MaybeDocumentMap } from '../model/collections'; import { MaybeDocument } from '../model/document'; @@ -466,11 +467,18 @@ export class LocalStore { } ); - let changedDocKeys = documentKeySet(); + let changedDocs = maybeDocumentMap(); + let updatedKeys = documentKeySet(); remoteEvent.documentUpdates.forEach((key, doc) => { - changedDocKeys = changedDocKeys.add(key); - promises.push( - documentBuffer.getEntry(txn, key).next(existingDoc => { + updatedKeys = updatedKeys.add(key); + }); + + // Each loop iteration only affects its "own" doc, so it's safe to get all the remote + // documents in advance in a single call. + documentBuffer.getEntries(txn, updatedKeys) + .next(existingDocs => { + remoteEvent.documentUpdates.forEach((key, doc) => { + const existingDoc = existingDocs.get(key); // If a document update isn't authoritative, make sure we don't // apply an old document version to the remote cache. We make an // exception for SnapshotVersion.MIN which can happen for @@ -484,6 +492,7 @@ export class LocalStore { doc.version.compareTo(existingDoc.version) >= 0 ) { documentBuffer.addEntry(doc); + changedDocs = changedDocs.insert(key, doc); } else { log.debug( LOG_TAG, @@ -495,13 +504,13 @@ export class LocalStore { doc.version ); } - }) - ); + if (remoteEvent.resolvedLimboDocuments.has(key)) { promises.push( this.persistence.referenceDelegate.updateLimboDocument(txn, key) ); } + }); }); // HACK: The only reason we allow a null snapshot version is so that we @@ -532,7 +541,7 @@ export class LocalStore { return PersistencePromise.waitFor(promises) .next(() => documentBuffer.apply(txn)) .next(() => { - return this.localDocuments.getDocuments(txn, changedDocKeys); + return this.localDocuments.getLocalViewOfDocuments(txn, changedDocs); }); } ); diff --git a/packages/firestore/src/local/memory_remote_document_cache.ts b/packages/firestore/src/local/memory_remote_document_cache.ts index f1cfa3d2ef1..c21de8a82af 100644 --- a/packages/firestore/src/local/memory_remote_document_cache.ts +++ b/packages/firestore/src/local/memory_remote_document_cache.ts @@ -21,6 +21,7 @@ import { DocumentMap, documentMap, DocumentSizeEntry, + DocumentSizeEntries, MaybeDocumentMap, maybeDocumentMap, NullableMaybeDocumentMap, @@ -122,6 +123,26 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache { return PersistencePromise.resolve(results); } + /** + * Looks up an entry in the cache. + * + * @param documentKey The key of the entry to look up. + * @return The cached MaybeDocument entry and its size, or null if we have nothing cached. + */ + getSizedEntries( + transaction: PersistenceTransaction, + documentKeys: DocumentKeySet + ): PersistencePromise { + let results = nullableMaybeDocumentMap(); + let sizeMap = new SortedMap(DocumentKey.comparator); + documentKeys.forEach(documentKey => { + const entry = this.docs.get(documentKey); + results = results.insert(documentKey, entry ? entry.maybeDocument : null); + sizeMap = sizeMap.insert(documentKey, entry ? entry.size : 0); + }); + return PersistencePromise.resolve({maybeDocuments: results, sizeMap}); + } + getDocumentsMatchingQuery( transaction: PersistenceTransaction, query: Query @@ -218,4 +239,11 @@ export class MemoryRemoteDocumentChangeBuffer extends RemoteDocumentChangeBuffer ): PersistencePromise { return this.documentCache.getSizedEntry(transaction, documentKey); } + + protected getAllFromCache( + transaction: PersistenceTransaction, + documentKeys: DocumentKeySet + ): PersistencePromise { + return this.documentCache.getSizedEntries(transaction, documentKeys); + } } diff --git a/packages/firestore/src/local/remote_document_change_buffer.ts b/packages/firestore/src/local/remote_document_change_buffer.ts index 27b8113993e..b82f86d41cb 100644 --- a/packages/firestore/src/local/remote_document_change_buffer.ts +++ b/packages/firestore/src/local/remote_document_change_buffer.ts @@ -15,7 +15,10 @@ */ import { + DocumentKeySet, DocumentSizeEntry, + DocumentSizeEntries, + NullableMaybeDocumentMap, maybeDocumentMap, MaybeDocumentMap } from '../model/collections'; @@ -52,6 +55,11 @@ export abstract class RemoteDocumentChangeBuffer { documentKey: DocumentKey ): PersistencePromise; + protected abstract getAllFromCache( + transaction: PersistenceTransaction, + documentKeys: DocumentKeySet + ): PersistencePromise; + protected abstract applyChanges( transaction: PersistenceTransaction ): PersistencePromise; @@ -99,6 +107,34 @@ export abstract class RemoteDocumentChangeBuffer { } } + /** + * Looks up an entry in the cache. The buffered changes will first be checked, + * and if no buffered change applies, this will forward to + * `RemoteDocumentCache.getEntry()`. + * + * @param transaction The transaction in which to perform any persistence + * operations. + * @param documentKey The key of the entry to look up. + * @return The cached Document or NoDocument entry, or null if we have nothing + * cached. + */ + getEntries( + transaction: PersistenceTransaction, + documentKeys: DocumentKeySet + ): PersistencePromise { + const changes = this.assertChanges(); + + // TODO OBC: look up in the buffer + + // Record the size of everything we load from the cache so we can compute a delta later. + return this.getAllFromCache(transaction, documentKeys).next(getResult => { + getResult.sizeMap.forEach((documentKey, size) => { + this.documentSizes.set(documentKey, size); + }); + return getResult.maybeDocuments; + }); + } + /** * Applies buffered changes to the underlying RemoteDocumentCache, using * the provided transaction. diff --git a/packages/firestore/src/model/collections.ts b/packages/firestore/src/model/collections.ts index 64a2e162edf..f07a4c22248 100644 --- a/packages/firestore/src/model/collections.ts +++ b/packages/firestore/src/model/collections.ts @@ -46,6 +46,11 @@ export function nullableMaybeDocumentMap(): NullableMaybeDocumentMap { return maybeDocumentMap(); } +export type DocumentSizeEntries = { + maybeDocuments: NullableMaybeDocumentMap; + sizeMap: SortedMap; +}; + export type DocumentMap = SortedMap; const EMPTY_DOCUMENT_MAP = new SortedMap( DocumentKey.comparator From 62c627b614d824703bf9de05d8736c6274d1ee8f Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 17 Dec 2018 21:06:15 -0500 Subject: [PATCH 12/29] Fix failing tests --- packages/firestore/src/local/local_store.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index c236a0654ea..09b2b00ca9f 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -475,7 +475,7 @@ export class LocalStore { // Each loop iteration only affects its "own" doc, so it's safe to get all the remote // documents in advance in a single call. - documentBuffer.getEntries(txn, updatedKeys) + promises.push(documentBuffer.getEntries(txn, updatedKeys) .next(existingDocs => { remoteEvent.documentUpdates.forEach((key, doc) => { const existingDoc = existingDocs.get(key); @@ -505,13 +505,13 @@ export class LocalStore { ); } - if (remoteEvent.resolvedLimboDocuments.has(key)) { - promises.push( - this.persistence.referenceDelegate.updateLimboDocument(txn, key) - ); - } + if (remoteEvent.resolvedLimboDocuments.has(key)) { + promises.push( + this.persistence.referenceDelegate.updateLimboDocument(txn, key) + ); + } }); - }); + })); // HACK: The only reason we allow a null snapshot version is so that we // can synthesize remote events when we get permission denied errors while From 8f293ebb8ee649491e2adf67360f76d65edab3f5 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 17 Dec 2018 21:27:21 -0500 Subject: [PATCH 13/29] Serializer --- packages/firestore/src/local/local_serializer.ts | 7 ++++++- packages/firestore/src/model/document.ts | 9 ++++++++- packages/firestore/src/remote/serializer.ts | 6 ++++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/firestore/src/local/local_serializer.ts b/packages/firestore/src/local/local_serializer.ts index 0825e2a8fac..c6c34b325df 100644 --- a/packages/firestore/src/local/local_serializer.ts +++ b/packages/firestore/src/local/local_serializer.ts @@ -71,7 +71,12 @@ export class LocalSerializer { /** Encodes a document for storage locally. */ toDbRemoteDocument(maybeDoc: MaybeDocument): DbRemoteDocument { if (maybeDoc instanceof Document) { - const doc = this.remoteSerializer.toDocument(maybeDoc); + let doc: api.Document; + if (maybeDoc.proto) { + doc = maybeDoc.proto; + } else { + doc = this.remoteSerializer.toDocument(maybeDoc); + } const hasCommittedMutations = maybeDoc.hasCommittedMutations; return new DbRemoteDocument( /* unknownDocument= */ null, diff --git a/packages/firestore/src/model/document.ts b/packages/firestore/src/model/document.ts index ff0b1266f33..78e2f438a44 100644 --- a/packages/firestore/src/model/document.ts +++ b/packages/firestore/src/model/document.ts @@ -22,6 +22,8 @@ import { DocumentKey } from './document_key'; import { FieldValue, JsonObject, ObjectValue } from './field_value'; import { FieldPath } from './path'; +import * as api from '../protos/firestore_proto_api'; + export interface DocumentOptions { hasLocalMutations?: boolean; hasCommittedMutations?: boolean; @@ -59,7 +61,12 @@ export class Document extends MaybeDocument { key: DocumentKey, version: SnapshotVersion, readonly data: ObjectValue, - options: DocumentOptions + options: DocumentOptions, +/** + * Memoized serialized form of the document for optimization purposes (avoids repeated + * serialization). Might be nil. + */ + readonly proto?: api.Document ) { super(key, version); this.hasLocalMutations = !!options.hasLocalMutations; diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 0961d135c0b..74e8b1b2d04 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -581,7 +581,7 @@ export class JsonProtoSerializer { const key = this.fromName(doc.found!.name!); const version = this.fromVersion(doc.found!.updateTime!); const fields = this.fromFields(doc.found!.fields || {}); - return new Document(key, version, fields, {}); + return new Document(key, version, fields, {}, doc.found!); } private fromMissing(result: api.BatchGetDocumentsResponse): NoDocument { @@ -726,7 +726,9 @@ export class JsonProtoSerializer { const key = this.fromName(entityChange.document!.name!); const version = this.fromVersion(entityChange.document!.updateTime!); const fields = this.fromFields(entityChange.document!.fields || {}); - const doc = new Document(key, version, fields, {}); + // The document may soon be re-serialized back to protos in order to store it in local + // persistence. Memoize the encoded form to avoid encoding it again. + const doc = new Document(key, version, fields, {}, entityChange.document!); const updatedTargetIds = entityChange.targetIds || []; const removedTargetIds = entityChange.removedTargetIds || []; watchChange = new DocumentWatchChange( From 4e23f3b80df9edbf46c51a3f11af50c57f8ad8ad Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 17 Dec 2018 21:28:18 -0500 Subject: [PATCH 14/29] [AUTOMATED]: Prettier Code Styling --- .../local/indexeddb_remote_document_cache.ts | 14 +++++++++----- packages/firestore/src/local/local_store.ts | 17 ++++++++++++----- .../src/local/memory_remote_document_cache.ts | 2 +- packages/firestore/src/model/document.ts | 8 ++++---- packages/firestore/src/remote/serializer.ts | 8 +++++++- 5 files changed, 33 insertions(+), 16 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index 962f340ad35..04fa417a45a 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -187,7 +187,9 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { transaction: PersistenceTransaction, documentKeys: DocumentKeySet ): PersistencePromise { - return this.getSizedEntries(transaction, documentKeys).next(result => result.maybeDocuments); + return this.getSizedEntries(transaction, documentKeys).next( + result => result.maybeDocuments + ); } getSizedEntries( @@ -197,15 +199,17 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { let results = nullableMaybeDocumentMap(); let sizeMap = new SortedMap(DocumentKey.comparator); if (documentKeys.isEmpty()) { - return PersistencePromise.resolve({maybeDocuments: results, sizeMap}); + return PersistencePromise.resolve({ maybeDocuments: results, sizeMap }); } - const range = IDBKeyRange.bound(documentKeys.first()!.path.toArray(), - documentKeys.last()!.path.toArray()); + const range = IDBKeyRange.bound( + documentKeys.first()!.path.toArray(), + documentKeys.last()!.path.toArray() + ); let key = documentKeys.first(); return remoteDocumentsStore(transaction) - .iterate( {range}, (potentialKeyRaw, dbRemoteDoc, control) => { + .iterate({ range }, (potentialKeyRaw, dbRemoteDoc, control) => { const potentialKey = DocumentKey.fromSegments(potentialKeyRaw); while (DocumentKey.comparator(key!, potentialKey) != 1) { if (key!.isEqual(potentialKey)) { diff --git a/packages/firestore/src/local/local_store.ts b/packages/firestore/src/local/local_store.ts index 09b2b00ca9f..daa7b1ca574 100644 --- a/packages/firestore/src/local/local_store.ts +++ b/packages/firestore/src/local/local_store.ts @@ -475,8 +475,8 @@ export class LocalStore { // Each loop iteration only affects its "own" doc, so it's safe to get all the remote // documents in advance in a single call. - promises.push(documentBuffer.getEntries(txn, updatedKeys) - .next(existingDocs => { + promises.push( + documentBuffer.getEntries(txn, updatedKeys).next(existingDocs => { remoteEvent.documentUpdates.forEach((key, doc) => { const existingDoc = existingDocs.get(key); // If a document update isn't authoritative, make sure we don't @@ -507,11 +507,15 @@ export class LocalStore { if (remoteEvent.resolvedLimboDocuments.has(key)) { promises.push( - this.persistence.referenceDelegate.updateLimboDocument(txn, key) + this.persistence.referenceDelegate.updateLimboDocument( + txn, + key + ) ); } }); - })); + }) + ); // HACK: The only reason we allow a null snapshot version is so that we // can synthesize remote events when we get permission denied errors while @@ -541,7 +545,10 @@ export class LocalStore { return PersistencePromise.waitFor(promises) .next(() => documentBuffer.apply(txn)) .next(() => { - return this.localDocuments.getLocalViewOfDocuments(txn, changedDocs); + return this.localDocuments.getLocalViewOfDocuments( + txn, + changedDocs + ); }); } ); diff --git a/packages/firestore/src/local/memory_remote_document_cache.ts b/packages/firestore/src/local/memory_remote_document_cache.ts index c21de8a82af..f821c24efa9 100644 --- a/packages/firestore/src/local/memory_remote_document_cache.ts +++ b/packages/firestore/src/local/memory_remote_document_cache.ts @@ -140,7 +140,7 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache { results = results.insert(documentKey, entry ? entry.maybeDocument : null); sizeMap = sizeMap.insert(documentKey, entry ? entry.size : 0); }); - return PersistencePromise.resolve({maybeDocuments: results, sizeMap}); + return PersistencePromise.resolve({ maybeDocuments: results, sizeMap }); } getDocumentsMatchingQuery( diff --git a/packages/firestore/src/model/document.ts b/packages/firestore/src/model/document.ts index 78e2f438a44..b617b21cd89 100644 --- a/packages/firestore/src/model/document.ts +++ b/packages/firestore/src/model/document.ts @@ -62,10 +62,10 @@ export class Document extends MaybeDocument { version: SnapshotVersion, readonly data: ObjectValue, options: DocumentOptions, -/** - * Memoized serialized form of the document for optimization purposes (avoids repeated - * serialization). Might be nil. - */ + /** + * Memoized serialized form of the document for optimization purposes (avoids repeated + * serialization). Might be nil. + */ readonly proto?: api.Document ) { super(key, version); diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index 74e8b1b2d04..e37e9b25687 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -728,7 +728,13 @@ export class JsonProtoSerializer { const fields = this.fromFields(entityChange.document!.fields || {}); // The document may soon be re-serialized back to protos in order to store it in local // persistence. Memoize the encoded form to avoid encoding it again. - const doc = new Document(key, version, fields, {}, entityChange.document!); + const doc = new Document( + key, + version, + fields, + {}, + entityChange.document! + ); const updatedTargetIds = entityChange.targetIds || []; const removedTargetIds = entityChange.removedTargetIds || []; watchChange = new DocumentWatchChange( From fb751dd03695b7bc3421a5300f3268d250b324cd Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 17 Dec 2018 21:33:22 -0500 Subject: [PATCH 15/29] small cleanup --- .../src/local/indexeddb_remote_document_cache.ts | 8 ++++++++ .../firestore/src/local/memory_remote_document_cache.ts | 8 +++++--- .../firestore/src/local/remote_document_change_buffer.ts | 2 -- packages/firestore/src/model/document.ts | 2 +- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index 04fa417a45a..a6f42b5acfd 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -192,6 +192,14 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { ); } + /** + * Looks up several entries in the cache. + * + * @param documentKeys The set of keys entries to look up. + * @return A map of MaybeDocuments indexed by key (if a document cannot be + * found, the key will be mapped to null) and a map of sizes indexed by + * key (zero if the key cannot be found). + */ getSizedEntries( transaction: PersistenceTransaction, documentKeys: DocumentKeySet diff --git a/packages/firestore/src/local/memory_remote_document_cache.ts b/packages/firestore/src/local/memory_remote_document_cache.ts index f821c24efa9..9e259d36d92 100644 --- a/packages/firestore/src/local/memory_remote_document_cache.ts +++ b/packages/firestore/src/local/memory_remote_document_cache.ts @@ -124,10 +124,12 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache { } /** - * Looks up an entry in the cache. + * Looks up several entries in the cache. * - * @param documentKey The key of the entry to look up. - * @return The cached MaybeDocument entry and its size, or null if we have nothing cached. + * @param documentKeys The set of keys entries to look up. + * @return A map of MaybeDocuments indexed by key (if a document cannot be + * found, the key will be mapped to null) and a map of sizes indexed by + * key (zero if the key cannot be found). */ getSizedEntries( transaction: PersistenceTransaction, diff --git a/packages/firestore/src/local/remote_document_change_buffer.ts b/packages/firestore/src/local/remote_document_change_buffer.ts index b82f86d41cb..c7dc97e82ef 100644 --- a/packages/firestore/src/local/remote_document_change_buffer.ts +++ b/packages/firestore/src/local/remote_document_change_buffer.ts @@ -124,8 +124,6 @@ export abstract class RemoteDocumentChangeBuffer { ): PersistencePromise { const changes = this.assertChanges(); - // TODO OBC: look up in the buffer - // Record the size of everything we load from the cache so we can compute a delta later. return this.getAllFromCache(transaction, documentKeys).next(getResult => { getResult.sizeMap.forEach((documentKey, size) => { diff --git a/packages/firestore/src/model/document.ts b/packages/firestore/src/model/document.ts index b617b21cd89..44f3134fa0d 100644 --- a/packages/firestore/src/model/document.ts +++ b/packages/firestore/src/model/document.ts @@ -64,7 +64,7 @@ export class Document extends MaybeDocument { options: DocumentOptions, /** * Memoized serialized form of the document for optimization purposes (avoids repeated - * serialization). Might be nil. + * serialization). Might be undefined. */ readonly proto?: api.Document ) { From 431f6183fa444fcfe57777e308caa36bc56faaee Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 17 Dec 2018 21:47:50 -0500 Subject: [PATCH 16/29] Fix accidental --- packages/firestore/src/local/indexeddb_remote_document_cache.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index a6f42b5acfd..8bc872a47cb 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -236,6 +236,7 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { control.done(); return; } + control.skip(key!.path.toArray()); } }) .next(() => { From 1681f3d0ce026a30b855ad3727101cdf1f1829dc Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 17 Dec 2018 21:56:33 -0500 Subject: [PATCH 17/29] Comment --- .../src/local/remote_document_change_buffer.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/firestore/src/local/remote_document_change_buffer.ts b/packages/firestore/src/local/remote_document_change_buffer.ts index c7dc97e82ef..4dff2e46735 100644 --- a/packages/firestore/src/local/remote_document_change_buffer.ts +++ b/packages/firestore/src/local/remote_document_change_buffer.ts @@ -108,15 +108,16 @@ export abstract class RemoteDocumentChangeBuffer { } /** - * Looks up an entry in the cache. The buffered changes will first be checked, - * and if no buffered change applies, this will forward to + * Looks up several entries in the cache. + * checked, and if no buffered change applies, this will forward to * `RemoteDocumentCache.getEntry()`. * * @param transaction The transaction in which to perform any persistence * operations. - * @param documentKey The key of the entry to look up. - * @return The cached Document or NoDocument entry, or null if we have nothing - * cached. + * @param documentKeys The keys of the entries to look up. + * @return A map of cached `Document`s or `NoDocument`s, indexed by key. If an + * entry cannot be found, the corresponding key will be mapped to a null + * value. */ getEntries( transaction: PersistenceTransaction, @@ -124,7 +125,8 @@ export abstract class RemoteDocumentChangeBuffer { ): PersistencePromise { const changes = this.assertChanges(); - // Record the size of everything we load from the cache so we can compute a delta later. + // Record the size of everything we load from the cache so we can compute + // a delta later. return this.getAllFromCache(transaction, documentKeys).next(getResult => { getResult.sizeMap.forEach((documentKey, size) => { this.documentSizes.set(documentKey, size); From 55c7ff3bccb19b1b72041790ef3d23fa8dc865ce Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Tue, 18 Dec 2018 22:54:29 -0500 Subject: [PATCH 18/29] Review feedback 1, test --- .../src/local/indexeddb_mutation_queue.ts | 3 +- .../src/local/local_documents_view.ts | 7 +--- .../firestore/src/local/local_serializer.ts | 7 +--- .../src/local/memory_mutation_queue.ts | 3 +- .../firestore/src/local/mutation_queue.ts | 3 +- packages/firestore/src/util/sorted_set.ts | 13 ++++--- .../unit/local/remote_document_cache.test.ts | 34 ++++++++++++++++++- .../unit/local/test_remote_document_cache.ts | 8 ++++- 8 files changed, 57 insertions(+), 21 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_mutation_queue.ts b/packages/firestore/src/local/indexeddb_mutation_queue.ts index 3ce89384fcd..9258475ba85 100644 --- a/packages/firestore/src/local/indexeddb_mutation_queue.ts +++ b/packages/firestore/src/local/indexeddb_mutation_queue.ts @@ -25,6 +25,7 @@ import { BATCHID_UNKNOWN, MutationBatch } from '../model/mutation_batch'; import { ResourcePath } from '../model/path'; import { assert, fail } from '../util/assert'; import { primitiveComparator } from '../util/misc'; +import { SortedMap } from '../util/sorted_map'; import { SortedSet } from '../util/sorted_set'; import * as EncodedResourcePath from './encoded_resource_path'; @@ -325,7 +326,7 @@ export class IndexedDbMutationQueue implements MutationQueue { getAllMutationBatchesAffectingDocumentKeys( transaction: PersistenceTransaction, - documentKeys: DocumentKeySet + documentKeys: SortedMap ): PersistencePromise { let uniqueBatchIDs = new SortedSet(primitiveComparator); diff --git a/packages/firestore/src/local/local_documents_view.ts b/packages/firestore/src/local/local_documents_view.ts index 6e3c6a9ef3c..ca88c4de8ff 100644 --- a/packages/firestore/src/local/local_documents_view.ts +++ b/packages/firestore/src/local/local_documents_view.ts @@ -119,13 +119,8 @@ export class LocalDocumentsView { transaction: PersistenceTransaction, baseDocs: NullableMaybeDocumentMap ): PersistencePromise { - let allKeys = documentKeySet(); - baseDocs.forEach(key => { - allKeys = allKeys.add(key); - }); - return this.mutationQueue - .getAllMutationBatchesAffectingDocumentKeys(transaction, allKeys) + .getAllMutationBatchesAffectingDocumentKeys(transaction, baseDocs) .next(batches => this.applyLocalMutationsToDocuments(transaction, baseDocs, batches) ) diff --git a/packages/firestore/src/local/local_serializer.ts b/packages/firestore/src/local/local_serializer.ts index c6c34b325df..6b22362b461 100644 --- a/packages/firestore/src/local/local_serializer.ts +++ b/packages/firestore/src/local/local_serializer.ts @@ -71,12 +71,7 @@ export class LocalSerializer { /** Encodes a document for storage locally. */ toDbRemoteDocument(maybeDoc: MaybeDocument): DbRemoteDocument { if (maybeDoc instanceof Document) { - let doc: api.Document; - if (maybeDoc.proto) { - doc = maybeDoc.proto; - } else { - doc = this.remoteSerializer.toDocument(maybeDoc); - } + const doc = maybeDoc.proto ? maybeDoc.proto : this.remoteSerializer.toDocument(maybeDoc); const hasCommittedMutations = maybeDoc.hasCommittedMutations; return new DbRemoteDocument( /* unknownDocument= */ null, diff --git a/packages/firestore/src/local/memory_mutation_queue.ts b/packages/firestore/src/local/memory_mutation_queue.ts index 442b57ec51c..1d130a80a94 100644 --- a/packages/firestore/src/local/memory_mutation_queue.ts +++ b/packages/firestore/src/local/memory_mutation_queue.ts @@ -24,6 +24,7 @@ import { BATCHID_UNKNOWN, MutationBatch } from '../model/mutation_batch'; import { emptyByteString } from '../platform/platform'; import { assert } from '../util/assert'; import { primitiveComparator } from '../util/misc'; +import { SortedMap } from '../util/sorted_map'; import { SortedSet } from '../util/sorted_set'; import { MutationQueue } from './mutation_queue'; @@ -203,7 +204,7 @@ export class MemoryMutationQueue implements MutationQueue { getAllMutationBatchesAffectingDocumentKeys( transaction: PersistenceTransaction, - documentKeys: DocumentKeySet + documentKeys: SortedMap ): PersistencePromise { let uniqueBatchIDs = new SortedSet(primitiveComparator); diff --git a/packages/firestore/src/local/mutation_queue.ts b/packages/firestore/src/local/mutation_queue.ts index 91c48ed4e49..8c9ffba8e22 100644 --- a/packages/firestore/src/local/mutation_queue.ts +++ b/packages/firestore/src/local/mutation_queue.ts @@ -21,6 +21,7 @@ import { DocumentKeySet } from '../model/collections'; import { DocumentKey } from '../model/document_key'; import { Mutation } from '../model/mutation'; import { MutationBatch } from '../model/mutation_batch'; +import { SortedMap } from '../util/sorted_map'; import { PersistenceTransaction } from './persistence'; import { PersistencePromise } from './persistence_promise'; @@ -133,7 +134,7 @@ export interface MutationQueue { // TODO(mcg): This should really return an enumerator getAllMutationBatchesAffectingDocumentKeys( transaction: PersistenceTransaction, - documentKeys: DocumentKeySet + documentKeys: SortedMap ): PersistencePromise; /** diff --git a/packages/firestore/src/util/sorted_set.ts b/packages/firestore/src/util/sorted_set.ts index ea86626ece3..2b0aa3fb601 100644 --- a/packages/firestore/src/util/sorted_set.ts +++ b/packages/firestore/src/util/sorted_set.ts @@ -103,11 +103,16 @@ export class SortedSet { if (!iter.hasNext()) { return null; } - if (this.comparator(iter.peek()!.key, elem) == 0) { - // Skip - iter.getNext(); + + const next = iter.getNext(); + if (this.comparator(next.key, elem) !== 0) { + return next.key; } - return iter.hasNext() ? iter.getNext().key : null; + if (iter.hasNext()) { + return iter.getNext().key; + } + + return null; } /** Finds the least element greater than or equal to `elem`. */ 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 84d6dd418c7..4944ee063db 100644 --- a/packages/firestore/test/unit/local/remote_document_cache.test.ts +++ b/packages/firestore/test/unit/local/remote_document_cache.test.ts @@ -38,7 +38,7 @@ import { DbRemoteDocumentChanges, DbRemoteDocumentChangesKey } from '../../../src/local/indexeddb_schema'; -import { MaybeDocumentMap } from '../../../src/model/collections'; +import { documentKeySet, MaybeDocumentMap } from '../../../src/model/collections'; import { fail } from '../../../src/util/assert'; import * as persistenceHelpers from './persistence_test_helpers'; import { @@ -326,6 +326,38 @@ function genericRemoteDocumentCacheTests( }); }); + it('can set and read several documents', () => { + const docs = [doc(DOC_PATH, VERSION, DOC_DATA), doc(LONG_DOC_PATH, VERSION, DOC_DATA)]; + const key1 = key(DOC_PATH); + const key2 = key(LONG_DOC_PATH); + return cache + .addEntries(docs) + .then(() => { + return cache.getEntries(documentKeySet().add(key1).add(key2)); + }) + .then(read => { + expectEqual(read.get(key1), docs[0]); + expectEqual(read.get(key2), docs[1]); + }); + }); + + it('can set and read several documents including missing document', () => { + const docs = [doc(DOC_PATH, VERSION, DOC_DATA), doc(LONG_DOC_PATH, VERSION, DOC_DATA)]; + const key1 = key(DOC_PATH); + const key2 = key(LONG_DOC_PATH); + const missing_key = key('foo/nonexistent'); + return cache + .addEntries(docs) + .then(() => { + return cache.getEntries(documentKeySet().add(key1).add(key2).add(missing_key)); + }) + .then(read => { + expectEqual(read.get(key1), docs[0]); + expectEqual(read.get(key2), docs[1]); + expect(read.get(missing_key)).to.be.null; + }); + }); + it('can remove document', () => { return cache .addEntries([doc(DOC_PATH, VERSION, DOC_DATA)]) 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 8298cf3034e..8448607c1b4 100644 --- a/packages/firestore/test/unit/local/test_remote_document_cache.ts +++ b/packages/firestore/test/unit/local/test_remote_document_cache.ts @@ -26,7 +26,7 @@ import { import { PersistencePromise } from '../../../src/local/persistence_promise'; import { RemoteDocumentCache } from '../../../src/local/remote_document_cache'; import { RemoteDocumentChangeBuffer } from '../../../src/local/remote_document_change_buffer'; -import { DocumentMap, MaybeDocumentMap } from '../../../src/model/collections'; +import { DocumentKeySet, DocumentMap, MaybeDocumentMap, NullableMaybeDocumentMap } from '../../../src/model/collections'; import { MaybeDocument } from '../../../src/model/document'; import { DocumentKey } from '../../../src/model/document_key'; @@ -83,6 +83,12 @@ export abstract class TestRemoteDocumentCache { }); } + getEntries(documentKeys: DocumentKeySet): Promise { + return this.persistence.runTransaction('getEntries', 'readonly', txn => { + return this.cache.getEntries(txn, documentKeys); + }); + } + getDocumentsMatchingQuery(query: Query): Promise { return this.persistence.runTransaction( 'getDocumentsMatchingQuery', From 5afa305be8dbbc4102ed40fe34a003fdc1d9205b Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Tue, 18 Dec 2018 22:55:05 -0500 Subject: [PATCH 19/29] [AUTOMATED]: Prettier Code Styling --- .../firestore/src/local/local_serializer.ts | 4 ++- .../unit/local/remote_document_cache.test.ts | 28 +++++++++++++++---- .../unit/local/test_remote_document_cache.ts | 7 ++++- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/packages/firestore/src/local/local_serializer.ts b/packages/firestore/src/local/local_serializer.ts index 6b22362b461..13532e34aa8 100644 --- a/packages/firestore/src/local/local_serializer.ts +++ b/packages/firestore/src/local/local_serializer.ts @@ -71,7 +71,9 @@ export class LocalSerializer { /** Encodes a document for storage locally. */ toDbRemoteDocument(maybeDoc: MaybeDocument): DbRemoteDocument { if (maybeDoc instanceof Document) { - const doc = maybeDoc.proto ? maybeDoc.proto : this.remoteSerializer.toDocument(maybeDoc); + const doc = maybeDoc.proto + ? maybeDoc.proto + : this.remoteSerializer.toDocument(maybeDoc); const hasCommittedMutations = maybeDoc.hasCommittedMutations; return new DbRemoteDocument( /* unknownDocument= */ null, 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 4944ee063db..12a4074ef1f 100644 --- a/packages/firestore/test/unit/local/remote_document_cache.test.ts +++ b/packages/firestore/test/unit/local/remote_document_cache.test.ts @@ -38,7 +38,10 @@ import { DbRemoteDocumentChanges, DbRemoteDocumentChangesKey } from '../../../src/local/indexeddb_schema'; -import { documentKeySet, MaybeDocumentMap } from '../../../src/model/collections'; +import { + documentKeySet, + MaybeDocumentMap +} from '../../../src/model/collections'; import { fail } from '../../../src/util/assert'; import * as persistenceHelpers from './persistence_test_helpers'; import { @@ -327,13 +330,20 @@ function genericRemoteDocumentCacheTests( }); it('can set and read several documents', () => { - const docs = [doc(DOC_PATH, VERSION, DOC_DATA), doc(LONG_DOC_PATH, VERSION, DOC_DATA)]; + const docs = [ + doc(DOC_PATH, VERSION, DOC_DATA), + doc(LONG_DOC_PATH, VERSION, DOC_DATA) + ]; const key1 = key(DOC_PATH); const key2 = key(LONG_DOC_PATH); return cache .addEntries(docs) .then(() => { - return cache.getEntries(documentKeySet().add(key1).add(key2)); + return cache.getEntries( + documentKeySet() + .add(key1) + .add(key2) + ); }) .then(read => { expectEqual(read.get(key1), docs[0]); @@ -342,14 +352,22 @@ function genericRemoteDocumentCacheTests( }); it('can set and read several documents including missing document', () => { - const docs = [doc(DOC_PATH, VERSION, DOC_DATA), doc(LONG_DOC_PATH, VERSION, DOC_DATA)]; + const docs = [ + doc(DOC_PATH, VERSION, DOC_DATA), + doc(LONG_DOC_PATH, VERSION, DOC_DATA) + ]; const key1 = key(DOC_PATH); const key2 = key(LONG_DOC_PATH); const missing_key = key('foo/nonexistent'); return cache .addEntries(docs) .then(() => { - return cache.getEntries(documentKeySet().add(key1).add(key2).add(missing_key)); + return cache.getEntries( + documentKeySet() + .add(key1) + .add(key2) + .add(missing_key) + ); }) .then(read => { expectEqual(read.get(key1), docs[0]); 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 8448607c1b4..bfe49a40757 100644 --- a/packages/firestore/test/unit/local/test_remote_document_cache.ts +++ b/packages/firestore/test/unit/local/test_remote_document_cache.ts @@ -26,7 +26,12 @@ import { import { PersistencePromise } from '../../../src/local/persistence_promise'; import { RemoteDocumentCache } from '../../../src/local/remote_document_cache'; import { RemoteDocumentChangeBuffer } from '../../../src/local/remote_document_change_buffer'; -import { DocumentKeySet, DocumentMap, MaybeDocumentMap, NullableMaybeDocumentMap } from '../../../src/model/collections'; +import { + DocumentKeySet, + DocumentMap, + MaybeDocumentMap, + NullableMaybeDocumentMap +} from '../../../src/model/collections'; import { MaybeDocument } from '../../../src/model/document'; import { DocumentKey } from '../../../src/model/document_key'; From 56eefbcca23f2adde92da28f4474c8ab06b29007 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Thu, 20 Dec 2018 16:55:32 -0500 Subject: [PATCH 20/29] Review feedback 1 --- .../local/indexeddb_remote_document_cache.ts | 53 +++++++++++-------- .../src/local/local_documents_view.ts | 20 +++---- .../local/remote_document_change_buffer.ts | 9 ++-- packages/firestore/src/util/sorted_set.ts | 39 +++++++------- 4 files changed, 64 insertions(+), 57 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index 8bc872a47cb..34c9fc7cf3c 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -214,36 +214,45 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { documentKeys.first()!.path.toArray(), documentKeys.last()!.path.toArray() ); - let key = documentKeys.first(); + const keyIter = documentKeys.getIterator(); + let nextKey = keyIter.getNext(); return remoteDocumentsStore(transaction) .iterate({ range }, (potentialKeyRaw, dbRemoteDoc, control) => { const potentialKey = DocumentKey.fromSegments(potentialKeyRaw); - while (DocumentKey.comparator(key!, potentialKey) != 1) { - if (key!.isEqual(potentialKey)) { - results = results.insert( - key!, - this.serializer.fromDbRemoteDocument(dbRemoteDoc) - ); - sizeMap = sizeMap.insert(key!, dbDocumentSize(dbRemoteDoc)); - } else { - results = results.insert(key!, null); - sizeMap = sizeMap.insert(key!, 0); - } - key = documentKeys.firstAfter(key!); - if (!key) { - control.done(); - return; - } - control.skip(key!.path.toArray()); + let compResult = 0; + // Go through keys not found in cache. + for (compResult = DocumentKey.comparator(nextKey!, potentialKey); compResult < 0;) { + results = results.insert(nextKey!, null); + sizeMap = sizeMap.insert(nextKey!, 0); + nextKey = keyIter.hasNext() ? keyIter.getNext() : null; + } + + if (nextKey && nextKey!.isEqual(potentialKey)) { + // Key found in cache. + results = results.insert( + nextKey!, + this.serializer.fromDbRemoteDocument(dbRemoteDoc) + ); + sizeMap = sizeMap.insert(nextKey!, dbDocumentSize(dbRemoteDoc)); + nextKey = keyIter.hasNext() ? keyIter.getNext() : null; + } + + // Skip to the next key (if there is one). + if (nextKey) { + control.skip(nextKey!.path.toArray()); + } else { + control.done(); } }) .next(() => { - while (key) { - results = results.insert(key, null); - sizeMap = sizeMap.insert(key, 0); - key = documentKeys.firstAfter(key!); + // The rest of the keys are not in the cache. One case where `iterate` + // above won't go through them is when the cache is empty. + while (nextKey) { + results = results.insert(nextKey, null); + sizeMap = sizeMap.insert(nextKey, 0); + nextKey = keyIter.hasNext() ? keyIter.getNext() : null; } return { maybeDocuments: results, sizeMap }; }); diff --git a/packages/firestore/src/local/local_documents_view.ts b/packages/firestore/src/local/local_documents_view.ts index ca88c4de8ff..4b39ec60d9a 100644 --- a/packages/firestore/src/local/local_documents_view.ts +++ b/packages/firestore/src/local/local_documents_view.ts @@ -83,17 +83,15 @@ export class LocalDocumentsView { transaction: PersistenceTransaction, docs: NullableMaybeDocumentMap, batches: MutationBatch[] - ): PersistencePromise { + ): NullableMaybeDocumentMap { let results = nullableMaybeDocumentMap(); - return new PersistencePromise(resolve => { - docs.forEach((key, localView) => { - for (const batch of batches) { - localView = batch.applyToLocalView(key, localView); - } - results = results.insert(key, localView); - }); - resolve(results); + docs.forEach((key, localView) => { + for (const batch of batches) { + localView = batch.applyToLocalView(key, localView); + } + results = results.insert(key, localView); }); + return result; } /** @@ -122,9 +120,7 @@ export class LocalDocumentsView { return this.mutationQueue .getAllMutationBatchesAffectingDocumentKeys(transaction, baseDocs) .next(batches => - this.applyLocalMutationsToDocuments(transaction, baseDocs, batches) - ) - .next(docs => { + const docs = this.applyLocalMutationsToDocuments(transaction, baseDocs, batches); let results = maybeDocumentMap(); docs.forEach((key, maybeDoc) => { // TODO(http://b/32275378): Don't conflate missing / deleted. diff --git a/packages/firestore/src/local/remote_document_change_buffer.ts b/packages/firestore/src/local/remote_document_change_buffer.ts index 4dff2e46735..9350e514b48 100644 --- a/packages/firestore/src/local/remote_document_change_buffer.ts +++ b/packages/firestore/src/local/remote_document_change_buffer.ts @@ -108,8 +108,7 @@ export abstract class RemoteDocumentChangeBuffer { } /** - * Looks up several entries in the cache. - * checked, and if no buffered change applies, this will forward to + * Looks up several entries in the cache, orwarding to * `RemoteDocumentCache.getEntry()`. * * @param transaction The transaction in which to perform any persistence @@ -127,11 +126,11 @@ export abstract class RemoteDocumentChangeBuffer { // Record the size of everything we load from the cache so we can compute // a delta later. - return this.getAllFromCache(transaction, documentKeys).next(getResult => { - getResult.sizeMap.forEach((documentKey, size) => { + return this.getAllFromCache(transaction, documentKeys).next((maybeDocuments, sizeMap) => { + sizeMap.forEach((documentKey, size) => { this.documentSizes.set(documentKey, size); }); - return getResult.maybeDocuments; + return maybeDocuments; }); } diff --git a/packages/firestore/src/util/sorted_set.ts b/packages/firestore/src/util/sorted_set.ts index 2b0aa3fb601..266aea3cdc3 100644 --- a/packages/firestore/src/util/sorted_set.ts +++ b/packages/firestore/src/util/sorted_set.ts @@ -97,30 +97,20 @@ export class SortedSet { } } - /** Finds the least element greater than `elem`. */ - firstAfter(elem: T): T | null { - const iter = this.data.getIteratorFrom(elem); - if (!iter.hasNext()) { - return null; - } - - const next = iter.getNext(); - if (this.comparator(next.key, elem) !== 0) { - return next.key; - } - if (iter.hasNext()) { - return iter.getNext().key; - } - - return null; - } - /** Finds the least element greater than or equal to `elem`. */ firstAfterOrEqual(elem: T): T | null { const iter = this.data.getIteratorFrom(elem); return iter.hasNext() ? iter.getNext().key : null; } + getIterator(): SortedSetIterator { + return new SortedSetIterator(this.data.getIterator()); + } + + getIteratorFrom(key: T): SortedSetIterator { + return new SortedSetIterator(this.data.getIteratorFrom(key)); + } + /** Inserts or updates an element */ add(elem: T): SortedSet { return this.copy(this.data.remove(elem).insert(elem, true)); @@ -178,3 +168,16 @@ export class SortedSet { return result; } } + +export class SortedSetIterator { + constructor(private iter: SortedMapIterator) { + } + + getNext(): T { + return this.iter.getNext().key; + } + + hasNext(): boolean { + return this.iter.hasNext(); + } +} From 5e506fa2abc9061ec2b741cd4d0d3e9884c98d73 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Thu, 20 Dec 2018 17:42:44 -0500 Subject: [PATCH 21/29] Review feedback 2 --- .../local/indexeddb_remote_document_cache.ts | 68 ++++++++++++------- .../src/local/local_documents_view.ts | 4 +- .../local/remote_document_change_buffer.ts | 2 +- 3 files changed, 48 insertions(+), 26 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index 34c9fc7cf3c..8493fbae76e 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -20,16 +20,16 @@ import { documentKeySet, DocumentMap, documentMap, - DocumentSizeEntry, DocumentSizeEntries, - nullableMaybeDocumentMap, - NullableMaybeDocumentMap, + DocumentSizeEntry, MaybeDocumentMap, - maybeDocumentMap + maybeDocumentMap, + nullableMaybeDocumentMap, + NullableMaybeDocumentMap } from '../model/collections'; import { Document, MaybeDocument, NoDocument } from '../model/document'; -import { SortedMap } from '../util/sorted_map'; import { DocumentKey } from '../model/document_key'; +import { SortedMap } from '../util/sorted_map'; import { SnapshotVersion } from '../core/snapshot_version'; import { assert, fail } from '../util/assert'; @@ -187,9 +187,17 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { transaction: PersistenceTransaction, documentKeys: DocumentKeySet ): PersistencePromise { - return this.getSizedEntries(transaction, documentKeys).next( - result => result.maybeDocuments - ); + let results = nullableMaybeDocumentMap(); + return this.forEachDbEntry(transaction, documentKeys, (key, dbRemoteDoc) => { + if (dbRemoteDoc) { + results = results.insert( + key, + this.serializer.fromDbRemoteDocument(dbRemoteDoc) + ); + } else { + results = results.insert(key, null); + } + }).next(() => results); } /** @@ -206,8 +214,30 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { ): PersistencePromise { let results = nullableMaybeDocumentMap(); let sizeMap = new SortedMap(DocumentKey.comparator); + return this.forEachDbEntry(transaction, documentKeys, (key, dbRemoteDoc) => { + if (dbRemoteDoc) { + results = results.insert( + key, + this.serializer.fromDbRemoteDocument(dbRemoteDoc) + ); + sizeMap = sizeMap.insert(key, dbDocumentSize(dbRemoteDoc)); + } else { + results = results.insert(key, null); + sizeMap = sizeMap.insert(key, 0); + } + }) + .next(() => { + return { maybeDocuments:results, sizeMap }; + }); + } + + forEachDbEntry( + transaction: PersistenceTransaction, + documentKeys: DocumentKeySet, + callback: (key: DocumentKey, doc: DbRemoteDocument | null) => void + ) : PersistencePromise { if (documentKeys.isEmpty()) { - return PersistencePromise.resolve({ maybeDocuments: results, sizeMap }); + return PersistencePromise.resolve(); } const range = IDBKeyRange.bound( @@ -215,27 +245,21 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { documentKeys.last()!.path.toArray() ); const keyIter = documentKeys.getIterator(); - let nextKey = keyIter.getNext(); + let nextKey: DocumentKey | null = keyIter.getNext(); return remoteDocumentsStore(transaction) .iterate({ range }, (potentialKeyRaw, dbRemoteDoc, control) => { const potentialKey = DocumentKey.fromSegments(potentialKeyRaw); - let compResult = 0; // Go through keys not found in cache. - for (compResult = DocumentKey.comparator(nextKey!, potentialKey); compResult < 0;) { - results = results.insert(nextKey!, null); - sizeMap = sizeMap.insert(nextKey!, 0); - nextKey = keyIter.hasNext() ? keyIter.getNext() : null; + while (nextKey && DocumentKey.comparator(nextKey!, potentialKey) < 0) { + callback(nextKey!, null); + nextKey = keyIter.getNext(); } if (nextKey && nextKey!.isEqual(potentialKey)) { // Key found in cache. - results = results.insert( - nextKey!, - this.serializer.fromDbRemoteDocument(dbRemoteDoc) - ); - sizeMap = sizeMap.insert(nextKey!, dbDocumentSize(dbRemoteDoc)); + callback(nextKey!, dbRemoteDoc); nextKey = keyIter.hasNext() ? keyIter.getNext() : null; } @@ -250,11 +274,9 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { // The rest of the keys are not in the cache. One case where `iterate` // above won't go through them is when the cache is empty. while (nextKey) { - results = results.insert(nextKey, null); - sizeMap = sizeMap.insert(nextKey, 0); + callback(nextKey!, null); nextKey = keyIter.hasNext() ? keyIter.getNext() : null; } - return { maybeDocuments: results, sizeMap }; }); } diff --git a/packages/firestore/src/local/local_documents_view.ts b/packages/firestore/src/local/local_documents_view.ts index 4b39ec60d9a..1cdbe0f645f 100644 --- a/packages/firestore/src/local/local_documents_view.ts +++ b/packages/firestore/src/local/local_documents_view.ts @@ -91,7 +91,7 @@ export class LocalDocumentsView { } results = results.insert(key, localView); }); - return result; + return results; } /** @@ -119,7 +119,7 @@ export class LocalDocumentsView { ): PersistencePromise { return this.mutationQueue .getAllMutationBatchesAffectingDocumentKeys(transaction, baseDocs) - .next(batches => + .next(batches => { const docs = this.applyLocalMutationsToDocuments(transaction, baseDocs, batches); let results = maybeDocumentMap(); docs.forEach((key, maybeDoc) => { diff --git a/packages/firestore/src/local/remote_document_change_buffer.ts b/packages/firestore/src/local/remote_document_change_buffer.ts index 9350e514b48..f80ab4b2352 100644 --- a/packages/firestore/src/local/remote_document_change_buffer.ts +++ b/packages/firestore/src/local/remote_document_change_buffer.ts @@ -126,7 +126,7 @@ export abstract class RemoteDocumentChangeBuffer { // Record the size of everything we load from the cache so we can compute // a delta later. - return this.getAllFromCache(transaction, documentKeys).next((maybeDocuments, sizeMap) => { + return this.getAllFromCache(transaction, documentKeys).next(({maybeDocuments, sizeMap}) => { sizeMap.forEach((documentKey, size) => { this.documentSizes.set(documentKey, size); }); From 7b11cec132089642d42edee76dc3051b65ba0fe6 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Thu, 20 Dec 2018 17:43:31 -0500 Subject: [PATCH 22/29] [AUTOMATED]: Prettier Code Styling --- .../local/indexeddb_remote_document_cache.ts | 33 +++++++++++-------- .../src/local/local_documents_view.ts | 8 +++-- .../local/remote_document_change_buffer.ts | 14 ++++---- packages/firestore/src/util/sorted_set.ts | 3 +- 4 files changed, 35 insertions(+), 23 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index 8493fbae76e..da14e8c9832 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -188,16 +188,20 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { documentKeys: DocumentKeySet ): PersistencePromise { let results = nullableMaybeDocumentMap(); - return this.forEachDbEntry(transaction, documentKeys, (key, dbRemoteDoc) => { - if (dbRemoteDoc) { + return this.forEachDbEntry( + transaction, + documentKeys, + (key, dbRemoteDoc) => { + if (dbRemoteDoc) { results = results.insert( key, this.serializer.fromDbRemoteDocument(dbRemoteDoc) ); - } else { + } else { results = results.insert(key, null); + } } - }).next(() => results); + ).next(() => results); } /** @@ -214,20 +218,23 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { ): PersistencePromise { let results = nullableMaybeDocumentMap(); let sizeMap = new SortedMap(DocumentKey.comparator); - return this.forEachDbEntry(transaction, documentKeys, (key, dbRemoteDoc) => { - if (dbRemoteDoc) { + return this.forEachDbEntry( + transaction, + documentKeys, + (key, dbRemoteDoc) => { + if (dbRemoteDoc) { results = results.insert( key, this.serializer.fromDbRemoteDocument(dbRemoteDoc) ); - sizeMap = sizeMap.insert(key, dbDocumentSize(dbRemoteDoc)); - } else { + sizeMap = sizeMap.insert(key, dbDocumentSize(dbRemoteDoc)); + } else { results = results.insert(key, null); - sizeMap = sizeMap.insert(key, 0); + sizeMap = sizeMap.insert(key, 0); + } } - }) - .next(() => { - return { maybeDocuments:results, sizeMap }; + ).next(() => { + return { maybeDocuments: results, sizeMap }; }); } @@ -235,7 +242,7 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { transaction: PersistenceTransaction, documentKeys: DocumentKeySet, callback: (key: DocumentKey, doc: DbRemoteDocument | null) => void - ) : PersistencePromise { + ): PersistencePromise { if (documentKeys.isEmpty()) { return PersistencePromise.resolve(); } diff --git a/packages/firestore/src/local/local_documents_view.ts b/packages/firestore/src/local/local_documents_view.ts index 1cdbe0f645f..b0153a67903 100644 --- a/packages/firestore/src/local/local_documents_view.ts +++ b/packages/firestore/src/local/local_documents_view.ts @@ -119,8 +119,12 @@ export class LocalDocumentsView { ): PersistencePromise { return this.mutationQueue .getAllMutationBatchesAffectingDocumentKeys(transaction, baseDocs) - .next(batches => { - const docs = this.applyLocalMutationsToDocuments(transaction, baseDocs, batches); + .next(batches => { + const docs = this.applyLocalMutationsToDocuments( + transaction, + baseDocs, + batches + ); let results = maybeDocumentMap(); docs.forEach((key, maybeDoc) => { // TODO(http://b/32275378): Don't conflate missing / deleted. diff --git a/packages/firestore/src/local/remote_document_change_buffer.ts b/packages/firestore/src/local/remote_document_change_buffer.ts index f80ab4b2352..b39653d5206 100644 --- a/packages/firestore/src/local/remote_document_change_buffer.ts +++ b/packages/firestore/src/local/remote_document_change_buffer.ts @@ -126,12 +126,14 @@ export abstract class RemoteDocumentChangeBuffer { // Record the size of everything we load from the cache so we can compute // a delta later. - return this.getAllFromCache(transaction, documentKeys).next(({maybeDocuments, sizeMap}) => { - sizeMap.forEach((documentKey, size) => { - this.documentSizes.set(documentKey, size); - }); - return maybeDocuments; - }); + return this.getAllFromCache(transaction, documentKeys).next( + ({ maybeDocuments, sizeMap }) => { + sizeMap.forEach((documentKey, size) => { + this.documentSizes.set(documentKey, size); + }); + return maybeDocuments; + } + ); } /** diff --git a/packages/firestore/src/util/sorted_set.ts b/packages/firestore/src/util/sorted_set.ts index 266aea3cdc3..b834df6a355 100644 --- a/packages/firestore/src/util/sorted_set.ts +++ b/packages/firestore/src/util/sorted_set.ts @@ -170,8 +170,7 @@ export class SortedSet { } export class SortedSetIterator { - constructor(private iter: SortedMapIterator) { - } + constructor(private iter: SortedMapIterator) {} getNext(): T { return this.iter.getNext().key; From 1a32b222fb5739ea5f2ffdba306441898c783f4c Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Thu, 20 Dec 2018 20:32:46 -0500 Subject: [PATCH 23/29] Review feedback 3 --- packages/firestore/src/local/indexeddb_mutation_queue.ts | 4 +++- packages/firestore/src/local/memory_mutation_queue.ts | 4 +++- packages/firestore/src/local/mutation_queue.ts | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_mutation_queue.ts b/packages/firestore/src/local/indexeddb_mutation_queue.ts index 9258475ba85..618bb36101e 100644 --- a/packages/firestore/src/local/indexeddb_mutation_queue.ts +++ b/packages/firestore/src/local/indexeddb_mutation_queue.ts @@ -47,6 +47,8 @@ import { PersistenceTransaction, ReferenceDelegate } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { SimpleDbStore, SimpleDbTransaction } from './simple_db'; +import { AnyJs } from '../../src/util/misc'; + /** A mutation queue for a specific user, backed by IndexedDB. */ export class IndexedDbMutationQueue implements MutationQueue { /** @@ -326,7 +328,7 @@ export class IndexedDbMutationQueue implements MutationQueue { getAllMutationBatchesAffectingDocumentKeys( transaction: PersistenceTransaction, - documentKeys: SortedMap + documentKeys: SortedMap ): PersistencePromise { let uniqueBatchIDs = new SortedSet(primitiveComparator); diff --git a/packages/firestore/src/local/memory_mutation_queue.ts b/packages/firestore/src/local/memory_mutation_queue.ts index 1d130a80a94..89e338874a7 100644 --- a/packages/firestore/src/local/memory_mutation_queue.ts +++ b/packages/firestore/src/local/memory_mutation_queue.ts @@ -32,6 +32,8 @@ import { PersistenceTransaction, ReferenceDelegate } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { DocReference } from './reference_set'; +import { AnyJs } from '../../src/util/misc'; + export class MemoryMutationQueue implements MutationQueue { /** * The set of all mutations that have been sent but not yet been applied to @@ -204,7 +206,7 @@ export class MemoryMutationQueue implements MutationQueue { getAllMutationBatchesAffectingDocumentKeys( transaction: PersistenceTransaction, - documentKeys: SortedMap + documentKeys: SortedMap ): PersistencePromise { let uniqueBatchIDs = new SortedSet(primitiveComparator); diff --git a/packages/firestore/src/local/mutation_queue.ts b/packages/firestore/src/local/mutation_queue.ts index 8c9ffba8e22..2633873586c 100644 --- a/packages/firestore/src/local/mutation_queue.ts +++ b/packages/firestore/src/local/mutation_queue.ts @@ -26,6 +26,8 @@ import { SortedMap } from '../util/sorted_map'; import { PersistenceTransaction } from './persistence'; import { PersistencePromise } from './persistence_promise'; +import { AnyJs } from '../../src/util/misc'; + /** A queue of mutations to apply to the remote store. */ export interface MutationQueue { /** Returns true if this queue contains no mutation batches. */ @@ -134,7 +136,7 @@ export interface MutationQueue { // TODO(mcg): This should really return an enumerator getAllMutationBatchesAffectingDocumentKeys( transaction: PersistenceTransaction, - documentKeys: SortedMap + documentKeys: SortedMap ): PersistencePromise; /** From 17a69a6524541b161c4c1e63c0b47ac813b4d230 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 21 Dec 2018 20:52:43 -0500 Subject: [PATCH 24/29] Review feedback --- packages/firestore/src/local/indexeddb_remote_document_cache.ts | 2 +- packages/firestore/src/local/remote_document_change_buffer.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/firestore/src/local/indexeddb_remote_document_cache.ts b/packages/firestore/src/local/indexeddb_remote_document_cache.ts index da14e8c9832..87114e713ec 100644 --- a/packages/firestore/src/local/indexeddb_remote_document_cache.ts +++ b/packages/firestore/src/local/indexeddb_remote_document_cache.ts @@ -238,7 +238,7 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache { }); } - forEachDbEntry( + private forEachDbEntry( transaction: PersistenceTransaction, documentKeys: DocumentKeySet, callback: (key: DocumentKey, doc: DbRemoteDocument | null) => void diff --git a/packages/firestore/src/local/remote_document_change_buffer.ts b/packages/firestore/src/local/remote_document_change_buffer.ts index b39653d5206..25020318c96 100644 --- a/packages/firestore/src/local/remote_document_change_buffer.ts +++ b/packages/firestore/src/local/remote_document_change_buffer.ts @@ -108,7 +108,7 @@ export abstract class RemoteDocumentChangeBuffer { } /** - * Looks up several entries in the cache, orwarding to + * Looks up several entries in the cache, forwarding to * `RemoteDocumentCache.getEntry()`. * * @param transaction The transaction in which to perform any persistence From 4c270c7635175f6bdee2a23de0c0d0bd49ec5c19 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 21 Dec 2018 21:01:01 -0500 Subject: [PATCH 25/29] Appease linter --- packages/firestore/src/local/local_documents_view.ts | 1 - .../firestore/src/local/memory_remote_document_cache.ts | 8 ++++---- packages/firestore/src/local/remote_document_cache.ts | 4 ++-- .../firestore/src/local/remote_document_change_buffer.ts | 8 +++----- .../test/unit/local/remote_document_cache.test.ts | 6 +++--- 5 files changed, 12 insertions(+), 15 deletions(-) diff --git a/packages/firestore/src/local/local_documents_view.ts b/packages/firestore/src/local/local_documents_view.ts index b0153a67903..9b5e77f28e0 100644 --- a/packages/firestore/src/local/local_documents_view.ts +++ b/packages/firestore/src/local/local_documents_view.ts @@ -17,7 +17,6 @@ import { Query } from '../core/query'; import { SnapshotVersion } from '../core/snapshot_version'; import { - documentKeySet, DocumentKeySet, DocumentMap, documentMap, diff --git a/packages/firestore/src/local/memory_remote_document_cache.ts b/packages/firestore/src/local/memory_remote_document_cache.ts index 9e259d36d92..fce6202fdf7 100644 --- a/packages/firestore/src/local/memory_remote_document_cache.ts +++ b/packages/firestore/src/local/memory_remote_document_cache.ts @@ -20,12 +20,12 @@ import { documentKeySet, DocumentMap, documentMap, - DocumentSizeEntry, DocumentSizeEntries, - MaybeDocumentMap, - maybeDocumentMap, + DocumentSizeEntry, NullableMaybeDocumentMap, - nullableMaybeDocumentMap + nullableMaybeDocumentMap, + MaybeDocumentMap, + maybeDocumentMap } from '../model/collections'; import { Document, MaybeDocument, NoDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; diff --git a/packages/firestore/src/local/remote_document_cache.ts b/packages/firestore/src/local/remote_document_cache.ts index 8cd41f4bbf3..2709e99a601 100644 --- a/packages/firestore/src/local/remote_document_cache.ts +++ b/packages/firestore/src/local/remote_document_cache.ts @@ -18,8 +18,8 @@ import { Query } from '../core/query'; import { DocumentKeySet, DocumentMap, - NullableMaybeDocumentMap, - MaybeDocumentMap + MaybeDocumentMap, + NullableMaybeDocumentMap } from '../model/collections'; import { MaybeDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; diff --git a/packages/firestore/src/local/remote_document_change_buffer.ts b/packages/firestore/src/local/remote_document_change_buffer.ts index 25020318c96..e00f5f69772 100644 --- a/packages/firestore/src/local/remote_document_change_buffer.ts +++ b/packages/firestore/src/local/remote_document_change_buffer.ts @@ -16,11 +16,11 @@ import { DocumentKeySet, - DocumentSizeEntry, DocumentSizeEntries, - NullableMaybeDocumentMap, + DocumentSizeEntry, maybeDocumentMap, - MaybeDocumentMap + MaybeDocumentMap, + NullableMaybeDocumentMap } from '../model/collections'; import { MaybeDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; @@ -122,8 +122,6 @@ export abstract class RemoteDocumentChangeBuffer { transaction: PersistenceTransaction, documentKeys: DocumentKeySet ): PersistencePromise { - const changes = this.assertChanges(); - // Record the size of everything we load from the cache so we can compute // a delta later. return this.getAllFromCache(transaction, documentKeys).next( 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 12a4074ef1f..91f6e88c671 100644 --- a/packages/firestore/test/unit/local/remote_document_cache.test.ts +++ b/packages/firestore/test/unit/local/remote_document_cache.test.ts @@ -358,7 +358,7 @@ function genericRemoteDocumentCacheTests( ]; const key1 = key(DOC_PATH); const key2 = key(LONG_DOC_PATH); - const missing_key = key('foo/nonexistent'); + const missingKey = key('foo/nonexistent'); return cache .addEntries(docs) .then(() => { @@ -366,13 +366,13 @@ function genericRemoteDocumentCacheTests( documentKeySet() .add(key1) .add(key2) - .add(missing_key) + .add(missingKey) ); }) .then(read => { expectEqual(read.get(key1), docs[0]); expectEqual(read.get(key2), docs[1]); - expect(read.get(missing_key)).to.be.null; + expect(read.get(missingKey)).to.be.null; }); }); From 512667e6ff537c233c029f236a58233638544f37 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 21 Dec 2018 21:10:30 -0500 Subject: [PATCH 26/29] Fix node tests --- packages/firestore/test/unit/local/test_mutation_queue.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/firestore/test/unit/local/test_mutation_queue.ts b/packages/firestore/test/unit/local/test_mutation_queue.ts index fd214d4e71f..ced8bfaebbf 100644 --- a/packages/firestore/test/unit/local/test_mutation_queue.ts +++ b/packages/firestore/test/unit/local/test_mutation_queue.ts @@ -23,6 +23,7 @@ import { DocumentKeySet } from '../../../src/model/collections'; import { DocumentKey } from '../../../src/model/document_key'; import { Mutation } from '../../../src/model/mutation'; import { MutationBatch } from '../../../src/model/mutation_batch'; +import { SortedMap } from '../../../src/util/sorted_map'; import { AnyDuringMigration } from '../../../src/util/misc'; /** @@ -139,13 +140,18 @@ export class TestMutationQueue { getAllMutationBatchesAffectingDocumentKeys( documentKeys: DocumentKeySet ): Promise { + let keyMap = new SortedMap(DocumentKey.comparator); + documentKeys.forEach(key => { + keyMap = keyMap.insert(key, null); + }); + return this.persistence.runTransaction( 'getAllMutationBatchesAffectingDocumentKeys', 'readonly', txn => { return this.queue.getAllMutationBatchesAffectingDocumentKeys( txn, - documentKeys + keyMap ); } ); From 6cb4bfb18039e4ba1d761ae16216ca07ffdc74e3 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 21 Dec 2018 21:12:55 -0500 Subject: [PATCH 27/29] Appease linter 2 --- packages/firestore/src/local/memory_remote_document_cache.ts | 4 ++-- packages/firestore/test/unit/local/test_mutation_queue.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/firestore/src/local/memory_remote_document_cache.ts b/packages/firestore/src/local/memory_remote_document_cache.ts index fce6202fdf7..7836eea76d5 100644 --- a/packages/firestore/src/local/memory_remote_document_cache.ts +++ b/packages/firestore/src/local/memory_remote_document_cache.ts @@ -22,10 +22,10 @@ import { documentMap, DocumentSizeEntries, DocumentSizeEntry, + MaybeDocumentMap, + maybeDocumentMap, NullableMaybeDocumentMap, nullableMaybeDocumentMap, - MaybeDocumentMap, - maybeDocumentMap } from '../model/collections'; import { Document, MaybeDocument, NoDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; diff --git a/packages/firestore/test/unit/local/test_mutation_queue.ts b/packages/firestore/test/unit/local/test_mutation_queue.ts index ced8bfaebbf..fd426a90792 100644 --- a/packages/firestore/test/unit/local/test_mutation_queue.ts +++ b/packages/firestore/test/unit/local/test_mutation_queue.ts @@ -23,8 +23,8 @@ import { DocumentKeySet } from '../../../src/model/collections'; import { DocumentKey } from '../../../src/model/document_key'; import { Mutation } from '../../../src/model/mutation'; import { MutationBatch } from '../../../src/model/mutation_batch'; -import { SortedMap } from '../../../src/util/sorted_map'; import { AnyDuringMigration } from '../../../src/util/misc'; +import { SortedMap } from '../../../src/util/sorted_map'; /** * A wrapper around a MutationQueue that automatically creates a From 77bf92ef5d796c08f5dba75da1b1fb02a4369138 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 21 Dec 2018 21:13:45 -0500 Subject: [PATCH 28/29] [AUTOMATED]: Prettier Code Styling --- packages/firestore/src/local/memory_remote_document_cache.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/src/local/memory_remote_document_cache.ts b/packages/firestore/src/local/memory_remote_document_cache.ts index 7836eea76d5..9e43f0321b5 100644 --- a/packages/firestore/src/local/memory_remote_document_cache.ts +++ b/packages/firestore/src/local/memory_remote_document_cache.ts @@ -25,7 +25,7 @@ import { MaybeDocumentMap, maybeDocumentMap, NullableMaybeDocumentMap, - nullableMaybeDocumentMap, + nullableMaybeDocumentMap } from '../model/collections'; import { Document, MaybeDocument, NoDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; From e7b8c8e7c6c02de2697d97ab3afd580a69ecb908 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 21 Dec 2018 22:58:05 -0500 Subject: [PATCH 29/29] Comment --- packages/firestore/src/local/remote_document_change_buffer.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/firestore/src/local/remote_document_change_buffer.ts b/packages/firestore/src/local/remote_document_change_buffer.ts index e00f5f69772..ac837e8ff6a 100644 --- a/packages/firestore/src/local/remote_document_change_buffer.ts +++ b/packages/firestore/src/local/remote_document_change_buffer.ts @@ -126,6 +126,9 @@ export abstract class RemoteDocumentChangeBuffer { // a delta later. return this.getAllFromCache(transaction, documentKeys).next( ({ maybeDocuments, sizeMap }) => { + // Note: `getAllFromCache` returns two maps instead of a single map from + // keys to `DocumentSizeEntry`s. This is to allow returning the + // `NullableMaybeDocumentMap` directly, without a conversion. sizeMap.forEach((documentKey, size) => { this.documentSizes.set(documentKey, size); });