DiffBuilder: fix reindexing of operations involving nested paths

jsonpatch.py

@@ -223,17 +223,29 @@ def path(self):
 
     @property
     def key(self):
-        try:
-            return int(self.pointer.parts[-1])
-        except ValueError:
-            return self.pointer.parts[-1]
+        return self.get_part(-1)
 
     @key.setter
     def key(self, value):
-        self.pointer.parts[-1] = str(value)
+        self.set_part(-1, value)
+
+    def get_part(self, index):
+        try:
+            return int(self.pointer.parts[index])
+        except ValueError:
+            return self.pointer.parts[index]
+
+    def set_part(self, index, value):
+        self.pointer.parts[index] = str(value)
         self.location = self.pointer.path
         self.operation['path'] = self.location
 
+    def _increment_part(self, index):
+        self.set_part(index, self.get_part(index) + 1)
+
+    def _decrement_part(self, index):
+        self.set_part(index, self.get_part(index) - 1)
+
 
 class RemoveOperation(PatchOperation):
     """Removes an object property or an array element."""
@@ -252,18 +264,20 @@ def apply(self, obj):
 
         return obj
 
-    def _on_undo_remove(self, path, key):
-        if self.path == path:
-            if self.key >= key:
-                self.key += 1
+    def _on_undo_remove(self, sub_parts, key):
+        if _is_prefix(sub_parts, self.pointer.parts):
+            affected_index = len(sub_parts)
+            if self.get_part(affected_index) >= key:
+                self._increment_part(affected_index)
             else:
                 key -= 1
         return key
 
-    def _on_undo_add(self, path, key):
-        if self.path == path:
-            if self.key > key:
-                self.key -= 1
+    def _on_undo_add(self, sub_parts, key):
+        if _is_prefix(sub_parts, self.pointer.parts):
+            affected_index = len(sub_parts)
+            if self.get_part(affected_index) > key:
+                self._decrement_part(affected_index)
             else:
                 key -= 1
         return key
@@ -304,18 +318,20 @@ def apply(self, obj):
                 raise JsonPatchConflict("unable to fully resolve json pointer {0}, part {1}".format(self.location, part))
         return obj
 
-    def _on_undo_remove(self, path, key):
-        if self.path == path:
-            if self.key > key:
-                self.key += 1
+    def _on_undo_remove(self, sub_parts, key):
+        if _is_prefix(sub_parts, self.pointer.parts):
+            affected_index = len(sub_parts)
+            if self.get_part(affected_index) > key:
+                self._increment_part(affected_index)
             else:
                 key += 1
         return key
 
-    def _on_undo_add(self, path, key):
-        if self.path == path:
-            if self.key > key:
-                self.key -= 1
+    def _on_undo_add(self, sub_parts, key):
+        if _is_prefix(sub_parts, self.pointer.parts):
+            affected_index = len(sub_parts)
+            if self.get_part(affected_index) > key:
+                self._decrement_part(affected_index)
             else:
                 key += 1
         return key
@@ -356,10 +372,10 @@ def apply(self, obj):
         subobj[part] = value
         return obj
 
-    def _on_undo_remove(self, path, key):
+    def _on_undo_remove(self, sub_parts, key):
         return key
 
-    def _on_undo_add(self, path, key):
+    def _on_undo_add(self, sub_parts, key):
         return key
 
 
@@ -410,40 +426,58 @@ def from_path(self):
 
     @property
     def from_key(self):
-        from_ptr = self.pointer_cls(self.operation['from'])
-        try:
-            return int(from_ptr.parts[-1])
-        except TypeError:
-            return from_ptr.parts[-1]
+        return self.get_from_part(-1)
 
     @from_key.setter
     def from_key(self, value):
+        self.set_from_part(-1, value)
+
+    def get_from_part(self, index):
+        from_ptr = self.pointer_cls(self.operation['from'])
+        try:
+            return int(from_ptr.parts[index])
+        except ValueError:
+            return from_ptr.parts[index]
+
+    def set_from_part(self, index, value):
         from_ptr = self.pointer_cls(self.operation['from'])
-        from_ptr.parts[-1] = str(value)
+        from_ptr.parts[index] = str(value)
         self.operation['from'] = from_ptr.path
 
-    def _on_undo_remove(self, path, key):
-        if self.from_path == path:
-            if self.from_key >= key:
-                self.from_key += 1
+    def _increment_from_part(self, index):
+        self.set_from_part(index, self.get_from_part(index) + 1)
+
+    def _decrement_from_part(self, index):
+        self.set_from_part(index, self.get_from_part(index) - 1)
+
+    def _on_undo_remove(self, sub_parts, key):
+        from_ptr = self.pointer_cls(self.operation['from'])
+        if _is_prefix(sub_parts, from_ptr.parts):
+            affected_index = len(sub_parts)
+            if self.get_from_part(affected_index) >= key:
+                self._increment_from_part(affected_index)
             else:
                 key -= 1
-        if self.path == path:
-            if self.key > key:
-                self.key += 1
+        if _is_prefix(sub_parts, self.pointer.parts):
+            affected_index = len(sub_parts)
+            if self.get_part(affected_index) > key:
+                self._increment_part(affected_index)
             else:
                 key += 1
         return key
 
-    def _on_undo_add(self, path, key):
-        if self.from_path == path:
-            if self.from_key > key:
-                self.from_key -= 1
+    def _on_undo_add(self, sub_parts, key):
+        from_ptr = self.pointer_cls(self.operation['from'])
+        if _is_prefix(sub_parts, from_ptr.parts):
+            affected_index = len(sub_parts)
+            if self.get_from_part(affected_index) > key:
+                self._decrement_from_part(affected_index)
             else:
                 key -= 1
-        if self.path == path:
-            if self.key > key:
-                self.key -= 1
+        if _is_prefix(sub_parts, self.pointer.parts):
+            affected_index = len(sub_parts)
+            if self.get_part(affected_index) > key:
+                self._decrement_part(affected_index)
             else:
                 key += 1
         return key
@@ -799,7 +833,7 @@ def _item_added(self, path, key, item):
             op = index[2]
             if type(op.key) == int and type(key) == int:
                 for v in self.iter_from(index):
-                    op.key = v._on_undo_remove(op.path, op.key)
+                    op.key = v._on_undo_remove(op.pointer.parts[:-1], op.key)
 
             self.remove(index)
             if op.location != _path_join(path, key):
@@ -834,7 +868,7 @@ def _item_removed(self, path, key, item):
             added_item = op.pointer.to_last(self.dst_doc)[0]
             if type(added_item) == list:
                 for v in self.iter_from(index):
-                    op.key = v._on_undo_add(op.path, op.key)
+                    op.key = v._on_undo_add(op.pointer.parts[:-1], op.key)
 
             self.remove(index)
             if new_op.location != op.location:
@@ -929,3 +963,6 @@ def _path_join(path, key):
         return path
 
     return path + '/' + str(key).replace('~', '~0').replace('/', '~1')
+
+def _is_prefix(sub_parts, parts):
+    return sub_parts == parts[:len(sub_parts)]

tests.py

@@ -566,6 +566,52 @@ def test_issue120(self):
         res = jsonpatch.apply_patch(src, patch)
         self.assertEqual(res, dst)
 
+    def test_issue_138(self):
+        """
+        The _on_undo methods should update its operation's path if it is
+        affected by the removal of a prior operation.
+        """
+        old = [
+            {"x": ["a", {"y": ["b"]}], "z": "a"},
+            {"x": ["c", {"d": ["d"]}], "z": "c"},
+            {},
+        ]
+        new = [
+            {"x": ["c", {"y": ["d"]}], "z": "c"},
+            {},
+        ]
+        patch = jsonpatch.make_patch(old, new)
+        result = jsonpatch.apply_patch(old, patch)
+        self.assertEqual(result, new)
+
+    def test_issue_138b(self):
+        """Additionally tests escaping special characters."""
+        old = {"/":
+            [
+                {"x": ["a", {"y": ["b"]}], "z": "a"},
+                {"x": ["c", {"d": ["d"]}], "z": "c"},
+                {},
+            ]
+        }
+        new = {"/":
+            [
+                {"x": ["c", {"y": ["d"]}], "z": "c"},
+                {},
+            ]
+        }
+        patch = jsonpatch.make_patch(old, new)
+        result = jsonpatch.apply_patch(old, patch)
+        self.assertEqual(result, new)
+
+
+    def test_issue_124(self):
+        """Similar to issue 138, but for different operations."""
+        old = ['a', 'b', ['d', 'e'], 'f']
+        new = ['a', 'd', ['e', 'g']]
+        patch = jsonpatch.make_patch(old, new)
+        result = jsonpatch.apply_patch(old, patch)
+        self.assertEqual(result, new)
+
     def test_custom_types_diff(self):
         old = {'value': decimal.Decimal('1.0')}
         new = {'value': decimal.Decimal('1.00')}