Merge pull request #35 from Valian/bugfix-changing-map-type

Bugfix: prepare_map returning non-map works correctly + small refactor.

Author: corka149

lib/jsonpatch.ex

@@ -208,44 +208,38 @@ defmodule Jsonpatch do
       |> Keyword.update(:object_hash, nil, &make_safe_hash_fn/1)
       |> Keyword.validate!(
         ancestor_path: "",
-        prepare_map: fn struct -> struct end,
+        prepare_map: &Function.identity/1,
         object_hash: nil
       )
 
-    cond do
-      is_map(source) and is_map(destination) ->
-        do_map_diff(destination, source, opts[:ancestor_path], [], opts)
-
-      is_list(source) and is_list(destination) ->
-        do_list_diff(destination, source, opts[:ancestor_path], [], 0, opts)
-
-      # type of value changed, eg set to nil
-      source != destination ->
-        destination = maybe_prepare_map(destination, opts)
-        [%{op: "replace", path: opts[:ancestor_path], value: destination}]
-
-      true ->
-        []
-    end
+    do_diff(destination, source, opts[:ancestor_path], nil, [], opts)
   end
 
   defguardp are_unequal_maps(val1, val2) when val1 != val2 and is_map(val2) and is_map(val1)
   defguardp are_unequal_lists(val1, val2) when val1 != val2 and is_list(val2) and is_list(val1)
 
   defp do_diff(dest, source, path, key, patches, opts) when are_unequal_lists(dest, source) do
     # uneqal lists, let's use a specialized function for that
-    do_list_diff(dest, source, "#{path}/#{escape(key)}", patches, 0, opts)
+    do_list_diff(dest, source, join_key(path, key), patches, opts)
   end
 
   defp do_diff(dest, source, path, key, patches, opts) when are_unequal_maps(dest, source) do
-    # uneqal maps, let's use a specialized function for that
-    do_map_diff(dest, source, "#{path}/#{escape(key)}", patches, opts)
+    # Convert structs to maps if prepare_map function is provided
+    dest = maybe_prepare_map(dest, opts)
+    source = maybe_prepare_map(source, opts)
+
+    if not is_map(dest) or not is_map(source) do
+      # type changed, let's process it again
+      do_diff(dest, source, path, key, patches, opts)
+    else
+      # uneqal maps, let's use a specialized function for that
+      do_map_diff(dest, source, join_key(path, key), patches, opts)
+    end
   end
 
   defp do_diff(dest, source, path, key, patches, opts) when dest != source do
     # scalar values or change of type (map -> list etc), let's just make a replace patch
-    value = maybe_prepare_map(dest, opts)
-    [%{op: "replace", path: "#{path}/#{escape(key)}", value: value} | patches]
+    [%{op: "replace", path: join_key(path, key), value: maybe_prepare_map(dest, opts)} | patches]
   end
 
   defp do_diff(_dest, _source, _path, _key, patches, _opts) do
@@ -254,10 +248,6 @@ defmodule Jsonpatch do
   end
 
   defp do_map_diff(%{} = destination, %{} = source, ancestor_path, patches, opts) do
-    # Convert structs to maps if prepare_map function is provided
-    destination = maybe_prepare_map(destination, opts)
-    source = maybe_prepare_map(source, opts)
-
     # entrypoint for map diff, let's convert the map to a list of {k, v} tuples
     destination
     |> Map.to_list()
@@ -271,7 +261,7 @@ defmodule Jsonpatch do
       if k in checked_keys do
         patches
       else
-        [%{op: "remove", path: "#{ancestor_path}/#{escape(k)}"} | patches]
+        [%{op: "remove", path: join_key(ancestor_path, k)} | patches]
       end
     end)
   end
@@ -285,23 +275,23 @@ defmodule Jsonpatch do
 
         :error ->
           value = maybe_prepare_map(val, opts)
-          [%{op: "add", path: "#{ancestor_path}/#{escape(key)}", value: value} | patches]
+          [%{op: "add", path: join_key(ancestor_path, key), value: value} | patches]
       end
 
     # Diff next value of same level
     do_map_diff(rest, source, ancestor_path, patches, [key | checked_keys], opts)
   end
 
-  defp do_list_diff(destination, source, ancestor_path, patches, idx, opts) do
+  defp do_list_diff(destination, source, ancestor_path, patches, opts) do
     if opts[:object_hash] do
       do_hash_list_diff(destination, source, ancestor_path, patches, opts)
     else
-      do_pairwise_list_diff(destination, source, ancestor_path, patches, idx, opts)
+      do_pairwise_list_diff(destination, source, ancestor_path, patches, 0, opts)
     end
   catch
     # happens if we've got a nil hash or we tried to hash a non-map
     :hash_not_implemented ->
-      do_pairwise_list_diff(destination, source, ancestor_path, patches, idx, opts)
+      do_pairwise_list_diff(destination, source, ancestor_path, patches, 0, opts)
   end
 
   defp do_pairwise_list_diff(destination, source, ancestor_path, patches, idx, opts)
@@ -310,15 +300,15 @@ defmodule Jsonpatch do
 
   defp do_pairwise_list_diff([], [_item | source_rest], ancestor_path, patches, idx, opts) do
     # if we find any leftover items in source, we have to remove them
-    patches = [%{op: "remove", path: "#{ancestor_path}/#{idx}"} | patches]
+    patches = [%{op: "remove", path: join_key(ancestor_path, idx)} | patches]
     do_pairwise_list_diff([], source_rest, ancestor_path, patches, idx + 1, opts)
   end
 
   defp do_pairwise_list_diff(items, [], ancestor_path, patches, idx, opts) do
     # we have to do it without recursion, because we have to keep the order of the items
     items
     |> Enum.map_reduce(idx, fn val, idx ->
-      {%{op: "add", path: "#{ancestor_path}/#{idx}", value: maybe_prepare_map(val, opts)},
+      {%{op: "add", path: join_key(ancestor_path, idx), value: maybe_prepare_map(val, opts)},
        idx + 1}
     end)
     |> elem(0)
@@ -469,15 +459,15 @@ defmodule Jsonpatch do
   @compile {:inline, add_removals: 4}
   defp add_removals(from_idx, to_idx, path, removals) do
     Enum.reduce(from_idx..to_idx//1, removals, fn idx, removals ->
-      [%{op: "remove", path: "#{path}/#{idx}"} | removals]
+      [%{op: "remove", path: join_key(path, idx)} | removals]
     end)
   end
 
   @compile {:inline, add_additions: 6}
   defp add_additions(from_idx, to_idx, path, dest_tuple, additions, opts) do
     Enum.reduce(from_idx..to_idx//1, additions, fn idx, additions ->
       value = dest_tuple |> elem(idx) |> maybe_prepare_map(opts)
-      [%{op: "add", path: "#{path}/#{idx}", value: value} | additions]
+      [%{op: "add", path: join_key(path, idx), value: value} | additions]
     end)
   end
 
@@ -503,6 +493,10 @@ defmodule Jsonpatch do
 
   defp escape(fragment), do: fragment
 
+  @compile {:inline, join_key: 2}
+  defp join_key(path, nil), do: path
+  defp join_key(path, key), do: "#{path}/#{escape(key)}"
+
   defp make_safe_hash_fn(hash_fn) do
     # we want to compare only maps, and returning nil should mean
     # we should compare lists pairwise instead

test/jsonpatch_test.exs

@@ -324,6 +324,23 @@ defmodule JsonpatchTest do
              ]
     end
 
+    test "replace_map changing type of value is also supported" do
+      source = %{"a" => ~D[2025-01-01]}
+      destination = %{"a" => ~D[2025-01-02]}
+
+      patches =
+        Jsonpatch.diff(source, destination,
+          prepare_map: fn
+            %Date{year: year, month: month, day: day} -> "#{year}-#{month}-#{day}"
+            map -> map
+          end
+        )
+
+      assert patches == [
+               %{op: "replace", path: "/a", value: "2025-1-2"}
+             ]
+    end
+
     test "Create diff with ancestor_path when changing type of base value (map to nil)" do
       source = %{"key" => "value"}
       destination = nil