Prepare struct option

[Valian]

I've added a new option to diff letting to customize structs before diffing them. By default it does nothing, but can be used to nicely customize diff behaviour for your structs.

I've built that PR on top of #33, so that other one should be merged first.

Fixes #29

[Valian]

@corka149 I've noticed one missing thing - I forgot to process add and remove patches, now it's fixed :) Once you'll decide it's ready to merge, I'll rebase #32 on top of all these changes.

[corka149]

@Valian looks good. But what do you think about a prepare_target and prepare_source in general and not only limited to structs?

[Valian]

I agree we could make it work for any data. I suggested structs since they're easy to pattern match, and usually should be enough.

Not sure about different transforms for source and destination. Do you have an use case for different transforms for source and destination?

[corka149]

When someone is using regular maps which should be filtered for passwords or api tokens.

Or someone wants a "ignore case" for strings and wants to lower case all strings before diffing.

Do you think there is any disadvantage to allowing such functionality for any type?🤔

[Valian]

The only disadvantage I see is potentially a small performance hit - instead of doing it only for structs we would be doing it for each value, including strings, numbers etc. Unless I misunderstood and you'd like to do it only for maps and not for individual simple values (strings etc) inside maps?

Do you think there is any disadvantage to allowing such functionality for any type?🤔

I only expressed concern about two separate handlers, prepare_source and prepare_target. Preparing them in a different way seems very niche and I can't really think about a use case when you'd like to prepare source in a different way than target.

In other words, for me prepare_struct is an optimization. Everyone can quite easily code their own mapper of source and destination, run it and then call Jsonpatch.diff with resulting values:

source = prepare_for_diff(source)
destination = prepare_for_diff(destination)
Jsonpatch.diff(source, destination)

The difference is in:

1. Convenience - one less thing to write
2. Performance - we delay transformation until really needed, when we know given values are really equal.

I think library API should cover 90% of the most common, reasonable requirements. With prepare_struct I wanted to cover my own need, and it seems reasonable to me. It's quite similar to what eg Jason.encode does, you can customize how structs will be dumped into JSON but you cannot do it for regular maps.

---

After a lengthy response, here what I'd suggest:

• let's rename prepare_struct to prepare_value (or prepare_map?)
• let's run it for both regular maps and structs
• if someone would need to eg transform all strings or numbers in some way, it's trivial to write prepare_value to do it.

What do you think @corka149?

[corka149]

Sounds reasonable @Valian . I agree with your suggestions. Let's use prepare_map and focus on maps and structs.

[Valian]

Nice! Will adjust soon

[Valian]

Sorry for it taking some time @corka149.

New approach is almost ready. Just, I realized now that function has to almost always include a passthrough clause eg

test "Create diff with prepare_map option using subset of fields" do
      source = %TestStruct{
        field1: "value1",
        field2: "value2",
        inner: %{nested: "old"},
        field: "ignored"
      }

      destination = %TestStruct{
        field1: "new_value",
        field2: "value2",
        inner: %{nested: "new"},
        field: "also_ignored"
      }

      patches =
        Jsonpatch.diff(source, destination,
          prepare_map: fn
            %TestStruct{field1: field1, inner: inner} -> %{field1: field1, inner: inner}
            map -> map
          end
        )

      expected_patches = [
        %{op: "replace", path: "/field1", value: "new_value"},
        %{op: "replace", path: "/inner/nested", value: "new"}
      ]

      assert_equal_patches(patches, expected_patches)
    end

If I'd write simply &Map.take(&1, [:field1, :inner]) it wouldn't work, because it would be applied also to nested values effectively clearing them up. But I guess it's fine 👍🏻

[Valian]

@corka149 I've added one more change here. When I was using Jsonpatch in live_vue, especially with ancestor_path specified, I was not getting expected replace path when type of value was changed.

You can check my latest commit, but in short I'm now generating a replace operation when type of root value was updated. I think it makes sense? According to AI, this is correct based on RFC.

    test "Create full replace operation when type of root value changes" do
      assert [%{op: "replace", path: "", value: 1}] = Jsonpatch.diff("unexpected", 1)
    end

The merge-base changed after approval.

[corka149]

Thx for all the effort @Valian 🙏

[Valian]

no problem! Now onto rebasing #32 and hopefully merge & release <3