diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e97881..6eebf65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Python JSONPath Change Log +# Version 2.2.2 (unreleased) + +**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. + # Version 2.2.1 **Fixes** diff --git a/jsonpath/patch.py b/jsonpath/patch.py index 4b5956b..863a8fa 100644 --- a/jsonpath/patch.py +++ b/jsonpath/patch.py @@ -59,32 +59,7 @@ 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: - # Replace the root object. - # The following op, if any, will raise a JSONPatchError if needed. - return self.value # type: ignore - - target = self.path.parts[-1] - if isinstance(parent, MutableSequence): - if obj is UNDEFINED: - if target == "-": - parent.append(self.value) - else: - index = self.path._index(target) # type: ignore # noqa: SLF001 - if index == len(parent): - parent.append(self.value) - else: - raise JSONPatchError("index out of range") - else: - parent.insert(int(target), self.value) - elif isinstance(parent, MutableMapping): - parent[str(target)] = self.value - else: - raise JSONPatchError( - f"unexpected operation on {parent.__class__.__name__!r}" - ) - return data + return _add(data=data, path=self.path, value=self.value) def asdict(self) -> Dict[str, object]: """Return a dictionary representation of this operation.""" @@ -266,22 +241,7 @@ def apply( if isinstance(source_parent, MutableMapping): del source_parent[str(self.source.parts[-1])] - dest_parent, _ = self.dest.resolve_parent(data) - - if dest_parent is None: - # Move source to root - return source_obj # type: ignore - - if isinstance(dest_parent, MutableSequence): - dest_parent.insert(int(self.dest.parts[-1]), source_obj) - elif isinstance(dest_parent, MutableMapping): - dest_parent[str(self.dest.parts[-1])] = source_obj - else: - raise JSONPatchError( - f"unexpected operation on {dest_parent.__class__.__name__!r}" - ) - - return data + return _add(path=self.dest, value=source_obj, data=data) def asdict(self) -> Dict[str, object]: """Return a dictionary representation of this operation.""" @@ -308,22 +268,8 @@ def apply( if source_obj is UNDEFINED: raise JSONPatchError("source object does not exist") - dest_parent, _ = self.dest.resolve_parent(data) - - if dest_parent is None: - # Copy source to root - return copy.deepcopy(source_obj) # type: ignore - - if isinstance(dest_parent, MutableSequence): - dest_parent.insert(int(self.dest.parts[-1]), copy.deepcopy(source_obj)) - elif isinstance(dest_parent, MutableMapping): - dest_parent[str(self.dest.parts[-1])] = copy.deepcopy(source_obj) - else: - raise JSONPatchError( - f"unexpected operation on {dest_parent.__class__.__name__!r}" - ) - - return data + # return OpAdd(self.dest, copy.deepcopy(source_obj)).apply(data) + return _add(path=self.dest, value=copy.deepcopy(source_obj), data=data) def asdict(self) -> Dict[str, object]: """Return a dictionary representation of this operation.""" @@ -836,3 +782,39 @@ def patched( unicode_escape=unicode_escape, uri_decode=uri_decode, ).patched(data) + + +_DataT = Union[MutableSequence[object], MutableMapping[str, object]] + + +def _add(*, data: _DataT, path: JSONPointer, value: object) -> _DataT: + """Add _value_ to _data_ at _path_. + + This is semantically the `add` operation, used by `OpAdd`, `OpMove` and + `OpCopy`. + """ + parent, obj = path.resolve_parent(data) + if parent is None: + # Replace the root object. + # The following op, if any, will raise a JSONPatchError if needed. + return value # type: ignore + + target = path.parts[-1] + + if isinstance(parent, MutableSequence): + if obj is UNDEFINED: + if target == "-": + parent.append(value) + else: + index = path._index(target) # type: ignore # noqa: SLF001 + if index == len(parent): + parent.append(value) + else: + raise JSONPatchError("index out of range") + else: + parent.insert(int(target), value) + elif isinstance(parent, MutableMapping): + parent[str(target)] = value + else: + raise JSONPatchError(f"unexpected operation on {parent.__class__.__name__!r}") + return data diff --git a/tests/test_json_patch.py b/tests/test_json_patch.py index 16659c5..377d54b 100644 --- a/tests/test_json_patch.py +++ b/tests/test_json_patch.py @@ -127,6 +127,11 @@ def test_move_to_root() -> None: assert patch.apply({"foo": {"bar": "baz"}}) == {"bar": "baz"} +def test_move_appends_to_array() -> None: + patch = JSONPatch().move(from_="/foo/0", path="/foo/-") + assert patch.apply({"foo": ["bar", "baz"]}) == {"foo": ["baz", "bar"]} + + def test_move_to_immutable_mapping() -> None: patch = JSONPatch().move(from_="/foo/bar", path="/baz/bar") with pytest.raises( @@ -148,6 +153,11 @@ def test_copy_to_root() -> None: assert patch.apply({"foo": {"bar": [1, 2, 3]}}) == [1, 2, 3] +def test_copy_appends_to_array() -> None: + patch = JSONPatch().copy(from_="/foo/0", path="/foo/-") + assert patch.apply({"foo": ["bar", "baz"]}) == {"foo": ["bar", "baz", "bar"]} + + def test_copy_to_immutable_mapping() -> None: with pytest.raises( JSONPatchError, @@ -158,6 +168,20 @@ def test_copy_to_immutable_mapping() -> None: ) +def test_move_past_array_end() -> None: + patch = JSONPatch().move("/foo/0", "/foo/3") + + with pytest.raises(JSONPatchError, match=re.escape("index out of range (move:0)")): + patch.apply({"foo": ["bar", "baz", "qux"]}) + + +def test_copy_past_array_end() -> None: + patch = JSONPatch().copy("/foo/0", "/foo/4") + + with pytest.raises(JSONPatchError, match=re.escape("index out of range (copy:0)")): + patch.apply({"foo": ["bar", "baz", "qux"]}) + + def test_patch_from_file_like() -> None: patch_doc = StringIO( json.dumps(