Separate out mergeMutation from patchMutation (#2306)

Author: Brian Chen

firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java

@@ -22,6 +22,7 @@
 import static com.google.firebase.firestore.testutil.TestUtil.filter;
 import static com.google.firebase.firestore.testutil.TestUtil.key;
 import static com.google.firebase.firestore.testutil.TestUtil.map;
+import static com.google.firebase.firestore.testutil.TestUtil.mergeMutation;
 import static com.google.firebase.firestore.testutil.TestUtil.noChangeEvent;
 import static com.google.firebase.firestore.testutil.TestUtil.patchMutation;
 import static com.google.firebase.firestore.testutil.TestUtil.query;
@@ -1298,7 +1299,7 @@ public void testHandlesMergeMutationWithTransformThenRemoteEvent() {
     assertTargetId(2);
 
     writeMutation(
-        patchMutation("foo/bar", map("sum", FieldValue.increment(1)), Collections.EMPTY_LIST));
+        mergeMutation("foo/bar", map("sum", FieldValue.increment(1)), Collections.EMPTY_LIST));
     assertChanged(doc("foo/bar", 0, map("sum", 1), Document.DocumentState.LOCAL_MUTATIONS));
     assertContains(doc("foo/bar", 0, map("sum", 1), Document.DocumentState.LOCAL_MUTATIONS));
 

firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java

@@ -21,6 +21,7 @@
 import static com.google.firebase.firestore.testutil.TestUtil.fieldMask;
 import static com.google.firebase.firestore.testutil.TestUtil.key;
 import static com.google.firebase.firestore.testutil.TestUtil.map;
+import static com.google.firebase.firestore.testutil.TestUtil.mergeMutation;
 import static com.google.firebase.firestore.testutil.TestUtil.mutationResult;
 import static com.google.firebase.firestore.testutil.TestUtil.patchMutation;
 import static com.google.firebase.firestore.testutil.TestUtil.setMutation;
@@ -84,7 +85,7 @@ public void testAppliesPatchToDocuments() {
   public void testAppliesPatchWithMergeToDocuments() {
     MaybeDocument baseDoc = deletedDoc("collection/key", 0);
     Mutation upsert =
-        patchMutation(
+        mergeMutation(
             "collection/key", map("foo.bar", "new-bar-value"), Arrays.asList(field("foo.bar")));
     MaybeDocument newDoc = upsert.applyToLocalView(baseDoc, baseDoc, Timestamp.now());
     Map<String, Object> expectedData = map("foo", map("bar", "new-bar-value"));
@@ -96,7 +97,7 @@ public void testAppliesPatchWithMergeToDocuments() {
   public void testAppliesPatchToNullDocWithMergeToDocuments() {
     MaybeDocument baseDoc = null;
     Mutation upsert =
-        patchMutation(
+        mergeMutation(
             "collection/key", map("foo.bar", "new-bar-value"), Arrays.asList(field("foo.bar")));
     MaybeDocument newDoc = upsert.applyToLocalView(baseDoc, baseDoc, Timestamp.now());
     Map<String, Object> expectedData = map("foo", map("bar", "new-bar-value"));

firebase-firestore/src/test/java/com/google/firebase/firestore/remote/RemoteSerializerTest.java

@@ -21,6 +21,7 @@
 import static com.google.firebase.firestore.testutil.TestUtil.filter;
 import static com.google.firebase.firestore.testutil.TestUtil.key;
 import static com.google.firebase.firestore.testutil.TestUtil.map;
+import static com.google.firebase.firestore.testutil.TestUtil.mergeMutation;
 import static com.google.firebase.firestore.testutil.TestUtil.orderBy;
 import static com.google.firebase.firestore.testutil.TestUtil.patchMutation;
 import static com.google.firebase.firestore.testutil.TestUtil.query;
@@ -362,7 +363,7 @@ public void testEncodesPatchMutation() {
   @Test
   public void testEncodesPatchMutationWithFieldMask() {
     Mutation mutation =
-        patchMutation("docs/1", map("key", "value", "key2", true), asList(field("key")));
+        mergeMutation("docs/1", map("key", "value", "key2", true), asList(field("key")));
 
     Write expected =
         Write.newBuilder()

firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java

@@ -486,12 +486,20 @@ public static SetMutation setMutation(String path, Map<String, Object> values) {
   }
 
   public static PatchMutation patchMutation(String path, Map<String, Object> values) {
-    return patchMutation(path, values, null);
+    return patchMutationHelper(path, values, Precondition.exists(true), null);
   }
 
-  public static PatchMutation patchMutation(
-      String path, Map<String, Object> values, @Nullable List<FieldPath> updateMask) {
-    // Replace '<DELETE>' from JSON with FieldValue
+  public static PatchMutation mergeMutation(
+      String path, Map<String, Object> values, List<FieldPath> updateMask) {
+    return patchMutationHelper(path, values, Precondition.NONE, updateMask);
+  }
+
+  private static PatchMutation patchMutationHelper(
+      String path,
+      Map<String, Object> values,
+      Precondition precondition,
+      @Nullable List<FieldPath> updateMask) {
+    // Replace '<DELETE>' from JSON
     for (Entry<String, Object> entry : values.entrySet()) {
       if (entry.getValue().equals(DELETE_SENTINEL)) {
         values.put(entry.getKey(), FieldValue.delete());
@@ -500,13 +508,15 @@ public static PatchMutation patchMutation(
 
     UserDataReader dataReader = new UserDataReader(DatabaseId.forProject("project"));
     ParsedUpdateData parsed = dataReader.parseUpdateData(values);
-    boolean merge = updateMask != null;
+
+    // `mergeMutation()` provides an update mask for the merged fields, whereas `patchMutation()`
+    // requires the update mask to be parsed from the values.
+    Collection<FieldPath> mask = updateMask != null ? updateMask : parsed.getFieldMask().getMask();
 
     // We sort the fieldMaskPaths to make the order deterministic in tests. (Otherwise, when we
     // flatten a Set to a proto repeated field, we'll end up comparing in iterator order and
     // possibly consider {foo,bar} != {bar,foo}.)
-    SortedSet<FieldPath> fieldMaskPaths =
-        new TreeSet<>(merge ? updateMask : parsed.getFieldMask().getMask());
+    SortedSet<FieldPath> fieldMaskPaths = new TreeSet<>(mask);
 
     // The order of the transforms doesn't matter, but we sort them so tests can assume a particular
     // order.
@@ -518,7 +528,7 @@ public static PatchMutation patchMutation(
         key(path),
         parsed.getData(),
         FieldMask.fromSet(fieldMaskPaths),
-        merge ? Precondition.NONE : Precondition.exists(true),
+        precondition,
         fieldTransforms);
   }