From 299eff707c5451b621087684f75b9bba973ec99e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 22:07:06 +0000 Subject: [PATCH] Support whole document operations on scalar documents (#213) A JSON document can be a scalar, but operations on the whole document (path "") raised a bare TypeError for one, which is not part of the documented exceptions: - 'add' (and 'copy', which goes through it) to "" now replaces the document whatever its type, as 'replace' already did. - 'remove' of "" now raises JsonPatchConflict for every type of document, instead of a TypeError for scalars and a misleading "non-existent object 'None'" / "invalid array index 'None'" for objects and arrays. - 'move' from "" resolves the whole document as 'copy' does, so moving it onto itself is a no-op and moving it elsewhere fails with JsonPatchConflict. This also removes the TypeError for array documents from #214; its check for moves into a location's own children is still missing. The TypeError branches in 'replace' could no longer be reached and are removed. test_whole_document_as_target now holds; test_raises_only_documented_exceptions still fails, only because of #215. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01964EYaTCKUya9tcPjhytDq --- jsonpatch.py | 28 +++++++++++----------------- property_tests.py | 7 +++---- tests.py | 29 +++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 21 deletions(-) diff --git a/jsonpatch.py b/jsonpatch.py index 5383aec..54f9a10 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -230,6 +230,9 @@ class RemoveOperation(PatchOperation): def apply(self, obj): subobj, part = _to_last(self.pointer, obj) + if part is None: + raise JsonPatchConflict("can't remove the whole document") + if isinstance(subobj, Sequence) and not isinstance(part, int): raise JsonPointerException("invalid array index '{0}'".format(part)) @@ -259,11 +262,11 @@ def apply(self, obj): def _add(self, obj, value): subobj, part = _to_last(self.pointer, obj) - if isinstance(subobj, MutableSequence): - if part is None: - return value # we're replacing the root + if part is None: + return value # we're replacing the root, whatever its type - elif part == '-': + if isinstance(subobj, MutableSequence): + if part == '-': subobj.append(value) # pylint: disable=E1103 elif part > len(subobj) or part < 0: @@ -273,16 +276,10 @@ def _add(self, obj, value): subobj.insert(part, value) # pylint: disable=E1103 elif isinstance(subobj, MutableMapping): - if part is None: - obj = value # we're replacing the root - else: - subobj[part] = value + subobj[part] = value else: - if part is None: - raise TypeError("invalid document type {0}".format(type(subobj))) - else: - raise JsonPatchConflict("unable to fully resolve json pointer {0}, part {1}".format(self.location, part)) + raise JsonPatchConflict("unable to fully resolve json pointer {0}, part {1}".format(self.location, part)) return obj @@ -316,10 +313,7 @@ def apply(self, obj): msg = "can't replace a non-existent object '{0}'".format(part) raise JsonPatchConflict(msg) else: - if part is None: - raise TypeError("invalid document type {0}".format(type(subobj))) - else: - raise JsonPatchConflict("unable to fully resolve json pointer {0}, part {1}".format(self.location, part)) + raise JsonPatchConflict("unable to fully resolve json pointer {0}, part {1}".format(self.location, part)) subobj[part] = value return obj @@ -340,7 +334,7 @@ def apply(self, obj): subobj, part = _to_last(from_ptr, obj) try: - value = subobj[part] + value = subobj if part is None else subobj[part] except (KeyError, IndexError) as ex: raise JsonPatchConflict(str(ex)) diff --git a/property_tests.py b/property_tests.py index 9e92ebf..0b9a4eb 100644 --- a/property_tests.py +++ b/property_tests.py @@ -335,11 +335,11 @@ def test_does_not_modify_patch(self, case): @unittest.expectedFailure @given(docs_with_patches()) - # 'move' from the whole document crashes for array roots + # 'move' from the whole document crashed for array roots, #213 @example(case=([], [{'op': 'move', 'from': '', 'path': '/-'}])) # 'copy' or 'move' from the '-' of an array crashes @example(case=([0], [{'op': 'copy', 'from': '/-', 'path': '/0'}])) - # operations on the whole document crash if it is a scalar + # operations on the whole document crashed if it was a scalar, #213 @example(case=(None, [{'op': 'remove', 'path': ''}])) def test_raises_only_documented_exceptions(self, case): doc, patch = case @@ -386,9 +386,8 @@ def test_patch_as_string_gives_same_result(self, case): class OperationProperties(unittest.TestCase): - @unittest.expectedFailure @given(whole_document_operations()) - # 'add' crashes if the document is a scalar, unlike 'replace' + # 'add' crashed if the document was a scalar, unlike 'replace', #213 @example(case=(None, {'op': 'add', 'path': '', 'value': 0}, 0)) def test_whole_document_as_target(self, case): doc, operation, expected = case diff --git a/tests.py b/tests.py index af42fbc..7ee6f96 100755 --- a/tests.py +++ b/tests.py @@ -139,6 +139,35 @@ def test_add_whole_document_list_root_raises_no_bare_typeerror(self): except TypeError: self.fail("root add on an array root raised a bare TypeError") + def test_whole_document_scalar_root(self): + # a JSON document can also be a scalar (#213); operations on the whole + # document have to work on it as on an object or array root + for obj in (None, True, 5, 1.5, 'foo'): + with self.subTest(obj=obj): + for op in ('add', 'replace'): + res = jsonpatch.apply_patch( + obj, [{'op': op, 'path': '', 'value': {'baz': 'qux'}}]) + self.assertEqual(res, {'baz': 'qux'}) + for op in ('copy', 'move'): + res = jsonpatch.apply_patch( + obj, [{'op': op, 'from': '', 'path': ''}]) + self.assertEqual(res, obj) + + def test_remove_whole_document(self): + # there would be no document left, whatever the type of the root + for obj in (None, True, 5, 1.5, 'foo', {'foo': 'bar'}, ['foo']): + with self.subTest(obj=obj): + self.assertRaises(jsonpatch.JsonPatchConflict, + jsonpatch.apply_patch, + obj, [{'op': 'remove', 'path': ''}]) + + def test_move_whole_document_scalar_root_into_child(self): + for obj in (None, 5, 'foo'): + with self.subTest(obj=obj): + self.assertRaises(jsonpatch.JsonPatchConflict, + jsonpatch.apply_patch, + obj, [{'op': 'move', 'from': '', 'path': '/foo'}]) + def test_replace_array_item(self): obj = {'foo': ['bar', 'qux', 'baz']} res = jsonpatch.apply_patch(obj, [{'op': 'replace', 'path': '/foo/1',