Skip to content

Commit

Permalink
throw a TypeError when trying to merge a list with a mapping when `me…
Browse files Browse the repository at this point in the history
…rge_lists=True`
  • Loading branch information
Tom Doherty committed Dec 13, 2024
1 parent 9d10d69 commit 3acaf41
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 1 deletion.
1 change: 1 addition & 0 deletions changelog/67092.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
dictupdate.update: throw a TypeError when trying to merge a list with a mapping when ``merge_lists=True``.
17 changes: 16 additions & 1 deletion salt/utils/dictupdate.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
log = logging.getLogger(__name__)


def update(dest, upd, recursive_update=True, merge_lists=False):
def update(dest, upd, recursive_update=True, merge_lists=False, strict=False):
"""
Recursive version of the default dict.update
Expand All @@ -30,9 +30,15 @@ def update(dest, upd, recursive_update=True, merge_lists=False):
is ``dest[key] + upd[key]``. This behavior is only activated when
recursive_update=True. By default merge_lists=False.
if strict=True, then we will raise a TypeError when trying to merge a list
with a mapping when ``merge_lists=True``.
.. versionchanged:: 2016.11.6
When merging lists, duplicate values are removed. Values already
present in the ``dest`` list are not added from the ``upd`` list.
.. versionchanged:: 3008.0
Throw a TypeError when trying to merge a list with a mapping when
``merge_lists=True``.
"""
if (not isinstance(dest, Mapping)) or (not isinstance(upd, Mapping)):
raise TypeError("Cannot update using non-dict types in dictupdate.update()")
Expand All @@ -56,6 +62,15 @@ def update(dest, upd, recursive_update=True, merge_lists=False):
dest[key] = merged
else:
dest[key] = upd[key]
elif (
strict
and merge_lists
and isinstance(dest_subkey, list)
and isinstance(val, Mapping)
):
raise TypeError(
f"With {merge_lists=} we cannot update list under {key=} with mapping containing keys {val.keys()=} in dictupdate.update()"
)
else:
dest[key] = upd[key]
return dest
Expand Down
12 changes: 12 additions & 0 deletions tests/unit/utils/test_dictupdate.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,3 +343,15 @@ def test_deep_update_illegal_update(self):
dictupdate.update_dict_key_value(
{}, "foo", update_with, ordered_dict=True
)

def test_update_with_merge_lists_and_mapping(self):
"""
Test that updating a list with a mapping raises a TypeError
when merge_lists=True.
"""
mdict = copy.deepcopy(self.dict1)
mdict["A"] = [1, 2]
with self.assertRaises(TypeError):
dictupdate.update(
copy.deepcopy(mdict), {"A": {"key": "value"}}, merge_lists=True, strict=True
)

0 comments on commit 3acaf41

Please sign in to comment.