Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix RecursiveDictDiffer #63213

Merged
merged 9 commits into from
Dec 6, 2022
Merged

Fix RecursiveDictDiffer #63213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d586714
Migrate dictdiffer tests to pytest
lkubb Dec 6, 2022
1ae1142
Add test for issue 59017
lkubb Dec 6, 2022
aafa0e1
Fix RecursiveDictDiffer with added nested dicts
lkubb Dec 6, 2022
f775f69
Add tests for added empty dicts
lkubb Dec 6, 2022
3332fea
Correctly handle type change from/to dict
lkubb Dec 6, 2022
5302697
Refine tests
lkubb Dec 6, 2022
d4e8621
Fix nested with separator
lkubb Dec 6, 2022
dd0eb98
Cleanup
lkubb Dec 6, 2022
1536257
Add changelog
lkubb Dec 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/59017.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed RecursiveDictDiffer with added nested dicts
267 changes: 176 additions & 91 deletions salt/utils/dictdiffer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def _get_diffs(cls, dict1, dict2, ignore_missing_keys):
Simple compares are done on lists
"""
ret_dict = {}
for p in dict1.keys():
for p in dict1:
if p not in dict2:
ret_dict.update({p: {"new": dict1[p], "old": cls.NONE_VALUE}})
elif dict1[p] != dict2[p]:
Expand All @@ -186,8 +186,8 @@ def _get_diffs(cls, dict1, dict2, ignore_missing_keys):
else:
ret_dict.update({p: {"new": dict1[p], "old": dict2[p]}})
if not ignore_missing_keys:
for p in dict2.keys():
if p not in dict1.keys():
for p in dict2:
if p not in dict1:
ret_dict.update({p: {"new": cls.NONE_VALUE, "old": dict2[p]}})
return ret_dict

Expand All @@ -200,8 +200,8 @@ def _get_values(cls, diff_dict, type="new"):
Which values to return, 'new' or 'old'
"""
ret_dict = {}
for p in diff_dict.keys():
if type in diff_dict[p].keys():
for p in diff_dict:
if type in diff_dict[p]:
ret_dict.update({p: diff_dict[p][type]})
else:
ret_dict.update({p: cls._get_values(diff_dict[p], type=type)})
Expand All @@ -215,101 +215,172 @@ def _get_changes(cls, diff_dict):
Each inner difference is tabulated two space deeper
"""
changes_strings = []
for p in sorted(diff_dict.keys()):
if sorted(diff_dict[p].keys()) == ["new", "old"]:
# Some string formatting
old_value = diff_dict[p]["old"]
if diff_dict[p]["old"] == cls.NONE_VALUE:
old_value = "nothing"
elif isinstance(diff_dict[p]["old"], str):
old_value = "'{}'".format(diff_dict[p]["old"])
elif isinstance(diff_dict[p]["old"], list):
old_value = "'{}'".format(", ".join(diff_dict[p]["old"]))
new_value = diff_dict[p]["new"]
if diff_dict[p]["new"] == cls.NONE_VALUE:
new_value = "nothing"
elif isinstance(diff_dict[p]["new"], str):
new_value = "'{}'".format(diff_dict[p]["new"])
elif isinstance(diff_dict[p]["new"], list):
new_value = "'{}'".format(", ".join(diff_dict[p]["new"]))
for p in sorted(diff_dict):
if set(diff_dict[p]) == {"new", "old"}:
changes = {
"old_value": diff_dict[p]["old"],
"new_value": diff_dict[p]["new"],
}
for ref in ("old_value", "new_value"):
val = changes[ref]
# Some string formatting
if val == cls.NONE_VALUE:
changes[ref] = "nothing"
elif isinstance(val, str):
changes[ref] = f"'{val}'"
elif isinstance(val, list):
changes[ref] = f"'{', '.join(val)}'"
changes_strings.append(
"{} from {} to {}".format(p, old_value, new_value)
f"{p} from {changes['old_value']} to {changes['new_value']}"
)
else:
sub_changes = cls._get_changes(diff_dict[p])
if sub_changes:
changes_strings.append("{}:".format(p))
changes_strings.extend([" {}".format(c) for c in sub_changes])
changes_strings.append(f"{p}:")
changes_strings.extend([f" {c}" for c in sub_changes])
return changes_strings

def added(self):
def _it_addrm(
self,
key_a,
key_b,
include_nested=False,
diffs=None,
prefix="",
is_nested=False,
separator=".",
):
keys = []
if diffs is None:
diffs = self.diffs

for key in diffs:
if is_nested:
keys.append(f"{prefix}{key}")

if not isinstance(diffs[key], dict):
continue

if is_nested and include_nested:
keys.extend(
self._it_addrm(
key_a,
key_b,
diffs=diffs[key],
prefix=f"{prefix}{key}{separator}",
is_nested=is_nested,
include_nested=include_nested,
separator=separator,
)
)
elif "old" not in diffs[key]:
keys.extend(
self._it_addrm(
key_a,
key_b,
diffs=diffs[key],
prefix=f"{prefix}{key}{separator}",
is_nested=is_nested,
include_nested=include_nested,
separator=separator,
)
)
elif diffs[key][key_a] == self.NONE_VALUE:
keys.append(f"{prefix}{key}")

if isinstance(diffs[key][key_b], dict) and include_nested:
keys.extend(
self._it_addrm(
key_a,
key_b,
diffs=diffs[key][key_b],
is_nested=True,
prefix=f"{prefix}{key}{separator}",
include_nested=include_nested,
separator=separator,
)
)
# type change from/to dict
elif (
not is_nested
and not isinstance(diffs[key][key_a], dict)
and isinstance(diffs[key][key_b], dict)
):
keys.extend(
self._it_addrm(
key_a,
key_b,
diffs=diffs[key][key_b],
is_nested=True,
prefix=f"{prefix}{key}{separator}",
include_nested=include_nested,
separator=separator,
)
)

return keys

def added(
self, include_nested=False, separator="."
): # pylint: disable=arguments-differ
"""
Returns all keys that have been added.

If the keys are in child dictionaries they will be represented with
. notation
"""
include_nested
If an added key contains a dictionary, include its
keys in dot notation as well. Defaults to false.

def _added(diffs, prefix):
keys = []
for key in diffs.keys():
if isinstance(diffs[key], dict) and "old" not in diffs[key]:
keys.extend(_added(diffs[key], prefix="{}{}.".format(prefix, key)))
elif diffs[key]["old"] == self.NONE_VALUE:
if isinstance(diffs[key]["new"], dict):
keys.extend(
_added(
diffs[key]["new"], prefix="{}{}.".format(prefix, key)
)
)
else:
keys.append("{}{}".format(prefix, key))
return keys
.. versionadded:: 3006

return sorted(_added(self._diffs, prefix=""))
separator
Separator used to indicate nested keys. Defaults to ``.``.

def removed(self):
.. versionadded:: 3006
"""
Returns all keys that have been removed.
return sorted(self._it_addrm("old", "new", include_nested, separator=separator))

If the keys are in child dictionaries they will be represented with
. notation
def removed(
self, include_nested=False, separator="."
): # pylint: disable=arguments-differ
"""
Returns all keys that have been removed.

def _removed(diffs, prefix):
keys = []
for key in diffs.keys():
if isinstance(diffs[key], dict) and "old" not in diffs[key]:
keys.extend(
_removed(diffs[key], prefix="{}{}.".format(prefix, key))
)
elif diffs[key]["new"] == self.NONE_VALUE:
keys.append("{}{}".format(prefix, key))
elif isinstance(diffs[key]["new"], dict):
keys.extend(
_removed(diffs[key]["new"], prefix="{}{}.".format(prefix, key))
)
return keys
include_nested
If an added key contains a dictionary, include its
keys in dot notation as well. Defaults to false.

return sorted(_removed(self._diffs, prefix=""))
.. versionadded:: 3006

def changed(self):
separator
Separator used to indicate nested keys. Defaults to ``.``.

.. versionadded:: 3006
"""
return sorted(self._it_addrm("new", "old", include_nested, separator=separator))

def changed(self, separator="."): # pylint: disable=arguments-differ
"""
Returns all keys that have been changed.

If the keys are in child dictionaries they will be represented with
. notation
separator
Separator used to indicate nested keys. Defaults to ``.``.

.. versionadded:: 3006
"""

def _changed(diffs, prefix):
def _changed(diffs, prefix, separator):
keys = []
for key in diffs.keys():
for key in diffs:
if not isinstance(diffs[key], dict):
continue

if isinstance(diffs[key], dict) and "old" not in diffs[key]:
keys.extend(
_changed(diffs[key], prefix="{}{}.".format(prefix, key))
_changed(
diffs[key],
prefix=f"{prefix}{key}{separator}",
separator=separator,
)
)
continue
if self.ignore_unset_values:
Expand All @@ -323,64 +394,78 @@ def _changed(diffs, prefix):
keys.extend(
_changed(
diffs[key]["new"],
prefix="{}{}.".format(prefix, key),
prefix=f"{prefix}{key}{separator}",
separator=separator,
)
)
else:
keys.append("{}{}".format(prefix, key))
keys.append(f"{prefix}{key}")
elif isinstance(diffs[key], dict):
keys.extend(
_changed(diffs[key], prefix="{}{}.".format(prefix, key))
_changed(
diffs[key],
prefix=f"{prefix}{key}{separator}",
separator=separator,
)
)
else:
if "old" in diffs[key] and "new" in diffs[key]:
if isinstance(diffs[key]["new"], dict):
keys.extend(
_changed(
diffs[key]["new"],
prefix="{}{}.".format(prefix, key),
prefix=f"{prefix}{key}{separator}",
separator=separator,
)
)
else:
keys.append("{}{}".format(prefix, key))
keys.append(f"{prefix}{key}")
elif isinstance(diffs[key], dict):
keys.extend(
_changed(diffs[key], prefix="{}{}.".format(prefix, key))
_changed(
diffs[key],
prefix=f"{prefix}{key}{separator}",
separator=separator,
)
)

return keys

return sorted(_changed(self._diffs, prefix=""))
return sorted(_changed(self._diffs, prefix="", separator=separator))

def unchanged(self):
def unchanged(self, separator="."): # pylint: disable=arguments-differ
"""
Returns all keys that have been unchanged.

If the keys are in child dictionaries they will be represented with
. notation
separator
Separator used to indicate nested keys. Defaults to ``.``.

.. versionadded:: 3006
"""

def _unchanged(current_dict, diffs, prefix):
def _unchanged(current_dict, diffs, prefix, separator):
keys = []
for key in current_dict.keys():
for key in current_dict:
if key not in diffs:
keys.append("{}{}".format(prefix, key))
keys.append(f"{prefix}{key}")
elif isinstance(current_dict[key], dict):
if "new" in diffs[key]:
# There is a diff
continue
else:
keys.extend(
_unchanged(
current_dict[key],
diffs[key],
prefix="{}{}.".format(prefix, key),
)
keys.extend(
_unchanged(
current_dict[key],
diffs[key],
prefix=f"{prefix}{key}{separator}",
separator=separator,
)
)

return keys

return sorted(_unchanged(self.current_dict, self._diffs, prefix=""))
return sorted(
_unchanged(self.current_dict, self._diffs, prefix="", separator=separator)
)

@property
def diffs(self):
Expand Down
Loading