Don't drop empty objects during merge (#26)

Author: schmidt-sebastian

firebase-firestore/CHANGELOG.md

@@ -4,6 +4,8 @@
 - [changed] Changed `get()` to only make 1 attempt to reach the backend before
   returning cached data, potentially reducing delays while offline. Previously
   it would make 2 attempts, to work around a backend bug.
+- [fixed] Fixed an issue that caused us to drop empty objects from calls to
+  `set(..., SetOptions.merge())`.
 - [changed] Some SDK errors that represent common mistakes (such as permission
   denied or a missing index) will automatically be logged as a warning in
   addition to being surfaced via the API.

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java

@@ -39,9 +39,11 @@
 import com.google.firebase.Timestamp;
 import com.google.firebase.firestore.FirebaseFirestoreException.Code;
 import com.google.firebase.firestore.Query.Direction;
+import com.google.firebase.firestore.testutil.EventAccumulator;
 import com.google.firebase.firestore.testutil.IntegrationTestUtil;
 import com.google.firebase.firestore.util.AsyncQueue.TimerId;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
@@ -110,6 +112,31 @@ public void testCanMergeServerTimestamps() {
     assertTrue(doc.get("nested.time") instanceof Timestamp);
   }
 
+  @Test
+  public void testCanMergeEmptyObject() {
+    DocumentReference documentReference = testDocument();
+    EventAccumulator<DocumentSnapshot> eventAccumulator = new EventAccumulator<>();
+    ListenerRegistration listenerRegistration =
+        documentReference.addSnapshotListener(eventAccumulator.listener());
+    eventAccumulator.await();
+
+    documentReference.set(Collections.emptyMap());
+    DocumentSnapshot snapshot = eventAccumulator.await();
+    assertEquals(Collections.emptyMap(), snapshot.getData());
+
+    waitFor(documentReference.set(map("a", Collections.emptyMap()), SetOptions.mergeFields("a")));
+    snapshot = eventAccumulator.await();
+    assertEquals(map("a", Collections.emptyMap()), snapshot.getData());
+
+    waitFor(documentReference.set(map("b", Collections.emptyMap()), SetOptions.merge()));
+    snapshot = eventAccumulator.await();
+    assertEquals(map("a", Collections.emptyMap(), "b", Collections.emptyMap()), snapshot.getData());
+
+    snapshot = waitFor(documentReference.get(Source.SERVER));
+    assertEquals(map("a", Collections.emptyMap(), "b", Collections.emptyMap()), snapshot.getData());
+    listenerRegistration.remove();
+  }
+
   @Test
   public void testCanDeleteFieldUsingMerge() {
     DocumentReference documentReference = testCollection("rooms").document("eros");

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

@@ -258,15 +258,23 @@ private FieldValue parseData(Object input, ParseContext context) {
 
   private <K, V> ObjectValue parseMap(Map<K, V> map, ParseContext context) {
     Map<String, FieldValue> result = new HashMap<>();
-    for (Entry<K, V> entry : map.entrySet()) {
-      if (!(entry.getKey() instanceof String)) {
-        throw context.createError(
-            String.format("Non-String Map key (%s) is not allowed", entry.getValue()));
+
+    if (map.isEmpty()) {
+      if (context.getPath() != null && !context.getPath().isEmpty()) {
+        context.addToFieldMask(context.getPath());
       }
-      String key = (String) entry.getKey();
-      @Nullable FieldValue parsedValue = parseData(entry.getValue(), context.childContext(key));
-      if (parsedValue != null) {
-        result.put(key, parsedValue);
+      return ObjectValue.emptyObject();
+    } else {
+      for (Entry<K, V> entry : map.entrySet()) {
+        if (!(entry.getKey() instanceof String)) {
+          throw context.createError(
+              String.format("Non-String Map key (%s) is not allowed", entry.getValue()));
+        }
+        String key = (String) entry.getKey();
+        @Nullable FieldValue parsedValue = parseData(entry.getValue(), context.childContext(key));
+        if (parsedValue != null) {
+          result.put(key, parsedValue);
+        }
       }
     }
     return ObjectValue.fromMap(result);

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

@@ -147,11 +147,13 @@ private ObjectValue patchDocument(@Nullable MaybeDocument maybeDoc) {
 
   private ObjectValue patchObject(ObjectValue obj) {
     for (FieldPath path : mask.getMask()) {
-      FieldValue newValue = value.get(path);
-      if (newValue == null) {
-        obj = obj.delete(path);
-      } else {
-        obj = obj.set(path, newValue);
+      if (!path.isEmpty()) {
+        FieldValue newValue = value.get(path);
+        if (newValue == null) {
+          obj = obj.delete(path);
+        } else {
+          obj = obj.set(path, newValue);
+        }
       }
     }
     return obj;