diff --git a/doc/tutorial.rst b/doc/tutorial.rst index 1af72fa..3bfe930 100644 --- a/doc/tutorial.rst +++ b/doc/tutorial.rst @@ -70,14 +70,16 @@ The operations of a patch refer to locations in the object it is applied to. A patch created with ``make_patch(src, dst)`` or ``JsonPatch.from_diff(src, dst)`` must therefore be applied to ``src`` (or to an object equal to it). Applying it to any other object, such as ``dst``, usually fails with a -``JsonPatchConflict``, because the locations it refers to don't exist there: +``JsonPatchConflict``, because the locations it refers to don't exist there. +Here it is a ``JsonPatchPathNotFound``, the subclass of ``JsonPatchConflict`` +that is raised when the location an operation acts on doesn't exist: .. code-block:: python >>> patch.apply(dst) Traceback (most recent call last): ... - jsonpatch.JsonPatchConflict: can't remove a non-existent object 'foo' + jsonpatch.JsonPatchPathNotFound: can't remove a non-existent object 'foo' Applying it to an object with the same structure but different values may succeed, though, and overwrite those values. With ``generate_test_ops=True`` @@ -152,6 +154,69 @@ do not roll back changes already made in place: {'value': 2} +Ignoring Missing Locations +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +`RFC 6902 `_ requires a +``remove`` to fail if the location it removes doesn't exist, and this library +follows it: such an operation raises ``JsonPatchPathNotFound``. A ``replace`` +of a missing location, and a ``move`` or ``copy`` from one, raise it as well. + +.. code-block:: python + + >>> patch = [{'op': 'remove', 'path': '/data/another'}] + >>> try: + ... jsonpatch.apply_patch({'data': {}}, patch) + ... except jsonpatch.JsonPatchPathNotFound as ex: + ... print(ex, ex.op.location) + can't remove a non-existent object 'another' /data/another + +Catching the exception around ``apply`` does not skip just that operation, +though: application stops, and the operations after it are not applied. + +If removing a member that is already absent should count as success (an +idempotent ``remove``), the recommended way is a ``JsonPatch`` subclass that +uses its own ``remove`` operation. The ``operations`` attribute of a +``JsonPatch`` class maps each operation name to the class implementing it: + +.. code-block:: python + + >>> from types import MappingProxyType + >>> class IdempotentRemoveOperation(jsonpatch.RemoveOperation): + ... def apply(self, obj): + ... try: + ... return super().apply(obj) + ... except jsonpatch.JsonPatchPathNotFound: + ... return obj # nothing to remove + >>> class IdempotentJsonPatch(jsonpatch.JsonPatch): + ... operations = MappingProxyType(dict( + ... jsonpatch.JsonPatch.operations, + ... remove=IdempotentRemoveOperation, + ... )) + + >>> patch = IdempotentJsonPatch([ + ... {'op': 'remove', 'path': '/data/another'}, + ... {'op': 'add', 'path': '/data/new', 'value': 1}, + ... ]) + >>> patch.apply({'data': {}}) + {'data': {'new': 1}} + +Only a missing member or element is ignored. Other errors still raise, such +as a ``JsonPatchConflict`` for removing the whole document, or a +``JsonPointerException`` if a parent of the location doesn't exist: + +.. code-block:: python + + >>> patch = IdempotentJsonPatch([{'op': 'remove', 'path': '/missing/key'}]) + >>> patch.apply({'data': {}}) + Traceback (most recent call last): + ... + jsonpointer.JsonPointerException: member 'missing' not found in {'data': {}} + +Patches applied this way don't conform to RFC 6902, so only use this where +both sides of an exchange agree on it. + + Replacing the Whole Document ---------------------------- diff --git a/jsonpatch.py b/jsonpatch.py index 32c896a..6764c2b 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -90,6 +90,24 @@ class JsonPatchConflict(JsonPatchException): """ +class JsonPatchPathNotFound(JsonPatchConflict): + """Raised if the location an operation has to act on does not exist. + + This is the case for + + - a ``remove`` or ``replace`` whose ``path`` refers to a non-existent + object member or array element, and + - a ``move`` or ``copy`` whose ``from`` refers to a non-existent object + member or array element. + + It is a subclass of :class:`JsonPatchConflict`, so existing code that + catches :class:`JsonPatchConflict` continues to work. It can be caught on + its own to tell a missing location apart from other conflicts, for + example to implement a ``remove`` that ignores members which are already + absent (see the tutorial). + """ + + class JsonPatchTestFailed(JsonPatchException, AssertionError): """ A Test operation failed """ @@ -282,7 +300,7 @@ def apply(self, obj): del subobj[part] except (KeyError, IndexError): msg = "can't remove a non-existent object '{0}'".format(part) - raise JsonPatchConflict(msg, op=self) + raise JsonPatchPathNotFound(msg, op=self) except TypeError as ex: raise JsonPatchConflict(str(ex), op=self) @@ -352,12 +370,12 @@ def apply(self, obj): raise InvalidJsonPatch("'path' with '-' can't be applied to 'replace' operation", op=self) if part >= len(subobj) or part < 0: - raise JsonPatchConflict("can't replace outside of list", op=self) + raise JsonPatchPathNotFound("can't replace outside of list", op=self) elif isinstance(subobj, MutableMapping): if part not in subobj: msg = "can't replace a non-existent object '{0}'".format(part) - raise JsonPatchConflict(msg, op=self) + raise JsonPatchPathNotFound(msg, op=self) else: raise JsonPatchConflict("unable to fully resolve json pointer {0}, part {1}".format(self.location, part), op=self) @@ -386,7 +404,7 @@ def apply(self, obj): try: value = subobj[part] except (KeyError, IndexError) as ex: - raise JsonPatchConflict(str(ex), op=self) + raise JsonPatchPathNotFound(str(ex), op=self) # If source and target are equal, this is a no-op if self.pointer == from_ptr: @@ -475,7 +493,7 @@ def apply(self, obj): try: value = copy.deepcopy(subobj if part is None else subobj[part]) except (KeyError, IndexError) as ex: - raise JsonPatchConflict(str(ex), op=self) + raise JsonPatchPathNotFound(str(ex), op=self) # value is already a deep copy, no need to copy it again try: diff --git a/tests.py b/tests.py index 8a74962..0c90437 100755 --- a/tests.py +++ b/tests.py @@ -2528,7 +2528,7 @@ def test_jsonpatch_in_place_failing_patch(self): patch = self.write('patch.json', [{'op': 'remove', 'path': '/b'}]) res = self.run_script('jsonpatch', '-i', '-b', doc, patch) self.assertNotEqual(res.returncode, 0) - self.assertIn('JsonPatchConflict', res.stderr) + self.assertIn('JsonPatchPathNotFound', res.stderr) self.assertEqual(self.read(doc), {'a': 1}) self.assertEqual(sorted(os.listdir(self.dir)), ['doc.json', 'patch.json']) @@ -2539,6 +2539,87 @@ def test_version(self): self.assertEqual(res.stdout, '{0} {1}\n'.format(script, jsonpatch.__version__)) +class PathNotFoundTests(unittest.TestCase): + """ JsonPatchPathNotFound is raised for locations that don't exist """ + + def assertNotFound(self, doc, operation): + with self.assertRaises(jsonpatch.JsonPatchPathNotFound) as cm: + jsonpatch.apply_patch(doc, [operation]) + self.assertIsInstance(cm.exception, jsonpatch.JsonPatchConflict) + self.assertEqual(cm.exception.op.operation, operation) + + def assertOtherConflict(self, doc, operation): + with self.assertRaises(jsonpatch.JsonPatchConflict) as cm: + jsonpatch.apply_patch(doc, [operation]) + self.assertNotIsInstance(cm.exception, jsonpatch.JsonPatchPathNotFound) + + def test_is_conflict(self): + self.assertTrue(issubclass(jsonpatch.JsonPatchPathNotFound, + jsonpatch.JsonPatchConflict)) + + def test_remove_missing_member(self): + self.assertNotFound({'data': {}}, + {'op': 'remove', 'path': '/data/another'}) + + def test_remove_missing_element(self): + self.assertNotFound({'a': [1]}, {'op': 'remove', 'path': '/a/1'}) + + def test_replace_missing_member(self): + self.assertNotFound({'a': {}}, + {'op': 'replace', 'path': '/a/b', 'value': 1}) + + def test_replace_missing_element(self): + self.assertNotFound({'a': [1]}, + {'op': 'replace', 'path': '/a/1', 'value': 1}) + + def test_move_missing_from(self): + self.assertNotFound({'a': {}}, + {'op': 'move', 'from': '/a/b', 'path': '/c'}) + self.assertNotFound({'a': []}, + {'op': 'move', 'from': '/a/0', 'path': '/c'}) + + def test_copy_missing_from(self): + self.assertNotFound({'a': {}}, + {'op': 'copy', 'from': '/a/b', 'path': '/c'}) + self.assertNotFound({'a': []}, + {'op': 'copy', 'from': '/a/0', 'path': '/c'}) + + def test_other_conflicts(self): + self.assertOtherConflict({}, {'op': 'remove', 'path': ''}) + self.assertOtherConflict({'a': []}, + {'op': 'add', 'path': '/a/5', 'value': 1}) + self.assertOtherConflict({'a': (1, 2)}, + {'op': 'remove', 'path': '/a/0'}) + self.assertOtherConflict({'a': {}}, + {'op': 'move', 'from': '/a', 'path': '/a/b'}) + + def test_idempotent_remove_subclass(self): + """ The recipe from the tutorial """ + class IdempotentRemoveOperation(jsonpatch.RemoveOperation): + def apply(self, obj): + try: + return super().apply(obj) + except jsonpatch.JsonPatchPathNotFound: + return obj + + class IdempotentJsonPatch(jsonpatch.JsonPatch): + operations = MappingProxyType(dict( + jsonpatch.JsonPatch.operations, + remove=IdempotentRemoveOperation, + )) + + patch = IdempotentJsonPatch([ + {'op': 'remove', 'path': '/data/another'}, + {'op': 'remove', 'path': '/list/3'}, + {'op': 'add', 'path': '/x', 'value': 1}, + ]) + doc = {'data': {}, 'list': [1]} + self.assertEqual(patch.apply(doc), + {'data': {}, 'list': [1], 'x': 1}) + with self.assertRaises(jsonpatch.JsonPatchConflict): + IdempotentJsonPatch([{'op': 'remove', 'path': ''}]).apply(doc) + + class DocumentationTests(ScriptRunner, unittest.TestCase): """ The code examples in the documentation run and give the shown output """ @@ -2649,6 +2730,7 @@ def get_suite(): suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(CustomJsonPointerTests)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(CustomOperationTests)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(ExceptionOperationTests)) + suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PathNotFoundTests)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(CommandLineTests)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(DocumentationTests)) return suite