Replace usage of transform with updateTransforms (#2262)

Author: Brian Chen

firebase-firestore/CHANGELOG.md

@@ -3,6 +3,10 @@
   This more closely aligns the SDK with the Firestore backend, which has always
   accepted null and NaN for all operators, even though this isn't necessarily
   useful.
+- [changed] 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.
 
 # (22.0.0)
 - [changed] Removed the deprecated `timestampsInSnapshotsEnabled` setting.

firebase-firestore/src/main/java/com/google/firebase/firestore/DocumentReference.java

@@ -42,6 +42,7 @@
 import com.google.firebase.firestore.util.Assert;
 import com.google.firebase.firestore.util.Executors;
 import com.google.firebase.firestore.util.Util;
+import java.util.Collections;
 import java.util.Map;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
@@ -166,7 +167,7 @@ public Task<Void> set(@NonNull Object data, @NonNull SetOptions options) {
             : firestore.getUserDataReader().parseSetData(data);
     return firestore
         .getClient()
-        .write(parsed.toMutationList(key, Precondition.NONE))
+        .write(Collections.singletonList(parsed.toMutation(key, Precondition.NONE)))
         .continueWith(Executors.DIRECT_EXECUTOR, voidErrorTransformer());
   }
 
@@ -230,7 +231,7 @@ public Task<Void> update(
   private Task<Void> update(@NonNull ParsedUpdateData parsedData) {
     return firestore
         .getClient()
-        .write(parsedData.toMutationList(key, Precondition.exists(true)))
+        .write(Collections.singletonList(parsedData.toMutation(key, Precondition.exists(true))))
         .continueWith(Executors.DIRECT_EXECUTOR, voidErrorTransformer());
   }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataReader.java

@@ -208,7 +208,7 @@ public Value parseQueryValue(Object input, boolean allowArrays) {
   }
 
   /** Converts a POJO to native types and then parses it into model types. */
-  private Value convertAndParseFieldData(Object input, ParseContext context) {
+  public Value convertAndParseFieldData(Object input, ParseContext context) {
     Object converted = CustomClassMapper.convertToPlainJavaTypes(input);
     return parseData(converted, context);
   }

firebase-firestore/src/main/java/com/google/firebase/firestore/WriteBatch.java

@@ -88,7 +88,7 @@ public WriteBatch set(
         options.isMerge()
             ? firestore.getUserDataReader().parseMergeData(data, options.getFieldMask())
             : firestore.getUserDataReader().parseSetData(data);
-    mutations.addAll(parsed.toMutationList(documentRef.getKey(), Precondition.NONE));
+    mutations.add(parsed.toMutation(documentRef.getKey(), Precondition.NONE));
     return this;
   }
 
@@ -163,7 +163,7 @@ private WriteBatch update(
       @NonNull DocumentReference documentRef, @NonNull ParsedUpdateData updateData) {
     firestore.validateReference(documentRef);
     verifyNotCommitted();
-    mutations.addAll(updateData.toMutationList(documentRef.getKey(), Precondition.exists(true)));
+    mutations.add(updateData.toMutation(documentRef.getKey(), Precondition.exists(true)));
     return this;
   }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/core/Transaction.java

@@ -103,7 +103,7 @@ public Task<List<MaybeDocument>> lookup(List<DocumentKey> keys) {
 
   /** Stores a set mutation for the given key and value, to be committed when commit() is called. */
   public void set(DocumentKey key, ParsedSetData data) {
-    write(data.toMutationList(key, precondition(key)));
+    write(Collections.singletonList(data.toMutation(key, precondition(key))));
     writtenDocs.add(key);
   }
 
@@ -113,7 +113,7 @@ public void set(DocumentKey key, ParsedSetData data) {
    */
   public void update(DocumentKey key, ParsedUpdateData data) {
     try {
-      write(data.toMutationList(key, preconditionForUpdate(key)));
+      write(Collections.singletonList(data.toMutation(key, preconditionForUpdate(key))));
     } catch (FirebaseFirestoreException e) {
       lastWriteError = e;
     }

firebase-firestore/src/main/java/com/google/firebase/firestore/core/UserData.java

@@ -26,7 +26,6 @@
 import com.google.firebase.firestore.model.mutation.PatchMutation;
 import com.google.firebase.firestore.model.mutation.Precondition;
 import com.google.firebase.firestore.model.mutation.SetMutation;
-import com.google.firebase.firestore.model.mutation.TransformMutation;
 import com.google.firebase.firestore.model.mutation.TransformOperation;
 import com.google.firebase.firestore.util.Assert;
 import java.util.ArrayList;
@@ -335,23 +334,32 @@ public static class ParsedSetData {
       this.fieldTransforms = fieldTransforms;
     }
 
-    public List<Mutation> toMutationList(DocumentKey key, Precondition precondition) {
-      ArrayList<Mutation> mutations = new ArrayList<>();
+    public ObjectValue getData() {
+      return data;
+    }
+
+    @Nullable
+    public FieldMask getFieldMask() {
+      return fieldMask;
+    }
+
+    public List<FieldTransform> getFieldTransforms() {
+      return fieldTransforms;
+    }
+
+    public Mutation toMutation(DocumentKey key, Precondition precondition) {
       if (fieldMask != null) {
-        mutations.add(new PatchMutation(key, data, fieldMask, precondition));
+        return new PatchMutation(key, data, fieldMask, precondition, fieldTransforms);
       } else {
-        mutations.add(new SetMutation(key, data, precondition));
-      }
-      if (!fieldTransforms.isEmpty()) {
-        mutations.add(new TransformMutation(key, fieldTransforms));
+        return new SetMutation(key, data, precondition, fieldTransforms);
       }
-      return mutations;
     }
   }
 
   /** The result of parsing "update" data (i.e. for an updateData call). */
   public static class ParsedUpdateData {
     private final ObjectValue data;
+    // The fieldMask does not include document transforms.
     private final FieldMask fieldMask;
     private final List<FieldTransform> fieldTransforms;
 
@@ -361,17 +369,20 @@ public static class ParsedUpdateData {
       this.fieldTransforms = fieldTransforms;
     }
 
+    public ObjectValue getData() {
+      return data;
+    }
+
+    public FieldMask getFieldMask() {
+      return fieldMask;
+    }
+
     public List<FieldTransform> getFieldTransforms() {
       return fieldTransforms;
     }
 
-    public List<Mutation> toMutationList(DocumentKey key, Precondition precondition) {
-      ArrayList<Mutation> mutations = new ArrayList<>();
-      mutations.add(new PatchMutation(key, data, fieldMask, precondition));
-      if (!fieldTransforms.isEmpty()) {
-        mutations.add(new TransformMutation(key, fieldTransforms));
-      }
-      return mutations;
+    public Mutation toMutation(DocumentKey key, Precondition precondition) {
+      return new PatchMutation(key, data, fieldMask, precondition, fieldTransforms);
     }
   }
 }

firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalSerializer.java

@@ -29,8 +29,12 @@
 import com.google.firebase.firestore.model.mutation.Mutation;
 import com.google.firebase.firestore.model.mutation.MutationBatch;
 import com.google.firebase.firestore.remote.RemoteSerializer;
+import com.google.firestore.v1.DocumentTransform.FieldTransform;
+import com.google.firestore.v1.Write;
+import com.google.firestore.v1.Write.Builder;
 import com.google.protobuf.ByteString;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 /** Serializer for values stored in the LocalStore. */
@@ -171,11 +175,35 @@ MutationBatch decodeMutationBatch(com.google.firebase.firestore.proto.WriteBatch
     for (int i = 0; i < baseMutationsCount; i++) {
       baseMutations.add(rpcSerializer.decodeMutation(batch.getBaseWrites(i)));
     }
-    int mutationsCount = batch.getWritesCount();
-    List<Mutation> mutations = new ArrayList<>(mutationsCount);
-    for (int i = 0; i < mutationsCount; i++) {
-      mutations.add(rpcSerializer.decodeMutation(batch.getWrites(i)));
+
+    List<Mutation> mutations = new ArrayList<>(batch.getWritesCount());
+
+    // 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 (int i = batch.getWritesCount() - 1; i >= 0; --i) {
+      Write mutation = batch.getWrites(i);
+      if (mutation.hasTransform()) {
+        hardAssert(
+            i >= 1 && !batch.getWrites(i - 1).hasTransform() && batch.getWrites(i - 1).hasUpdate(),
+            "TransformMutation should be preceded by a patch or set mutation");
+        Write mutationToJoin = batch.getWrites(i - 1);
+        Builder newMutationBuilder = Write.newBuilder(mutationToJoin);
+        for (FieldTransform fieldTransform : mutation.getTransform().getFieldTransformsList()) {
+          newMutationBuilder.addUpdateTransforms(fieldTransform);
+        }
+        mutations.add(rpcSerializer.decodeMutation(newMutationBuilder.build()));
+        --i;
+      } else {
+        mutations.add(rpcSerializer.decodeMutation(mutation));
+      }
     }
+
+    // Reverse the mutations to preserve the original ordering since the above for-loop iterates in
+    // reverse order. We use reverse() instead of prepending the elements into the mutations array
+    // since prepending to a List is O(n).
+    Collections.reverse(mutations);
     return new MutationBatch(batchId, localWriteTime, baseMutations, mutations);
   }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java

@@ -219,7 +219,7 @@ public LocalWriteResult writeLocally(List<Mutation> mutations) {
           List<Mutation> baseMutations = new ArrayList<>();
           for (Mutation mutation : mutations) {
             ObjectValue baseValue =
-                mutation.extractBaseValue(existingDocuments.get(mutation.getKey()));
+                mutation.extractTransformBaseValue(existingDocuments.get(mutation.getKey()));
             if (baseValue != null) {
               // NOTE: The base state should only be applied if there's some existing
               // document to override, so use a Precondition of exists=true

firebase-firestore/src/main/java/com/google/firebase/firestore/model/ServerTimestamps.java

@@ -28,9 +28,8 @@
  * `__previous_value__` and `__local_write_time__` fields respectively.
  *
  * <p>Notes:
- * <li>ServerTimestamp Values 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.
+ * <li>ServerTimestamp Values are created as the result of applying a transform. They can only exist
+ *     in the local view of a document. Therefore they do not need to be parsed or serialized.
  * <li>When evaluated locally (e.g. via DocumentSnapshot data), they evaluate to null.
  * <li>They sort after all Timestamp Values. With respect to other ServerTimestamp Values, they sort
  *     by their localWriteTime.

firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/DeleteMutation.java

@@ -21,7 +21,6 @@
 import com.google.firebase.firestore.model.DocumentKey;
 import com.google.firebase.firestore.model.MaybeDocument;
 import com.google.firebase.firestore.model.NoDocument;
-import com.google.firebase.firestore.model.ObjectValue;
 import com.google.firebase.firestore.model.SnapshotVersion;
 
 /** Represents a Delete operation */
@@ -83,10 +82,4 @@ public MaybeDocument applyToLocalView(
 
     return new NoDocument(getKey(), SnapshotVersion.NONE, /*hasCommittedMutations=*/ false);
   }
-
-  @Nullable
-  @Override
-  public ObjectValue extractBaseValue(@Nullable MaybeDocument maybeDoc) {
-    return null;
-  }
 }