PISTON-1005: make kz_json:merge_recursive an option of kz_json:merge (2…

…600hz#6254)

core/kazoo_stdlib/doc/kz_json.md

@@ -74,34 +74,14 @@ You can merge two (or more) objects together and specify which objects' value wi
 -   `merge([JObj1, JObj2,...])`: Defaults to using the `merge_right` strategy where the object on the "right" will have its value used when both objects have a value at a give key path.
 -   `merge(Strategy, JObjs)`: Choose a strategy, either `fun kz_json:merge_right/2` or `fun kz_json:merge_left/2` (or define your own), and apply it to the list of objects.
 
+Both `merge_left/2` and `merge_right/2` will keep the left or right empty objects (respectively) when they exist at a given key path if the other value is also an object (empty or not). To override this behavior and always recursively merge the values when both are objects, specify `'recursive' => 'true'` in the `Options` map passed in `merge(JObjs, Options)`, `merge(JObj1, JObj2, Options)`, `merge(Strategy, JObjs, Options)`, or `merge(JObj1, JObj2, Options)`, as arg 4 to `merge/4`, or as arg 5 to `merge/5`.
+
 ### Handling `null`
 
 Sometimes it is beneficial to include the `null` atom as a value (for downstream processing perhaps) versus triggering a `delete_key` equivalent.
 
 `set_value` and `merge` can take an optional map `#{'keep_null' => 'true'}` to ensure `null` is kept in the resulting data structure.
 
-### `merge/3` vs `merge_recursive/2`
-
-There is a second, older implementation of merging objects called `merge_recursive/2`. The main difference is that it operates differently when merging a path in the second object that has an empty object as the value:
-
-```erlang
-> J1 = {[{<<"foo">>,false}]},
-> J2 = {[{<<"foo">>,false}, {<<"bar">>,{[{<<"foo">>,{[]}}]}}]}.
-> kz_json:merge(fun kz_json:merge_right/2, J1, J2).
-{[{<<"foo">>,false},{<<"bar">>,{[{<<"foo">>,{[]}}]}}]}
-> kz_json:merge(fun kz_json:merge_left/2, J1, J2).
-{[{<<"foo">>,false},{<<"bar">>,{[{<<"foo">>,{[]}}]}}]}
-
-> kz_json:merge_recursive(J1, J2).
-{[{<<"foo">>,false}]}
-> kz_json:merge_recursive(J2, J1).
-{[{<<"foo">>,false},{<<"bar">>,{[{<<"foo">>,{[]}}]}}]}
-```
-
-Since `bar.foo` in `J2` is an empty object, the values won't be set because folding over an empty object returns the accumulator (`J1` in this case). Nothing in `merge_recursive/4` sets `bar` or `bar.foo` onto `J1`.
-
-More generally, the longest key path suffix with an empty object as the value on the right side of the merge will be stripped from the merged object when using `merge_recursive/2`.
-
 ## List-like operations
 
 JSON objects are iterable and `kz_json` has equivalents to the `lists` module for `filter/{2,3}`, `filtermap/2`, `map/2`, `fold{l,r}/3`, `foreach/2`, `all/2`, and `any/2` among others.

core/kazoo_stdlib/src/kz_json.erl

@@ -351,10 +351,14 @@ recursive_from_list(_Else, #{invalid_as_null := 'false'}) ->
 -type merge_fun() :: fun((key(), merge_arg_2()) -> merge_fun_result()) |
                      fun((key(), merge_arg_2(), merge_options()) -> merge_fun_result()).
 
--type merge_options() :: #{'keep_null' => boolean()}.
+-type merge_options() :: #{'keep_null' => boolean()
+                          ,'recursive' => boolean() %% Supports `merge_recursive'-like behaviour - recursively merge even on empty objects
+                          }.
 -spec merge_options() -> merge_options().
 merge_options() ->
-    #{'keep_null' => 'false'}.
+    #{'keep_null' => 'false'
+     ,'recursive' => 'false'
+     }.
 
 -spec merge(objects()) -> object().
 merge(JObjs) ->
@@ -456,7 +460,7 @@ merge_left(_K, {'right', ?JSON_WRAPPER(_)=Right}, Options) ->
     {'ok', merge(fun merge_left/3, Right, new(), Options)};
 merge_left(_K, {'right', V}, _Options) -> {'ok', V};
 
-merge_left(_K, {'both', ?EMPTY_JSON_OBJECT=Left, ?JSON_WRAPPER(_)=_Right}, _Options) ->
+merge_left(_K, {'both', ?EMPTY_JSON_OBJECT=Left, ?JSON_WRAPPER(_)=_Right}, #{'recursive' := 'false'}) ->
     {'ok', Left};
 merge_left(_K, {'both', ?JSON_WRAPPER(_)=Left, ?JSON_WRAPPER(_)=Right}, Options) ->
     {'ok', merge(fun merge_left/3, Left, Right, Options)};
@@ -482,7 +486,7 @@ merge_right(_K, {'right', ?JSON_WRAPPER(_)=Right}, Options) ->
     {'ok', merge(fun merge_right/3, new(), Right, Options)};
 merge_right(_K, {'right', V}, _Options) -> {'ok', V};
 
-merge_right(_K, {'both', ?JSON_WRAPPER(_)=_Left, ?EMPTY_JSON_OBJECT=Right}, _Options) ->
+merge_right(_K, {'both', ?JSON_WRAPPER(_)=_Left, ?EMPTY_JSON_OBJECT=Right}, #{'recursive' := 'false'}) ->
     {'ok', Right};
 merge_right(_K, {'both', ?JSON_WRAPPER(_)=Left, ?JSON_WRAPPER(_)=Right}, Options) ->
     {'ok', merge(fun merge_right/3, Left, Right, Options)};
@@ -505,6 +509,11 @@ merge_jobjs(?JSON_WRAPPER(Props1), ?JSON_WRAPPER(_)=JObj2) ->
 -spec merge_true(any(), any()) -> 'true'.
 merge_true(_, _) -> 'true'.
 
+%%------------------------------------------------------------------------------
+%% @doc Deprecated in favor of `kz_json:merge/2' with the `Options' map
+%% containing 'recursive' => 'true'.
+%% @end
+%%------------------------------------------------------------------------------
 -spec merge_recursive(objects()) -> object().
 merge_recursive(JObjs) when is_list(JObjs) ->
     merge_recursive(JObjs, fun merge_true/2).

core/kazoo_stdlib/test/kz_json_tests.erl

@@ -265,6 +265,26 @@ prop_merge_left() ->
             end
            ).
 
+prop_merge_right_recursive() ->
+    Options = #{'keep_null' => 'false'
+               ,'recursive' => 'true'
+               },
+    ?FORALL({LeftJObj, RightJObj}
+           ,{resize(?MAX_OBJECT_DEPTH, kz_json_generators:deep_object())
+            ,resize(?MAX_OBJECT_DEPTH, kz_json_generators:deep_object())
+            }
+           ,begin
+                MergedJObj = kz_json:merge(LeftJObj, RightJObj, Options),
+
+                ?WHENFAIL(?debugFmt("Failed to merge_right recursively (~p, ~p)~nmerge-r/3: ~p~n"
+                                   ,[LeftJObj, RightJObj, MergedJObj]
+                                   )
+                         ,are_all_properties_found(MergedJObj, RightJObj)
+                          andalso are_all_keys_found(MergedJObj, LeftJObj)
+                         )
+            end
+           ).
+
 prop_key_with_null() ->
     ?FORALL({JObj, Path}
            ,{resize(?MAX_OBJECT_DEPTH, kz_json_generators:deep_object())
@@ -378,6 +398,24 @@ is_property_found(Key, Value, Merged) ->
             'false'
     end.
 
+%%------------------------------------------------------------------------------
+%% @doc Ensures that all keys (recursively) from `Favored' are found in
+%% `Merged'. Values need not be the same.
+%% @end
+%%------------------------------------------------------------------------------
+are_all_keys_found(Merged, Favored) ->
+    kz_json:all(fun({K, V}) -> is_key_found(K, V, Merged) end, Favored).
+
+is_key_found(Key, ?JSON_WRAPPER(_)=Value, Merged) ->
+    case kz_json:get_value(Key, Merged) of
+        ?JSON_WRAPPER(_)=MergedV -> are_all_keys_found(MergedV, Value);
+        Missing ->
+            log_failure(Key, Value, Missing),
+            'false'
+    end;
+is_key_found(Key, _, Merged) ->
+    kz_json:is_defined(Key, Merged).
+
 log_failure(Key, Value, Missing) ->
     ?debugFmt("failed to find ~p~nexpected: ~p~nfound: ~p~n", [Key, Value, Missing]).