Raise JsonPatchPathNotFound for operations on missing locations

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 <https://tools.ietf.org/html/rfc6902#section-4.2>`_ 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
 ----------------------------
 

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:

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