json-patch-php icon indicating copy to clipboard operation
json-patch-php copied to clipboard

Incorrect diff result

Open Dimona opened this issue 9 years ago • 2 comments

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

Dimona avatar Aug 22 '16 14:08 Dimona

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.

before
array (size=1)
  0 => 
    array (size=1)
      'value' => string '3' (length=1)
after
array (size=2)
  0 => 
    array (size=1)
      'value' => string '3' (length=1)
  'test' => string 'works' (length=5)
diff
array (size=0)
  empty

ghost avatar Nov 16 '17 15:11 ghost

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

If the target location references an element of an existing array, 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.

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.

joseph-montanez avatar Nov 28 '18 14:11 joseph-montanez