Skip to content
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

Diff generates invalid patch if an object becomes an array #37

Closed
vileanco opened this issue May 5, 2024 · 3 comments
Closed

Diff generates invalid patch if an object becomes an array #37

vileanco opened this issue May 5, 2024 · 3 comments

Comments

@vileanco
Copy link

vileanco commented May 5, 2024

There seems to be an issue with the json_patch::diff function. The path op Add is returned before Remove. Added failing test case as an example:

    async fn failing_test() {
        let left = json!({ "style": { "ref": "name" } });
        let right = json!({ "style": [{ "ref": "hello" }]});

        let patch = json_patch::diff(&left, &right);

        let mut initial = left.clone();
        json_patch::patch(&mut initial, &patch);

        assert_eq!(
            serde_json::to_string(&right).unwrap(),
            serde_json::to_string(&initial).unwrap()
        );
    }

gives output

assertion `left == right` failed
  left: "{\"style\":[{\"ref\":\"hello\"}]}"
 right: "{\"style\":{\"0\":{\"ref\":\"hello\"}}}"

If I try this same with some other diff tool eg (https://json-patch-builder-online.github.io/) it gives the patches in right order eg.:

[
    {
        "op": "remove",
        "path": "/style/ref"
    },
    {
        "op": "add",
        "path": "/style/0",
        "value": {
            "ref": "hello"
        }
    }
]

while this lib gives the Add op first.

@idubrov
Copy link
Owner

idubrov commented May 5, 2024

Ah, that's annoying. Seems like library I use, treediff, does not distinguish enough between arrays and objects.

I'll try to rewrite the whole diffing myself. Doing it directly against serde_json::Value should be easier than try to coerce this library for my needs... And then I would only "diff" same-kinded nodes, any time node kind changes ("object" vs "string" or "object" vs "array"), I would simply issue patch "replace" operation.

idubrov added a commit that referenced this issue May 6, 2024
Re-implementing the diffing algorithm to fix an issue when "kind" of
the node changes (array into an object or vice-versa), #37.
idubrov added a commit that referenced this issue May 6, 2024
Re-implementing the diffing algorithm to fix an issue when "kind" of
the node changes (array into an object or vice-versa), #37.
idubrov added a commit that referenced this issue May 6, 2024
Re-implementing the diffing algorithm to fix an issue when "kind" of
the node changes (array into an object or vice-versa), #37.
idubrov added a commit that referenced this issue May 6, 2024
Re-implementing the diffing algorithm to fix an issue when "kind" of
the node changes (array into an object or vice-versa), #37.

Fix some invalid tests, add more tests.
idubrov added a commit that referenced this issue May 6, 2024
Re-implementing the diffing algorithm to fix an issue when "kind" of
the node changes (array into an object or vice-versa), #37.

Fix some invalid tests, add more tests.
@idubrov
Copy link
Owner

idubrov commented May 6, 2024

Should be fixed in 1.3.0.

@idubrov idubrov closed this as completed May 6, 2024
@vileanco
Copy link
Author

vileanco commented May 6, 2024

Should be fixed in 1.3.0.

that was fast, I'll try it out! thanks 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants