diff --git a/packages/firestore/src/model/mutation.ts b/packages/firestore/src/model/mutation.ts index d5432b81267..b0c9d49dfd0 100644 --- a/packages/firestore/src/model/mutation.ts +++ b/packages/firestore/src/model/mutation.ts @@ -87,11 +87,11 @@ export class MutationResult { */ readonly version: SnapshotVersion, /** - * The resulting fields returned from the backend after a - * TransformMutation has been committed. Contains one FieldValue for each - * FieldTransform that was in the mutation. + * The resulting fields returned from the backend after a mutation + * containing field transforms has been committed. Contains one FieldValue + * for each FieldTransform that was in the mutation. * - * Will be null if the mutation was not a TransformMutation. + * Will be null if the mutation did not contain any field transforms. */ readonly transformResults: Array | null ) {} @@ -178,8 +178,8 @@ export function preconditionIsValidForDocument( * Mutations not only act on the value of the document but also its version. * * For local mutations (mutations that haven't been committed yet), we preserve - * the existing version for Set, Patch, and Transform mutations. For Delete - * mutations, we reset the version to 0. + * the existing version for Set and Patch mutations. For Delete mutations, we + * reset the version to 0. * * Here's the expected transition table. * @@ -191,28 +191,22 @@ export function preconditionIsValidForDocument( * PatchMutation Document(v3) Document(v3) * PatchMutation NoDocument(v3) NoDocument(v3) * PatchMutation null null - * TransformMutation Document(v3) Document(v3) - * TransformMutation NoDocument(v3) NoDocument(v3) - * TransformMutation null null * DeleteMutation Document(v3) NoDocument(v0) * DeleteMutation NoDocument(v3) NoDocument(v0) * DeleteMutation null NoDocument(v0) * * For acknowledged mutations, we use the updateTime of the WriteResponse as - * the resulting version for Set, Patch, and Transform mutations. As deletes - * have no explicit update time, we use the commitTime of the WriteResponse for + * the resulting version for Set and Patch mutations. As deletes have no + * explicit update time, we use the commitTime of the WriteResponse for * Delete mutations. * * If a mutation is acknowledged by the backend but fails the precondition check * locally, we return an `UnknownDocument` and rely on Watch to send us the * updated version. * - * Note that TransformMutations don't create Documents (in the case of being - * applied to a NoDocument), even though they would on the backend. This is - * because the client always combines the TransformMutation with a SetMutation - * or PatchMutation and we only want to apply the transform if the prior - * mutation resulted in a Document (always true for a SetMutation, but not - * necessarily for a PatchMutation). + * Field transforms are used only with Patch and Set Mutations. We use the + * `updateTransforms` message to store transforms, rather than the `transforms`s + * messages. * * ## Subclassing Notes * @@ -335,13 +329,7 @@ export function extractMutationBaseValue( mutation: Mutation, maybeDoc: MaybeDocument | null ): ObjectValue | null { - if (mutation.fieldTransforms !== undefined) { - return extractTransformMutationBaseValue( - mutation.fieldTransforms, - maybeDoc - ); - } - return null; + return extractTransformMutationBaseValue(mutation.fieldTransforms, maybeDoc); } function extractTransformMutationBaseValue( @@ -489,19 +477,17 @@ function applySetMutationToLocalView( } let newData = mutation.value; - if (mutation.fieldTransforms) { - const transformResults = localTransformResults( - mutation.fieldTransforms, - localWriteTime, - maybeDoc, - baseDoc - ); - newData = transformObject( - mutation.fieldTransforms, - newData, - transformResults - ); - } + const transformResults = localTransformResults( + mutation.fieldTransforms, + localWriteTime, + maybeDoc, + baseDoc + ); + newData = transformObject( + mutation.fieldTransforms, + newData, + transformResults + ); const version = getPostMutationVersion(maybeDoc); return new Document(mutation.key, version, newData, { @@ -623,8 +609,8 @@ function patchObject(mutation: PatchMutation, data: ObjectValue): ObjectValue { /** * Creates a list of "transform results" (a transform result is a field value - * representing the result of applying a transform) for use after a - * TransformMutation has been acknowledged by the server. + * representing the result of applying a transform) for use after a mutation + * containing transforms has been acknowledged by the server. * * @param fieldTransforms - The field transforms to apply the result to. * @param baseDoc - The document prior to applying this mutation batch. @@ -664,10 +650,10 @@ function serverTransformResults( /** * Creates a list of "transform results" (a transform result is a field value * representing the result of applying a transform) for use when applying a - * TransformMutation locally. + * transform locally. * * @param fieldTransforms - The field transforms to apply the result to. - * @param localWriteTime - The local time of the transform mutation (used to + * @param localWriteTime - The local time of the mutation (used to * generate ServerTimestampValues). * @param maybeDoc - The current state of the document after applying all * previous mutations. @@ -689,14 +675,6 @@ function localTransformResults( previousValue = maybeDoc.field(fieldTransform.field); } - if (previousValue === null && baseDoc instanceof Document) { - // If the current document does not contain a value for the mutated - // field, use the value that existed before applying this mutation - // batch. This solves an edge case where a PatchMutation clears the - // values in a nested map before the TransformMutation is applied. - previousValue = baseDoc.field(fieldTransform.field); - } - transformResults.push( applyTransformOperationToLocalView( transform, diff --git a/packages/firestore/src/model/server_timestamps.ts b/packages/firestore/src/model/server_timestamps.ts index e6c3e2bd4fb..1aa5fc772cb 100644 --- a/packages/firestore/src/model/server_timestamps.ts +++ b/packages/firestore/src/model/server_timestamps.ts @@ -33,9 +33,8 @@ import { normalizeTimestamp } from './normalize'; * * Notes: * - ServerTimestampValue instances are created as the result of applying a - * TransformMutation (see TransformMutation.applyTo()). They can only exist in - * the local view of a document. Therefore they do not need to be parsed or - * serialized. + * transform. They can only exist in the local view of a document. Therefore + * they do not need to be parsed or serialized. * - When evaluated locally (e.g. for snapshot.data()), they by default * evaluate to `null`. This behavior can be configured by passing custom * FieldValueOptions to value(). diff --git a/packages/firestore/src/model/transform_operation.ts b/packages/firestore/src/model/transform_operation.ts index e4fffb909c3..4a2a220b9d6 100644 --- a/packages/firestore/src/model/transform_operation.ts +++ b/packages/firestore/src/model/transform_operation.ts @@ -25,7 +25,7 @@ import { normalizeNumber } from './normalize'; import { serverTimestamp } from './server_timestamps'; import { isArray, isInteger, isNumber, valueEquals } from './values'; -/** Represents a transform within a TransformMutation. */ +/** Used to represent a field transform on a mutation. */ export class TransformOperation { // Make sure that the structural type of `TransformOperation` is unique. // See https://github.com/microsoft/TypeScript/issues/5451 diff --git a/packages/firestore/test/integration/api/server_timestamp.test.ts b/packages/firestore/test/integration/api/server_timestamp.test.ts index d4313116308..4c451e6a91a 100644 --- a/packages/firestore/test/integration/api/server_timestamp.test.ts +++ b/packages/firestore/test/integration/api/server_timestamp.test.ts @@ -104,25 +104,6 @@ apiDescribe('Server Timestamps', (persistence: boolean) => { ); } - /** - * Verifies a snapshot containing setData but using the previous field value - * for the timestamps. - */ - function verifyTimestampsUsePreviousValue( - current: firestore.DocumentSnapshot, - prev: firestore.DocumentSnapshot | null - ): void { - if (!prev) { - verifyTimestampsAreNull(current); - } else { - expect(current.exists).to.equal(true); - const when = prev.get('when'); - expect(current.data({ serverTimestamps: 'previous' })).to.deep.equal( - expectedDataWithTimestamp(when) - ); - } - } - /** * Wraps a test, getting a docRef and event accumulator, and cleaning them * up when done. @@ -207,34 +188,6 @@ apiDescribe('Server Timestamps', (persistence: boolean) => { }); }); - it('can return previous value', () => { - // The following test includes an update of the nested map "deep", which - // updates it to contain a single ServerTimestamp. This update is split - // into two mutations: One that sets "deep" to an empty map and overwrites - // the previous ServerTimestamp value and a second transform that writes - // the new ServerTimestamp. This step in the test verifies that we can - // still access the old ServerTimestamp value (from `previousSnapshot`) even - // though it was removed in an intermediate step. - - let previousSnapshot: firestore.DocumentSnapshot; - - return withTestSetup(() => { - return writeInitialData() - .then(() => docRef.update(updateData)) - .then(() => accumulator.awaitLocalEvent()) - .then(snapshot => verifyTimestampsUsePreviousValue(snapshot, null)) - .then(() => accumulator.awaitRemoteEvent()) - .then(snapshot => { - previousSnapshot = snapshot; - }) - .then(() => docRef.update(updateData)) - .then(() => accumulator.awaitLocalEvent()) - .then(snapshot => - verifyTimestampsUsePreviousValue(snapshot, previousSnapshot) - ); - }); - }); - it('can return previous value of different type', () => { return withTestSetup(() => { return writeInitialData()