Fix: optimization bugs stefankoegl#55, stefankoegl#54

Author: mrhadenough

jsonpatch.py

@@ -563,9 +563,7 @@ def apply(self, obj):
 
 def _compare_lists(path, src, dst):
     """Compares two lists objects and return JSON patch about."""
-    # TODO: commented while optimization has bugs
-    # return _optimize(_compare(path, src, dst, *_split_by_common_seq(src, dst)))
-    return _compare(path, src, dst, *_split_by_common_seq(src, dst))
+    return _optimize(_compare(path, src, dst, *_split_by_common_seq(src, dst)))
 
 
 def _longest_common_subseq(src, dst):
@@ -770,9 +768,17 @@ def _optimize_using_replace(prev, cur):
     For nested strucures, tries to recurse replacement, see #36 """
     prev['op'] = 'replace'
     if cur['op'] == 'add':
-        # make recursive patch
+        # check case when dict "remove" is less than "add" and has a same key
         patch = make_patch(prev['value'], cur['value'])
-        if len(patch.patch) == 1 and patch.patch[0]['op'] != 'remove':
+        if isinstance(prev['value'], dict) and isinstance(cur['value'], dict) and len(prev['value'].keys()) == 1:
+            prev_set = set(prev['value'].keys())
+            cur_set = set(cur['value'].keys())
+            if prev_set & cur_set == prev_set:
+                patch = make_patch(cur['value'], prev['value'])
+
+        if len(patch.patch) == 1 and \
+                patch.patch[0]['op'] != 'remove' and \
+                patch.patch[0]['path'] and patch.patch[0]['path'].split('/')[1] in prev['value']:
             prev['path'] = prev['path'] + patch.patch[0]['path']
             prev['value'] = patch.patch[0]['value']
         else:

tests.py

@@ -369,58 +369,85 @@ def test_issue40(self):
         patch = jsonpatch.make_patch(src, dest)
 
 
-# class OptimizationTests(unittest.TestCase):
-#     def test_use_replace_instead_of_remove_add(self):
-#         src = {'foo': [1, 2, 3]}
-#         dst = {'foo': [3, 2, 3]}
-#         patch = list(jsonpatch.make_patch(src, dst))
-#         self.assertEqual(len(patch), 1)
-#         self.assertEqual(patch[0]['op'], 'replace')
-#         res = jsonpatch.apply_patch(src, patch)
-#         self.assertEqual(res, dst)
-
-#     def test_use_replace_instead_of_remove_add_nested(self):
-#         src = {'foo': [{'bar': 1, 'baz': 2}, {'bar': 2, 'baz': 3}]}
-#         dst = {'foo': [{'bar': 1}, {'bar': 2, 'baz': 3}]}
-#         patch = list(jsonpatch.make_patch(src, dst))
-#         self.assertEqual(len(patch), 1)
-#         self.assertEqual(patch[0]['op'], 'replace')
-#         res = jsonpatch.apply_patch(src, patch)
-#         self.assertEqual(res, dst)
-
-#     def test_use_move_instead_of_remove_add(self):
-#         src = {'foo': [4, 1, 2, 3]}
-#         dst = {'foo': [1, 2, 3, 4]}
-#         patch = list(jsonpatch.make_patch(src, dst))
-#         self.assertEqual(len(patch), 1)
-#         self.assertEqual(patch[0]['op'], 'move')
-#         res = jsonpatch.apply_patch(src, patch)
-#         self.assertEqual(res, dst)
-
-#     def test_use_move_instead_of_add_remove(self):
-#         src = {'foo': [1, 2, 3]}
-#         dst = {'foo': [3, 1, 2]}
-#         patch = list(jsonpatch.make_patch(src, dst))
-#         self.assertEqual(len(patch), 1)
-#         self.assertEqual(patch[0]['op'], 'move')
-#         res = jsonpatch.apply_patch(src, patch)
-#         self.assertEqual(res, dst)
-
-#     def test_minimal_patch(self):
-#         """ Test whether a minimal patch is created, see #36 """
-#         src = [{"foo": 1, "bar": 2}]
-#         dst = [{"foo": 2, "bar": 2}]
-#         patch = jsonpatch.make_patch(src, dst)
-
-#         exp = [
-#             {
-#                 "path": "/0/foo",
-#                 "value": 2,
-#                 "op": "replace"
-#             }
-#         ]
-
-#         self.assertEqual(patch.patch, exp)
+class OptimizationTests(unittest.TestCase):
+    def test_use_replace_instead_of_remove_add(self):
+        src = {'foo': [1, 2, 3]}
+        dst = {'foo': [3, 2, 3]}
+        patch = list(jsonpatch.make_patch(src, dst))
+        self.assertEqual(len(patch), 1)
+        self.assertEqual(patch[0]['op'], 'replace')
+        res = jsonpatch.apply_patch(src, patch)
+        self.assertEqual(res, dst)
+
+    def test_use_replace_instead_of_remove_add_nested(self):
+        src = {'foo': [{'bar': 1, 'baz': 2}, {'bar': 2, 'baz': 3}]}
+        dst = {'foo': [{'bar': 1}, {'bar': 2, 'baz': 3}]}
+        patch = list(jsonpatch.make_patch(src, dst))
+        self.assertEqual(len(patch), 1)
+        self.assertEqual(patch[0]['op'], 'replace')
+        res = jsonpatch.apply_patch(src, patch)
+        self.assertEqual(res, dst)
+
+    def test_use_move_instead_of_remove_add(self):
+        src = {'foo': [4, 1, 2, 3]}
+        dst = {'foo': [1, 2, 3, 4]}
+        patch = list(jsonpatch.make_patch(src, dst))
+        self.assertEqual(len(patch), 1)
+        self.assertEqual(patch[0]['op'], 'move')
+        res = jsonpatch.apply_patch(src, patch)
+        self.assertEqual(res, dst)
+
+    def test_use_move_instead_of_add_remove(self):
+        src = {'foo': [1, 2, 3]}
+        dst = {'foo': [3, 1, 2]}
+        patch = list(jsonpatch.make_patch(src, dst))
+        self.assertEqual(len(patch), 1)
+        self.assertEqual(patch[0]['op'], 'move')
+        res = jsonpatch.apply_patch(src, patch)
+        self.assertEqual(res, dst)
+
+    def test_minimal_patch(self):
+        """ Test whether a minimal patch is created, see #36 """
+        src = [{'a': 1, 'foo': {'b': 2, 'd': 5}}]
+        dst = [{'a': 1, 'foo': {'b': 3, 'd': 6}}]
+        patch = jsonpatch.make_patch(src, dst)
+        self.assertEqual(patch.apply(src), dst)
+
+        src = [{'a': 1, 'b': 2, 'd': 5}]
+        dst = [{'a': 1, 'c': 3, 'd': 5}]
+        patch = jsonpatch.make_patch(src, dst)
+        self.assertEqual(patch.apply(src), dst)
+
+        src = [{'a': 1, 'b': 2, 'd': 5}]
+        dst = [{'d': 6}]
+        patch = jsonpatch.make_patch(src, dst)
+        self.assertEqual(patch.apply(src), dst)
+
+        src = [{'a': 1}, {'b': 2}]
+        dst = [{'a': 1, 'b': 2}]
+        patch = jsonpatch.make_patch(src, dst)
+        self.assertEqual(patch.apply(src), dst)
+
+        src = [{"a": 1, "b": 2}]
+        dst = [{"b": 2, "c": 2}]
+        exp = [{u'path': u'/0', u'value': {u'c': 2, u'b': 2}, u'op': u'replace'}]
+        patch = jsonpatch.make_patch(src, dst)
+        self.assertEqual(patch.patch, exp)
+
+        src = [{"foo": 1, "bar": 2}]
+        dst = [{"foo": 2, "bar": 2}]
+
+        patch = jsonpatch.make_patch(src, dst)
+
+        exp = [
+            {
+                "path": "/0/foo",
+                "value": 2,
+                "op": "replace"
+            }
+        ]
+
+        self.assertEqual(patch.patch, exp)
 
 
 class InvalidInputTests(unittest.TestCase):
@@ -489,7 +516,7 @@ def get_suite():
         suite.addTest(unittest.makeSuite(MakePatchTestCase))
         suite.addTest(unittest.makeSuite(InvalidInputTests))
         suite.addTest(unittest.makeSuite(ConflictTests))
-        # suite.addTest(unittest.makeSuite(OptimizationTests))
+        suite.addTest(unittest.makeSuite(OptimizationTests))
         return suite