Skip to content
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

JsonPatch is mutable #137

Open
samueldeklund opened this issue Oct 12, 2021 · 1 comment
Open

JsonPatch is mutable #137

samueldeklund opened this issue Oct 12, 2021 · 1 comment

Comments

@samueldeklund
Copy link

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
Copy link
Owner

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants