Simplify Mutations

Author: schmidt-sebastian

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

@@ -19,15 +19,12 @@
 import com.google.firebase.Timestamp;
 import com.google.firebase.firestore.model.Document;
 import com.google.firebase.firestore.model.DocumentKey;
-import com.google.firebase.firestore.model.FieldPath;
 import com.google.firebase.firestore.model.MutableDocument;
 import com.google.firebase.firestore.model.ObjectValue;
 import com.google.firebase.firestore.model.SnapshotVersion;
 import com.google.firestore.v1.Value;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 
 /**
  * Represents a Mutation of a document. Different subclasses of Mutation will perform different
@@ -150,17 +147,14 @@ static SnapshotVersion getPostMutationVersion(MutableDocument document) {
   }
 
   /**
-   * Creates a list of "transform results" (a transform result is a field value representing the
-   * result of applying a transform) for use after a mutation containing transforms has been
-   * acknowledged by the server.
+   * Applies the result of applying a transform by the backend.
    *
-   * @param mutableDocument The current state of the document after applying all previous mutations.
+   * @param newValue The object value to mutate.
+   * @param existingData The current state of the document after applying all previous mutations.
    * @param serverTransformResults The transform results received by the server.
-   * @return A map of fields to transform results.
    */
-  protected Map<FieldPath, Value> serverTransformResults(
-      MutableDocument mutableDocument, List<Value> serverTransformResults) {
-    Map<FieldPath, Value> transformResults = new HashMap<>(fieldTransforms.size());
+  protected void applyServerTransformResults(
+      ObjectValue newValue, Document existingData, List<Value> serverTransformResults) {
     hardAssert(
         fieldTransforms.size() == serverTransformResults.size(),
         "server transform count (%d) should match field transform count (%d)",
@@ -170,42 +164,28 @@ protected Map<FieldPath, Value> serverTransformResults(
     for (int i = 0; i < serverTransformResults.size(); i++) {
       FieldTransform fieldTransform = fieldTransforms.get(i);
       TransformOperation transform = fieldTransform.getOperation();
-
-      Value previousValue = null;
-      if (mutableDocument.isFoundDocument()) {
-        previousValue = mutableDocument.getField(fieldTransform.getFieldPath());
-      }
-
-      transformResults.put(
-          fieldTransform.getFieldPath(),
-          transform.applyToRemoteDocument(previousValue, serverTransformResults.get(i)));
+      Value previousValue = existingData.getField(fieldTransform.getFieldPath());
+      Value transformedValue =
+          transform.applyToRemoteDocument(previousValue, serverTransformResults.get(i));
+      newValue.set(fieldTransform.getFieldPath(), transformedValue);
     }
-    return transformResults;
   }
 
   /**
-   * 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 transform locally.
+   * Applies the result a transform locally.
    *
+   * @param newValue The object value to mutate.
+   * @param existingData The current state of the document after applying all previous mutations.
    * @param localWriteTime The local time of the mutation (used to generate ServerTimestampValues).
-   * @param mutableDocument The current state of the document after applying all previous mutations.
-   * @return A map of fields to transform results.
    */
-  protected Map<FieldPath, Value> localTransformResults(
-      Timestamp localWriteTime, MutableDocument mutableDocument) {
-    Map<FieldPath, Value> transformResults = new HashMap<>(fieldTransforms.size());
+  protected void applyLocalTransformResults(
+      ObjectValue newValue, Document existingData, Timestamp localWriteTime) {
     for (FieldTransform fieldTransform : fieldTransforms) {
       TransformOperation transform = fieldTransform.getOperation();
-
-      Value previousValue = null;
-      if (mutableDocument.isFoundDocument()) {
-        previousValue = mutableDocument.getField(fieldTransform.getFieldPath());
-      }
-
-      transformResults.put(
-          fieldTransform.getFieldPath(), transform.applyToLocalView(previousValue, localWriteTime));
+      Value previousValue = existingData.getField(fieldTransform.getFieldPath());
+      Value transformedValue = transform.applyToLocalView(previousValue, localWriteTime);
+      newValue.set(fieldTransform.getFieldPath(), transformedValue);
     }
-    return transformResults;
   }
 
   public ObjectValue extractTransformBaseValue(Document document) {

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

@@ -115,11 +115,9 @@ public void applyToRemoteDocument(MutableDocument document, MutationResult mutat
       return;
     }
 
-    Map<FieldPath, Value> transformResults =
-        serverTransformResults(document, mutationResult.getTransformResults());
     ObjectValue value = document.getData();
+    applyServerTransformResults(value, document, mutationResult.getTransformResults());
     value.setAll(getPatch());
-    value.setAll(transformResults);
     document
         .convertToFoundDocument(mutationResult.getVersion(), document.getData())
         .setHasCommittedMutations();
@@ -133,10 +131,9 @@ public void applyToLocalView(MutableDocument document, Timestamp localWriteTime)
       return;
     }
 
-    Map<FieldPath, Value> transformResults = localTransformResults(localWriteTime, document);
     ObjectValue value = document.getData();
+    applyLocalTransformResults(value, document, localWriteTime);
     value.setAll(getPatch());
-    value.setAll(transformResults);
     document
         .convertToFoundDocument(getPostMutationVersion(document), document.getData())
         .setHasLocalMutations();

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

@@ -16,13 +16,10 @@
 
 import com.google.firebase.Timestamp;
 import com.google.firebase.firestore.model.DocumentKey;
-import com.google.firebase.firestore.model.FieldPath;
 import com.google.firebase.firestore.model.MutableDocument;
 import com.google.firebase.firestore.model.ObjectValue;
-import com.google.firestore.v1.Value;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
 
 /**
  * A mutation that creates or replaces the document at the given key with the object value contents.
@@ -77,9 +74,7 @@ public void applyToRemoteDocument(MutableDocument document, MutationResult mutat
     // Unlike applyToLocalView, if we're applying a mutation to a remote document the server has
     // accepted the mutation so the precondition must have held.
     ObjectValue newData = value.clone();
-    Map<FieldPath, Value> transformResults =
-        serverTransformResults(document, mutationResult.getTransformResults());
-    newData.setAll(transformResults);
+    applyServerTransformResults(newData, document, mutationResult.getTransformResults());
     document
         .convertToFoundDocument(mutationResult.getVersion(), newData)
         .setHasCommittedMutations();
@@ -93,9 +88,8 @@ public void applyToLocalView(MutableDocument document, Timestamp localWriteTime)
       return;
     }
 
-    Map<FieldPath, Value> transformResults = localTransformResults(localWriteTime, document);
     ObjectValue localValue = value.clone();
-    localValue.setAll(transformResults);
+    applyLocalTransformResults(localValue, document, localWriteTime);
     document
         .convertToFoundDocument(getPostMutationVersion(document), localValue)
         .setHasLocalMutations();