| Subject: | Possible bug in JSON::Patch |
| Date: | Tue, 7 Jul 2020 13:46:45 +0200 |
| To: | bug-json-patch [...] rt.cpan.org |
| From: | Javier Elices <jelices [...] hdi2.com> |
Hello Michael,
First of all, a big thank you for all the useful libraries that you
have available!
I have a web development that uses Perl for the batch work and Node
and Javascript for the backend and frontend respectively. One of the
Perl scripts uses your JSON::Patch library to compare a JSON that gets
modified by the client App. This allows showing the changes diff-like to
the admin as well as doing and undoing them. For all to work, the
patches need to be correct and interoperable across Perl and Javascript.
JSON Patch seems to be the way to go as that is exactly what promises...
Consider the following script:
#!/usr/bin/env perl
# JSON::Patch operations order bug?
use strict;
use warnings;
use utf8;
use JSON::XS;
use JSON::Patch qw(diff patch);
my $json1 = '
{
"one": [
{ "a": [1, 2] },
{ "b": [3, 4] },
{ "c": [5, 6] }
]
}
';
my $json2 = '
{
"one": [
{ "a": [1, 2] }
]
}
';
my $j1 = decode_json $json1;
my $j2 = decode_json $json2;
my $p2 = diff($j2, $j1);
print JSON::XS->new->utf8->pretty->encode( $p2 ), "\n";
patch($j2, $p2);
The output of the script is (I am using version 0.04 of JSON::Patch):
[
{
"path" : "/one/2",
"value" : {
"c" : [
5,
6
]
},
"op" : "add"
},
{
"path" : "/one/1",
"value" : {
"b" : [
3,
4
]
},
"op" : "add"
}
]
Index is out of range, step #1 at
/home/xxx/perl5/lib/perl5/Struct/Path.pm line 336.
I have seen that JSON::Patch::diff generates three types of operation
based on the output of Struct::Diff::list_diff($diff, sort => 1). I
think that "add" operations should be generated in the oposite order,
while "remove" operations are being generated in the right order and
"replace" operations could be generated in any order. That is assuming
that Struct::Diff does not produce redundant differences.
I have checked the RFC 6902 specification to ensure and found (about
"add" op's):
o An element to add to an existing array - whereupon the supplied
value is added to the array at the indicated location. Any
elements at or above the specified index are shifted one position
to the right. The specified index MUST NOT be greater than the
number of elements in the array. If the "-" character is used to
index the end of the array (see [RFC6901 <https://tools.ietf.org/html/rfc6901>]), this has the effect of
appending the value to the array.
I have also tried a different JSON Patch library (Node this one,
which I also use): https://json8.github.io/patch/demos/apply/ and it
also does not like the order of the operations of the patch above.
I have thought about the problem and I think that a sorting step
after the patch generation should be all it is required:
* Group operations by type
* Sort "add" operations in alphabetical path order
* Sort "remove" operations in reverse alphabetical path order
I think this function takes care of it:
sub patch_sort {
my $p = shift || [];
return [ sort {
( $b->{op} cmp $a->{op} ) ||
(
$a->{op} eq 'remove'
? $b->{path} cmp $a->{path}
: $a->{path} cmp $b->{path}
)
} @$p ];
}
It first sorts by "op", then for equal "op" it uses "path" in reverse
alpha order for "remove" and in alpha order for the rest. This also
sorts "replace" op's which I think does not harm.
In the above script, it will take this change:
my $p2 = patch_sort( diff($j2, $j1) );
To get the patch in order. With that change it works.
I have tried more complex examples and the ordering seems to work. I
share all this with you so you can think about the problem, see if you
agree with me about the ordering and hopefully fix the library for other
people to use. I have not studied your other libraries which JSON::Patch
uses, so I may be missing something.
What do you think? Any feedback would be appreciated.
Thanks a lot for your time!
Best Regards,
Javier Elices.