Allowing empty merges

schmidt-sebastian · schmidt-sebastian · commit 447dbddebbcb · 2018-02-26T07:55:37.000-08:00
diff --git a/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UpdateBuilder.java b/google-cloud-firestore/src/main/java/com/google/cloud/firestore/UpdateBuilder.java
@@ -245,7 +245,6 @@ private T performSet(
         DocumentTransform.fromFieldPathMap(documentReference, documentData);
 
     if (options.isMerge()) {
-      Preconditions.checkArgument(!fields.isEmpty(), "Data to merge cannot be empty.");
       if (options.getFieldMask() != null) {
         List<FieldPath> fieldMask = new ArrayList<>(options.getFieldMask());
         fieldMask.removeAll(documentTransform.getFields());
@@ -257,11 +256,12 @@ private T performSet(
 
     Mutation mutation = addMutation();
 
-    if (!options.isMerge() || !documentSnapshot.isEmpty() || !documentMask.isEmpty()) {
-      mutation.document = documentSnapshot.toPb();
-    }
+    boolean hasDocumentData = !documentSnapshot.isEmpty() || !documentMask.isEmpty();
 
-    if (!documentMask.isEmpty()) {
+    if (!options.isMerge()) {
+      mutation.document = documentSnapshot.toPb();
+    } else if (hasDocumentData || documentTransform.isEmpty()) {
+      mutation.document = documentSnapshot.toPb();
       mutation.document.setUpdateMask(documentMask.toPb());
     }
 
diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/ConformanceTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/ConformanceTest.java
@@ -85,7 +85,8 @@ public class ConformanceTest {
   private interface ConformanceTestCase extends Test, Describable {}
 
   /** Excluded tests by test description. */
-  private final Set<String> excludedTests = Collections.emptySet();
+  private final Set<String> excludedTests =
+      Collections.singleton("set: MergeAll cannot be specified with empty data."); // b/73495873
 
   /** If non-empty, only runs tests included in this set. */
   private final Set<String> includedTests = Collections.emptySet();
diff --git a/google-cloud-firestore/src/test/java/com/google/cloud/firestore/DocumentReferenceTest.java b/google-cloud-firestore/src/test/java/com/google/cloud/firestore/DocumentReferenceTest.java
@@ -411,6 +411,20 @@ public void setDocumentWithMerge() throws Exception {
     }
   }
 
+  @Test
+  public void setDocumentWithEmptyMerge() throws Exception {
+    doReturn(SINGLE_WRITE_COMMIT_RESPONSE)
+        .when(firestoreMock)
+        .sendRequest(
+            commitCapture.capture(), Matchers.<UnaryCallable<CommitRequest, CommitResponse>>any());
+
+    documentReference.set(map(), SetOptions.merge()).get();
+
+    assertCommitEquals(
+        commit(set(Collections.<String, Value>emptyMap(), Collections.<String>emptyList())),
+        commitCapture.getValue());
+  }
+
   @Test
   public void setDocumentWithNestedMerge() throws Exception {
     doReturn(SINGLE_WRITE_COMMIT_RESPONSE)
