stefankoegl / python-json-patch

Applying JSON Patches in Python
https://python-json-patch.readthedocs.org/
BSD 3-Clause "New" or "Revised" License
459 stars 97 forks source link

Patch representation unnecessary complex #78

Open markoh1 opened 6 years ago

markoh1 commented 6 years ago

With ver 1.20 Patch representation becomes unnecessary complex when inserting new element into list of lists. Using old implementation it was very simple to determine added element from "value" field.

Example:

import jsonpatch
a = [[1,'a']]
b = [[2,'b'], [1,'a']]
print jsonpatch.make_patch(a, b)

Ver 1.16 prints:

[{"path": "/0", "value": [2, "b"], "op": "add"}]

Ver 1.20 prints:

[{"path": "/0/0", "value": 2, "op": "replace"}, {"path": "/0/1", "value": "b", "op": "replace"}, {"path": "/1", "value": [1, "a"], "op": "add"}]

wisotzky commented 6 years ago

Actually it looks, that this is okay for simple lists:

>>> a = [1,3]
>>> b = [1,2,3]
>>> print jsonpatch.make_patch(a, b)
[{"path": "/1", "value": 2, "op": "add"}]

While it fails with all lists of complex types, including dicts:

>>> a = [{'a': 1}, {'a': 3}]
>>> b = [{'a': 1}, {'a': 2}, {'a': 3}]
>>> print jsonpatch.make_patch(a, b)
[{"path": "/1/a", "value": 2, "op": "replace"}, {"path": "/2", "value": {"a": 3}, "op": "add"}]

If intend is to use this for human operators to show the differences between two documents, it is absolute need to get this fixed. Especially for very long lists or more complex list entries, it is essential doing a deepdiff to reduce the output to a minimum.

Example:

>>> a = [{'a': 1}, {'a': 3}, {'a': 4}, {'a': 5}, {'a': 6}, {'a': 7}, {'a': 8}, {'a': 9}, {'a': 10}]
>>> b = [{'a': 1}, {'a': 2}, {'a': 3}, {'a': 4}, {'a': 5}, {'a': 6}, {'a': 7}, {'a': 8}, {'a': 9}, {'a': 10}]
>>> print jsonpatch.make_patch(a, b)
[{"path": "/1/a", "value": 2, "op": "replace"}, {"path": "/2/a", "value": 3, "op": "replace"}, {"path": "/3/a", "value": 4, "op": "replace"}, {"path": "/4/a", "value": 5, "op": "replace"}, {"path": "/5/a", "value": 6, "op": "replace"}, {"path": "/6/a", "value": 7, "op": "replace"}, {"path": "/7/a", "value": 8, "op": "replace"}, {"path": "/8/a", "value": 9, "op": "replace"}, {"path": "/9", "value": {"a": 10}, "op": "add"}]

We've just inserted the 2nd item to the list, so output should just be:

[{"path": "/1", "value": {"a": 2}, "op": "add"}]
darrickyee commented 4 years ago

I am experiencing the same issue. Here is an example where two patches result in the same object values, but the one generated by make_patch is "unnecessarily complex":

d1 = {'mylist': [1, 2, 3]}
d2 = {'mylist': []}
print(jsonpatch.make_patch(d1, d2))
# Result: [{"op": "remove", "path": "/mylist/0"}, {"op": "remove", "path": "/mylist/0"}, {"op": "remove", "path": "/mylist/0"}]

newpatch = [{'op': 'replace', 'path': '/mylist', 'value': []}]
newd2 = jsonpatch.apply_patch(d1, newpatch)
newd2 == d2
# Result: True

In this case it seems to be a question of mutating the existing list vs. replacing with a new list. It would be good to be able to specify whether mutation or replacement is preferred. Does the spec say anything about this?