skip unmodified keys in dynamic::merge_diff

Summary:
`dynamic::merge_diff` currently fully replicates `target` within the
patch.

Reviewed By: psjanani, luciang

Differential Revision: D26100408

fbshipit-source-id: f6800e665281d87393fed0bb48bda888e24bbcfc

folly/dynamic.cpp

@@ -333,7 +333,7 @@ void dynamic::destroy() noexcept {
 }
 
 dynamic dynamic::merge_diff(const dynamic& source, const dynamic& target) {
-  if (!source.isObject() || source.type() != target.type()) {
+  if (!source.isObject() || !target.isObject()) {
     return target;
   }
 
@@ -345,7 +345,16 @@ dynamic dynamic::merge_diff(const dynamic& source, const dynamic& target) {
     if (it == source.items().end()) {
       diff[pair.first] = pair.second;
     } else {
-      diff[pair.first] = merge_diff(source[pair.first], target[pair.first]);
+      const auto& ssource = it->second;
+      const auto& starget = pair.second;
+      if (ssource.isObject() && starget.isObject()) {
+        auto sdiff = merge_diff(ssource, starget);
+        if (!sdiff.empty()) {
+          diff[pair.first] = std::move(sdiff);
+        }
+      } else if (ssource != starget) {
+        diff[pair.first] = merge_diff(ssource, starget);
+      }
     }
   }
 

folly/test/DynamicTest.cpp

@@ -1013,8 +1013,8 @@ TEST(Dynamic, MergePatchRemoveNonExistent) {
 }
 
 TEST(Dynamic, MergeDiffFlatObjects) {
-  dynamic source = dynamic::object("a", 0)("b", 1)("c", 2);
-  dynamic target = dynamic::object("a", 1)("b", 2);
+  dynamic source = dynamic::object("a", 0)("b", 1)("c", 2)("d", 3);
+  dynamic target = dynamic::object("a", 1)("b", 2)("d", 3);
   auto patch = dynamic::merge_diff(source, target);
 
   EXPECT_EQ(3, patch.size());
@@ -1027,10 +1027,16 @@ TEST(Dynamic, MergeDiffFlatObjects) {
 }
 
 TEST(Dynamic, MergeDiffNestedObjects) {
-  dynamic source = dynamic::object("a", dynamic::object("b", 1)("c", 2))(
-      "d", dynamic::array(1, 2, 3));
-  dynamic target = dynamic::object("a", dynamic::object("b", 2))(
-      "d", dynamic::array(2, 3, 4));
+  // clang-format off
+  dynamic source = dynamic::object
+    ("a", dynamic::object("b", 1)("c", 2)("d", 3))
+    ("e", dynamic::array(1, 2, 3))
+    ("f", dynamic::array(4, 5, 6));
+  dynamic target = dynamic::object
+    ("a", dynamic::object("b", 2)("d", 3))
+    ("e", dynamic::array(2, 3, 4))
+    ("f", dynamic::array(4, 5, 6));
+  // clang-format on
 
   auto patch = dynamic::merge_diff(source, target);
 
@@ -1040,11 +1046,11 @@ TEST(Dynamic, MergeDiffNestedObjects) {
   EXPECT_EQ(2, patch["a"]["b"].getInt());
   EXPECT_TRUE(patch["a"]["c"].isNull());
 
-  EXPECT_TRUE(patch["d"].isArray());
-  EXPECT_EQ(3, patch["d"].size());
-  EXPECT_EQ(2, patch["d"][0].getInt());
-  EXPECT_EQ(3, patch["d"][1].getInt());
-  EXPECT_EQ(4, patch["d"][2].getInt());
+  EXPECT_TRUE(patch["e"].isArray());
+  EXPECT_EQ(3, patch["e"].size());
+  EXPECT_EQ(2, patch["e"][0].getInt());
+  EXPECT_EQ(3, patch["e"][1].getInt());
+  EXPECT_EQ(4, patch["e"][2].getInt());
 
   source.merge_patch(patch);
   EXPECT_EQ(source, target);