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/App/GoogleService-Info.plist

@@ -123,7 +123,6 @@
 		54A0353020A3B3D8003E0143 /* statusor_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54A0352D20A3B3D7003E0143 /* statusor_test.cc */; };
 		54A0353520A3D8CB003E0143 /* iterator_adaptors_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54A0353420A3D8CB003E0143 /* iterator_adaptors_test.cc */; };
 		54C2294F1FECABAE007D065B /* log_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54C2294E1FECABAE007D065B /* log_test.cc */; };
-		54D400D42148BACE001D2BCC /* GoogleService-Info.plist in Resources */ = {isa = PBXBuildFile; fileRef = 54D400D32148BACE001D2BCC /* GoogleService-Info.plist */; };
 		54DA12A61F315EE100DD57A1 /* collection_spec_test.json in Resources */ = {isa = PBXBuildFile; fileRef = 54DA129C1F315EE100DD57A1 /* collection_spec_test.json */; };
 		54DA12A71F315EE100DD57A1 /* existence_filter_spec_test.json in Resources */ = {isa = PBXBuildFile; fileRef = 54DA129D1F315EE100DD57A1 /* existence_filter_spec_test.json */; };
 		54DA12A81F315EE100DD57A1 /* limbo_spec_test.json in Resources */ = {isa = PBXBuildFile; fileRef = 54DA129E1F315EE100DD57A1 /* limbo_spec_test.json */; };
@@ -217,7 +216,10 @@
 		C80B10E79CDD7EF7843C321E /* type_traits_apple_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2A0CF41BA5AED6049B0BEB2C /* type_traits_apple_test.mm */; };
 		C8D3CE2343E53223E6487F2C /* Pods_Firestore_Example_iOS.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 5918805E993304321A05E82B /* Pods_Firestore_Example_iOS.framework */; };
 		CA989C0E6020C372A62B7062 /* testutil.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54A0352820A3B3BD003E0143 /* testutil.cc */; };
+		D5B2529BEB307D0725979F62 /* GoogleService-Info.plist in Resources */ = {isa = PBXBuildFile; fileRef = D5B259789B257368234FA2F6 /* GoogleService-Info.plist */; };
 		D5B25CBF07F65E885C9D68AB /* perf_spec_test.json in Resources */ = {isa = PBXBuildFile; fileRef = D5B2593BCB52957D62F1C9D3 /* perf_spec_test.json */; };
+		D5B25DDFD27670BEA5AD70EE /* GoogleService-Info.plist in Resources */ = {isa = PBXBuildFile; fileRef = D5B259789B257368234FA2F6 /* GoogleService-Info.plist */; };
+		D5B25EE88A01AC7768B6D98F /* GoogleService-Info.plist in Resources */ = {isa = PBXBuildFile; fileRef = D5B259789B257368234FA2F6 /* GoogleService-Info.plist */; };
 		D94A1862B8FB778225DB54A1 /* filesystem_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = F51859B394D01C0C507282F1 /* filesystem_test.cc */; };
 		DD5976A45071455FF3FE74B8 /* string_win_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 79507DF8378D3C42F5B36268 /* string_win_test.cc */; };
 		DE03B2D41F2149D600A30B9C /* XCTest.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 6003F5AF195388D20070C39A /* XCTest.framework */; };
@@ -408,7 +410,6 @@
 		54C2294E1FECABAE007D065B /* log_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = log_test.cc; sourceTree = "<group>"; };
 		54C9EDF12040E16300A969CD /* Firestore_SwiftTests_iOS.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = Firestore_SwiftTests_iOS.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
 		54C9EDF52040E16300A969CD /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
-		54D400D32148BACE001D2BCC /* GoogleService-Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; name = "GoogleService-Info.plist"; path = "App/GoogleService-Info.plist"; sourceTree = SOURCE_ROOT; };
 		54DA129C1F315EE100DD57A1 /* collection_spec_test.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = collection_spec_test.json; sourceTree = "<group>"; };
 		54DA129D1F315EE100DD57A1 /* existence_filter_spec_test.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = existence_filter_spec_test.json; sourceTree = "<group>"; };
 		54DA129E1F315EE100DD57A1 /* limbo_spec_test.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = limbo_spec_test.json; sourceTree = "<group>"; };
@@ -532,6 +533,7 @@
 		BB92EB03E3F92485023F64ED /* Pods_Firestore_Example_iOS_Firestore_SwiftTests_iOS.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_Example_iOS_Firestore_SwiftTests_iOS.framework; sourceTree = BUILT_PRODUCTS_DIR; };
 		D3CC3DC5338DCAF43A211155 /* README.md */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = net.daringfireball.markdown; name = README.md; path = ../README.md; sourceTree = "<group>"; };
 		D5B2593BCB52957D62F1C9D3 /* perf_spec_test.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = perf_spec_test.json; sourceTree = "<group>"; };
+		D5B259789B257368234FA2F6 /* GoogleService-Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.info; path = "GoogleService-Info.plist"; sourceTree = "<group>"; };
 		DE03B2E91F2149D600A30B9C /* Firestore_IntegrationTests_iOS.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = Firestore_IntegrationTests_iOS.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
 		DE03B3621F215E1600A30B9C /* CAcert.pem */ = {isa = PBXFileReference; lastKnownFileType = text; path = CAcert.pem; sourceTree = "<group>"; };
 		DE0761F61F2FE68D003233AF /* BasicCompileTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BasicCompileTests.swift; sourceTree = "<group>"; };
@@ -625,7 +627,7 @@
 			isa = PBXGroup;
 			children = (
 				6003F593195388D20070C39A /* iOS */,
-				54D400D32148BACE001D2BCC /* GoogleService-Info.plist */,
+				D5B259789B257368234FA2F6 /* GoogleService-Info.plist */,
 			);
 			path = App;
 			sourceTree = "<group>";
@@ -1440,11 +1442,11 @@
 			isa = PBXResourcesBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
-				54D400D42148BACE001D2BCC /* GoogleService-Info.plist in Resources */,
 				6003F5A9195388D20070C39A /* Images.xcassets in Resources */,
 				873B8AEB1B1F5CCA007FD442 /* Main.storyboard in Resources */,
 				71719F9F1E33DC2100824A3D /* LaunchScreen.storyboard in Resources */,
 				6003F598195388D20070C39A /* InfoPlist.strings in Resources */,
+				D5B25DDFD27670BEA5AD70EE /* GoogleService-Info.plist in Resources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};
@@ -1465,6 +1467,7 @@
 				3B843E4C1F3A182900548890 /* remote_store_spec_test.json in Resources */,
 				54DA12AE1F315EE100DD57A1 /* resume_token_spec_test.json in Resources */,
 				54DA12AF1F315EE100DD57A1 /* write_spec_test.json in Resources */,
+				D5B2529BEB307D0725979F62 /* GoogleService-Info.plist in Resources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};
@@ -1482,6 +1485,7 @@
 			files = (
 				DE03B3631F215E1A00A30B9C /* CAcert.pem in Resources */,
 				DE03B2DD1F2149D600A30B9C /* InfoPlist.strings in Resources */,
+				D5B25EE88A01AC7768B6D98F /* GoogleService-Info.plist in Resources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};

Firestore/Example/Firestore.xcodeproj/project.pbxproj

@@ -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/Integration/API/FIRDatabaseTests.mm

@@ -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.h

@@ -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/Example/Tests/Util/FSTIntegrationTestCase.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/API/FSTUserDataConverter.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;