diff --git a/jsonpatch.py b/jsonpatch.py index d3fc26d..4c09935 100644 --- a/jsonpatch.py +++ b/jsonpatch.py @@ -774,24 +774,91 @@ def __iter__(self): curr = curr[1] def execute(self): + # First, collect all operations + operations = [] root = self.__root curr = root[1] + + # First pass: collect operations and handle replace optimizations while curr is not root: if curr[1] is not root: op_first, op_second = curr[2], curr[1][2] if op_first.location == op_second.location and \ type(op_first) == RemoveOperation and \ type(op_second) == AddOperation: - yield ReplaceOperation({ + operations.append(ReplaceOperation({ 'op': 'replace', 'path': op_second.location, 'value': op_second.operation['value'], - }, pointer_cls=self.pointer_cls).operation + }, pointer_cls=self.pointer_cls).operation) curr = curr[1][1] continue - - yield curr[2].operation + + operations.append(curr[2].operation) curr = curr[1] + + # Second pass: identify and sort move operations + move_indices = [] + move_ops = [] + + for i, op in enumerate(operations): + if op['op'] == 'move': + move_indices.append(i) + move_ops.append(op) + + # Sort move operations based on dependencies + if move_ops: + # Create dependency graph: if op1's target is op2's source, op2 depends on op1 + dependencies = {} + for i, op1 in enumerate(move_ops): + dependencies[i] = [] + src1 = op1['from'] + tgt1 = op1['path'] + + for j, op2 in enumerate(move_ops): + if i == j: + continue # Skip self-comparison + + src2 = op2['from'] + + # If op2's source is the same as op1's target, op2 should come after op1 + if src2 == tgt1 or src2.startswith(tgt1 + '/'): + dependencies[i].append(j) + + # Topological sort to determine execution order + # We're using a simple approach: if op1 depends on op2, op2 should be executed first + sorted_indices = [] + visited = set() + temp_visited = set() + + def visit(i): + if i in temp_visited: + # Cyclic dependency - maintain original order + return + if i in visited: + return + + temp_visited.add(i) + + for j in dependencies.get(i, []): + visit(j) + + temp_visited.remove(i) + visited.add(i) + sorted_indices.append(i) + + for i in range(len(move_ops)): + if i not in visited: + visit(i) + + # Replace original move operations with sorted ones + sorted_move_ops = [move_ops[i] for i in sorted_indices] + for idx, op in zip(move_indices, sorted_move_ops): + operations[idx] = op + + # Yield operations + for op in operations: + yield op def _item_added(self, path, key, item): index = self.take_index(item, _ST_REMOVE) diff --git a/patch_test.py b/patch_test.py new file mode 100644 index 0000000..4d47edb --- /dev/null +++ b/patch_test.py @@ -0,0 +1,15 @@ +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 +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) + +# Check reverse application of the patch +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) \ No newline at end of file diff --git a/tests.py b/tests.py index d9eea92..d6df64e 100755 --- a/tests.py +++ b/tests.py @@ -455,6 +455,24 @@ def test_arrays_one_element_sequences(self): patch = jsonpatch.make_patch(src, dst) res = jsonpatch.apply_patch(src, patch) self.assertEqual(res, dst) + + def test_move_operations_order(self): + """Test that move operations are properly ordered to ensure correct application""" + src_obj = {'a': [{'id': [1]}, {'id': [2]}], 'b': [{'id': 5}]} + tgt_obj = {'a': [{'id': []}, {'id': [1]}], 'b': [{'id': 5, 'newKey': 2}]} + + # Generate patch + patch = jsonpatch.make_patch(src_obj, tgt_obj) + + # Apply patch forward and check result + result = jsonpatch.apply_patch(src_obj, patch) + self.assertEqual(result, tgt_obj, "Patch application should produce the expected target object") + + # Apply patch in reverse and verify it fails (to confirm we're testing the right scenario) + patch_ops = list(patch) + reverse_patch = jsonpatch.JsonPatch(patch_ops[::-1]) + reverse_result = jsonpatch.apply_patch(src_obj, reverse_patch) + self.assertNotEqual(reverse_result, tgt_obj, "Reverse patch should not work - confirms we're testing the right issue") def test_list_in_dict(self): """ Test patch creation with a list within a dict, as reported in #74