Repository: stefankoegl/python-json-patch

Issue:
Feature Request - patch creation should fail when given an invalid patch
I use this library to implement `PATCH` in a number of JSON-based APIs and it works quite well.  I do have one nagging issue though.  I do not have a good way to verify that the document I am given is a valid `application/json-patch+json` instance until I try to apply the patch to another document.  Usually this means that I have retrieved a representation to update from somewhere (e.g, database), then I apply the patch.  If the patch is not a valid document, then I bothered my database unnecessarily.

Would you accept a PR to add a new method, `jsonpatch.JsonPatch.validate`, that performs the sanity checking done in the [_ops property](https://github.com/stefankoegl/python-json-patch/blob/9e4d423c22bedad6ffc260541ae587114dfed70a/jsonpatch.py#L291-L293) today?

Discussion:

Patch:
diff --git a/.coveragerc b/.coveragerc
index 2a98e09..f0d91db 100644
--- a/.coveragerc
+++ b/.coveragerc
@@ -1,8 +1,10 @@
 # .coveragerc to control coverage.py
 [run]
 branch = True
+source = jsonpatch
 
 [report]
+show_missing = True
 # Regexes for lines to exclude from consideration
 exclude_lines =
     # Have to re-enable the standard pragma
diff --git a/jsonpatch.py b/jsonpatch.py
index 7d5489a..2b8678f 100644
--- a/jsonpatch.py
+++ b/jsonpatch.py
@@ -222,6 +222,13 @@ class JsonPatch(object):
             'copy': CopyOperation,
         }
 
+        # Verify that the structure of the patch document
+        # is correct by retrieving each patch element.
+        # Much of the validation is done in the initializer
+        # though some is delayed until the patch is applied.
+        for op in self.patch:
+            self._get_operation(op)
+
     def __str__(self):
         """str(self) -> self.to_string()"""
         return self.to_string()
diff --git a/requirements-dev.txt b/requirements-dev.txt
index 21daf9a..c729ece 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -1,2 +1,3 @@
+coverage
 wheel
 pypandoc


Test patch:
diff --git a/tests.py b/tests.py
index 0abf4d2..58e3ce2 100755
--- a/tests.py
+++ b/tests.py
@@ -671,6 +671,85 @@ class JsonPointerTests(unittest.TestCase):
         self.assertEqual(result, expected)
 
 
+class JsonPatchCreationTest(unittest.TestCase):
+
+    def test_creation_fails_with_invalid_patch(self):
+        invalid_patches = [
+            {             'path': '/foo', 'value': 'bar'},
+            {'op': 0xADD, 'path': '/foo', 'value': 'bar'},
+            {'op': 'boo', 'path': '/foo', 'value': 'bar'},
+            {'op': 'add',                 'value': 'bar'},
+        ]
+        for patch in invalid_patches:
+            with self.assertRaises(jsonpatch.InvalidJsonPatch):
+                jsonpatch.JsonPatch([patch])
+
+        with self.assertRaises(jsonpointer.JsonPointerException):
+            jsonpatch.JsonPatch([{'op': 'add', 'path': 'foo', 'value': 'bar'}])
+
+
+class UtilityMethodTests(unittest.TestCase):
+
+    def test_boolean_coercion(self):
+        empty_patch = jsonpatch.JsonPatch([])
+        self.assertFalse(empty_patch)
+
+    def test_patch_equality(self):
+        p = jsonpatch.JsonPatch([{'op': 'add', 'path': '/foo', 'value': 'bar'}])
+        q = jsonpatch.JsonPatch([{'op': 'add', 'path': '/foo', 'value': 'bar'}])
+        different_op = jsonpatch.JsonPatch([{'op': 'remove', 'path': '/foo'}])
+        different_path = jsonpatch.JsonPatch([{'op': 'add', 'path': '/bar', 'value': 'bar'}])
+        different_value = jsonpatch.JsonPatch([{'op': 'add', 'path': '/foo', 'value': 'foo'}])
+        self.assertNotEqual(p, different_op)
+        self.assertNotEqual(p, different_path)
+        self.assertNotEqual(p, different_value)
+        self.assertEqual(p, q)
+
+    def test_operation_equality(self):
+        add = jsonpatch.AddOperation({'path': '/new-element', 'value': 'new-value'})
+        add2 = jsonpatch.AddOperation({'path': '/new-element', 'value': 'new-value'})
+        rm = jsonpatch.RemoveOperation({'path': '/target'})
+        self.assertEqual(add, add2)
+        self.assertNotEqual(add, rm)
+
+    def test_add_operation_structure(self):
+        with self.assertRaises(jsonpatch.InvalidJsonPatch):
+            jsonpatch.AddOperation({'path': '/'}).apply({})
+
+    def test_replace_operation_structure(self):
+        with self.assertRaises(jsonpatch.InvalidJsonPatch):
+            jsonpatch.ReplaceOperation({'path': '/'}).apply({})
+
+        with self.assertRaises(jsonpatch.InvalidJsonPatch):
+            jsonpatch.ReplaceOperation({'path': '/top/-', 'value': 'foo'}).apply({'top': {'inner': 'value'}})
+
+        with self.assertRaises(jsonpatch.JsonPatchConflict):
+            jsonpatch.ReplaceOperation({'path': '/top/missing', 'value': 'foo'}).apply({'top': {'inner': 'value'}})
+
+    def test_move_operation_structure(self):
+        with self.assertRaises(jsonpatch.InvalidJsonPatch):
+            jsonpatch.MoveOperation({'path': '/target'}).apply({})
+
+        with self.assertRaises(jsonpatch.JsonPatchConflict):
+            jsonpatch.MoveOperation({'from': '/source', 'path': '/target'}).apply({})
+
+    def test_test_operation_structure(self):
+        with self.assertRaises(jsonpatch.JsonPatchTestFailed):
+            jsonpatch.TestOperation({'path': '/target'}).apply({})
+
+        with self.assertRaises(jsonpatch.InvalidJsonPatch):
+            jsonpatch.TestOperation({'path': '/target'}).apply({'target': 'value'})
+
+    def test_copy_operation_structure(self):
+        with self.assertRaises(jsonpatch.InvalidJsonPatch):
+            jsonpatch.CopyOperation({'path': '/target'}).apply({})
+
+        with self.assertRaises(jsonpatch.JsonPatchConflict):
+            jsonpatch.CopyOperation({'path': '/target', 'from': '/source'}).apply({})
+
+        with self.assertRaises(jsonpatch.JsonPatchConflict):
+            jsonpatch.CopyOperation({'path': '/target', 'from': '/source'}).apply({})
+
 
 if __name__ == '__main__':
     modules = ['jsonpatch']
@@ -687,6 +766,8 @@ if __name__ == '__main__':
         suite.addTest(unittest.makeSuite(ConflictTests))
         suite.addTest(unittest.makeSuite(OptimizationTests))
         suite.addTest(unittest.makeSuite(JsonPointerTests))
+        suite.addTest(unittest.makeSuite(JsonPatchCreationTest))
+        suite.addTest(unittest.makeSuite(UtilityMethodTests))
         return suite