Replace usage of transform with updateTransforms (#4153)

Author: Brian Chen

.changeset/rich-birds-wink.md

@@ -0,0 +1,4 @@
+---
+'@firebase/firestore': minor
+---
+A write to a document that contains FieldValue transforms is no longer split up into two separate operations. This reduces the number of writes the backend performs and allows each WriteBatch to hold 500 writes regardless of how many FieldValue transformations are attached.

packages/firestore/exp/src/api/reference.ts

@@ -314,8 +314,8 @@ export function setDoc<T>(
     options
   );
 
-  const mutations = parsed.toMutations(reference._key, Precondition.none());
-  return executeWrite(firestore, mutations);
+  const mutation = parsed.toMutation(reference._key, Precondition.none());
+  return executeWrite(firestore, [mutation]);
 }
 
 /**
@@ -394,11 +394,8 @@ export function updateDoc(
     );
   }
 
-  const mutations = parsed.toMutations(
-    reference._key,
-    Precondition.exists(true)
-  );
-  return executeWrite(firestore, mutations);
+  const mutation = parsed.toMutation(reference._key, Precondition.exists(true));
+  return executeWrite(firestore, [mutation]);
 }
 
 /**
@@ -448,8 +445,8 @@ export function addDoc<T>(
     {}
   );
 
-  const mutations = parsed.toMutations(docRef._key, Precondition.exists(false));
-  return executeWrite(firestore, mutations).then(() => docRef);
+  const mutation = parsed.toMutation(docRef._key, Precondition.exists(false));
+  return executeWrite(firestore, [mutation]).then(() => docRef);
 }
 
 // TODO(firestorexp): Make sure these overloads are tested via the Firestore

packages/firestore/exp/src/api/snapshot.ts

@@ -176,9 +176,9 @@ export interface DocumentChange<T = DocumentData> {
  * access will return 'undefined'. You can use the `exists()` method to
  * explicitly verify a document's existence.
  */
-export class DocumentSnapshot<
-  T = DocumentData
-> extends LiteDocumentSnapshot<T> {
+export class DocumentSnapshot<T = DocumentData> extends LiteDocumentSnapshot<
+  T
+> {
   private readonly _firestoreImpl: FirebaseFirestore;
 
   /**
@@ -291,9 +291,9 @@ export class DocumentSnapshot<
  * `exists` property will always be true and `data()` will never return
  * 'undefined'.
  */
-export class QueryDocumentSnapshot<
-  T = DocumentData
-> extends DocumentSnapshot<T> {
+export class QueryDocumentSnapshot<T = DocumentData> extends DocumentSnapshot<
+  T
+> {
   /**
    * Retrieves all fields in the document as an `Object`.
    *

packages/firestore/lite/src/api/reference.ts

@@ -1478,10 +1478,9 @@ export function setDoc<T>(
   );
 
   const datastore = getDatastore(reference.firestore);
-  return invokeCommitRpc(
-    datastore,
-    parsed.toMutations(reference._key, Precondition.none())
-  );
+  return invokeCommitRpc(datastore, [
+    parsed.toMutation(reference._key, Precondition.none())
+  ]);
 }
 
 /**
@@ -1569,10 +1568,9 @@ export function updateDoc(
   }
 
   const datastore = getDatastore(reference.firestore);
-  return invokeCommitRpc(
-    datastore,
-    parsed.toMutations(reference._key, Precondition.exists(true))
-  );
+  return invokeCommitRpc(datastore, [
+    parsed.toMutation(reference._key, Precondition.exists(true))
+  ]);
 }
 
 /**
@@ -1634,10 +1632,9 @@ export function addDoc<T>(
   );
 
   const datastore = getDatastore(reference.firestore);
-  return invokeCommitRpc(
-    datastore,
-    parsed.toMutations(docRef._key, Precondition.exists(false))
-  ).then(() => docRef);
+  return invokeCommitRpc(datastore, [
+    parsed.toMutation(docRef._key, Precondition.exists(false))
+  ]).then(() => docRef);
 }
 
 /**

packages/firestore/lite/src/api/snapshot.ts

@@ -211,9 +211,9 @@ export class DocumentSnapshot<T = DocumentData> {
  * `exists` property will always be true and `data()` will never return
  * 'undefined'.
  */
-export class QueryDocumentSnapshot<
-  T = DocumentData
-> extends DocumentSnapshot<T> {
+export class QueryDocumentSnapshot<T = DocumentData> extends DocumentSnapshot<
+  T
+> {
   /**
    * Retrieves all fields in the document as an `Object`.
    *

packages/firestore/lite/src/api/write_batch.ts

@@ -111,9 +111,7 @@ export class WriteBatch {
       ref._converter !== null,
       options
     );
-    this._mutations = this._mutations.concat(
-      parsed.toMutations(ref._key, Precondition.none())
-    );
+    this._mutations.push(parsed.toMutation(ref._key, Precondition.none()));
     return this;
   }
 
@@ -186,8 +184,8 @@ export class WriteBatch {
       );
     }
 
-    this._mutations = this._mutations.concat(
-      parsed.toMutations(ref._key, Precondition.exists(true))
+    this._mutations.push(
+      parsed.toMutation(ref._key, Precondition.exists(true))
     );
     return this;
   }

packages/firestore/src/api/user_data_reader.ts

@@ -17,13 +17,13 @@
 
 import {
   DocumentData,
-  SetOptions,
-  FieldPath as PublicFieldPath
+  FieldPath as PublicFieldPath,
+  SetOptions
 } from '@firebase/firestore-types';
 
 import {
-  Value as ProtoValue,
-  MapValue as ProtoMapValue
+  MapValue as ProtoMapValue,
+  Value as ProtoValue
 } from '../protos/firestore_proto_api';
 import { Timestamp } from './timestamp';
 import { DatabaseId } from '../core/database_info';
@@ -34,8 +34,7 @@ import {
   Mutation,
   PatchMutation,
   Precondition,
-  SetMutation,
-  TransformMutation
+  SetMutation
 } from '../model/mutation';
 import { FieldPath as InternalFieldPath } from '../model/path';
 import { debugAssert, fail } from '../util/assert';
@@ -79,38 +78,43 @@ export class ParsedSetData {
     readonly fieldTransforms: FieldTransform[]
   ) {}
 
-  toMutations(key: DocumentKey, precondition: Precondition): Mutation[] {
-    const mutations = [] as Mutation[];
+  toMutation(key: DocumentKey, precondition: Precondition): Mutation {
     if (this.fieldMask !== null) {
-      mutations.push(
-        new PatchMutation(key, this.data, this.fieldMask, precondition)
+      return new PatchMutation(
+        key,
+        this.data,
+        this.fieldMask,
+        precondition,
+        this.fieldTransforms
       );
     } else {
-      mutations.push(new SetMutation(key, this.data, precondition));
-    }
-    if (this.fieldTransforms.length > 0) {
-      mutations.push(new TransformMutation(key, this.fieldTransforms));
+      return new SetMutation(
+        key,
+        this.data,
+        precondition,
+        this.fieldTransforms
+      );
     }
-    return mutations;
   }
 }
 
 /** The result of parsing "update" data (i.e. for an updateData call). */
 export class ParsedUpdateData {
   constructor(
     readonly data: ObjectValue,
+    // The fieldMask does not include document transforms.
     readonly fieldMask: FieldMask,
     readonly fieldTransforms: FieldTransform[]
   ) {}
 
-  toMutations(key: DocumentKey, precondition: Precondition): Mutation[] {
-    const mutations = [
-      new PatchMutation(key, this.data, this.fieldMask, precondition)
-    ] as Mutation[];
-    if (this.fieldTransforms.length > 0) {
-      mutations.push(new TransformMutation(key, this.fieldTransforms));
-    }
-    return mutations;
+  toMutation(key: DocumentKey, precondition: Precondition): Mutation {
+    return new PatchMutation(
+      key,
+      this.data,
+      this.fieldMask,
+      precondition,
+      this.fieldTransforms
+    );
   }
 }
 

packages/firestore/src/core/transaction.ts

@@ -81,21 +81,21 @@ export class Transaction {
   }
 
   set(key: DocumentKey, data: ParsedSetData): void {
-    this.write(data.toMutations(key, this.precondition(key)));
+    this.write(data.toMutation(key, this.precondition(key)));
     this.writtenDocs.add(key.toString());
   }
 
   update(key: DocumentKey, data: ParsedUpdateData): void {
     try {
-      this.write(data.toMutations(key, this.preconditionForUpdate(key)));
+      this.write(data.toMutation(key, this.preconditionForUpdate(key)));
     } catch (e) {
       this.lastWriteError = e;
     }
     this.writtenDocs.add(key.toString());
   }
 
   delete(key: DocumentKey): void {
-    this.write([new DeleteMutation(key, this.precondition(key))]);
+    this.write(new DeleteMutation(key, this.precondition(key)));
     this.writtenDocs.add(key.toString());
   }
 
@@ -193,9 +193,9 @@ export class Transaction {
     }
   }
 
-  private write(mutations: Mutation[]): void {
+  private write(mutation: Mutation): void {
     this.ensureCommitNotCalled();
-    this.mutations = this.mutations.concat(mutations);
+    this.mutations.push(mutation);
   }
 
   private ensureCommitNotCalled(): void {

packages/firestore/src/local/local_serializer.ts

@@ -194,6 +194,28 @@ export function fromDbMutationBatch(
   const baseMutations = (dbBatch.baseMutations || []).map(m =>
     fromMutation(localSerializer.remoteSerializer, m)
   );
+
+  // Squash old transform mutations into existing patch or set mutations.
+  // The replacement of representing `transforms` with `update_transforms`
+  // on the SDK means that old `transform` mutations stored in IndexedDB need
+  // to be updated to `update_transforms`.
+  // TODO(b/174608374): Remove this code once we perform a schema migration.
+  for (let i = dbBatch.mutations.length - 1; i >= 0; --i) {
+    const mutationProto = dbBatch.mutations[i];
+    if (mutationProto?.transform !== undefined) {
+      debugAssert(
+        i >= 1 &&
+          dbBatch.mutations[i - 1].transform === undefined &&
+          dbBatch.mutations[i - 1].update !== undefined,
+        'TransformMutation should be preceded by a patch or set mutation'
+      );
+      const mutationToJoin = dbBatch.mutations[i - 1];
+      mutationToJoin.updateTransforms = mutationProto.transform.fieldTransforms;
+      dbBatch.mutations.splice(i, 1);
+      --i;
+    }
+  }
+
   const mutations = dbBatch.mutations.map(m =>
     fromMutation(localSerializer.remoteSerializer, m)
   );