Don't drop empty object during merge

Author: schmidt-sebastian

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
+  `setData(..., merge:true)`.
 
 # v0.13.3
 - [changed] Internal improvements.

Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm

@@ -18,6 +18,7 @@
 
 #import <XCTest/XCTest.h>
 
+#import "Firestore/Example/Tests/Util/FSTEventAccumulator.h"
 #import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h"
 #import "Firestore/Source/API/FIRFirestore+Internal.h"
 #import "Firestore/Source/Core/FSTFirestoreClient.h"
@@ -155,6 +156,31 @@ - (void)testCanMergeDataWithAnExistingDocumentUsingSet {
   XCTAssertEqualObjects(document.data, finalData);
 }
 
+- (void)testCanMergeEmptyObject {
+  FIRDocumentReference *doc = [[self.db collectionWithPath:@"rooms"] documentWithAutoID];
+
+  FSTEventAccumulator *accumulator = [FSTEventAccumulator accumulatorForTest:self];
+  id<FIRListenerRegistration> listenerRegistration =
+      [doc addSnapshotListener:[accumulator valueEventHandler]];
+
+  [self writeDocumentRef:doc data:@{}];
+  FIRDocumentSnapshot *snapshot = [accumulator awaitEventWithName:@"Snapshot"];
+  XCTAssertEqualObjects(snapshot.data, @{});
+
+  [self mergeDocumentRef:doc data:@{@"a" : @{}} fields:@[ @"a" ]];
+  snapshot = [accumulator awaitEventWithName:@"Snapshot"];
+  XCTAssertEqualObjects(snapshot.data, @{@"a" : @{}});
+
+  [self mergeDocumentRef:doc data:@{@"b" : @{}}];
+  snapshot = [accumulator awaitEventWithName:@"Snapshot"];
+  XCTAssertEqualObjects(snapshot.data, (@{@"a" : @{}, @"b" : @{}}));
+
+  snapshot = [self readDocumentForRef:doc source:FIRFirestoreSourceServer];
+  XCTAssertEqualObjects(snapshot.data, (@{@"a" : @{}, @"b" : @{}}));
+
+  [listenerRegistration remove];
+}
+
 - (void)testCanMergeServerTimestamps {
   FIRDocumentReference *doc = [[self.db collectionWithPath:@"rooms"] documentWithAutoID];
 

Firestore/Example/Tests/Util/FSTIntegrationTestCase.h

@@ -90,6 +90,10 @@ extern "C" {
 
 - (void)mergeDocumentRef:(FIRDocumentReference *)ref data:(NSDictionary<NSString *, id> *)data;
 
+- (void)mergeDocumentRef:(FIRDocumentReference *)ref
+                    data:(NSDictionary<NSString *, id> *)data
+                  fields:(NSArray<id> *)fields;
+
 - (void)disableNetwork;
 
 - (void)enableNetwork;

Firestore/Example/Tests/Util/FSTIntegrationTestCase.mm

@@ -331,6 +331,15 @@ - (void)mergeDocumentRef:(FIRDocumentReference *)ref data:(NSDictionary<NSString
   [self awaitExpectations];
 }
 
+- (void)mergeDocumentRef:(FIRDocumentReference *)ref
+                    data:(NSDictionary<NSString *, id> *)data
+                  fields:(NSArray<id> *)fields {
+  [ref setData:data
+      mergeFields:fields
+       completion:[self completionForExpectationWithName:@"setDataWithMerge"]];
+  [self awaitExpectations];
+}
+
 - (void)disableNetwork {
   [self.db.client
       disableNetworkWithCompletion:[self completionForExpectationWithName:@"Disable Network."]];

Firestore/Source/API/FSTUserDataConverter.mm

@@ -257,13 +257,22 @@ - (nullable FSTFieldValue *)parseData:(id)input context:(ParseContext &&)context
 - (FSTFieldValue *)parseDictionary:(NSDictionary *)dict context:(ParseContext &&)context {
   NSMutableDictionary<NSString *, FSTFieldValue *> *result =
       [NSMutableDictionary dictionaryWithCapacity:dict.count];
-  [dict enumerateKeysAndObjectsUsingBlock:^(NSString *key, id value, BOOL *stop) {
-    FSTFieldValue *_Nullable parsedValue =
-        [self parseData:value context:context.ChildContext(util::MakeString(key))];
-    if (parsedValue) {
-      result[key] = parsedValue;
+
+  if ([dict count] == 0) {
+    const FieldPath *path = context.path();
+    if (path && !path->empty()) {
+      context.AddToFieldMask(*path);
     }
-  }];
+    return [FSTObjectValue objectValue];
+  } else {
+    [dict enumerateKeysAndObjectsUsingBlock:^(NSString *key, id value, BOOL *stop) {
+      FSTFieldValue *_Nullable parsedValue =
+          [self parseData:value context:context.ChildContext(util::MakeString(key))];
+      if (parsedValue) {
+        result[key] = parsedValue;
+      }
+    }];
+  }
   return [[FSTObjectValue alloc] initWithDictionary:result];
 }
 

Firestore/Source/Model/FSTMutation.mm

@@ -271,11 +271,13 @@ - (nullable FSTMaybeDocument *)applyTo:(nullable FSTMaybeDocument *)maybeDoc
 - (FSTObjectValue *)patchObjectValue:(FSTObjectValue *)objectValue {
   FSTObjectValue *result = objectValue;
   for (const FieldPath &fieldPath : self.fieldMask) {
-    FSTFieldValue *newValue = [self.value valueForPath:fieldPath];
-    if (newValue) {
-      result = [result objectBySettingValue:newValue forPath:fieldPath];
-    } else {
-      result = [result objectByDeletingPath:fieldPath];
+    if (!fieldPath.empty()) {
+      FSTFieldValue *newValue = [self.value valueForPath:fieldPath];
+      if (newValue) {
+        result = [result objectBySettingValue:newValue forPath:fieldPath];
+      } else {
+        result = [result objectByDeletingPath:fieldPath];
+      }
     }
   }
   return result;