Merge pull request #473 from surefyresystems/iterable_compare_func_fixes

Fix for iterable moved items what are found with iterable_compare_func
seperman · Aug 27, 2024 · fe70fa0 · fe70fa0
2 parents 5a7bccb + 78d478a
commit fe70fa0
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 45 deletions.
diff --git a/deepdiff/diff.py b/deepdiff/diff.py
@@ -854,13 +854,15 @@ def _diff_by_forming_pairs_and_comparing_one_by_one(
             if self._count_diff() is StopIteration:
                 return  # pragma: no cover. This is already covered for addition.
 
+            reference_param1 = i
+            reference_param2 = j
             if y is ListItemRemovedOrAdded:  # item removed completely
                 change_level = level.branch_deeper(
                     x,
                     notpresent,
                     child_relationship_class=child_relationship_class,
-                    child_relationship_param=i,
-                    child_relationship_param2=j,
+                    child_relationship_param=reference_param1,
+                    child_relationship_param2=reference_param2,
                     )
                 self._report_result('iterable_item_removed', change_level, local_tree=local_tree)
 
@@ -869,8 +871,8 @@ def _diff_by_forming_pairs_and_comparing_one_by_one(
                     notpresent,
                     y,
                     child_relationship_class=child_relationship_class,
-                    child_relationship_param=i,
-                    child_relationship_param2=j,
+                    child_relationship_param=reference_param1,
+                    child_relationship_param2=reference_param2,
                     )
                 self._report_result('iterable_item_added', change_level, local_tree=local_tree)
 
@@ -881,11 +883,17 @@ def _diff_by_forming_pairs_and_comparing_one_by_one(
                         x,
                         y,
                         child_relationship_class=child_relationship_class,
-                        child_relationship_param=i,
-                        child_relationship_param2=j
+                        child_relationship_param=reference_param1,
+                        child_relationship_param2=reference_param2
                     )
                     self._report_result('iterable_item_moved', change_level, local_tree=local_tree)
-                    continue
+
+                    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
 
                 item_id = id(x)
                 if parents_ids and item_id in parents_ids:
@@ -897,8 +905,8 @@ def _diff_by_forming_pairs_and_comparing_one_by_one(
                     x,
                     y,
                     child_relationship_class=child_relationship_class,
-                    child_relationship_param=i,
-                    child_relationship_param2=j,
+                    child_relationship_param=reference_param1,
+                    child_relationship_param2=reference_param2
                 )
                 self._diff(next_level, parents_ids_added, local_tree=local_tree)
 

diff --git a/tests/fixtures/compare_func_result1.json b/tests/fixtures/compare_func_result1.json
@@ -1,40 +1,59 @@
 {
-    "dictionary_item_added": [
-        "root['Cars'][3]['dealers']"
-    ],
-    "dictionary_item_removed": [
-        "root['Cars'][3]['production']"
-    ],
-    "values_changed": {
-        "root['Cars'][3]['model']": {
-            "new_value": "Supra",
-            "old_value": "supra"
-        }
+  "dictionary_item_added": [
+    "root['Cars'][3]['dealers']"
+  ],
+  "dictionary_item_removed": [
+    "root['Cars'][3]['production']"
+  ],
+  "values_changed": {
+    "root['Cars'][2]['dealers'][0]['quantity']": {
+      "new_value": 50,
+      "old_value": 20
     },
-    "iterable_item_added": {
-        "root['Cars'][0]": {
-            "id": "7",
-            "make": "Toyota",
-            "model": "8Runner"
-        }
+    "root['Cars'][1]['model_numbers'][2]": {
+      "new_value": 3,
+      "old_value": 4
+    },
+    "root['Cars'][3]['model']": {
+      "new_value": "Supra",
+      "old_value": "supra"
+    }
+  },
+  "iterable_item_added": {
+    "root['Cars'][2]['dealers'][1]": {
+      "id": 200,
+      "address": "200 Fake St",
+      "quantity": 10
+    },
+    "root['Cars'][1]['model_numbers'][3]": 4,
+    "root['Cars'][0]": {
+      "id": "7",
+      "make": "Toyota",
+      "model": "8Runner"
+    }
+  },
+  "iterable_item_removed": {
+    "root['Cars'][2]['dealers'][0]": {
+      "id": 103,
+      "address": "103 Fake St",
+      "quantity": 50
     },
-    "iterable_item_removed": {
-        "root['Cars'][1]": {
-            "id": "2",
-            "make": "Toyota",
-            "model": "Highlander",
-            "dealers": [
-                {
-                    "id": 123,
-                    "address": "123 Fake St",
-                    "quantity": 50
-                },
-                {
-                    "id": 125,
-                    "address": "125 Fake St",
-                    "quantity": 20
-                }
-            ]
+    "root['Cars'][1]": {
+      "id": "2",
+      "make": "Toyota",
+      "model": "Highlander",
+      "dealers": [
+        {
+          "id": 123,
+          "address": "123 Fake St",
+          "quantity": 50
+        },
+        {
+          "id": 125,
+          "address": "125 Fake St",
+          "quantity": 20
         }
+      ]
     }
+  }
 }
diff --git a/tests/test_delta.py b/tests/test_delta.py
@@ -1894,7 +1894,14 @@ def test_compare_func_with_duplicates_removed(self):
                         "val": 3
                     }
                 }
-            }
+            },
+            'values_changed': {
+                "root[2]['val']": {
+                    'new_value': 3,
+                    'old_value': 1,
+                    'new_path': "root[0]['val']"
+                }
+            },
         }
         assert expected == ddiff
         delta = Delta(ddiff)
@@ -1903,6 +1910,7 @@ 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], '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': [3], 'value': {'id': 3, 'val': 3}, 'action': 'iterable_item_removed', 'type': dict},
@@ -1945,6 +1953,12 @@ def test_compare_func_with_duplicates_removed(self):
                         'val': 3
                     }
                 }
+            },
+            'values_changed': {
+                "root[2]['val']": {
+                    'new_value': 3,
+                    'new_path': "root[0]['val']"
+                }
             }
         }
         assert expected_delta_dict == delta_again.diff
@@ -1976,7 +1990,14 @@ def test_compare_func_with_duplicates_added(self):
                         'val': 1
                     }
                 }
-            }
+            },
+            'values_changed': {
+                "root[0]['val']": {
+                    'new_value': 1,
+                    'old_value': 3,
+                    'new_path': "root[2]['val']"
+                }
+            },
         }
         assert expected == ddiff
         delta = Delta(ddiff)