-
Notifications
You must be signed in to change notification settings - Fork 27
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
Incorrect diff result #14
Comments
I've just tried this lib for the first time and it does not work rigt from the get go. Shame, since it is the only library that I've found that can produce and apply the patch.
|
This is actually a really important issue as it breaks JSON-Patch ADD compatibility with other libraries. https://tools.ietf.org/id/draft-ietf-appsawg-json-patch-05.html
Because array ADD operations are in reverse order, it becomes an issue with other JSON-Patch libraries as they will not preserving the index of the array, instead, other libraries will do a shift, one position to the right, not preserving the array order. [
{
"op": "add",
"path": "\/trip\/countries\/2",
"value": {
"id": 122
}
},
{
"op": "add",
"path": "\/trip\/countries\/1",
"value": {
"id": 523
}
},
{
"op": "add",
"path": "\/trip\/countries\/0",
"value": #{
"id": 123
}
}
] So other JSON-Patch libraires will produce the following result, using the shift to one right logic: // Actual Outcome
({"trip":{"countries":[{"id":123},{"id":122},{"id":523}]}}); Which the expected outcome should be: // Expected Outcome
({"trip":{"countries":[{"id":123},{"id":523},{"id":122}]}}); At this time a quick fix I've done is simply reverse the order of the patches which seems to work for the time being. Looking throught the code here is the issue: https://github.com/mikemccabe/json-patch-php/blob/master/src/JsonPatch.php#L336 The code walks through the array backwards, so its adding all operations from last to first. I even took the output added a json test and it failed within its own library with the error: "test failed with exception: Can't operate outside of array bounds" so even this library is aware of this limitation. However if with the corrected results, the patch also fails with the same error: "Can't operate outside of array bounds". I've created a pull request #21 , but the test I've created fails in its patch apply function. So I've omitted the test for now. |
I added some objects to array as values and execute diff function. Returns patches which sorted descending, that's why function patch crashed because greater indexes in patches are above than lower
http://screen.mindklab.com/incoming/a1b6263b4df85100418eba577c32.png
The text was updated successfully, but these errors were encountered: