diff --git a/CHANGELOG.md b/CHANGELOG.md index 6eebf65..bd0b489 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ **Fixes** - JSON Patch `move` and `copy` operations now use `add` semantics for their target locations. Previously it was not possible to move or copy to the end of an array with the special `-` index, and we would silently append to arrays given an index greater than the array's length. We now raise a `JSONPatchError` if the target index is greater than the number of elements in the array. +- Fixed JSONPatch `move` operations with immutable Mapping or Sequence source objects. Previously we silently ignored the internal `remove` op if the source was immutable,making the operation a copy. Now we raise a `JSONPatchError`. # Version 2.2.1 diff --git a/jsonpath/__about__.py b/jsonpath/__about__.py index 3aaeb51..493659b 100644 --- a/jsonpath/__about__.py +++ b/jsonpath/__about__.py @@ -1,4 +1,4 @@ # SPDX-FileCopyrightText: 2023-present James Prior # # SPDX-License-Identifier: MIT -__version__ = "2.2.1" +__version__ = "2.2.2" diff --git a/jsonpath/patch.py b/jsonpath/patch.py index 863a8fa..b56a7e7 100644 --- a/jsonpath/patch.py +++ b/jsonpath/patch.py @@ -152,22 +152,8 @@ def apply( self, data: Union[MutableSequence[object], MutableMapping[str, object]] ) -> Union[MutableSequence[object], MutableMapping[str, object]]: """Apply this patch operation to _data_.""" - parent, obj = self.path.resolve_parent(data) - if parent is None: - raise JSONPatchError("can't remove root") - - if isinstance(parent, MutableSequence): - if obj is UNDEFINED: - raise JSONPatchError("can't remove nonexistent item") - del parent[int(self.path.parts[-1])] - elif isinstance(parent, MutableMapping): - if obj is UNDEFINED: - raise JSONPatchError("can't remove nonexistent property") - del parent[str(self.path.parts[-1])] - else: - raise JSONPatchError( - f"unexpected operation on {parent.__class__.__name__!r}" - ) + # Note that it's not possible to remove the root value. + _remove(data=data, path=self.path, op=self.name) return data def asdict(self) -> Dict[str, object]: @@ -231,17 +217,8 @@ def apply( if self.dest.is_relative_to(self.source): raise JSONPatchError("can't move object to one of its own children") - source_parent, source_obj = self.source.resolve_parent(data) - - if source_obj is UNDEFINED: - raise JSONPatchError("source object does not exist") - - if isinstance(source_parent, MutableSequence): - del source_parent[int(self.source.parts[-1])] - if isinstance(source_parent, MutableMapping): - del source_parent[str(self.source.parts[-1])] - - return _add(path=self.dest, value=source_obj, data=data) + obj = _remove(path=self.source, data=data, op=self.name) + return _add(path=self.dest, value=obj, data=data) def asdict(self) -> Dict[str, object]: """Return a dictionary representation of this operation.""" @@ -788,7 +765,7 @@ def patched( def _add(*, data: _DataT, path: JSONPointer, value: object) -> _DataT: - """Add _value_ to _data_ at _path_. + """Add _value_ to _data_ at _path_ and return modified _data_. This is semantically the `add` operation, used by `OpAdd`, `OpMove` and `OpCopy`. @@ -816,5 +793,32 @@ def _add(*, data: _DataT, path: JSONPointer, value: object) -> _DataT: elif isinstance(parent, MutableMapping): parent[str(target)] = value else: - raise JSONPatchError(f"unexpected operation on {parent.__class__.__name__!r}") + raise JSONPatchError( + f"unexpected operation on {parent.__class__.__name__!r}", + ) return data + + +def _remove(*, data: _DataT, path: JSONPointer, op: str) -> object: + """Remove value at _path_ from _data_ and return the removed value. + + This is semantically the `remove` operation used by `OpRemove` and + `OpMove`. + """ + parent, obj = path.resolve_parent(data) + if parent is None: + raise JSONPatchError(f"can't {op} root") + + if isinstance(parent, MutableSequence): + if obj is UNDEFINED: + raise JSONPatchError(f"can't {op} nonexistent item") + del parent[int(path.parts[-1])] + elif isinstance(parent, MutableMapping): + if obj is UNDEFINED: + raise JSONPatchError(f"can't {op} nonexistent property") + del parent[str(path.parts[-1])] + else: + raise JSONPatchError( + f"unexpected operation on {parent.__class__.__name__!r}", + ) + return obj diff --git a/tests/test_json_patch.py b/tests/test_json_patch.py index 377d54b..1e6c94f 100644 --- a/tests/test_json_patch.py +++ b/tests/test_json_patch.py @@ -6,6 +6,7 @@ from collections.abc import Mapping from contextlib import suppress from io import StringIO +from types import MappingProxyType from typing import Any from typing import Dict from typing import Iterator @@ -117,7 +118,7 @@ def test_move_to_child() -> None: def test_move_nonexistent_value() -> None: with pytest.raises( - JSONPatchError, match=re.escape("source object does not exist (move:0)") + JSONPatchError, match=re.escape("can't move nonexistent property (move:0)") ): JSONPatch().move(from_="/foo/bar", path="/bar").apply({"foo": {"baz": 1}}) @@ -141,6 +142,24 @@ def test_move_to_immutable_mapping() -> None: patch.apply({"foo": {"bar": "hello"}, "baz": MockMapping()}) +def test_move_from_immutable_mapping() -> None: + patch = JSONPatch().move(from_="/foo/bar", path="/baz/bar") + with pytest.raises( + JSONPatchError, + match=re.escape("unexpected operation on 'mappingproxy' (move:0)"), + ): + patch.apply({"foo": MappingProxyType({"bar": "hello"}), "baz": {}}) + + +def test_move_from_immutable_sequence() -> None: + patch = JSONPatch().move(from_="/foo/0", path="/baz/0") + with pytest.raises( + JSONPatchError, + match=re.escape("unexpected operation on 'tuple' (move:0)"), + ): + patch.apply({"foo": ("hello",), "baz": []}) + + def test_copy_nonexistent_value() -> None: with pytest.raises( JSONPatchError, match=re.escape("source object does not exist (copy:0)")