Added fix and unit test for (bug) issue 457, seperman#457

sf-tcalhoun · Mar 25, 2024 · a9bfc08 · a9bfc08
1 parent 71fde30
commit a9bfc08
Show file tree

Hide file tree

Showing 2 changed files with 153 additions and 1 deletion.
diff --git a/deepdiff/delta.py b/deepdiff/delta.py
@@ -1,3 +1,4 @@
+import copy
 import logging
 from functools import partial
 from collections.abc import Mapping
@@ -125,7 +126,8 @@ def _deserializer(obj, safe_to_import=None):
                 raise ValueError(BINIARY_MODE_NEEDED_MSG.format(e)) from None
             self.diff = _deserializer(content, safe_to_import=safe_to_import)
         elif flat_dict_list:
-            self.diff = self._from_flat_dicts(flat_dict_list)
+            # Use copy to preserve original value of flat_dict_list in calling module
+            self.diff = self._from_flat_dicts(copy.deepcopy(flat_dict_list))
         else:
             raise ValueError(DELTA_AT_LEAST_ONE_ARG_NEEDED)
 

diff --git a/tests/test_delta.py b/tests/test_delta.py
@@ -1,3 +1,5 @@
+import copy
+
 import pytest
 import os
 import io
@@ -461,6 +463,154 @@ def test_delta_dict_items_added_retain_order(self):
         delta2 = Delta(diff=diff, bidirectional=True)
         assert t1 == t2 - delta2
 
+    def test_delta_constr_flat_dict_list_param_preserve(self):
+        """
+        Issue: https://github.com/seperman/deepdiff/issues/457
+
+        Scenario:
+        We found that when a flat_dict_list was provided as a constructor
+        parameter for instantiating a new delta, the provided flat_dict_list
+        is unexpectedly being mutated/changed, which can be troublesome for the
+        caller if they were expecting the flat_dict_list to be used BY COPY
+        rather than BY REFERENCE.
+
+        Intent:
+        Preserve the original value of the flat_dict_list variable within the
+        calling module/function after instantiating the new delta.
+        """
+
+        t1 = {
+            "individualNames": [
+                {
+                    "firstName": "Johnathan",
+                    "lastName": "Doe",
+                    "prefix": "COLONEL",
+                    "middleName": "A",
+                    "primaryIndicator": True,
+                    "professionalDesignation": "PHD",
+                    "suffix": "SR",
+                    "nameIdentifier": "00001"
+                },
+                {
+                    "firstName": "John",
+                    "lastName": "Doe",
+                    "prefix": "",
+                    "middleName": "",
+                    "primaryIndicator": False,
+                    "professionalDesignation": "",
+                    "suffix": "SR",
+                    "nameIdentifier": "00002"
+                }
+            ]
+        }
+
+        t2 = {
+            "individualNames": [
+                {
+                    "firstName": "Johnathan",
+                    "lastName": "Doe",
+                    "prefix": "COLONEL",
+                    "middleName": "A",
+                    "primaryIndicator": True,
+                    "professionalDesignation": "PHD",
+                    "suffix": "SR",
+                    "nameIdentifier": "00001"
+                },
+                {
+                    "firstName": "Johnny",
+                    "lastName": "Doe",
+                    "prefix": "",
+                    "middleName": "A",
+                    "primaryIndicator": False,
+                    "professionalDesignation": "",
+                    "suffix": "SR",
+                    "nameIdentifier": "00003"
+                }
+            ]
+        }
+
+        def compare_func(item1, item2, level=None):
+            print("*** inside compare ***")
+            it1_keys = item1.keys()
+
+            try:
+
+                # --- individualNames ---
+                if 'nameIdentifier' in it1_keys and 'lastName' in it1_keys:
+                    match_result = item1['nameIdentifier'] == item2['nameIdentifier']
+                    print("individualNames - matching result:", match_result)
+                    return match_result
+                else:
+                    print("Unknown list item...", "matching result:", item1 == item2)
+                    return item1 == item2
+            except Exception:
+                raise CannotCompare() from None
+        # ---------------------------- End of nested function
+
+        # This diff should show:
+        # 1 - list item (with an index on the path) being added
+        # 1 - list item (with an index on the path) being removed
+        diff = DeepDiff(t1, t2, report_repetition=True,
+                             ignore_order=True, iterable_compare_func=compare_func, cutoff_intersection_for_pairs=1)
+
+        # Now create a flat_dict_list from a delta instantiated from the diff...
+        temp_delta = Delta(diff, always_include_values=True, bidirectional=True, raise_errors=True)
+        flat_dict_list = temp_delta.to_flat_dicts()
+
+        # Note: the list index is provided on the path value...
+        assert flat_dict_list == [{'path': ['individualNames', 1],
+                                   'value': {'firstName': 'Johnny',
+                                             'lastName': 'Doe',
+                                             'prefix': '',
+                                             'middleName': 'A',
+                                             'primaryIndicator': False,
+                                             'professionalDesignation': '',
+                                             'suffix': 'SR',
+                                             'nameIdentifier': '00003'},
+                                   'action': 'unordered_iterable_item_added'},
+                                  {'path': ['individualNames', 1],
+                                   'value': {'firstName': 'John',
+                                             'lastName': 'Doe',
+                                             'prefix': '',
+                                             'middleName': '',
+                                             'primaryIndicator': False,
+                                             'professionalDesignation': '',
+                                             'suffix': 'SR',
+                                             'nameIdentifier': '00002'},
+                                   'action': 'unordered_iterable_item_removed'}]
+
+        preserved_flat_dict_list = copy.deepcopy(flat_dict_list)  # Use this later for assert comparison
+
+        # Now use the flat_dict_list to instantiate a new delta...
+        delta = Delta(flat_dict_list=flat_dict_list,
+                      always_include_values=True, bidirectional=True, raise_errors=True)
+
+        # if the flat_dict_list is (unexpectedly) mutated, it will be missing the list index number on the path value.
+        old_mutated_list_missing_indexes_on_path = [{'path': ['individualNames'],
+                                         'value': {'firstName': 'Johnny',
+                                                   'lastName': 'Doe',
+                                                   'prefix': '',
+                                                   'middleName': 'A',
+                                                   'primaryIndicator': False,
+                                                   'professionalDesignation': '',
+                                                   'suffix': 'SR',
+                                                   'nameIdentifier': '00003'},
+                                         'action': 'unordered_iterable_item_added'},
+                                        {'path': ['individualNames'],
+                                         'value': {'firstName': 'John',
+                                                   'lastName': 'Doe',
+                                                   'prefix': '',
+                                                   'middleName': '',
+                                                   'primaryIndicator': False,
+                                                   'professionalDesignation': '',
+                                                   'suffix': 'SR',
+                                                   'nameIdentifier': '00002'},
+                                         'action': 'unordered_iterable_item_removed'}]
+
+        # Verify that our fix in the delta constructor worked...
+        assert flat_dict_list != old_mutated_list_missing_indexes_on_path
+        assert flat_dict_list == preserved_flat_dict_list
+
 
 picklalbe_obj_without_item = PicklableClass(11)
 del picklalbe_obj_without_item.item