From d8d1766ebd461d46eba096df49fe38195edc12a6 Mon Sep 17 00:00:00 2001 From: Dustin Lorres Date: Thu, 17 Apr 2025 21:18:50 -0700 Subject: [PATCH] Fix for moving nested tables when using iterable_compare_func. This fixes issue #540. Before this change the reference params were being swapped right after there was a move. This is because the move needed to have the original paths, but child changes needed the new paths. The problem was that nested moves swapped the reference parameters again after the move was recorded. This made the paths inaccurate since the parent did not have the params swapped but the child did. Instead, we are no longer swapping when building the tree, but rather when we request the paths. The paths will not be swapped for the iterable_item_moved but it will be swapped for all other changes if there was a parent with an iterable_item_moved. --- deepdiff/diff.py | 9 +- deepdiff/model.py | 27 +++-- tests/test_delta.py | 260 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 277 insertions(+), 19 deletions(-) diff --git a/deepdiff/diff.py b/deepdiff/diff.py index d84ecc7e..d2664ef6 100755 --- a/deepdiff/diff.py +++ b/deepdiff/diff.py @@ -952,11 +952,10 @@ def _diff_by_forming_pairs_and_comparing_one_by_one( self._report_result('iterable_item_moved', change_level, local_tree=local_tree) if self.iterable_compare_func: - # Intentionally setting j as the first child relationship param in cases of a moved item. - # If the item was moved using an iterable_compare_func then we want to make sure that the index - # is relative to t2. - reference_param1 = j - reference_param2 = i + # Mark additional context denoting that we have moved an item. + # This will allow for correctly setting paths relative to t2 when using an iterable_compare_func + level.additional["moved"] = True + else: continue diff --git a/deepdiff/model.py b/deepdiff/model.py index 41dd7517..bba2fe8e 100644 --- a/deepdiff/model.py +++ b/deepdiff/model.py @@ -221,10 +221,11 @@ def _from_tree_value_changed(self, tree): def _from_tree_iterable_item_moved(self, tree): if 'iterable_item_moved' in tree and self.verbose_level > 1: + for change in tree['iterable_item_moved']: - the_changed = {'new_path': change.path(use_t2=True), 'value': change.t2} + the_changed = {'new_path': change.path(use_t2=True, reporting_move=True), 'value': change.t2} self['iterable_item_moved'][change.path( - force=FORCE_DEFAULT)] = the_changed + force=FORCE_DEFAULT, use_t2=False, reporting_move=True)] = the_changed def _from_tree_unprocessed(self, tree): if 'unprocessed' in tree: @@ -428,11 +429,11 @@ def _from_tree_iterable_item_moved(self, tree): if 'iterable_item_moved' in tree: for change in tree['iterable_item_moved']: if ( - change.up.path(force=FORCE_DEFAULT) not in self["_iterable_opcodes"] + change.up.path(force=FORCE_DEFAULT, reporting_move=True) not in self["_iterable_opcodes"] ): - the_changed = {'new_path': change.path(use_t2=True), 'value': change.t2} + the_changed = {'new_path': change.path(use_t2=True, reporting_move=True), 'value': change.t2} self['iterable_item_moved'][change.path( - force=FORCE_DEFAULT)] = the_changed + force=FORCE_DEFAULT, reporting_move=True)] = the_changed class DiffLevel: @@ -673,7 +674,7 @@ def get_root_key(self, use_t2=False): return next_rel.param return notpresent - def path(self, root="root", force=None, get_parent_too=False, use_t2=False, output_format='str'): + def path(self, root="root", force=None, get_parent_too=False, use_t2=False, output_format='str', reporting_move=False): """ A python syntax string describing how to descend to this level, assuming the top level object is called root. Returns None if the path is not representable as a string. @@ -699,6 +700,9 @@ def path(self, root="root", force=None, get_parent_too=False, use_t2=False, outp :param output_format: The format of the output. The options are 'str' which is the default and produces a string representation of the path or 'list' to produce a list of keys and attributes that produce the path. + + :param reporting_move: This should be set to true if and only if we are reporting on iterable_item_moved. + All other cases should leave this set to False. """ # TODO: We could optimize this by building on top of self.up's path if it is cached there cache_key = "{}{}{}{}".format(force, get_parent_too, use_t2, output_format) @@ -720,7 +724,16 @@ def path(self, root="root", force=None, get_parent_too=False, use_t2=False, outp # traverse all levels of this relationship while level and level is not self: # get this level's relationship object - if use_t2: + if level.additional.get("moved") and not reporting_move: + # To ensure we can properly replay items such as values_changed in items that may have moved, we + # need to make sure that all paths are reported relative to t2 if a level has reported a move. + # If we are reporting a move, the path is already correct and does not need to be swapped. + # Additional context of "moved" is only ever set if using iterable_compare_func and a move has taken place. + level_use_t2 = not use_t2 + else: + level_use_t2 = use_t2 + + if level_use_t2: next_rel = level.t2_child_rel or level.t1_child_rel else: next_rel = level.t1_child_rel or level.t2_child_rel # next relationship object to get a formatted param from diff --git a/tests/test_delta.py b/tests/test_delta.py index 737a7fbb..20f58aba 100644 --- a/tests/test_delta.py +++ b/tests/test_delta.py @@ -1880,14 +1880,59 @@ def test_compare_func1(self, compare_func_t1, compare_func_t2, compare_func_resu assert compare_func_t2 == recreated_t2 def test_compare_func_with_duplicates_removed(self): - t1 = [{'id': 1, 'val': 1}, {'id': 2, 'val': 2}, {'id': 1, 'val': 3}, {'id': 3, 'val': 3}] - t2 = [{'id': 3, 'val': 3}, {'id': 2, 'val': 2}, {'id': 1, 'val': 3}] + t1 = [ + { + 'id': 1, + 'val': 1, + "nested": [ + {"id": 1, "val": 1}, + {"id": 2, "val": 2}, + ] + }, + { + 'id': 2, + 'val': 2 + }, + { + 'id': 1, + 'val': 3 + }, + { + 'id': 3, + 'val': 3 + } + ] + t2 = [ + { + 'id': 3, + 'val': 3 + }, + { + 'id': 2, + 'val': 2 + }, + { + 'id': 1, + 'val': 3, + "nested":[ + { + "id": 2, + "val": 3 + }, + ] + } + ] ddiff = DeepDiff(t1, t2, iterable_compare_func=self.compare_func, verbose_level=2) expected = { "iterable_item_removed": { "root[2]": { "id": 1, "val": 3 + }, + + "root[2]['nested'][0]": { + "id": 1, + "val": 1 } }, "iterable_item_moved": { @@ -1895,6 +1940,14 @@ def test_compare_func_with_duplicates_removed(self): "new_path": "root[2]", "value": { "id": 1, + "val": 3, + "nested": [{"id": 2, "val": 3}, ] + }, + }, + "root[0]['nested'][1]": { + "new_path": "root[2]['nested'][0]", + "value": { + "id": 2, "val": 3 } }, @@ -1907,6 +1960,11 @@ def test_compare_func_with_duplicates_removed(self): } }, 'values_changed': { + "root[2]['nested'][0]['val']": { + 'new_path': "root[0]['nested'][1]['val']", + 'new_value': 3, + 'old_value': 2 + }, "root[2]['val']": { 'new_value': 3, 'old_value': 1, @@ -1914,6 +1972,7 @@ def test_compare_func_with_duplicates_removed(self): } }, } + assert expected == ddiff delta = Delta(ddiff) recreated_t2 = t1 + delta @@ -1922,10 +1981,14 @@ def test_compare_func_with_duplicates_removed(self): flat_result = delta.to_flat_rows() flat_expected = [ {'path': [2, 'val'], 'value': 3, 'action': 'values_changed', 'type': int, 'new_path': [0, 'val']}, + {'path': [2, 'nested', 0, 'val'], 'value': 3, 'action': 'values_changed', 'type': int, 'new_path': [0, 'nested', 1, 'val']}, + {'path': [2, 'nested', 0], 'value': {'id': 1, 'val': 1}, 'action': 'iterable_item_removed', 'type': dict}, {'path': [2], 'value': {'id': 1, 'val': 3}, 'action': 'iterable_item_removed', 'type': dict}, - {'path': [0], 'value': {'id': 1, 'val': 3}, 'action': 'iterable_item_removed', 'type': dict}, + {'path': [0], 'value': {'id': 1, 'val': 3, 'nested': [{'id': 2, 'val': 3}]}, 'action': 'iterable_item_removed', 'type': dict}, + {'path': [0, 'nested', 1], 'value': {'id': 2, 'val': 3}, 'action': 'iterable_item_removed', 'type': dict}, {'path': [3], 'value': {'id': 3, 'val': 3}, 'action': 'iterable_item_removed', 'type': dict}, - {'path': [0], 'action': 'iterable_item_moved', 'value': {'id': 1, 'val': 3}, 'new_path': [2], 'type': dict}, + {'path': [0], 'action': 'iterable_item_moved', 'value': {'id': 1, 'val': 3, 'nested': [{'id': 2, 'val': 3}]}, 'new_path': [2], 'type': dict}, + {'path': [0, 'nested', 1], 'value': {'id': 2, 'val': 3}, 'action': 'iterable_item_moved', 'type': dict, 'new_path': [2, 'nested', 0]}, {'path': [3], 'action': 'iterable_item_moved', 'value': {'id': 3, 'val': 3}, 'new_path': [0], 'type': dict}, ] flat_expected = [FlatDeltaRow(**i) for i in flat_expected] @@ -1942,11 +2005,20 @@ def test_compare_func_with_duplicates_removed(self): }, 'root[0]': { 'id': 1, - 'val': 3 + 'val': 3, + 'nested': [{'id': 2, 'val': 3}] }, 'root[3]': { 'id': 3, 'val': 3 + }, + "root[2]['nested'][0]": { + "id": 1, + "val": 1 + }, + "root[0]['nested'][1]": { + "id": 2, + "val": 3 } }, 'iterable_item_moved': { @@ -1954,6 +2026,14 @@ def test_compare_func_with_duplicates_removed(self): 'new_path': 'root[2]', 'value': { 'id': 1, + 'val': 3, + 'nested': [{'id': 2, 'val': 3}] + } + }, + "root[0]['nested'][1]": { + 'new_path': "root[2]['nested'][0]", + 'value': { + 'id': 2, 'val': 3 } }, @@ -1968,8 +2048,12 @@ def test_compare_func_with_duplicates_removed(self): 'values_changed': { "root[2]['val']": { 'new_value': 3, - 'new_path': "root[0]['val']" - } + 'new_path': "root[0]['val']", + }, + "root[2]['nested'][0]['val']": { + 'new_path': "root[0]['nested'][1]['val']", + 'new_value': 3, + }, } } assert expected_delta_dict == delta_again.diff @@ -2104,6 +2188,168 @@ def test_compare_func_nested_changes(self): recreated_t2 = t1 + delta assert t2 == recreated_t2 + def test_compare_func_deep_nested_changes(self): + + t1 = { + "Locations": [ + { + "id": "c4fa7b12-f365-42a9-9544-3efc11963558", + "Items": [ + { + "id": "2399528f-2556-4e2c-bf9b-c8ea17bc323f" + }, + { + "id": "2399528f-2556-4e2c-bf9b-c8ea17bc323f1", + }, + { + "id": "2399528f-2556-4e2c-bf9b-c8ea17bc323f2" + }, + { + "id": "2399528f-2556-4e2c-bf9b-c8ea17bc323f3" + } + ] + }, + { + "id": "d9095676-bc41-4cbf-9fd2-7148bb26bcc4", + "Items": [ + { + "id": "26b78305-df71-40c0-8e98-dcd40b7f716d" + }, + { + "id": "3235125d-0110-4d0e-847a-24912cf73feb" + }, + { + "id": "7699552a-add9-4338-aeb9-662bec14c175" + }, + { + "id": "015e74f0-2c2a-45c0-a172-21758d14bf3a" + } + ] + }, + { + "id": "41b38757-8984-47fd-890d-8c4ed18c3c47", + "Items": [ + { + "id": "494e839e-37b1-4cac-b1dc-a44f3e6e7ada" + }, + { + "id": "60547ca6-3ef0-4b67-8826-2c7b76e67011" + }, + { + "id": "cee762a0-fbd8-48bb-ba92-be32cf3cf250" + }, + { + "id": "7a0da2b7-c1e6-45b4-8810-fec7b4b6186d" + } + ] + }, + { + "id": "c0be071a-5457-497d-9a78-ff7cb561d4d3", + "Items": [ + { + "id": "e54dcdff-ec99-4941-92eb-c12bb3cbeb91" + } + ] + }, + { + "id": "dfe4b37b-8df3-4dc6-8686-0588937fbe10", + "Items": [ + { + "id": "27a574ae-08db-47f9-a9dc-18df59287f4d" + }, + { + "id": "23edf031-8c4e-43d6-b5bf-4d5ee9008a36", + "Containers": [ + {"id": "1", "val": 1}, + {"id": "2", "val": 2}, + {"id": "3", "val": 3}, + ] + }, + { + "id": "e1e54643-23ee-496d-b7d2-de67c4bb7d68" + }, + { + "id": "2f910da3-8cd0-4cf5-81c9-23668fc9477f" + }, + { + "id": "5e36d258-2a82-49ee-b4fc-db0a8c28b404" + }, + { + "id": "4bf2ce8d-05ed-4718-a529-8c9e4704e38f" + }, + ] + }, + ] + } + + t2 = { + "Locations": [ + { + "id": "41b38757-8984-47fd-890d-8c4ed18c3c47", + "Items": [ + { + "id": "60547ca6-3ef0-4b67-8826-2c7b76e67011" + }, + { + "id": "cee762a0-fbd8-48bb-ba92-be32cf3cf250" + }, + { + "id": "7a0da2b7-c1e6-45b4-8810-fec7b4b6186d" + } + ] + }, + { + "id": "c0be071a-5457-497d-9a78-ff7cb561d4d3", + "Items": [ + { + "id": "e54dcdff-ec99-4941-92eb-c12bb3cbeb91" + } + ] + }, + { + "id": "dfe4b37b-8df3-4dc6-8686-0588937fbe10", + "Items": [ + { + "id": "27a574ae-08db-47f9-a9dc-18df59287f4d" + }, + { + "id": "27a574ae-08db-47f9-a9dc-88df59287f4d" + }, + { + "id": "23edf031-8c4e-43d6-b5bf-4d5ee9008a36", + "Containers": [ + {"id": "1", "val": 1}, + {"id": "3", "val": 3}, + {"id": "2", "val": 2}, + ] + }, + { + "id": "e1e54643-23ee-496d-b7d2-de67c4bb7d68" + }, + { + "id": "2f910da3-8cd0-4cf5-81c9-23668fc9477f" + }, + { + "id": "5e36d258-2a82-49ee-b4fc-db0a8c28b404" + }, + { + "id": "4bf2ce8d-05ed-4718-a529-8c9e4704e38f" + }, + ] + }, + ] + } + + ddiff = DeepDiff(t1, t2, iterable_compare_func=self.compare_func, verbose_level=2) + + delta2 = Delta(ddiff) + expected_move_1 = {'new_path': "root['Locations'][2]['Items'][2]['Containers'][2]", 'value': {'id': '2', 'val': 2}} + expected_move_2 = {'new_path': "root['Locations'][2]['Items'][2]['Containers'][1]", 'value': {'id': '3', 'val': 3}} + assert ddiff["iterable_item_moved"]["root['Locations'][4]['Items'][1]['Containers'][1]"] == expected_move_1 + assert ddiff["iterable_item_moved"]["root['Locations'][4]['Items'][1]['Containers'][2]"] == expected_move_2 + recreated_t2 = t1 + delta2 + assert t2 == recreated_t2 + def test_delta_force1(self): t1 = { 'x': {