stefankoegl / python-json-patch

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

JsonPatch is mutable #137

Open samueldeklund opened 2 years ago

samueldeklund commented 2 years ago

I encountered unexpected behavior when running the example in the docs here. After applying, the patch itself has changed. I expected it to be immutable so I could re-use the patch object, but this is not the case.

Example:

patch = jsonpatch.JsonPatch([
    {'op': 'add', 'path': '/foo', 'value': 'bar'},
    {'op': 'add', 'path': '/baz', 'value': [1, 2, 3]},
    {'op': 'remove', 'path': '/baz/1'},
    {'op': 'test', 'path': '/baz', 'value': [1, 3]},
    {'op': 'replace', 'path': '/baz/0', 'value': 42},
    {'op': 'remove', 'path': '/baz/1'},
])

d = {}

patch.apply(d)

print(patch.patch)
> # Prints:
> # [{'op': 'add', 'path': '/foo', 'value': 'bar'}
> # {'op': 'add', 'path': '/baz', 'value': [42]},
> # {'op': 'remove', 'path': '/baz/1'},
> # {'op': 'test', 'path': '/baz', 'value': [1, 3]},
> # {'op': 'replace', 'path': '/baz/0', 'value': 42},
> # {'op': 'remove', 'path': '/baz/1'}]

# d is still the same empty dict
print(d)
> # Prints:
> # {}

# Try to patch again:
patch.apply(d)

> # ...omitted some traceback
> # JsonPatchConflict: can't remove a non-existent object '1'

Note that the second operation is now different from what it was originally.

In the docs here, it says

If a patch is only used once, it is not necessary to create a patch object explicitly.

which implies that a jsonpatch.JsonPatch object can be used more than once. However, it appears that a patch is potentially single-use, depending on whether it includes mutable values (lists/dicts).

I think this can be resolved by doing a deepcopy when initializing the operation attribute in the PatchOperation class.

stefankoegl commented 2 years ago

I fully agree that the patch object should be immutable. Instead of deepcopying I'd be in favor of changing apply so that it does not modify its state.

Would you be able to work on a pull request for such a change?