diff --git a/packages/firestore/src/local/indexeddb_schema_converter.ts b/packages/firestore/src/local/indexeddb_schema_converter.ts index 700d12de32c..f25d3a70c1e 100644 --- a/packages/firestore/src/local/indexeddb_schema_converter.ts +++ b/packages/firestore/src/local/indexeddb_schema_converter.ts @@ -18,8 +18,13 @@ import { User } from '../auth/user'; import { ListenSequence } from '../core/listen_sequence'; import { SnapshotVersion } from '../core/snapshot_version'; -import { DocumentKeySet, documentKeySet } from '../model/collections'; +import { + DocumentKeyMap, + DocumentKeySet, + documentKeySet +} from '../model/collections'; import { DocumentKey } from '../model/document_key'; +import { FieldMask } from '../model/field_mask'; import { ResourcePath } from '../model/path'; import { debugAssert, fail, hardAssert } from '../util/assert'; import { BATCHID_UNKNOWN } from '../util/types'; @@ -484,7 +489,9 @@ export class SchemaConverter implements SimpleDbSchemaConverter { this.serializer.remoteSerializer ); - const promises: Array> = []; + const promises: Array< + PersistencePromise> + > = []; const userToDocumentSet = new Map(); return mutationsStore diff --git a/packages/firestore/src/local/local_documents_view.ts b/packages/firestore/src/local/local_documents_view.ts index 8281344de2b..aaa289b3f69 100644 --- a/packages/firestore/src/local/local_documents_view.ts +++ b/packages/firestore/src/local/local_documents_view.ts @@ -33,7 +33,8 @@ import { newOverlayMap, documentMap, mutableDocumentMap, - documentKeySet + documentKeySet, + DocumentKeyMap } from '../model/collections'; import { Document, MutableDocument } from '../model/document'; import { DocumentKey } from '../model/document_key'; @@ -52,6 +53,7 @@ import { SortedMap } from '../util/sorted_map'; import { DocumentOverlayCache } from './document_overlay_cache'; import { IndexManager } from './index_manager'; import { MutationQueue } from './mutation_queue'; +import { OverlayedDocument } from './overlayed_document'; import { PersistencePromise } from './persistence_promise'; import { PersistenceTransaction } from './persistence_transaction'; import { RemoteDocumentCache } from './remote_document_cache'; @@ -92,7 +94,7 @@ export class LocalDocumentsView { mutationApplyToLocalView( overlay.mutation, document, - null, + FieldMask.empty(), Timestamp.now() ); } @@ -141,10 +143,34 @@ export class LocalDocumentsView { docs, overlays, existenceStateChanged - ); + ).next(computeViewsResult => { + let result = documentMap(); + computeViewsResult.forEach((documentKey, overlayedDocument) => { + result = result.insert( + documentKey, + overlayedDocument.overlayedDocument + ); + }); + return result; + }); }); } + /** + * Gets the overlayed documents for the given document map, which will include + * the local view of those documents and a `FieldMask` indicating which fields + * are mutated locally, `null` if overlay is a Set or Delete mutation. + */ + getOverlayedDocuments( + transaction: PersistenceTransaction, + docs: MutableDocumentMap + ): PersistencePromise> { + const overlays = newOverlayMap(); + return this.populateOverlays(transaction, overlays, docs).next(() => + this.computeViews(transaction, docs, overlays, documentKeySet()) + ); + } + /** * Fetches the overlays for {@code docs} and adds them to provided overlay map * if the map does not already contain an entry for the given document key. @@ -170,17 +196,26 @@ export class LocalDocumentsView { } /** - * Computes the local view for documents, applying overlays from both - * `memoizedOverlays` and the overlay cache. + * Computes the local view for the given documents. + * + * @param docs - The documents to compute views for. It also has the base + * version of the documents. + * @param overlays - The overlays that need to be applied to the given base + * version of the documents. + * @param existenceStateChanged - A set of documents whose existence states + * might have changed. This is used to determine if we need to re-calculate + * overlays from mutation queues. + * @return A map represents the local documents view. */ computeViews( transaction: PersistenceTransaction, docs: MutableDocumentMap, overlays: OverlayMap, existenceStateChanged: DocumentKeySet - ): PersistencePromise { - let results = documentMap(); + ): PersistencePromise> { let recalculateDocuments = mutableDocumentMap(); + const mutatedFields = newDocumentKeyMap(); + const results = newDocumentKeyMap(); docs.forEach((_, doc) => { const overlay = overlays.get(doc.key); // Recalculate an overlay if the document's existence state changed due to @@ -196,17 +231,32 @@ export class LocalDocumentsView { ) { recalculateDocuments = recalculateDocuments.insert(doc.key, doc); } else if (overlay !== undefined) { - mutationApplyToLocalView(overlay.mutation, doc, null, Timestamp.now()); + mutatedFields.set(doc.key, overlay.mutation.getFieldMask()); + mutationApplyToLocalView( + overlay.mutation, + doc, + overlay.mutation.getFieldMask(), + Timestamp.now() + ); } }); return this.recalculateAndSaveOverlays( transaction, recalculateDocuments - ).next(() => { - docs.forEach((key, value) => { - results = results.insert(key, value); - }); + ).next(recalculatedFields => { + recalculatedFields.forEach((documentKey, mask) => + mutatedFields.set(documentKey, mask) + ); + docs.forEach((documentKey, document) => + results.set( + documentKey, + new OverlayedDocument( + document, + mutatedFields.get(documentKey) ?? null + ) + ) + ); return results; }); } @@ -214,7 +264,7 @@ export class LocalDocumentsView { private recalculateAndSaveOverlays( transaction: PersistenceTransaction, docs: MutableDocumentMap - ): PersistencePromise { + ): PersistencePromise> { const masks = newDocumentKeyMap(); // A reverse lookup map from batch id to the documents within that batch. let documentsByBatchId = new SortedMap( @@ -274,7 +324,8 @@ export class LocalDocumentsView { ); } return PersistencePromise.waitFor(promises); - }); + }) + .next(() => masks); } /** @@ -284,7 +335,7 @@ export class LocalDocumentsView { recalculateAndSaveOverlaysForDocumentKeys( transaction: PersistenceTransaction, documentKeys: DocumentKeySet - ): PersistencePromise { + ): PersistencePromise> { return this.remoteDocumentCache .getEntries(transaction, documentKeys) .next(docs => this.recalculateAndSaveOverlays(transaction, docs)); @@ -407,7 +458,7 @@ export class LocalDocumentsView { mutationApplyToLocalView( overlay.mutation, document, - null, + FieldMask.empty(), Timestamp.now() ); } diff --git a/packages/firestore/src/local/local_store_impl.ts b/packages/firestore/src/local/local_store_impl.ts index 36b967838eb..7db432a4fb0 100644 --- a/packages/firestore/src/local/local_store_impl.ts +++ b/packages/firestore/src/local/local_store_impl.ts @@ -28,8 +28,10 @@ import { canonifyTarget, Target, targetEquals } from '../core/target'; import { BatchId, TargetId } from '../core/types'; import { Timestamp } from '../lite-api/timestamp'; import { + DocumentKeyMap, documentKeySet, DocumentKeySet, + documentMap, DocumentMap, mutableDocumentMap, MutableDocumentMap @@ -75,6 +77,7 @@ import { LocalStore } from './local_store'; import { LocalViewChanges } from './local_view_changes'; import { LruGarbageCollector, LruResults } from './lru_garbage_collector'; import { MutationQueue } from './mutation_queue'; +import { OverlayedDocument } from './overlayed_document'; import { Persistence } from './persistence'; import { PersistencePromise } from './persistence_promise'; import { PersistenceTransaction } from './persistence_transaction'; @@ -315,7 +318,7 @@ export function localStoreWriteLocally( const localWriteTime = Timestamp.now(); const keys = mutations.reduce((keys, m) => keys.add(m.key), documentKeySet()); - let existingDocs: DocumentMap; + let overlayedDocuments: DocumentKeyMap; let mutationBatch: MutationBatch; return localStoreImpl.persistence @@ -342,13 +345,13 @@ export function localStoreWriteLocally( // Load and apply all existing mutations. This lets us compute the // current base state for all non-idempotent transforms before applying // any additional user-provided writes. - return localStoreImpl.localDocuments.getLocalViewOfDocuments( + return localStoreImpl.localDocuments.getOverlayedDocuments( txn, remoteDocs ); }) .next(docs => { - existingDocs = docs; + overlayedDocuments = docs; // For non-idempotent mutations (such as `FieldValue.increment()`), // we record the base state in a separate patch mutation. This is @@ -360,7 +363,7 @@ export function localStoreWriteLocally( for (const mutation of mutations) { const baseValue = mutationExtractBaseValue( mutation, - existingDocs.get(mutation.key)! + overlayedDocuments.get(mutation.key)!.overlayedDocument ); if (baseValue != null) { // NOTE: The base state should only be applied if there's some @@ -387,7 +390,7 @@ export function localStoreWriteLocally( .next(batch => { mutationBatch = batch; const overlays = batch.applyToLocalDocumentSet( - existingDocs, + overlayedDocuments, docsWithoutRemoteVersion ); return localStoreImpl.documentOverlayCache.saveOverlays( @@ -398,7 +401,11 @@ export function localStoreWriteLocally( }); }) .then(() => { - return { batchId: mutationBatch.batchId, changes: existingDocs }; + let documents = documentMap(); + overlayedDocuments.forEach( + (key, val) => (documents = documents.insert(key, val.overlayedDocument)) + ); + return { batchId: mutationBatch.batchId, changes: documents }; }); } diff --git a/packages/firestore/src/local/overlayed_document.ts b/packages/firestore/src/local/overlayed_document.ts new file mode 100644 index 00000000000..e50e6a9132d --- /dev/null +++ b/packages/firestore/src/local/overlayed_document.ts @@ -0,0 +1,35 @@ +/** + * @license + * Copyright 2022 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Document } from '../model/document'; +import { FieldMask } from '../model/field_mask'; + +/** + * Represents a local view (overlay) of a document, and the fields that are + * locally mutated. + */ +export class OverlayedDocument { + constructor( + readonly overlayedDocument: Document, + + /** + * The fields that are locally mutated by patch mutations. If the overlayed + * document is from set or delete mutations, this returns null. + */ + readonly mutatedFields: FieldMask | null + ) {} +} diff --git a/packages/firestore/src/model/mutation.ts b/packages/firestore/src/model/mutation.ts index 3c5c20f8c64..119e9b9731b 100644 --- a/packages/firestore/src/model/mutation.ts +++ b/packages/firestore/src/model/mutation.ts @@ -213,6 +213,12 @@ export abstract class Mutation { abstract readonly key: DocumentKey; abstract readonly precondition: Precondition; abstract readonly fieldTransforms: FieldTransform[]; + /** + * Returns a `FieldMask` representing the fields that will be changed by + * applying this mutation. Returns `null` if the mutation will overwrite the + * entire document. + */ + abstract getFieldMask(): FieldMask | null; } /** @@ -444,6 +450,10 @@ export class SetMutation extends Mutation { } readonly type: MutationType = MutationType.Set; + + getFieldMask(): FieldMask | null { + return null; + } } function setMutationApplyToRemoteDocument( @@ -516,6 +526,10 @@ export class PatchMutation extends Mutation { } readonly type: MutationType = MutationType.Patch; + + getFieldMask(): FieldMask | null { + return this.fieldMask; + } } function patchMutationApplyToRemoteDocument( @@ -670,6 +684,10 @@ export class DeleteMutation extends Mutation { readonly type: MutationType = MutationType.Delete; readonly fieldTransforms: FieldTransform[] = []; + + getFieldMask(): FieldMask | null { + return null; + } } function deleteMutationApplyToRemoteDocument( @@ -720,4 +738,8 @@ export class VerifyMutation extends Mutation { readonly type: MutationType = MutationType.Verify; readonly fieldTransforms: FieldTransform[] = []; + + getFieldMask(): FieldMask | null { + return null; + } } diff --git a/packages/firestore/src/model/mutation_batch.ts b/packages/firestore/src/model/mutation_batch.ts index 5b8c2c18a28..7acc65e69f8 100644 --- a/packages/firestore/src/model/mutation_batch.ts +++ b/packages/firestore/src/model/mutation_batch.ts @@ -18,6 +18,7 @@ import { SnapshotVersion } from '../core/snapshot_version'; import { BatchId } from '../core/types'; import { Timestamp } from '../lite-api/timestamp'; +import { OverlayedDocument } from '../local/overlayed_document'; import { debugAssert, hardAssert } from '../util/assert'; import { arrayEquals } from '../util/misc'; @@ -25,10 +26,10 @@ import { documentKeySet, DocumentKeySet, MutationMap, - DocumentMap, DocumentVersionMap, documentVersionMap, - newMutationMap + newMutationMap, + DocumentKeyMap } from './collections'; import { MutableDocument } from './document'; import { FieldMask } from './field_mask'; @@ -104,7 +105,7 @@ export class MutationBatch { */ applyToLocalView( document: MutableDocument, - mutatedFields: FieldMask | null = FieldMask.empty() + mutatedFields: FieldMask | null ): FieldMask | null { // First, apply the base state. This allows us to apply non-idempotent // transform against a consistent set of values. @@ -139,7 +140,7 @@ export class MutationBatch { * replace all the mutation applications. */ applyToLocalDocumentSet( - documentMap: DocumentMap, + documentMap: DocumentKeyMap, documentsWithoutRemoteVersion: DocumentKeySet ): MutationMap { // TODO(mrschmidt): This implementation is O(n^2). If we apply the mutations @@ -147,11 +148,15 @@ export class MutationBatch { // to O(n). const overlays = newMutationMap(); this.mutations.forEach(m => { - const document = documentMap.get(m.key)!; + const overlayedDocument = documentMap.get(m.key)!; // TODO(mutabledocuments): This method should take a MutableDocumentMap // and we should remove this cast. - const mutableDocument = document as MutableDocument; - let mutatedFields = this.applyToLocalView(mutableDocument); + const mutableDocument = + overlayedDocument.overlayedDocument as MutableDocument; + let mutatedFields = this.applyToLocalView( + mutableDocument, + overlayedDocument.mutatedFields + ); // Set mutatedFields to null if the document is only from local mutations. // This creates a Set or Delete mutation, instead of trying to create a // patch mutation as the overlay. @@ -162,7 +167,7 @@ export class MutationBatch { if (overlay !== null) { overlays.set(m.key, overlay); } - if (!document.isValidDocument()) { + if (!mutableDocument.isValidDocument()) { mutableDocument.convertToNoDocument(SnapshotVersion.min()); } }); diff --git a/packages/firestore/test/unit/local/local_store.test.ts b/packages/firestore/test/unit/local/local_store.test.ts index 8de65a93f8a..9e14e553edf 100644 --- a/packages/firestore/test/unit/local/local_store.test.ts +++ b/packages/firestore/test/unit/local/local_store.test.ts @@ -1398,7 +1398,7 @@ function genericLocalStoreTests( ); }); - it('holds back only non-idempotent transforms', () => { + it('holds back transforms', () => { const query1 = query('foo'); return ( expectLocalStore() @@ -1425,21 +1425,22 @@ function genericLocalStoreTests( ) ) .toReturnChanged(doc('foo/bar', 1, { sum: 0, arrayUnion: [] })) - .afterMutations([ - patchMutation('foo/bar', { sum: increment(1) }), - patchMutation('foo/bar', { - arrayUnion: arrayUnion('foo') - }) - ]) + .after(patchMutation('foo/bar', { sum: increment(1) })) + .toReturnChanged( + doc('foo/bar', 1, { + sum: 1, + arrayUnion: [] + }).setHasLocalMutations() + ) + .after(patchMutation('foo/bar', { arrayUnion: arrayUnion('foo') })) .toReturnChanged( doc('foo/bar', 1, { sum: 1, arrayUnion: ['foo'] }).setHasLocalMutations() ) - // The sum transform is not idempotent and the backend's updated value - // is ignored. The ArrayUnion transform is recomputed and includes the - // backend value. + // The sum transform and array union transform make the SDK ignore the + // backend's updated value. .afterRemoteEvent( docUpdateRemoteEvent( doc('foo/bar', 2, { sum: 1337, arrayUnion: ['bar'] }), @@ -1452,10 +1453,23 @@ function genericLocalStoreTests( arrayUnion: ['foo'] }).setHasLocalMutations() ) + // With a field transform acknowledgement, the overlay is recalculated + // with the remaining local mutations. .afterAcknowledgingMutation({ documentVersion: 3, + transformResults: [{ integerValue: 1338 }] + }) + .toReturnChanged( + doc('foo/bar', 3, { + sum: 1338, + arrayUnion: ['bar', 'foo'] + }) + .setReadTime(SnapshotVersion.fromTimestamp(new Timestamp(0, 3000))) + .setHasLocalMutations() + ) + .afterAcknowledgingMutation({ + documentVersion: 4, transformResults: [ - { integerValue: 1338 }, { arrayValue: { values: [{ stringValue: 'bar' }, { stringValue: 'foo' }] @@ -1464,11 +1478,11 @@ function genericLocalStoreTests( ] }) .toReturnChanged( - doc('foo/bar', 3, { + doc('foo/bar', 4, { sum: 1338, arrayUnion: ['bar', 'foo'] }) - .setReadTime(SnapshotVersion.fromTimestamp(new Timestamp(0, 3000))) + .setReadTime(SnapshotVersion.fromTimestamp(new Timestamp(0, 4000))) .setHasCommittedMutations() ) .finish() @@ -1839,6 +1853,111 @@ function genericLocalStoreTests( .finish(); }); + it('can handle multiple field patches on remote docs', () => { + const query1 = query('foo', filter('matches', '==', true)); + return expectLocalStore() + .afterAllocatingQuery(query1) + .toReturnTargetId(2) + .afterRemoteEvent( + docAddedRemoteEvent( + [doc('foo/bar', 1, { 'likes': 0, 'stars': 0 })], + [2], + [] + ) + ) + .toReturnChanged(doc('foo/bar', 1, { 'likes': 0, 'stars': 0 })) + .toContain(doc('foo/bar', 1, { 'likes': 0, 'stars': 0 })) + .after(patchMutation('foo/bar', { 'likes': 1 })) + .toReturnChanged( + doc('foo/bar', 1, { 'likes': 1, 'stars': 0 }).setHasLocalMutations() + ) + .toContain( + doc('foo/bar', 1, { 'likes': 1, 'stars': 0 }).setHasLocalMutations() + ) + .after(patchMutation('foo/bar', { 'stars': 1 })) + .toReturnChanged( + doc('foo/bar', 1, { 'likes': 1, 'stars': 1 }).setHasLocalMutations() + ) + .toContain( + doc('foo/bar', 1, { 'likes': 1, 'stars': 1 }).setHasLocalMutations() + ) + .after(patchMutation('foo/bar', { 'stars': 2 })) + .toReturnChanged( + doc('foo/bar', 1, { 'likes': 1, 'stars': 2 }).setHasLocalMutations() + ) + .toContain( + doc('foo/bar', 1, { 'likes': 1, 'stars': 2 }).setHasLocalMutations() + ) + .finish(); + }); + + it('can handle multiple field patches in one batch on remote docs', () => { + const query1 = query('foo'); + return expectLocalStore() + .afterAllocatingQuery(query1) + .toReturnTargetId(2) + .afterRemoteEvent( + docAddedRemoteEvent( + [doc('foo/bar', 1, { 'likes': 0, 'stars': 0 })], + [2], + [] + ) + ) + .toReturnChanged(doc('foo/bar', 1, { 'likes': 0, 'stars': 0 })) + .toContain(doc('foo/bar', 1, { 'likes': 0, 'stars': 0 })) + .afterMutations([ + patchMutation('foo/bar', { 'likes': 1 }), + patchMutation('foo/bar', { 'stars': 1 }) + ]) + .toReturnChanged( + doc('foo/bar', 1, { 'likes': 1, 'stars': 1 }).setHasLocalMutations() + ) + .toContain( + doc('foo/bar', 1, { 'likes': 1, 'stars': 1 }).setHasLocalMutations() + ) + .after(patchMutation('foo/bar', { 'stars': 2 })) + .toReturnChanged( + doc('foo/bar', 1, { 'likes': 1, 'stars': 2 }).setHasLocalMutations() + ) + .toContain( + doc('foo/bar', 1, { 'likes': 1, 'stars': 2 }).setHasLocalMutations() + ) + .finish(); + }); + + it('can handle multiple field patches on local docs', () => { + return expectLocalStore() + .after(setMutation('foo/bar', { 'likes': 0, 'stars': 0 })) + .toReturnChanged( + doc('foo/bar', 1, { 'likes': 0, 'stars': 0 }).setHasLocalMutations() + ) + .toContain( + doc('foo/bar', 1, { 'likes': 0, 'stars': 0 }).setHasLocalMutations() + ) + .after(patchMutation('foo/bar', { 'likes': 1 })) + .toReturnChanged( + doc('foo/bar', 1, { 'likes': 1, 'stars': 0 }).setHasLocalMutations() + ) + .toContain( + doc('foo/bar', 1, { 'likes': 1, 'stars': 0 }).setHasLocalMutations() + ) + .after(patchMutation('foo/bar', { 'stars': 1 })) + .toReturnChanged( + doc('foo/bar', 1, { 'likes': 1, 'stars': 1 }).setHasLocalMutations() + ) + .toContain( + doc('foo/bar', 1, { 'likes': 1, 'stars': 1 }).setHasLocalMutations() + ) + .after(patchMutation('foo/bar', { 'stars': 2 })) + .toReturnChanged( + doc('foo/bar', 1, { 'likes': 1, 'stars': 2 }).setHasLocalMutations() + ) + .toContain( + doc('foo/bar', 1, { 'likes': 1, 'stars': 2 }).setHasLocalMutations() + ) + .finish(); + }); + it('uses target mapping to execute queries', () => { if (gcIsEager) { return;