diff --git a/packages/firestore/src/local/local_documents_view.ts b/packages/firestore/src/local/local_documents_view.ts index 2765ee42946..e5d50c18e25 100644 --- a/packages/firestore/src/local/local_documents_view.ts +++ b/packages/firestore/src/local/local_documents_view.ts @@ -46,7 +46,6 @@ import { FieldMask } from '../model/field_mask'; import { calculateOverlayMutation, mutationApplyToLocalView, - MutationType, PatchMutation } from '../model/mutation'; import { Overlay } from '../model/overlay'; @@ -92,7 +91,7 @@ export class LocalDocumentsView { .getOverlay(transaction, key) .next(value => { overlay = value; - return this.getBaseDocument(transaction, key, overlay); + return this.remoteDocumentCache.getEntry(transaction, key); }) .next(document => { if (overlay !== null) { @@ -424,11 +423,11 @@ export class LocalDocumentsView { if (originalDocs.get(key)) { return PersistencePromise.resolve(); } - return this.getBaseDocument(transaction, key, overlay).next( - doc => { + return this.remoteDocumentCache + .getEntry(transaction, key) + .next(doc => { modifiedDocs = modifiedDocs.insert(key, doc); - } - ); + }); } ) .next(() => @@ -550,15 +549,4 @@ export class LocalDocumentsView { return results; }); } - - /** Returns a base document that can be used to apply `overlay`. */ - private getBaseDocument( - transaction: PersistenceTransaction, - key: DocumentKey, - overlay: Overlay | null - ): PersistencePromise { - return overlay === null || overlay.mutation.type === MutationType.Patch - ? this.remoteDocumentCache.getEntry(transaction, key) - : PersistencePromise.resolve(MutableDocument.newInvalidDocument(key)); - } } diff --git a/packages/firestore/src/model/document.ts b/packages/firestore/src/model/document.ts index 279f5031371..59cd82fcf3a 100644 --- a/packages/firestore/src/model/document.ts +++ b/packages/firestore/src/model/document.ts @@ -101,6 +101,13 @@ export interface Document { */ readonly readTime: SnapshotVersion; + /** + * The timestamp at which the document was created. This value increases + * monotonically when a document is deleted then recreated. It can also be + * compared to `createTime` of other documents and the `readTime` of a query. + */ + readonly createTime: SnapshotVersion; + /** The underlying data of this document or an empty value if no data exists. */ readonly data: ObjectValue; @@ -163,6 +170,7 @@ export class MutableDocument implements Document { private documentType: DocumentType, public version: SnapshotVersion, public readTime: SnapshotVersion, + public createTime: SnapshotVersion, public data: ObjectValue, private documentState: DocumentState ) {} @@ -175,8 +183,9 @@ export class MutableDocument implements Document { return new MutableDocument( documentKey, DocumentType.INVALID, - SnapshotVersion.min(), - SnapshotVersion.min(), + /* version */ SnapshotVersion.min(), + /* readTime */ SnapshotVersion.min(), + /* createTime */ SnapshotVersion.min(), ObjectValue.empty(), DocumentState.SYNCED ); @@ -189,13 +198,15 @@ export class MutableDocument implements Document { static newFoundDocument( documentKey: DocumentKey, version: SnapshotVersion, + createTime: SnapshotVersion, value: ObjectValue ): MutableDocument { return new MutableDocument( documentKey, DocumentType.FOUND_DOCUMENT, - version, - SnapshotVersion.min(), + /* version */ version, + /* readTime */ SnapshotVersion.min(), + /* createTime */ createTime, value, DocumentState.SYNCED ); @@ -209,8 +220,9 @@ export class MutableDocument implements Document { return new MutableDocument( documentKey, DocumentType.NO_DOCUMENT, - version, - SnapshotVersion.min(), + /* version */ version, + /* readTime */ SnapshotVersion.min(), + /* createTime */ SnapshotVersion.min(), ObjectValue.empty(), DocumentState.SYNCED ); @@ -228,8 +240,9 @@ export class MutableDocument implements Document { return new MutableDocument( documentKey, DocumentType.UNKNOWN_DOCUMENT, - version, - SnapshotVersion.min(), + /* version */ version, + /* readTime */ SnapshotVersion.min(), + /* createTime */ SnapshotVersion.min(), ObjectValue.empty(), DocumentState.HAS_COMMITTED_MUTATIONS ); @@ -243,6 +256,18 @@ export class MutableDocument implements Document { version: SnapshotVersion, value: ObjectValue ): MutableDocument { + // If a document is switching state from being an invalid or deleted + // document to a valid (FOUND_DOCUMENT) document, either due to receiving an + // update from Watch or due to applying a local set mutation on top + // of a deleted document, our best guess about its createTime would be the + // version at which the document transitioned to a FOUND_DOCUMENT. + if ( + this.createTime.isEqual(SnapshotVersion.min()) && + (this.documentType === DocumentType.NO_DOCUMENT || + this.documentType === DocumentType.INVALID) + ) { + this.createTime = version; + } this.version = version; this.documentType = DocumentType.FOUND_DOCUMENT; this.data = value; @@ -340,6 +365,7 @@ export class MutableDocument implements Document { this.documentType, this.version, this.readTime, + this.createTime, this.data.clone(), this.documentState ); @@ -350,6 +376,7 @@ export class MutableDocument implements Document { `Document(${this.key}, ${this.version}, ${JSON.stringify( this.data.value )}, ` + + `{createTime: ${this.createTime}}), ` + `{documentType: ${this.documentType}}), ` + `{documentState: ${this.documentState}})` ); diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index c537648775f..e5f205ef3ef 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -404,7 +404,8 @@ export function toDocument( return { name: toName(serializer, document.key), fields: document.data.value.mapValue.fields, - updateTime: toTimestamp(serializer, document.version.toTimestamp()) + updateTime: toTimestamp(serializer, document.version.toTimestamp()), + createTime: toTimestamp(serializer, document.createTime.toTimestamp()) }; } @@ -415,8 +416,19 @@ export function fromDocument( ): MutableDocument { const key = fromName(serializer, document.name!); const version = fromVersion(document.updateTime!); + // If we read a document from persistence that is missing createTime, it's due + // to older SDK versions not storing this information. In such cases, we'll + // set the createTime to zero. This can be removed in the long term. + const createTime = document.createTime + ? fromVersion(document.createTime) + : SnapshotVersion.min(); const data = new ObjectValue({ mapValue: { fields: document.fields } }); - const result = MutableDocument.newFoundDocument(key, version, data); + const result = MutableDocument.newFoundDocument( + key, + version, + createTime, + data + ); if (hasCommittedMutations) { result.setHasCommittedMutations(); } @@ -435,8 +447,11 @@ function fromFound( assertPresent(doc.found.updateTime, 'doc.found.updateTime'); const key = fromName(serializer, doc.found.name); const version = fromVersion(doc.found.updateTime); + const createTime = doc.found.createTime + ? fromVersion(doc.found.createTime) + : SnapshotVersion.min(); const data = new ObjectValue({ mapValue: { fields: doc.found.fields } }); - return MutableDocument.newFoundDocument(key, version, data); + return MutableDocument.newFoundDocument(key, version, createTime, data); } function fromMissing( @@ -502,10 +517,18 @@ export function fromWatchChange( ); const key = fromName(serializer, entityChange.document.name); const version = fromVersion(entityChange.document.updateTime); + const createTime = entityChange.document.createTime + ? fromVersion(entityChange.document.createTime) + : SnapshotVersion.min(); const data = new ObjectValue({ mapValue: { fields: entityChange.document.fields } }); - const doc = MutableDocument.newFoundDocument(key, version, data); + const doc = MutableDocument.newFoundDocument( + key, + version, + createTime, + data + ); const updatedTargetIds = entityChange.targetIds || []; const removedTargetIds = entityChange.removedTargetIds || []; watchChange = new DocumentWatchChange( diff --git a/packages/firestore/test/unit/local/local_store.test.ts b/packages/firestore/test/unit/local/local_store.test.ts index 9e14e553edf..a7b4e519fd7 100644 --- a/packages/firestore/test/unit/local/local_store.test.ts +++ b/packages/firestore/test/unit/local/local_store.test.ts @@ -398,7 +398,10 @@ class LocalStoreTester { return this; } - toReturnChanged(...docs: Document[]): LocalStoreTester { + toReturnChangedInternal( + docs: Document[], + isEqual?: (lhs: Document | null, rhs: Document | null) => boolean + ): LocalStoreTester { this.promiseChain = this.promiseChain.then(() => { debugAssert( this.lastChanges !== null, @@ -407,13 +410,29 @@ class LocalStoreTester { expect(this.lastChanges.size).to.equal(docs.length, 'number of changes'); for (const doc of docs) { const returned = this.lastChanges.get(doc.key); - expectEqual(doc, returned, `Expected '${returned}' to equal '${doc}'.`); + const message = `Expected '${returned}' to equal '${doc}'.`; + if (isEqual) { + expect(isEqual(doc, returned)).to.equal(true, message); + } else { + expectEqual(doc, returned, message); + } } this.lastChanges = null; }); return this; } + toReturnChanged(...docs: Document[]): LocalStoreTester { + return this.toReturnChangedInternal(docs); + } + + toReturnChangedWithDocComparator( + isEqual: (lhs: Document | null, rhs: Document | null) => boolean, + ...docs: Document[] + ): LocalStoreTester { + return this.toReturnChangedInternal(docs, isEqual); + } + toReturnRemoved(...keyStrings: string[]): LocalStoreTester { this.promiseChain = this.promiseChain.then(() => { debugAssert( @@ -433,16 +452,20 @@ class LocalStoreTester { return this; } - toContain(doc: Document): LocalStoreTester { + toContain( + doc: Document, + isEqual?: (lhs: Document, rhs: Document) => boolean + ): LocalStoreTester { this.promiseChain = this.promiseChain.then(() => { return localStoreReadDocument(this.localStore, doc.key).then(result => { - expectEqual( - result, - doc, - `Expected ${ - result ? result.toString() : null - } to match ${doc.toString()}.` - ); + const message = `Expected ${ + result ? result.toString() : null + } to match ${doc.toString()}.`; + if (isEqual) { + expect(isEqual(result, doc)).to.equal(true, message); + } else { + expectEqual(result, doc, message); + } }); }); return this; @@ -539,6 +562,22 @@ class LocalStoreTester { } } +// The `isEqual` method for the Document class does not compare createTime and +// readTime. For some tests, we'd like to verify that a certain createTime has +// been calculated for documents. In such cases we can use this comparator. +function compareDocsWithCreateTime( + lhs: Document | null, + rhs: Document | null +): boolean { + return ( + (lhs === null && rhs === null) || + (lhs !== null && + rhs !== null && + lhs.isEqual(rhs) && + lhs.createTime.isEqual(rhs.createTime)) + ); +} + describe('LocalStore w/ Memory Persistence', () => { async function initialize(): Promise { const queryEngine = new CountingQueryEngine(); @@ -1958,6 +1997,253 @@ function genericLocalStoreTests( .finish(); }); + it('handles document creation time', () => { + return ( + expectLocalStore() + .afterAllocatingQuery(query('col')) + .toReturnTargetId(2) + .after(docAddedRemoteEvent(doc('col/doc1', 12, { foo: 'bar' }, 5), [2])) + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc('col/doc1', 12, { foo: 'bar' }, 5) + ) + .toContain( + doc('col/doc1', 12, { foo: 'bar' }, 5), + compareDocsWithCreateTime + ) + .after(setMutation('col/doc1', { foo: 'newBar' })) + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc('col/doc1', 12, { foo: 'newBar' }, 5).setHasLocalMutations() + ) + .toContain( + doc('col/doc1', 12, { foo: 'newBar' }, 5).setHasLocalMutations(), + compareDocsWithCreateTime + ) + .afterAcknowledgingMutation({ documentVersion: 13 }) + // We haven't seen the remote event yet + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc('col/doc1', 13, { foo: 'newBar' }, 5).setHasCommittedMutations() + ) + .toContain( + doc('col/doc1', 13, { foo: 'newBar' }, 5).setHasCommittedMutations(), + compareDocsWithCreateTime + ) + .finish() + ); + }); + + it('saves updateTime as createTime when receives ack for creating a new doc', () => { + if (gcIsEager) { + return; + } + + return expectLocalStore() + .after(setMutation('col/doc1', { foo: 'newBar' })) + .afterAcknowledgingMutation({ documentVersion: 13 }) + .afterExecutingQuery(query('col')) + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc('col/doc1', 13, { foo: 'newBar' }, 13).setHasCommittedMutations() + ) + .toContain( + doc('col/doc1', 13, { foo: 'newBar' }, 13).setHasCommittedMutations(), + compareDocsWithCreateTime + ) + .finish(); + }); + + it('handles createTime for Set -> Ack -> RemoteEvent', () => { + if (gcIsEager) { + return; + } + + return expectLocalStore() + .after(setMutation('col/doc1', { foo: 'newBar' })) + .afterAcknowledgingMutation({ documentVersion: 13 }) + .afterExecutingQuery(query('col')) + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc('col/doc1', 13, { foo: 'newBar' }, 13).setHasCommittedMutations() + ) + .toContain( + doc('col/doc1', 13, { foo: 'newBar' }, 13).setHasCommittedMutations(), + compareDocsWithCreateTime + ) + .after(docAddedRemoteEvent(doc('col/doc1', 14, { foo: 'baz' }, 5), [2])) + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc('col/doc1', 14, { foo: 'baz' }, 5) + ) + .toContain( + doc('col/doc1', 14, { foo: 'baz' }, 5), + compareDocsWithCreateTime + ) + .finish(); + }); + + it('handles createTime for Set -> RemoteEvent -> Ack', () => { + if (gcIsEager) { + return; + } + + return expectLocalStore() + .after(setMutation('col/doc1', { foo: 'newBar' })) + .after(docAddedRemoteEvent(doc('col/doc1', 13, { foo: 'baz' }, 5), [2])) + .afterAcknowledgingMutation({ documentVersion: 14 }) + .afterExecutingQuery(query('col')) + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc('col/doc1', 14, { foo: 'newBar' }, 5).setHasCommittedMutations() + ) + .toContain( + doc('col/doc1', 14, { foo: 'newBar' }, 5).setHasCommittedMutations(), + compareDocsWithCreateTime + ) + .finish(); + }); + + it('saves updateTime as createTime when recreating a deleted doc', async () => { + if (gcIsEager) { + return; + } + + return ( + expectLocalStore() + .afterAllocatingQuery(query('col')) + .toReturnTargetId(2) + .after(docAddedRemoteEvent(deletedDoc('col/doc1', 12), [2])) + .toReturnChanged(deletedDoc('col/doc1', 12)) + .toContain(deletedDoc('col/doc1', 12)) + .after(setMutation('col/doc1', { foo: 'newBar' })) + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc('col/doc1', 12, { foo: 'newBar' }, 12).setHasLocalMutations() + ) + .toContain( + doc('col/doc1', 12, { foo: 'newBar' }, 12).setHasLocalMutations(), + compareDocsWithCreateTime + ) + .afterAcknowledgingMutation({ documentVersion: 13 }) + // We haven't seen the remote event yet + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc('col/doc1', 13, { foo: 'newBar' }, 13).setHasCommittedMutations() + ) + .toContain( + doc('col/doc1', 13, { foo: 'newBar' }, 13).setHasCommittedMutations(), + compareDocsWithCreateTime + ) + .finish() + ); + }); + + it('document createTime is preserved through Set -> Ack -> Patch -> Ack', () => { + if (gcIsEager) { + return; + } + + return expectLocalStore() + .after(setMutation('col/doc1', { foo: 'newBar' })) + .afterAcknowledgingMutation({ documentVersion: 13 }) + .afterExecutingQuery(query('col')) + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc('col/doc1', 13, { foo: 'newBar' }, 13).setHasCommittedMutations() + ) + .toContain( + doc('col/doc1', 13, { foo: 'newBar' }, 13).setHasCommittedMutations(), + compareDocsWithCreateTime + ) + .afterMutations([patchMutation('col/doc1', { 'likes': 1 })]) + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc( + 'col/doc1', + 13, + { foo: 'newBar', likes: 1 }, + 13 + ).setHasLocalMutations() + ) + .toContain( + doc( + 'col/doc1', + 13, + { foo: 'newBar', likes: 1 }, + 13 + ).setHasLocalMutations(), + compareDocsWithCreateTime + ) + .afterAcknowledgingMutation({ documentVersion: 14 }) + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc( + 'col/doc1', + 14, + { foo: 'newBar', likes: 1 }, + 13 + ).setHasCommittedMutations() + ) + .toContain( + doc( + 'col/doc1', + 14, + { foo: 'newBar', likes: 1 }, + 13 + ).setHasCommittedMutations(), + compareDocsWithCreateTime + ) + .finish(); + }); + + it('document createTime is preserved through Doc Added -> Patch -> Ack', () => { + if (gcIsEager) { + return; + } + return expectLocalStore() + .afterAllocatingQuery(query('col')) + .toReturnTargetId(2) + .after(docAddedRemoteEvent(doc('col/doc1', 12, { foo: 'bar' }, 5), [2])) + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc('col/doc1', 12, { foo: 'bar' }, 5) + ) + .toContain( + doc('col/doc1', 12, { foo: 'bar' }, 5), + compareDocsWithCreateTime + ) + .afterMutations([patchMutation('col/doc1', { 'likes': 1 })]) + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc('col/doc1', 13, { foo: 'bar', likes: 1 }, 5).setHasLocalMutations() + ) + .toContain( + doc('col/doc1', 13, { foo: 'bar', likes: 1 }, 5).setHasLocalMutations(), + compareDocsWithCreateTime + ) + .afterAcknowledgingMutation({ documentVersion: 14 }) + .toReturnChangedWithDocComparator( + compareDocsWithCreateTime, + doc( + 'col/doc1', + 14, + { foo: 'bar', likes: 1 }, + 5 + ).setHasCommittedMutations() + ) + .toContain( + doc( + 'col/doc1', + 14, + { foo: 'bar', likes: 1 }, + 5 + ).setHasCommittedMutations(), + compareDocsWithCreateTime + ) + .finish(); + }); + it('uses target mapping to execute queries', () => { if (gcIsEager) { return; diff --git a/packages/firestore/test/unit/model/mutation.test.ts b/packages/firestore/test/unit/model/mutation.test.ts index 10a85a031a0..f6a707a72e2 100644 --- a/packages/firestore/test/unit/model/mutation.test.ts +++ b/packages/firestore/test/unit/model/mutation.test.ts @@ -1012,7 +1012,7 @@ describe('Mutation', () => { // There are (0! + 7*1! + 21*2! + 35*3! + 35*4! + 21*5! + 7*6! + 7!) * 3 = 41100 cases. expect(testCases).to.equal(41100); - }); + }).timeout(10000); it('overlay by combinations and permutations for array transforms', () => { const docs: MutableDocument[] = [ diff --git a/packages/firestore/test/unit/remote/serializer.helper.ts b/packages/firestore/test/unit/remote/serializer.helper.ts index 1c0a49edda9..79140e73999 100644 --- a/packages/firestore/test/unit/remote/serializer.helper.ts +++ b/packages/firestore/test/unit/remote/serializer.helper.ts @@ -804,11 +804,12 @@ export function serializerTest( }); it('toDocument() / fromDocument', () => { - const d = doc('foo/bar', 42, { a: 5, b: 'b' }); + const d = doc('foo/bar', 42, { a: 5, b: 'b' }, /* createTime */ 12); const proto = { name: toName(s, d.key), fields: d.data.value.mapValue.fields, - updateTime: toVersion(s, d.version) + updateTime: toVersion(s, d.version), + createTime: toVersion(s, d.createTime) }; const serialized = toDocument(s, d); expect(serialized).to.deep.equal(proto); diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index fe3f079edb4..39e397c0fbf 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -1056,6 +1056,7 @@ export class SpecBuilder { return { key: SpecBuilder.keyToSpec(doc.key), version: doc.version.toMicroseconds(), + createTime: doc.createTime.toMicroseconds(), value: userDataWriter.convertValue( doc.data.value ) as JsonObject, @@ -1068,6 +1069,7 @@ export class SpecBuilder { return { key: SpecBuilder.keyToSpec(doc.key), version: doc.version.toMicroseconds(), + createTime: doc.createTime.toMicroseconds(), value: null }; } diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 8a6909b3f2e..e64dcbfd214 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -663,7 +663,8 @@ abstract class TestRunner { ? doc( watchEntity.doc.key, watchEntity.doc.version, - watchEntity.doc.value + watchEntity.doc.value, + watchEntity.doc.createTime ) : deletedDoc(watchEntity.doc.key, watchEntity.doc.version); if (watchEntity.doc.options?.hasCommittedMutations) { @@ -1193,7 +1194,12 @@ abstract class TestRunner { type: ChangeType, change: SpecDocument ): DocumentViewChange { - const document = doc(change.key, change.version, change.value || {}); + const document = doc( + change.key, + change.version, + change.value || {}, + change.createTime + ); if (change.options?.hasCommittedMutations) { document.setHasCommittedMutations(); } else if (change.options?.hasLocalMutations) { @@ -1622,6 +1628,7 @@ export interface SpecQuery { export interface SpecDocument { key: string; version: TestSnapshotVersion; + createTime: TestSnapshotVersion; value: JsonObject | null; options?: DocumentOptions; } diff --git a/packages/firestore/test/util/helpers.ts b/packages/firestore/test/util/helpers.ts index 24cb7bccf0d..1e5eb7f5d60 100644 --- a/packages/firestore/test/util/helpers.ts +++ b/packages/firestore/test/util/helpers.ts @@ -156,11 +156,13 @@ export function ref(key: string, offset?: number): DocumentReference { export function doc( keyStr: string, ver: TestSnapshotVersion, - jsonOrObjectValue: JsonObject | ObjectValue + jsonOrObjectValue: JsonObject | ObjectValue, + createTime?: TestSnapshotVersion ): MutableDocument { return MutableDocument.newFoundDocument( key(keyStr), version(ver), + createTime ? version(createTime) : SnapshotVersion.min(), jsonOrObjectValue instanceof ObjectValue ? jsonOrObjectValue : wrapObject(jsonOrObjectValue) diff --git a/packages/firestore/test/util/spec_test_helpers.ts b/packages/firestore/test/util/spec_test_helpers.ts index c721d0e0675..f8b8715d4c7 100644 --- a/packages/firestore/test/util/spec_test_helpers.ts +++ b/packages/firestore/test/util/spec_test_helpers.ts @@ -57,7 +57,8 @@ export function encodeWatchChange( document: { name: toName(serializer, doc.key), fields: doc?.data.value.mapValue.fields, - updateTime: toVersion(serializer, doc.version) + updateTime: toVersion(serializer, doc.version), + createTime: toVersion(serializer, doc?.createTime) }, targetIds: watchChange.updatedTargetIds, removedTargetIds: watchChange.removedTargetIds