-
Notifications
You must be signed in to change notification settings - Fork 13
Bugfix: prepare_map returning non-map works correctly + small refactor. #35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Comment on lines
+215
to
245
|
||
|
|
@@ -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) | ||
|
Comment on lines
+285
to
+294
|
||
| 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 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in comment: 'uneqal' should be 'unequal'.