diff --git a/jsonpatch.py b/jsonpatch.py index 8a759a2..5645a1b 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -250,6 +250,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)) @@ -279,11 +282,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: @@ -293,16 +296,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 @@ -338,10 +335,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 diff --git a/property_tests.py b/property_tests.py index 67d10fb..545abd7 100644 --- a/property_tests.py +++ b/property_tests.py @@ -332,11 +332,10 @@ def test_does_not_modify_patch(self, case): outcome(jsonpatch.apply_patch, doc, patch) assert_json_equal(patch, saved) - @unittest.expectedFailure @given(docs_with_patches()) # 'move' from the whole document crashed for array roots, #214 @example(case=([], [{'op': 'move', 'from': '', 'path': '/-'}])) - # 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 @@ -382,9 +381,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 72f5759..ba929c1 100755 --- a/tests.py +++ b/tests.py @@ -166,6 +166,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',