diff --git a/firebase-firestore/CHANGELOG.md b/firebase-firestore/CHANGELOG.md index e768fc49283..0e3fb6dac41 100644 --- a/firebase-firestore/CHANGELOG.md +++ b/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. diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java index 12500fbd6eb..9f820f1ddf3 100644 --- a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java +++ b/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 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"); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataConverter.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataConverter.java index 67cc6b3e7d7..d02ae4eaac6 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataConverter.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataConverter.java @@ -258,15 +258,23 @@ private FieldValue parseData(Object input, ParseContext context) { private ObjectValue parseMap(Map map, ParseContext context) { Map result = new HashMap<>(); - for (Entry 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 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); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/PatchMutation.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/PatchMutation.java index bb3a9a8f17c..6b246a55040 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/PatchMutation.java +++ b/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;