idubrov / json-patch

RFC 6902 (JSON Patch) / RFC 7396 (JSON Merge Patch) implementation for Rust
Apache License 2.0
124 stars 17 forks source link

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

Closed vileanco closed 1 week ago

vileanco commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

Should be fixed in 1.3.0.

vileanco commented 1 week ago

Should be fixed in 1.3.0.

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