From 6709b90597f636a052bfa2472c44ce4fa574c7d2 Mon Sep 17 00:00:00 2001 From: Angela Liss <59097311+angela-tarantula@users.noreply.github.com> Date: Sat, 11 Oct 2025 14:17:12 -0400 Subject: [PATCH 1/5] create new test file --- issue.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 issue.py diff --git a/issue.py b/issue.py new file mode 100644 index 0000000..706f451 --- /dev/null +++ b/issue.py @@ -0,0 +1,21 @@ +import jsonpatch + +src_obj = {'a': [{'id': [1]}, {'id': [2]}], 'b': [{'id': 5}]} +tgt_obj = {'a': [{'id': []}, {'id': [1]}], 'b': [{'id': 5, 'newKey': 2}]} + +patch = jsonpatch.make_patch(src_obj, tgt_obj) +print(patch) + +# Check normal application of the patch +try: + tgt_obj_check = jsonpatch.apply_patch(src_obj, patch) + print('Forward', 'OK' if len(jsonpatch.make_patch(tgt_obj, tgt_obj_check).patch) == 0 else 'ERROR', '->', tgt_obj_check) +except Exception as e: + print('Forward', 'Error', '->', type(e).__name__) + +# Check reverse application of the patch +try: + tgt_obj_check = jsonpatch.apply_patch(src_obj, patch.patch[::-1]) + print('Reverse', 'OK' if len(jsonpatch.make_patch(tgt_obj, tgt_obj_check).patch) == 0 else 'ERROR', '->', tgt_obj_check) +except Exception as e: + print('Reverse', 'Error', '->', type(e).__name__) From 77e37cef17996001dbe1a54e519d44c6d7e2ccf7 Mon Sep 17 00:00:00 2001 From: Angela Liss <59097311+angela-tarantula@users.noreply.github.com> Date: Sat, 11 Oct 2025 14:24:37 -0400 Subject: [PATCH 2/5] fix item_added logic When an AddOperation would match a prior RemoveOperation, a MoveOperation is inserted instead, and the RemoveOperation is deleted. But when the RemoveOperation is deleted, subsequent operations that depended on it are supposed to have their paths adjusted by _on_undo_remove. Previously, _on_undo_remove was guarded by type(key) == int, which fails to consider cases where the key is a string but the operation path still needs to be adjusted. The correct guard should be type(op.pointer.to_last(self.dst_doc)[0]) == list, because _on_undo_remove should apply whenever the item removed was contained in a list. --- jsonpatch.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/jsonpatch.py b/jsonpatch.py index d3fc26d..76d7cb5 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -797,7 +797,8 @@ def _item_added(self, path, key, item): index = self.take_index(item, _ST_REMOVE) if index is not None: op = index[2] - if type(op.key) == int and type(key) == int: + parent_collection = op.pointer.to_last(self.dst_doc)[0] + if type(parent_collection) == list: for v in self.iter_from(index): op.key = v._on_undo_remove(op.path, op.key) @@ -831,8 +832,8 @@ def _item_removed(self, path, key, item): # the .key property to int and this path wrongly ends up being taken # for numeric string dict keys while the intention is to only handle lists. # So we do an explicit check on the item affected by the op instead. - added_item = op.pointer.to_last(self.dst_doc)[0] - if type(added_item) == list: + parent_collection = op.pointer.to_last(self.dst_doc)[0] + if type(parent_collection) == list: for v in self.iter_from(index): op.key = v._on_undo_add(op.path, op.key) From 3bb54cf73aeb43abf8b283ff736dc78f6e30993c Mon Sep 17 00:00:00 2001 From: Angela Liss <59097311+angela-tarantula@users.noreply.github.com> Date: Sat, 11 Oct 2025 17:54:34 -0400 Subject: [PATCH 3/5] Add new test --- tests.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests.py b/tests.py index d9eea92..32239dc 100755 --- a/tests.py +++ b/tests.py @@ -566,6 +566,14 @@ def test_issue120(self): res = jsonpatch.apply_patch(src, patch) self.assertEqual(res, dst) + def test_issue_160(self): + """Removal of an operation to an array should trigger _on_undo_add.""" + old = {'a': [{'id': [1]}, {'id': [2]}], 'b': [{'id': 5}]} + new = {'a': [{'id': []}, {'id': [1]}], 'b': [{'id': 5, 'newKey': 2}]} + 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')} From 3b1a94525dc4f2c704c43315ef16bdccaf294c0c Mon Sep 17 00:00:00 2001 From: Angela Liss <59097311+angela-tarantula@users.noreply.github.com> Date: Sat, 11 Oct 2025 17:55:01 -0400 Subject: [PATCH 4/5] remove unnecessary test file (pytest.py already updated) --- issue.py | 21 --------------------- 1 file changed, 21 deletions(-) delete mode 100644 issue.py diff --git a/issue.py b/issue.py deleted file mode 100644 index 706f451..0000000 --- a/issue.py +++ /dev/null @@ -1,21 +0,0 @@ -import jsonpatch - -src_obj = {'a': [{'id': [1]}, {'id': [2]}], 'b': [{'id': 5}]} -tgt_obj = {'a': [{'id': []}, {'id': [1]}], 'b': [{'id': 5, 'newKey': 2}]} - -patch = jsonpatch.make_patch(src_obj, tgt_obj) -print(patch) - -# Check normal application of the patch -try: - tgt_obj_check = jsonpatch.apply_patch(src_obj, patch) - print('Forward', 'OK' if len(jsonpatch.make_patch(tgt_obj, tgt_obj_check).patch) == 0 else 'ERROR', '->', tgt_obj_check) -except Exception as e: - print('Forward', 'Error', '->', type(e).__name__) - -# Check reverse application of the patch -try: - tgt_obj_check = jsonpatch.apply_patch(src_obj, patch.patch[::-1]) - print('Reverse', 'OK' if len(jsonpatch.make_patch(tgt_obj, tgt_obj_check).patch) == 0 else 'ERROR', '->', tgt_obj_check) -except Exception as e: - print('Reverse', 'Error', '->', type(e).__name__) From d02143e79dcea1fa0f01f98f602c009248f9476b Mon Sep 17 00:00:00 2001 From: Angela Liss <59097311+angela-tarantula@users.noreply.github.com> Date: Sun, 12 Oct 2025 11:32:51 -0400 Subject: [PATCH 5/5] check for MutableSequence, not list This grants flexility for subclassing. Also removed a comment that previously existed because of discrepancy between _item_added and _item_removed that no longer exists. --- jsonpatch.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/jsonpatch.py b/jsonpatch.py index 76d7cb5..df6e3c1 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -798,7 +798,7 @@ def _item_added(self, path, key, item): if index is not None: op = index[2] parent_collection = op.pointer.to_last(self.dst_doc)[0] - if type(parent_collection) == list: + if isinstance(parent_collection, MutableSequence): for v in self.iter_from(index): op.key = v._on_undo_remove(op.path, op.key) @@ -828,12 +828,8 @@ def _item_removed(self, path, key, item): new_index = self.insert(new_op) if index is not None: op = index[2] - # We can't rely on the op.key type since PatchOperation casts - # the .key property to int and this path wrongly ends up being taken - # for numeric string dict keys while the intention is to only handle lists. - # So we do an explicit check on the item affected by the op instead. parent_collection = op.pointer.to_last(self.dst_doc)[0] - if type(parent_collection) == list: + if isinstance(parent_collection, MutableSequence): for v in self.iter_from(index): op.key = v._on_undo_add(op.path, op.key)