From d32b8435dbc2a6aab0d5ebb365575dd96f08e4b1 Mon Sep 17 00:00:00 2001 From: Herbert Buurman Date: Thu, 4 Jul 2019 12:52:27 +0200 Subject: [PATCH 1/8] salt/utils/data.py: Added recursive diff that can diff any mapping or iterable recursively. Also added unittests. --- salt/utils/data.py | 129 ++++++++++++- tests/unit/utils/test_data.py | 352 +++++++++++++++++++++++++++++++++- 2 files changed, 478 insertions(+), 3 deletions(-) diff --git a/salt/utils/data.py b/salt/utils/data.py index 3a47cb28905f..635c2b2d9ed4 100644 --- a/salt/utils/data.py +++ b/salt/utils/data.py @@ -28,6 +28,7 @@ from salt.utils.odict import OrderedDict # Import 3rd-party libs +from salt.ext.six.moves import zip # pylint: disable=redefined-builtin from salt.ext import six from salt.ext.six.moves import range # pylint: disable=redefined-builtin @@ -1016,12 +1017,22 @@ def json_query(data, expr): return jmespath.search(expr, data) +def is_iterable(data): + ''' + Helper function to determine if something is an iterable. + Strings are manually excluded. + ''' + return hasattr(data, '__iter__') and not isinstance(data, six.string_types) + + def _is_not_considered_falsey(value, ignore_types=()): ''' Helper function for filter_falsey to determine if something is not to be considered falsey. + :param any value: The value to consider :param list ignore_types: The types to ignore when considering the value. + :return bool ''' return isinstance(value, bool) or type(value) in ignore_types or value @@ -1032,12 +1043,14 @@ def filter_falsey(data, recurse_depth=None, ignore_types=()): Helper function to remove items from an iterable with falsey value. Removes ``None``, ``{}`` and ``[]``, 0, '' (but does not remove ``False``). Recurses into sub-iterables if ``recurse`` is set to ``True``. + :param dict/list data: Source iterable (dict, OrderedDict, list, set, ...) to process. :param int recurse_depth: Recurse this many levels into values that are dicts or lists to also process those. Default: 0 (do not recurse) :param list ignore_types: Contains types that can be falsey but must not be filtered. Default: Only booleans are not filtered. :return type(data) + .. version-added:: Neon ''' filter_element = ( @@ -1054,10 +1067,124 @@ def filter_falsey(data, recurse_depth=None, ignore_types=()): for key, value in processed_elements if _is_not_considered_falsey(value, ignore_types=ignore_types) ]) - if hasattr(data, '__iter__') and not isinstance(data, six.string_types): + elif is_iterable(data): processed_elements = (filter_element(value) for value in data) return type(data)([ value for value in processed_elements if _is_not_considered_falsey(value, ignore_types=ignore_types) ]) return data + + +def recursive_diff(old, new, ignore=None, unordered_lists=False): + ''' + Performs a recursive diff on mappings and/or iterables and returns the result + in a {'old': values, 'new': values}-style. + Compares dicts and sets unordered (obviously), OrderedDicts and Lists ordered + (but only if both ``old`` and ``new`` are of the same type), + all other Mapping types unordered, and all other iterables ordered. + + :param mapping/iterable old: Mapping or Iterable to compare from. + :param mapping/iterable new: Mapping or Iterable to compare to. + :param list ignore: List of keys to ignore when comparing Mappings. + :param bool unordered_lists: Compare lists like sets. + + :return dict: Returns dict with keys 'old' and 'new' containing the differences. + ''' + ignore = ignore or [] + res = {} + ret_old = copy.deepcopy(old) + ret_new = copy.deepcopy(new) + print('Comparing {} with {}'.format(old, new)) + if isinstance(old, OrderedDict) and isinstance(new, OrderedDict): + append_old, append_new = [], [] + if len(old) != len(new): + min_length = min(len(old), len(new)) + # The list coercion is required for Py3 + append_old = list(old.keys())[min_length:] + append_new = list(new.keys())[min_length:] + # Compare ordered + for (key_old, key_new) in zip(old, new): + if key_old == key_new: + if key_old in ignore: + del ret_old[key_old] + del ret_new[key_new] + else: + res = recursive_diff( + old[key_old], + new[key_new], + ignore=ignore, + unordered_lists=unordered_lists) + if not res: # Equal + del ret_old[key_old] + del ret_new[key_new] + else: + ret_old[key_old] = res['old'] + ret_new[key_new] = res['new'] + else: + if key_old in ignore: + del ret_old[key_old] + if key_new in ignore: + del ret_new[key_new] + # If the OrderedDicts were of inequal length, add the remaining key/values. + for item in append_old: + ret_old[item] = old[item] + for item in append_new: + ret_new[item] = new[item] + ret = {'old': ret_old, 'new': ret_new} if ret_old or ret_new else {} + elif isinstance(old, Mapping) and isinstance(new, Mapping): + # Compare unordered + for key in set(list(old) + list(new)): + if key in ignore: + ret_old.pop(key, None) + ret_new.pop(key, None) + elif key in old and key in new: + res = recursive_diff( + old[key], + new[key], + ignore=ignore, + unordered_lists=unordered_lists) + if not res: # Equal + del ret_old[key] + del ret_new[key] + else: + ret_old[key] = res['old'] + ret_new[key] = res['new'] + ret = {'old': ret_old, 'new': ret_new} if ret_old or ret_new else {} + elif isinstance(old, set) and isinstance(new, set): + ret = {'old': old - new, 'new': new - old} if old - new or new - old else {} + elif is_iterable(old) and is_iterable(new): + # Create a list so we can edit on an index-basis. + list_old = list(ret_old) + list_new = list(ret_new) + if unordered_lists: + for item_old in old: + for item_new in new: + res = recursive_diff(item_old, item_new, ignore=ignore, unordered_lists=unordered_lists) + if not res: + list_old.remove(item_old) + list_new.remove(item_new) + continue + else: + remove_indices = [] + for index, (iter_old, iter_new) in enumerate(zip(old, new)): + res = recursive_diff( + iter_old, + iter_new, + ignore=ignore, + unordered_lists=unordered_lists) + if not res: # Equal + remove_indices.append(index) + else: + list_old[index] = res['old'] + list_new[index] = res['new'] + for index in reversed(remove_indices): + list_old.pop(index) + list_new.pop(index) + # Instantiate a new whatever-it-was using the list as iterable source. + # This may not be the most optimized in way of speed and memory usage, + # but it will work for all iterable types. + ret = {'old': type(old)(list_old), 'new': type(new)(list_new)} if list_old or list_new else {} + else: + ret = {} if old == new else {'old': ret_old, 'new': ret_new} + return ret diff --git a/tests/unit/utils/test_data.py b/tests/unit/utils/test_data.py index d0e4cb1e5f50..5ce687f1b778 100644 --- a/tests/unit/utils/test_data.py +++ b/tests/unit/utils/test_data.py @@ -6,13 +6,15 @@ # Import Python libs from __future__ import absolute_import, print_function, unicode_literals import logging +from tests.support.unit import TestCase, LOREM_IPSUM +from tests.support.mock import patch # Import Salt libs import salt.utils.data import salt.utils.stringutils from salt.utils.odict import OrderedDict -from tests.support.unit import TestCase, LOREM_IPSUM -from tests.support.mock import patch + +# Import 3rd party libs from salt.ext.six.moves import builtins # pylint: disable=import-error,redefined-builtin from salt.ext import six @@ -875,3 +877,349 @@ def test_filter_exclude_types(self): old_list = ['foo', ['foo'], ['foo', None], {'foo': 0}, {'foo': 'bar', 'baz': []}, [{'foo': ''}]] new_list = salt.utils.data.filter_falsey(old_list, recurse_depth=3, ignore_types=[type(None)]) self.assertEqual(['foo', ['foo'], ['foo', None], {'foo': 'bar'}], new_list) + + +class FilterRecursiveDiff(TestCase): + ''' + Test suite for salt.utils.data.recursive_diff + ''' + + def test_list_equality(self): + ''' + Test cases where equal lists are compared. + ''' + test_list = [0, 1, 2] + self.assertEqual({}, salt.utils.data.recursive_diff(test_list, test_list)) + + test_list = [[0], [1], [0, 1, 2]] + self.assertEqual({}, salt.utils.data.recursive_diff(test_list, test_list)) + + def test_dict_equality(self): + ''' + Test cases where equal dicts are compared. + ''' + test_dict = {'foo': 'bar', 'bar': {'baz': {'qux': 'quux'}}, 'frop': 0} + self.assertEqual({}, salt.utils.data.recursive_diff(test_dict, test_dict)) + + def test_ordereddict_equality(self): + ''' + Test cases where equal OrderedDicts are compared. + ''' + test_dict = OrderedDict([ + ('foo', 'bar'), + ('bar', OrderedDict([('baz', OrderedDict([('qux', 'quux')]))])), + ('frop', 0)]) + self.assertEqual({}, salt.utils.data.recursive_diff(test_dict, test_dict)) + + def test_mixed_equality(self): + ''' + Test cases where mixed nested lists and dicts are compared. + ''' + test_data = { + 'foo': 'bar', + 'baz': [0, 1, 2], + 'bar': {'baz': [{'qux': 'quux'}, {'froop', 0}]} + } + self.assertEqual({}, salt.utils.data.recursive_diff(test_data, test_data)) + + def test_set_equality(self): + ''' + Test cases where equal sets are compared. + ''' + test_set = set([0, 1, 2, 3, 'foo']) + self.assertEqual({}, salt.utils.data.recursive_diff(test_set, test_set)) + + # This is a bit of an oddity, as python seems to sort the sets in memory + # so both sets end up with the same ordering (0..3). + set_one = set([0, 1, 2, 3]) + set_two = set([3, 2, 1, 0]) + self.assertEqual({}, salt.utils.data.recursive_diff(set_one, set_two)) + + def test_tuple_equality(self): + ''' + Test cases where equal tuples are compared. + ''' + test_tuple = (0, 1, 2, 3, 'foo') + self.assertEqual({}, salt.utils.data.recursive_diff(test_tuple, test_tuple)) + + def test_list_inequality(self): + ''' + Test cases where two inequal lists are compared. + ''' + list_one = [0, 1, 2] + list_two = ['foo', 'bar', 'baz'] + expected_result = {'old': list_one, 'new': list_two} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(list_one, list_two)) + expected_result = {'new': list_one, 'old': list_two} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(list_two, list_one)) + + list_one = [0, 'foo', 1, 'bar'] + list_two = [1, 'foo', 1, 'qux'] + expected_result = {'old': [0, 'bar'], 'new': [1, 'qux']} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(list_one, list_two)) + expected_result = {'new': [0, 'bar'], 'old': [1, 'qux']} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(list_two, list_one)) + + list_one = [0, 1, [2, 3]] + list_two = [0, 1, ['foo', 'bar']] + expected_result = {'old': [[2, 3]], 'new': [['foo', 'bar']]} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(list_one, list_two)) + expected_result = {'new': [[2, 3]], 'old': [['foo', 'bar']]} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(list_two, list_one)) + + def test_dict_inequality(self): + ''' + Test cases where two inequal dicts are compared. + ''' + dict_one = {'foo': 1, 'bar': 2, 'baz': 3} + dict_two = {'foo': 2, 1: 'bar', 'baz': 3} + expected_result = {'old': {'foo': 1, 'bar': 2}, 'new': {'foo': 2, 1: 'bar'}} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(dict_one, dict_two)) + expected_result = {'new': {'foo': 1, 'bar': 2}, 'old': {'foo': 2, 1: 'bar'}} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(dict_two, dict_one)) + + dict_one = {'foo': {'bar': {'baz': 1}}} + dict_two = {'foo': {'qux': {'baz': 1}}} + expected_result = {'old': dict_one, 'new': dict_two} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(dict_one, dict_two)) + expected_result = {'new': dict_one, 'old': dict_two} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(dict_two, dict_one)) + + def test_ordereddict_inequality(self): + ''' + Test cases where two inequal OrderedDicts are compared. + ''' + odict_one = OrderedDict([('foo', 'bar'), ('bar', 'baz')]) + odict_two = OrderedDict([('bar', 'baz'), ('foo', 'bar')]) + expected_result = {'old': odict_one, 'new': odict_two} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(odict_one, odict_two)) + + def test_set_inequality(self): + ''' + Test cases where two inequal sets are compared. + Tricky as the sets are compared zipped, so shuffled sets of equal values + are considered different. + ''' + set_one = set([0, 1, 2, 4]) + set_two = set([0, 1, 3, 4]) + expected_result = {'old': set([2]), 'new': set([3])} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(set_one, set_two)) + expected_result = {'new': set([2]), 'old': set([3])} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(set_two, set_one)) + + # It is unknown how different python versions will store sets in memory. + # Python 2.7 seems to sort it (i.e. set_one below becomes {0, 1, 'foo', 'bar'} + # However Python 3.6.8 stores it differently each run. + # So just test for "not equal" here. + set_one = set([0, 'foo', 1, 'bar']) + set_two = set(['foo', 1, 'bar', 2]) + expected_result = {} + self.assertNotEqual(expected_result, salt.utils.data.recursive_diff(set_one, set_two)) + + def test_mixed_inequality(self): + ''' + Test cases where two mixed dicts/iterables that are different are compared. + ''' + dict_one = {'foo': [1, 2, 3]} + dict_two = {'foo': [3, 2, 1]} + expected_result = {'old': {'foo': [1, 3]}, 'new': {'foo': [3, 1]}} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(dict_one, dict_two)) + expected_result = {'new': {'foo': [1, 3]}, 'old': {'foo': [3, 1]}} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(dict_two, dict_one)) + + list_one = [1, 2, {'foo': ['bar', {'foo': 1, 'bar': 2}]}] + list_two = [3, 4, {'foo': ['qux', {'foo': 1, 'bar': 2}]}] + expected_result = {'old': [1, 2, {'foo': ['bar']}], 'new': [3, 4, {'foo': ['qux']}]} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(list_one, list_two)) + expected_result = {'new': [1, 2, {'foo': ['bar']}], 'old': [3, 4, {'foo': ['qux']}]} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(list_two, list_one)) + + mixed_one = {'foo': set([0, 1, 2]), 'bar': [0, 1, 2]} + mixed_two = {'foo': set([1, 2, 3]), 'bar': [1, 2, 3]} + expected_result = { + 'old': {'foo': set([0]), 'bar': [0, 1, 2]}, + 'new': {'foo': set([3]), 'bar': [1, 2, 3]} + } + self.assertEqual(expected_result, salt.utils.data.recursive_diff(mixed_one, mixed_two)) + expected_result = { + 'new': {'foo': set([0]), 'bar': [0, 1, 2]}, + 'old': {'foo': set([3]), 'bar': [1, 2, 3]} + } + self.assertEqual(expected_result, salt.utils.data.recursive_diff(mixed_two, mixed_one)) + + def test_tuple_inequality(self): + ''' + Test cases where two tuples that are different are compared. + ''' + tuple_one = (1, 2, 3) + tuple_two = (3, 2, 1) + expected_result = {'old': (1, 3), 'new': (3, 1)} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(tuple_one, tuple_two)) + + def test_list_vs_set(self): + ''' + Test case comparing a list with a set, will be compared unordered. + ''' + mixed_one = [1, 2, 3] + mixed_two = set([3, 2, 1]) + expected_result = {} + self.assertEqual(expected_result, salt.utils.data.recursive_diff(mixed_one, mixed_two)) + self.assertEqual(expected_result, salt.utils.data.recursive_diff(mixed_two, mixed_one)) + + def test_dict_vs_ordereddict(self): + ''' + Test case comparing a dict with an ordereddict, will be compared unordered. + ''' + test_dict = {'foo': 'bar', 'bar': 'baz'} + test_odict = OrderedDict([('foo', 'bar'), ('bar', 'baz')]) + self.assertEqual({}, salt.utils.data.recursive_diff(test_dict, test_odict)) + self.assertEqual({}, salt.utils.data.recursive_diff(test_odict, test_dict)) + + test_odict2 = OrderedDict([('bar', 'baz'), ('foo', 'bar')]) + self.assertEqual({}, salt.utils.data.recursive_diff(test_dict, test_odict2)) + self.assertEqual({}, salt.utils.data.recursive_diff(test_odict2, test_dict)) + + def test_list_ignore_ignored(self): + ''' + Test case comparing two lists with ignore-list supplied (which is not used + when comparing lists). + ''' + list_one = [1, 2, 3] + list_two = [3, 2, 1] + expected_result = {'old': [1, 3], 'new': [3, 1]} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(list_one, list_two, ignore=[1, 3]) + ) + + def test_dict_ignore(self): + ''' + Test case comparing two dicts with ignore-list supplied. + ''' + dict_one = {'foo': 1, 'bar': 2, 'baz': 3} + dict_two = {'foo': 3, 'bar': 2, 'baz': 1} + expected_result = {'old': {'baz': 3}, 'new': {'baz': 1}} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(dict_one, dict_two, ignore=['foo']) + ) + + def test_ordereddict_ignore(self): + ''' + Test case comparing two OrderedDicts with ignore-list supplied. + ''' + odict_one = OrderedDict([('foo', 1), ('bar', 2), ('baz', 3)]) + odict_two = OrderedDict([('baz', 1), ('bar', 2), ('foo', 3)]) + # The key 'foo' will be ignored, which means the key from the other OrderedDict + # will always be considered "different" since OrderedDicts are compared ordered. + expected_result = {'old': OrderedDict([('baz', 3)]), 'new': OrderedDict([('baz', 1)])} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(odict_one, odict_two, ignore=['foo']) + ) + + def test_dict_vs_ordereddict_ignore(self): + ''' + Test case comparing a dict with an OrderedDict with ignore-list supplied. + ''' + dict_one = {'foo': 1, 'bar': 2, 'baz': 3} + odict_two = OrderedDict([('foo', 3), ('bar', 2), ('baz', 1)]) + expected_result = {'old': {'baz': 3}, 'new': OrderedDict([('baz', 1)])} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(dict_one, odict_two, ignore=['foo']) + ) + + def test_mixed_nested_ignore(self): + ''' + Test case comparing mixed, nested items with ignore-list supplied. + ''' + dict_one = {'foo': [1], 'bar': {'foo': 1, 'bar': 2}, 'baz': 3} + dict_two = {'foo': [2], 'bar': {'foo': 3, 'bar': 2}, 'baz': 1} + expected_result = {'old': {'baz': 3}, 'new': {'baz': 1}} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(dict_one, dict_two, ignore=['foo']) + ) + + def test_ordered_dict_unequal_length(self): + ''' + Test case comparing two OrderedDicts of unequal length. + ''' + odict_one = OrderedDict([('foo', 1), ('bar', 2), ('baz', 3)]) + odict_two = OrderedDict([('foo', 1), ('bar', 2)]) + expected_result = {'old': OrderedDict([('baz', 3)]), 'new': {}} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(odict_one, odict_two) + ) + + def test_list_unequal_length(self): + ''' + Test case comparing two lists of unequal length. + ''' + list_one = [1, 2, 3] + list_two = [1, 2, 3, 4] + expected_result = {'old': [], 'new': [4]} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(list_one, list_two) + ) + + def test_set_unequal_length(self): + ''' + Test case comparing two sets of unequal length. + This does not do anything special, as it is unordered. + ''' + set_one = set([1, 2, 3]) + set_two = set([4, 3, 2, 1]) + expected_result = {'old': set([]), 'new': set([4])} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(set_one, set_two) + ) + + def test_tuple_unequal_length(self): + ''' + Test case comparing two tuples of unequal length. + This should be the same as comparing two ordered lists. + ''' + tuple_one = (1, 2, 3) + tuple_two = (1, 2, 3, 4) + expected_result = {'old': (), 'new': (4,)} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(tuple_one, tuple_two) + ) + + def test_list_unordered(self): + ''' + Test case comparing two lists unordered. + ''' + list_one = [1, 2, 3, 4] + list_two = [4, 3, 2] + expected_result = {'old': [1], 'new': []} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(list_one, list_two, unordered_lists=True) + ) + + def test_mixed_nested_unordered(self): + ''' + Test case comparing nested dicts/lists unordered. + ''' + dict_one = {'foo': {'bar': [1, 2, 3]}, 'bar': [{'foo': 4}, 0]} + dict_two = {'foo': {'bar': [3, 2, 1]}, 'bar': [0, {'foo': 4}]} + expected_result = {} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(dict_one, dict_two, unordered_lists=True) + ) + expected_result = { + 'old': {'foo': {'bar': [1, 3]}, 'bar': [{'foo': 4}, 0]}, + 'new': {'foo': {'bar': [3, 1]}, 'bar': [0, {'foo': 4}]}, + } + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(dict_one, dict_two) + ) From 269cb34ebaf4360db0543e4c69b3f4c539a85074 Mon Sep 17 00:00:00 2001 From: Herbert Buurman Date: Thu, 4 Jul 2019 14:00:10 +0200 Subject: [PATCH 2/8] salt/utils/data.py: Remove debug print. --- salt/utils/data.py | 1 - 1 file changed, 1 deletion(-) diff --git a/salt/utils/data.py b/salt/utils/data.py index 635c2b2d9ed4..bfd34f38634d 100644 --- a/salt/utils/data.py +++ b/salt/utils/data.py @@ -1095,7 +1095,6 @@ def recursive_diff(old, new, ignore=None, unordered_lists=False): res = {} ret_old = copy.deepcopy(old) ret_new = copy.deepcopy(new) - print('Comparing {} with {}'.format(old, new)) if isinstance(old, OrderedDict) and isinstance(new, OrderedDict): append_old, append_new = [], [] if len(old) != len(new): From f49354efa281eb95b98b35a5c9928d7e21bec6ca Mon Sep 17 00:00:00 2001 From: Herbert Buurman Date: Thu, 4 Jul 2019 15:38:47 +0200 Subject: [PATCH 3/8] salt/utils/data.py: Updated docstring for recursive_diff. --- salt/utils/data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/utils/data.py b/salt/utils/data.py index bfd34f38634d..4b055684ccbb 100644 --- a/salt/utils/data.py +++ b/salt/utils/data.py @@ -1087,7 +1087,7 @@ def recursive_diff(old, new, ignore=None, unordered_lists=False): :param mapping/iterable old: Mapping or Iterable to compare from. :param mapping/iterable new: Mapping or Iterable to compare to. :param list ignore: List of keys to ignore when comparing Mappings. - :param bool unordered_lists: Compare lists like sets. + :param bool unordered_lists: Compare lists without regard for order. :return dict: Returns dict with keys 'old' and 'new' containing the differences. ''' From bea90a301820a714991be395618ca75152b055fd Mon Sep 17 00:00:00 2001 From: Herbert Buurman Date: Thu, 4 Jul 2019 16:18:01 +0200 Subject: [PATCH 4/8] Added ignore_missing_keys parameter to recursive_diff. This will make it more compatible with the existing dictdiffer. Also added tests for the new parameter. --- salt/utils/data.py | 19 ++++++++++++---- tests/unit/utils/test_data.py | 41 +++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/salt/utils/data.py b/salt/utils/data.py index 4b055684ccbb..03c505495147 100644 --- a/salt/utils/data.py +++ b/salt/utils/data.py @@ -1076,7 +1076,7 @@ def filter_falsey(data, recurse_depth=None, ignore_types=()): return data -def recursive_diff(old, new, ignore=None, unordered_lists=False): +def recursive_diff(old, new, ignore=None, unordered_lists=False, ignore_missing_keys=False): ''' Performs a recursive diff on mappings and/or iterables and returns the result in a {'old': values, 'new': values}-style. @@ -1088,6 +1088,8 @@ def recursive_diff(old, new, ignore=None, unordered_lists=False): :param mapping/iterable new: Mapping or Iterable to compare to. :param list ignore: List of keys to ignore when comparing Mappings. :param bool unordered_lists: Compare lists without regard for order. + :param bool ignore_missing_keys: Do not return keys only present in ``old`` + but missing in ``new``. Only works for regular dicts. :return dict: Returns dict with keys 'old' and 'new' containing the differences. ''' @@ -1113,7 +1115,8 @@ def recursive_diff(old, new, ignore=None, unordered_lists=False): old[key_old], new[key_new], ignore=ignore, - unordered_lists=unordered_lists) + unordered_lists=unordered_lists, + ignore_missing_keys=ignore_missing_keys) if not res: # Equal del ret_old[key_old] del ret_new[key_new] @@ -1137,12 +1140,15 @@ def recursive_diff(old, new, ignore=None, unordered_lists=False): if key in ignore: ret_old.pop(key, None) ret_new.pop(key, None) + elif ignore_missing_keys and key in old and key not in new: + del ret_old[key] elif key in old and key in new: res = recursive_diff( old[key], new[key], ignore=ignore, - unordered_lists=unordered_lists) + unordered_lists=unordered_lists, + ignore_missing_keys=ignore_missing_keys) if not res: # Equal del ret_old[key] del ret_new[key] @@ -1159,7 +1165,12 @@ def recursive_diff(old, new, ignore=None, unordered_lists=False): if unordered_lists: for item_old in old: for item_new in new: - res = recursive_diff(item_old, item_new, ignore=ignore, unordered_lists=unordered_lists) + res = recursive_diff( + item_old, + item_new, + ignore=ignore, + unordered_lists=unordered_lists, + ignore_missing_keys=ignore_missing_keys) if not res: list_old.remove(item_old) list_new.remove(item_new) diff --git a/tests/unit/utils/test_data.py b/tests/unit/utils/test_data.py index 5ce687f1b778..de7798fffb5c 100644 --- a/tests/unit/utils/test_data.py +++ b/tests/unit/utils/test_data.py @@ -1223,3 +1223,44 @@ def test_mixed_nested_unordered(self): expected_result, salt.utils.data.recursive_diff(dict_one, dict_two) ) + + def test_ignore_missing_keys_dict(self): + ''' + Test case ignoring missing keys on a comparison of dicts. + ''' + dict_one = {'foo': 1, 'bar': 2, 'baz': 3} + dict_two = {'bar': 3} + expected_result = {'old': {'bar': 2}, 'new': {'bar': 3}} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(dict_one, dict_two, ignore_missing_keys=True) + ) + + def test_ignore_missing_keys_ordered_dict(self): + ''' + Test case not ignoring missing keys on a comparison of OrderedDicts. + ''' + odict_one = OrderedDict([('foo', 1), ('bar', 2), ('baz', 3)]) + odict_two = OrderedDict([('bar', 3)]) + expected_result = {'old': odict_one, 'new': odict_two} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(odict_one, odict_two, ignore_missing_keys=True) + ) + + def test_ignore_missing_keys_recursive(self): + ''' + Test case ignoring missing keys on a comparison of nested dicts. + ''' + dict_one = {'foo': {'bar': 2, 'baz': 3}} + dict_two = {'foo': {'baz': 3}} + expected_result = {} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(dict_one, dict_two, ignore_missing_keys=True) + ) + dit_two = {} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(dict_one, dict_two, ignore_missing_keys=True) + ) From a277e874f2dff80e30ac7f0f7692d98824efeba5 Mon Sep 17 00:00:00 2001 From: Herbert Buurman Date: Thu, 4 Jul 2019 17:07:25 +0200 Subject: [PATCH 5/8] Renamed ignore_* parameters for recursive_diff to be more clear. Updated and added unittests to match. --- salt/utils/data.py | 42 ++++++++++++++++++++--------------- tests/unit/utils/test_data.py | 36 +++++++++++++++++++++++------- 2 files changed, 52 insertions(+), 26 deletions(-) diff --git a/salt/utils/data.py b/salt/utils/data.py index 03c505495147..db429d72ed53 100644 --- a/salt/utils/data.py +++ b/salt/utils/data.py @@ -1076,7 +1076,12 @@ def filter_falsey(data, recurse_depth=None, ignore_types=()): return data -def recursive_diff(old, new, ignore=None, unordered_lists=False, ignore_missing_keys=False): +def recursive_diff( + old, + new, + ignore_keys=None, + ignore_order=False, + ignore_missing_keys=False): ''' Performs a recursive diff on mappings and/or iterables and returns the result in a {'old': values, 'new': values}-style. @@ -1086,18 +1091,18 @@ def recursive_diff(old, new, ignore=None, unordered_lists=False, ignore_missing_ :param mapping/iterable old: Mapping or Iterable to compare from. :param mapping/iterable new: Mapping or Iterable to compare to. - :param list ignore: List of keys to ignore when comparing Mappings. - :param bool unordered_lists: Compare lists without regard for order. + :param list ignore_keys: List of keys to ignore when comparing Mappings. + :param bool ignore_order: Compare ordered mapping/iterables as if they were unordered. :param bool ignore_missing_keys: Do not return keys only present in ``old`` but missing in ``new``. Only works for regular dicts. :return dict: Returns dict with keys 'old' and 'new' containing the differences. ''' - ignore = ignore or [] + ignore_keys = ignore_keys or [] res = {} ret_old = copy.deepcopy(old) ret_new = copy.deepcopy(new) - if isinstance(old, OrderedDict) and isinstance(new, OrderedDict): + if isinstance(old, OrderedDict) and isinstance(new, OrderedDict) and not ignore_order: append_old, append_new = [], [] if len(old) != len(new): min_length = min(len(old), len(new)) @@ -1107,15 +1112,15 @@ def recursive_diff(old, new, ignore=None, unordered_lists=False, ignore_missing_ # Compare ordered for (key_old, key_new) in zip(old, new): if key_old == key_new: - if key_old in ignore: + if key_old in ignore_keys: del ret_old[key_old] del ret_new[key_new] else: res = recursive_diff( old[key_old], new[key_new], - ignore=ignore, - unordered_lists=unordered_lists, + ignore_keys=ignore_keys, + ignore_order=ignore_order, ignore_missing_keys=ignore_missing_keys) if not res: # Equal del ret_old[key_old] @@ -1124,9 +1129,9 @@ def recursive_diff(old, new, ignore=None, unordered_lists=False, ignore_missing_ ret_old[key_old] = res['old'] ret_new[key_new] = res['new'] else: - if key_old in ignore: + if key_old in ignore_keys: del ret_old[key_old] - if key_new in ignore: + if key_new in ignore_keys: del ret_new[key_new] # If the OrderedDicts were of inequal length, add the remaining key/values. for item in append_old: @@ -1137,7 +1142,7 @@ def recursive_diff(old, new, ignore=None, unordered_lists=False, ignore_missing_ elif isinstance(old, Mapping) and isinstance(new, Mapping): # Compare unordered for key in set(list(old) + list(new)): - if key in ignore: + if key in ignore_keys: ret_old.pop(key, None) ret_new.pop(key, None) elif ignore_missing_keys and key in old and key not in new: @@ -1146,8 +1151,8 @@ def recursive_diff(old, new, ignore=None, unordered_lists=False, ignore_missing_ res = recursive_diff( old[key], new[key], - ignore=ignore, - unordered_lists=unordered_lists, + ignore_keys=ignore_keys, + ignore_order=ignore_order, ignore_missing_keys=ignore_missing_keys) if not res: # Equal del ret_old[key] @@ -1162,14 +1167,14 @@ def recursive_diff(old, new, ignore=None, unordered_lists=False, ignore_missing_ # Create a list so we can edit on an index-basis. list_old = list(ret_old) list_new = list(ret_new) - if unordered_lists: + if ignore_order: for item_old in old: for item_new in new: res = recursive_diff( item_old, item_new, - ignore=ignore, - unordered_lists=unordered_lists, + ignore_keys=ignore_keys, + ignore_order=ignore_order, ignore_missing_keys=ignore_missing_keys) if not res: list_old.remove(item_old) @@ -1181,8 +1186,9 @@ def recursive_diff(old, new, ignore=None, unordered_lists=False, ignore_missing_ res = recursive_diff( iter_old, iter_new, - ignore=ignore, - unordered_lists=unordered_lists) + ignore_keys=ignore_keys, + ignore_order=ignore_order, + ignore_missing_keys=ignore_missing_keys) if not res: # Equal remove_indices.append(index) else: diff --git a/tests/unit/utils/test_data.py b/tests/unit/utils/test_data.py index de7798fffb5c..35ec7c019d82 100644 --- a/tests/unit/utils/test_data.py +++ b/tests/unit/utils/test_data.py @@ -1089,7 +1089,7 @@ def test_list_ignore_ignored(self): expected_result = {'old': [1, 3], 'new': [3, 1]} self.assertEqual( expected_result, - salt.utils.data.recursive_diff(list_one, list_two, ignore=[1, 3]) + salt.utils.data.recursive_diff(list_one, list_two, ignore_keys=[1, 3]) ) def test_dict_ignore(self): @@ -1101,7 +1101,7 @@ def test_dict_ignore(self): expected_result = {'old': {'baz': 3}, 'new': {'baz': 1}} self.assertEqual( expected_result, - salt.utils.data.recursive_diff(dict_one, dict_two, ignore=['foo']) + salt.utils.data.recursive_diff(dict_one, dict_two, ignore_keys=['foo']) ) def test_ordereddict_ignore(self): @@ -1115,7 +1115,7 @@ def test_ordereddict_ignore(self): expected_result = {'old': OrderedDict([('baz', 3)]), 'new': OrderedDict([('baz', 1)])} self.assertEqual( expected_result, - salt.utils.data.recursive_diff(odict_one, odict_two, ignore=['foo']) + salt.utils.data.recursive_diff(odict_one, odict_two, ignore_keys=['foo']) ) def test_dict_vs_ordereddict_ignore(self): @@ -1127,7 +1127,7 @@ def test_dict_vs_ordereddict_ignore(self): expected_result = {'old': {'baz': 3}, 'new': OrderedDict([('baz', 1)])} self.assertEqual( expected_result, - salt.utils.data.recursive_diff(dict_one, odict_two, ignore=['foo']) + salt.utils.data.recursive_diff(dict_one, odict_two, ignore_keys=['foo']) ) def test_mixed_nested_ignore(self): @@ -1139,7 +1139,7 @@ def test_mixed_nested_ignore(self): expected_result = {'old': {'baz': 3}, 'new': {'baz': 1}} self.assertEqual( expected_result, - salt.utils.data.recursive_diff(dict_one, dict_two, ignore=['foo']) + salt.utils.data.recursive_diff(dict_one, dict_two, ignore_keys=['foo']) ) def test_ordered_dict_unequal_length(self): @@ -1201,7 +1201,7 @@ def test_list_unordered(self): expected_result = {'old': [1], 'new': []} self.assertEqual( expected_result, - salt.utils.data.recursive_diff(list_one, list_two, unordered_lists=True) + salt.utils.data.recursive_diff(list_one, list_two, ignore_order=True) ) def test_mixed_nested_unordered(self): @@ -1213,7 +1213,7 @@ def test_mixed_nested_unordered(self): expected_result = {} self.assertEqual( expected_result, - salt.utils.data.recursive_diff(dict_one, dict_two, unordered_lists=True) + salt.utils.data.recursive_diff(dict_one, dict_two, ignore_order=True) ) expected_result = { 'old': {'foo': {'bar': [1, 3]}, 'bar': [{'foo': 4}, 0]}, @@ -1224,6 +1224,18 @@ def test_mixed_nested_unordered(self): salt.utils.data.recursive_diff(dict_one, dict_two) ) + def test_ordered_dict_unordered(self): + ''' + Test case comparing OrderedDicts unordered. + ''' + odict_one = OrderedDict([('foo', 1), ('bar', 2), ('baz', 3)]) + odict_two = OrderedDict([('baz', 3), ('bar', 2), ('foo', 1)]) + expected_result = {} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(odict_one, odict_two, ignore_order=True) + ) + def test_ignore_missing_keys_dict(self): ''' Test case ignoring missing keys on a comparison of dicts. @@ -1259,7 +1271,15 @@ def test_ignore_missing_keys_recursive(self): expected_result, salt.utils.data.recursive_diff(dict_one, dict_two, ignore_missing_keys=True) ) - dit_two = {} + # Compare from dict-in-dict + dict_two = {} + self.assertEqual( + expected_result, + salt.utils.data.recursive_diff(dict_one, dict_two, ignore_missing_keys=True) + ) + # Compare from dict-in-list + dict_one = {'foo': ['bar', {'baz': 3}]} + dict_two = {'foo': ['bar', {}]} self.assertEqual( expected_result, salt.utils.data.recursive_diff(dict_one, dict_two, ignore_missing_keys=True) From 29faab6f3996e6a559458b5fe38cecd35bddf843 Mon Sep 17 00:00:00 2001 From: Herbert Buurman Date: Tue, 24 Dec 2019 16:26:11 +0100 Subject: [PATCH 6/8] Remove (failing) json_query test Also remove FilterFalseyTestCase class as this is part of another PR --- tests/unit/utils/test_data.py | 248 ---------------------------------- 1 file changed, 248 deletions(-) diff --git a/tests/unit/utils/test_data.py b/tests/unit/utils/test_data.py index 35ec7c019d82..4f5cf1ad7f33 100644 --- a/tests/unit/utils/test_data.py +++ b/tests/unit/utils/test_data.py @@ -630,254 +630,6 @@ def test_stringify(self): ['one', 'two', 'three', '4', '5'] ) - def test_json_query(self): - # Raises exception if jmespath module is not found - with patch('salt.utils.data.jmespath', None): - self.assertRaisesRegex( - RuntimeError, 'requires jmespath', - salt.utils.data.json_query, {}, '@' - ) - - # Test search - user_groups = { - 'user1': {'groups': ['group1', 'group2', 'group3']}, - 'user2': {'groups': ['group1', 'group2']}, - 'user3': {'groups': ['group3']}, - } - expression = '*.groups[0]' - primary_groups = ['group1', 'group1', 'group3'] - self.assertEqual( - sorted(salt.utils.data.json_query(user_groups, expression)), - primary_groups - ) - - -class FilterFalseyTestCase(TestCase): - ''' - Test suite for salt.utils.data.filter_falsey - ''' - - def test_nop(self): - ''' - Test cases where nothing will be done. - ''' - # Test with dictionary without recursion - old_dict = {'foo': 'bar', 'bar': {'baz': {'qux': 'quux'}}, 'baz': ['qux', {'foo': 'bar'}]} - new_dict = salt.utils.data.filter_falsey(old_dict) - self.assertEqual(old_dict, new_dict) - # Check returned type equality - self.assertIs(type(old_dict), type(new_dict)) - # Test dictionary with recursion - new_dict = salt.utils.data.filter_falsey(old_dict, recurse_depth=3) - self.assertEqual(old_dict, new_dict) - # Test with list - old_list = ['foo', 'bar'] - new_list = salt.utils.data.filter_falsey(old_list) - self.assertEqual(old_list, new_list) - # Check returned type equality - self.assertIs(type(old_list), type(new_list)) - # Test with set - old_set = set(['foo', 'bar']) - new_set = salt.utils.data.filter_falsey(old_set) - self.assertEqual(old_set, new_set) - # Check returned type equality - self.assertIs(type(old_set), type(new_set)) - # Test with OrderedDict - old_dict = OrderedDict([ - ('foo', 'bar'), - ('bar', OrderedDict([('qux', 'quux')])), - ('baz', ['qux', OrderedDict([('foo', 'bar')])]) - ]) - new_dict = salt.utils.data.filter_falsey(old_dict) - self.assertEqual(old_dict, new_dict) - self.assertIs(type(old_dict), type(new_dict)) - # Test excluding int - old_list = [0] - new_list = salt.utils.data.filter_falsey(old_list, ignore_types=[type(0)]) - self.assertEqual(old_list, new_list) - # Test excluding str (or unicode) (or both) - old_list = [''] - new_list = salt.utils.data.filter_falsey(old_list, ignore_types=[type('')]) - self.assertEqual(old_list, new_list) - # Test excluding list - old_list = [[]] - new_list = salt.utils.data.filter_falsey(old_list, ignore_types=[type([])]) - self.assertEqual(old_list, new_list) - # Test excluding dict - old_list = [{}] - new_list = salt.utils.data.filter_falsey(old_list, ignore_types=[type({})]) - self.assertEqual(old_list, new_list) - - def test_filter_dict_no_recurse(self): - ''' - Test filtering a dictionary without recursing. - This will only filter out key-values where the values are falsey. - ''' - old_dict = {'foo': None, - 'bar': {'baz': {'qux': None, 'quux': '', 'foo': []}}, - 'baz': ['qux'], - 'qux': {}, - 'quux': []} - new_dict = salt.utils.data.filter_falsey(old_dict) - expect_dict = {'bar': {'baz': {'qux': None, 'quux': '', 'foo': []}}, 'baz': ['qux']} - self.assertEqual(expect_dict, new_dict) - self.assertIs(type(expect_dict), type(new_dict)) - - def test_filter_dict_recurse(self): - ''' - Test filtering a dictionary with recursing. - This will filter out any key-values where the values are falsey or when - the values *become* falsey after filtering their contents (in case they - are lists or dicts). - ''' - old_dict = {'foo': None, - 'bar': {'baz': {'qux': None, 'quux': '', 'foo': []}}, - 'baz': ['qux'], - 'qux': {}, - 'quux': []} - new_dict = salt.utils.data.filter_falsey(old_dict, recurse_depth=3) - expect_dict = {'baz': ['qux']} - self.assertEqual(expect_dict, new_dict) - self.assertIs(type(expect_dict), type(new_dict)) - - def test_filter_list_no_recurse(self): - ''' - Test filtering a list without recursing. - This will only filter out items which are falsey. - ''' - old_list = ['foo', None, [], {}, 0, ''] - new_list = salt.utils.data.filter_falsey(old_list) - expect_list = ['foo'] - self.assertEqual(expect_list, new_list) - self.assertIs(type(expect_list), type(new_list)) - # Ensure nested values are *not* filtered out. - old_list = [ - 'foo', - ['foo'], - ['foo', None], - {'foo': 0}, - {'foo': 'bar', 'baz': []}, - [{'foo': ''}], - ] - new_list = salt.utils.data.filter_falsey(old_list) - self.assertEqual(old_list, new_list) - self.assertIs(type(old_list), type(new_list)) - - def test_filter_list_recurse(self): - ''' - Test filtering a list with recursing. - This will filter out any items which are falsey, or which become falsey - after filtering their contents (in case they are lists or dicts). - ''' - old_list = [ - 'foo', - ['foo'], - ['foo', None], - {'foo': 0}, - {'foo': 'bar', 'baz': []}, - [{'foo': ''}] - ] - new_list = salt.utils.data.filter_falsey(old_list, recurse_depth=3) - expect_list = ['foo', ['foo'], ['foo'], {'foo': 'bar'}] - self.assertEqual(expect_list, new_list) - self.assertIs(type(expect_list), type(new_list)) - - def test_filter_set_no_recurse(self): - ''' - Test filtering a set without recursing. - Note that a set cannot contain unhashable types, so recursion is not possible. - ''' - old_set = set([ - 'foo', - None, - 0, - '', - ]) - new_set = salt.utils.data.filter_falsey(old_set) - expect_set = set(['foo']) - self.assertEqual(expect_set, new_set) - self.assertIs(type(expect_set), type(new_set)) - - def test_filter_ordereddict_no_recurse(self): - ''' - Test filtering an OrderedDict without recursing. - ''' - old_dict = OrderedDict([ - ('foo', None), - ('bar', OrderedDict([('baz', OrderedDict([('qux', None), ('quux', ''), ('foo', [])]))])), - ('baz', ['qux']), - ('qux', {}), - ('quux', []) - ]) - new_dict = salt.utils.data.filter_falsey(old_dict) - expect_dict = OrderedDict([ - ('bar', OrderedDict([('baz', OrderedDict([('qux', None), ('quux', ''), ('foo', [])]))])), - ('baz', ['qux']), - ]) - self.assertEqual(expect_dict, new_dict) - self.assertIs(type(expect_dict), type(new_dict)) - - def test_filter_ordereddict_recurse(self): - ''' - Test filtering an OrderedDict with recursing. - ''' - old_dict = OrderedDict([ - ('foo', None), - ('bar', OrderedDict([('baz', OrderedDict([('qux', None), ('quux', ''), ('foo', [])]))])), - ('baz', ['qux']), - ('qux', {}), - ('quux', []) - ]) - new_dict = salt.utils.data.filter_falsey(old_dict, recurse_depth=3) - expect_dict = OrderedDict([ - ('baz', ['qux']), - ]) - self.assertEqual(expect_dict, new_dict) - self.assertIs(type(expect_dict), type(new_dict)) - - def test_filter_list_recurse_limit(self): - ''' - Test filtering a list with recursing, but with a limited depth. - Note that the top-level is always processed, so a recursion depth of 2 - means that two *additional* levels are processed. - ''' - old_list = [None, [None, [None, [None]]]] - new_list = salt.utils.data.filter_falsey(old_list, recurse_depth=2) - self.assertEqual([[[[None]]]], new_list) - - def test_filter_dict_recurse_limit(self): - ''' - Test filtering a dict with recursing, but with a limited depth. - Note that the top-level is always processed, so a recursion depth of 2 - means that two *additional* levels are processed. - ''' - old_dict = {'one': None, - 'foo': {'two': None, 'bar': {'three': None, 'baz': {'four': None}}}} - new_dict = salt.utils.data.filter_falsey(old_dict, recurse_depth=2) - self.assertEqual({'foo': {'bar': {'baz': {'four': None}}}}, new_dict) - - def test_filter_exclude_types(self): - ''' - Test filtering a list recursively, but also ignoring (i.e. not filtering) - out certain types that can be falsey. - ''' - # Ignore int, unicode - old_list = ['foo', ['foo'], ['foo', None], {'foo': 0}, {'foo': 'bar', 'baz': []}, [{'foo': ''}]] - new_list = salt.utils.data.filter_falsey(old_list, recurse_depth=3, ignore_types=[type(0), type('')]) - self.assertEqual(['foo', ['foo'], ['foo'], {'foo': 0}, {'foo': 'bar'}, [{'foo': ''}]], new_list) - # Ignore list - old_list = ['foo', ['foo'], ['foo', None], {'foo': 0}, {'foo': 'bar', 'baz': []}, [{'foo': ''}]] - new_list = salt.utils.data.filter_falsey(old_list, recurse_depth=3, ignore_types=[type([])]) - self.assertEqual(['foo', ['foo'], ['foo'], {'foo': 'bar', 'baz': []}, []], new_list) - # Ignore dict - old_list = ['foo', ['foo'], ['foo', None], {'foo': 0}, {'foo': 'bar', 'baz': []}, [{'foo': ''}]] - new_list = salt.utils.data.filter_falsey(old_list, recurse_depth=3, ignore_types=[type({})]) - self.assertEqual(['foo', ['foo'], ['foo'], {}, {'foo': 'bar'}, [{}]], new_list) - # Ignore NoneType - old_list = ['foo', ['foo'], ['foo', None], {'foo': 0}, {'foo': 'bar', 'baz': []}, [{'foo': ''}]] - new_list = salt.utils.data.filter_falsey(old_list, recurse_depth=3, ignore_types=[type(None)]) - self.assertEqual(['foo', ['foo'], ['foo', None], {'foo': 'bar'}], new_list) - class FilterRecursiveDiff(TestCase): ''' From 7005cf769b83a5ff3038a8013b47795cfc2c5453 Mon Sep 17 00:00:00 2001 From: Herbert Buurman Date: Mon, 30 Dec 2019 11:50:04 +0100 Subject: [PATCH 7/8] Restore missing tests that got dropped during rebasing --- tests/unit/utils/test_data.py | 252 +++++++++++++++++++++++++++++++++- 1 file changed, 250 insertions(+), 2 deletions(-) diff --git a/tests/unit/utils/test_data.py b/tests/unit/utils/test_data.py index 4f5cf1ad7f33..8fa352321cff 100644 --- a/tests/unit/utils/test_data.py +++ b/tests/unit/utils/test_data.py @@ -6,13 +6,13 @@ # Import Python libs from __future__ import absolute_import, print_function, unicode_literals import logging -from tests.support.unit import TestCase, LOREM_IPSUM -from tests.support.mock import patch # Import Salt libs import salt.utils.data import salt.utils.stringutils from salt.utils.odict import OrderedDict +from tests.support.unit import TestCase, LOREM_IPSUM +from tests.support.mock import patch # Import 3rd party libs from salt.ext.six.moves import builtins # pylint: disable=import-error,redefined-builtin @@ -630,6 +630,254 @@ def test_stringify(self): ['one', 'two', 'three', '4', '5'] ) + def test_json_query(self): + # Raises exception if jmespath module is not found + with patch('salt.utils.data.jmespath', None): + self.assertRaisesRegex( + RuntimeError, 'requires jmespath', + salt.utils.data.json_query, {}, '@' + ) + + # Test search + user_groups = { + 'user1': {'groups': ['group1', 'group2', 'group3']}, + 'user2': {'groups': ['group1', 'group2']}, + 'user3': {'groups': ['group3']}, + } + expression = '*.groups[0]' + primary_groups = ['group1', 'group1', 'group3'] + self.assertEqual( + sorted(salt.utils.data.json_query(user_groups, expression)), + primary_groups + ) + + +class FilterFalseyTestCase(TestCase): + ''' + Test suite for salt.utils.data.filter_falsey + ''' + + def test_nop(self): + ''' + Test cases where nothing will be done. + ''' + # Test with dictionary without recursion + old_dict = {'foo': 'bar', 'bar': {'baz': {'qux': 'quux'}}, 'baz': ['qux', {'foo': 'bar'}]} + new_dict = salt.utils.data.filter_falsey(old_dict) + self.assertEqual(old_dict, new_dict) + # Check returned type equality + self.assertIs(type(old_dict), type(new_dict)) + # Test dictionary with recursion + new_dict = salt.utils.data.filter_falsey(old_dict, recurse_depth=3) + self.assertEqual(old_dict, new_dict) + # Test with list + old_list = ['foo', 'bar'] + new_list = salt.utils.data.filter_falsey(old_list) + self.assertEqual(old_list, new_list) + # Check returned type equality + self.assertIs(type(old_list), type(new_list)) + # Test with set + old_set = set(['foo', 'bar']) + new_set = salt.utils.data.filter_falsey(old_set) + self.assertEqual(old_set, new_set) + # Check returned type equality + self.assertIs(type(old_set), type(new_set)) + # Test with OrderedDict + old_dict = OrderedDict([ + ('foo', 'bar'), + ('bar', OrderedDict([('qux', 'quux')])), + ('baz', ['qux', OrderedDict([('foo', 'bar')])]) + ]) + new_dict = salt.utils.data.filter_falsey(old_dict) + self.assertEqual(old_dict, new_dict) + self.assertIs(type(old_dict), type(new_dict)) + # Test excluding int + old_list = [0] + new_list = salt.utils.data.filter_falsey(old_list, ignore_types=[type(0)]) + self.assertEqual(old_list, new_list) + # Test excluding str (or unicode) (or both) + old_list = [''] + new_list = salt.utils.data.filter_falsey(old_list, ignore_types=[type('')]) + self.assertEqual(old_list, new_list) + # Test excluding list + old_list = [[]] + new_list = salt.utils.data.filter_falsey(old_list, ignore_types=[type([])]) + self.assertEqual(old_list, new_list) + # Test excluding dict + old_list = [{}] + new_list = salt.utils.data.filter_falsey(old_list, ignore_types=[type({})]) + self.assertEqual(old_list, new_list) + + def test_filter_dict_no_recurse(self): + ''' + Test filtering a dictionary without recursing. + This will only filter out key-values where the values are falsey. + ''' + old_dict = {'foo': None, + 'bar': {'baz': {'qux': None, 'quux': '', 'foo': []}}, + 'baz': ['qux'], + 'qux': {}, + 'quux': []} + new_dict = salt.utils.data.filter_falsey(old_dict) + expect_dict = {'bar': {'baz': {'qux': None, 'quux': '', 'foo': []}}, 'baz': ['qux']} + self.assertEqual(expect_dict, new_dict) + self.assertIs(type(expect_dict), type(new_dict)) + + def test_filter_dict_recurse(self): + ''' + Test filtering a dictionary with recursing. + This will filter out any key-values where the values are falsey or when + the values *become* falsey after filtering their contents (in case they + are lists or dicts). + ''' + old_dict = {'foo': None, + 'bar': {'baz': {'qux': None, 'quux': '', 'foo': []}}, + 'baz': ['qux'], + 'qux': {}, + 'quux': []} + new_dict = salt.utils.data.filter_falsey(old_dict, recurse_depth=3) + expect_dict = {'baz': ['qux']} + self.assertEqual(expect_dict, new_dict) + self.assertIs(type(expect_dict), type(new_dict)) + + def test_filter_list_no_recurse(self): + ''' + Test filtering a list without recursing. + This will only filter out items which are falsey. + ''' + old_list = ['foo', None, [], {}, 0, ''] + new_list = salt.utils.data.filter_falsey(old_list) + expect_list = ['foo'] + self.assertEqual(expect_list, new_list) + self.assertIs(type(expect_list), type(new_list)) + # Ensure nested values are *not* filtered out. + old_list = [ + 'foo', + ['foo'], + ['foo', None], + {'foo': 0}, + {'foo': 'bar', 'baz': []}, + [{'foo': ''}], + ] + new_list = salt.utils.data.filter_falsey(old_list) + self.assertEqual(old_list, new_list) + self.assertIs(type(old_list), type(new_list)) + + def test_filter_list_recurse(self): + ''' + Test filtering a list with recursing. + This will filter out any items which are falsey, or which become falsey + after filtering their contents (in case they are lists or dicts). + ''' + old_list = [ + 'foo', + ['foo'], + ['foo', None], + {'foo': 0}, + {'foo': 'bar', 'baz': []}, + [{'foo': ''}] + ] + new_list = salt.utils.data.filter_falsey(old_list, recurse_depth=3) + expect_list = ['foo', ['foo'], ['foo'], {'foo': 'bar'}] + self.assertEqual(expect_list, new_list) + self.assertIs(type(expect_list), type(new_list)) + + def test_filter_set_no_recurse(self): + ''' + Test filtering a set without recursing. + Note that a set cannot contain unhashable types, so recursion is not possible. + ''' + old_set = set([ + 'foo', + None, + 0, + '', + ]) + new_set = salt.utils.data.filter_falsey(old_set) + expect_set = set(['foo']) + self.assertEqual(expect_set, new_set) + self.assertIs(type(expect_set), type(new_set)) + + def test_filter_ordereddict_no_recurse(self): + ''' + Test filtering an OrderedDict without recursing. + ''' + old_dict = OrderedDict([ + ('foo', None), + ('bar', OrderedDict([('baz', OrderedDict([('qux', None), ('quux', ''), ('foo', [])]))])), + ('baz', ['qux']), + ('qux', {}), + ('quux', []) + ]) + new_dict = salt.utils.data.filter_falsey(old_dict) + expect_dict = OrderedDict([ + ('bar', OrderedDict([('baz', OrderedDict([('qux', None), ('quux', ''), ('foo', [])]))])), + ('baz', ['qux']), + ]) + self.assertEqual(expect_dict, new_dict) + self.assertIs(type(expect_dict), type(new_dict)) + + def test_filter_ordereddict_recurse(self): + ''' + Test filtering an OrderedDict with recursing. + ''' + old_dict = OrderedDict([ + ('foo', None), + ('bar', OrderedDict([('baz', OrderedDict([('qux', None), ('quux', ''), ('foo', [])]))])), + ('baz', ['qux']), + ('qux', {}), + ('quux', []) + ]) + new_dict = salt.utils.data.filter_falsey(old_dict, recurse_depth=3) + expect_dict = OrderedDict([ + ('baz', ['qux']), + ]) + self.assertEqual(expect_dict, new_dict) + self.assertIs(type(expect_dict), type(new_dict)) + + def test_filter_list_recurse_limit(self): + ''' + Test filtering a list with recursing, but with a limited depth. + Note that the top-level is always processed, so a recursion depth of 2 + means that two *additional* levels are processed. + ''' + old_list = [None, [None, [None, [None]]]] + new_list = salt.utils.data.filter_falsey(old_list, recurse_depth=2) + self.assertEqual([[[[None]]]], new_list) + + def test_filter_dict_recurse_limit(self): + ''' + Test filtering a dict with recursing, but with a limited depth. + Note that the top-level is always processed, so a recursion depth of 2 + means that two *additional* levels are processed. + ''' + old_dict = {'one': None, + 'foo': {'two': None, 'bar': {'three': None, 'baz': {'four': None}}}} + new_dict = salt.utils.data.filter_falsey(old_dict, recurse_depth=2) + self.assertEqual({'foo': {'bar': {'baz': {'four': None}}}}, new_dict) + + def test_filter_exclude_types(self): + ''' + Test filtering a list recursively, but also ignoring (i.e. not filtering) + out certain types that can be falsey. + ''' + # Ignore int, unicode + old_list = ['foo', ['foo'], ['foo', None], {'foo': 0}, {'foo': 'bar', 'baz': []}, [{'foo': ''}]] + new_list = salt.utils.data.filter_falsey(old_list, recurse_depth=3, ignore_types=[type(0), type('')]) + self.assertEqual(['foo', ['foo'], ['foo'], {'foo': 0}, {'foo': 'bar'}, [{'foo': ''}]], new_list) + # Ignore list + old_list = ['foo', ['foo'], ['foo', None], {'foo': 0}, {'foo': 'bar', 'baz': []}, [{'foo': ''}]] + new_list = salt.utils.data.filter_falsey(old_list, recurse_depth=3, ignore_types=[type([])]) + self.assertEqual(['foo', ['foo'], ['foo'], {'foo': 'bar', 'baz': []}, []], new_list) + # Ignore dict + old_list = ['foo', ['foo'], ['foo', None], {'foo': 0}, {'foo': 'bar', 'baz': []}, [{'foo': ''}]] + new_list = salt.utils.data.filter_falsey(old_list, recurse_depth=3, ignore_types=[type({})]) + self.assertEqual(['foo', ['foo'], ['foo'], {}, {'foo': 'bar'}, [{}]], new_list) + # Ignore NoneType + old_list = ['foo', ['foo'], ['foo', None], {'foo': 0}, {'foo': 'bar', 'baz': []}, [{'foo': ''}]] + new_list = salt.utils.data.filter_falsey(old_list, recurse_depth=3, ignore_types=[type(None)]) + self.assertEqual(['foo', ['foo'], ['foo', None], {'foo': 'bar'}], new_list) + class FilterRecursiveDiff(TestCase): ''' From a88400b628d3cf6c75d1fe7e2e591002ad4fb18d Mon Sep 17 00:00:00 2001 From: Herbert Buurman Date: Mon, 30 Dec 2019 12:33:09 +0100 Subject: [PATCH 8/8] Apply pylint-inspired changes: Remove is_iterable as it is already implemented in is_iter; Replaced all elif after return with if; Remove else after return or continue; Added function docstrings where they were missing. --- salt/utils/data.py | 161 ++++++++++++++++++++++----------------------- 1 file changed, 77 insertions(+), 84 deletions(-) diff --git a/salt/utils/data.py b/salt/utils/data.py index db429d72ed53..74ba2472f686 100644 --- a/salt/utils/data.py +++ b/salt/utils/data.py @@ -93,6 +93,11 @@ def copy(self): def __change_case(data, attr, preserve_dict_class=False): + ''' + Calls data.attr() if data has an attribute/method called attr. + Processes data recursively if data is a Mapping or Sequence. + For Mapping, processes both keys and values. + ''' try: return getattr(data, attr)() except AttributeError: @@ -106,18 +111,23 @@ def __change_case(data, attr, preserve_dict_class=False): __change_case(val, attr, preserve_dict_class)) for key, val in six.iteritems(data) ) - elif isinstance(data, Sequence): + if isinstance(data, Sequence): return data_type( __change_case(item, attr, preserve_dict_class) for item in data) - else: - return data + return data def to_lowercase(data, preserve_dict_class=False): + ''' + Recursively changes everything in data to lowercase. + ''' return __change_case(data, 'lower', preserve_dict_class) def to_uppercase(data, preserve_dict_class=False): + ''' + Recursively changes everything in data to uppercase. + ''' return __change_case(data, 'upper', preserve_dict_class) @@ -196,27 +206,26 @@ def decode(data, encoding=None, errors='strict', keep=False, if isinstance(data, Mapping): return decode_dict(data, encoding, errors, keep, normalize, preserve_dict_class, preserve_tuples, to_str) - elif isinstance(data, list): + if isinstance(data, list): return decode_list(data, encoding, errors, keep, normalize, preserve_dict_class, preserve_tuples, to_str) - elif isinstance(data, tuple): + if isinstance(data, tuple): return decode_tuple(data, encoding, errors, keep, normalize, preserve_dict_class, to_str) \ if preserve_tuples \ else decode_list(data, encoding, errors, keep, normalize, preserve_dict_class, preserve_tuples, to_str) - else: - try: - data = _decode_func(data, encoding, errors, normalize) - except TypeError: - # to_unicode raises a TypeError when input is not a - # string/bytestring/bytearray. This is expected and simply means we - # are going to leave the value as-is. - pass - except UnicodeDecodeError: - if not keep: - raise - return data + try: + data = _decode_func(data, encoding, errors, normalize) + except TypeError: + # to_unicode raises a TypeError when input is not a + # string/bytestring/bytearray. This is expected and simply means we + # are going to leave the value as-is. + pass + except UnicodeDecodeError: + if not keep: + raise + return data def decode_dict(data, encoding=None, errors='strict', keep=False, @@ -230,7 +239,7 @@ def decode_dict(data, encoding=None, errors='strict', keep=False, if not to_str \ else salt.utils.stringutils.to_str # Make sure we preserve OrderedDicts - rv = data.__class__() if preserve_dict_class else {} + ret = data.__class__() if preserve_dict_class else {} for key, value in six.iteritems(data): if isinstance(key, tuple): key = decode_tuple(key, encoding, errors, keep, normalize, @@ -274,8 +283,8 @@ def decode_dict(data, encoding=None, errors='strict', keep=False, if not keep: raise - rv[key] = value - return rv + ret[key] = value + return ret def decode_list(data, encoding=None, errors='strict', keep=False, @@ -288,7 +297,7 @@ def decode_list(data, encoding=None, errors='strict', keep=False, _decode_func = salt.utils.stringutils.to_unicode \ if not to_str \ else salt.utils.stringutils.to_str - rv = [] + ret = [] for item in data: if isinstance(item, list): item = decode_list(item, encoding, errors, keep, normalize, @@ -314,8 +323,8 @@ def decode_list(data, encoding=None, errors='strict', keep=False, if not keep: raise - rv.append(item) - return rv + ret.append(item) + return ret def decode_tuple(data, encoding=None, errors='strict', keep=False, @@ -344,26 +353,25 @@ def encode(data, encoding=None, errors='strict', keep=False, if isinstance(data, Mapping): return encode_dict(data, encoding, errors, keep, preserve_dict_class, preserve_tuples) - elif isinstance(data, list): + if isinstance(data, list): return encode_list(data, encoding, errors, keep, preserve_dict_class, preserve_tuples) - elif isinstance(data, tuple): + if isinstance(data, tuple): return encode_tuple(data, encoding, errors, keep, preserve_dict_class) \ if preserve_tuples \ else encode_list(data, encoding, errors, keep, preserve_dict_class, preserve_tuples) - else: - try: - return salt.utils.stringutils.to_bytes(data, encoding, errors) - except TypeError: - # to_bytes raises a TypeError when input is not a - # string/bytestring/bytearray. This is expected and simply - # means we are going to leave the value as-is. - pass - except UnicodeEncodeError: - if not keep: - raise - return data + try: + return salt.utils.stringutils.to_bytes(data, encoding, errors) + except TypeError: + # to_bytes raises a TypeError when input is not a + # string/bytestring/bytearray. This is expected and simply + # means we are going to leave the value as-is. + pass + except UnicodeEncodeError: + if not keep: + raise + return data @jinja_filter('json_decode_dict') # Remove this for Aluminium @@ -373,7 +381,7 @@ def encode_dict(data, encoding=None, errors='strict', keep=False, ''' Encode all string values to bytes ''' - rv = data.__class__() if preserve_dict_class else {} + ret = data.__class__() if preserve_dict_class else {} for key, value in six.iteritems(data): if isinstance(key, tuple): key = encode_tuple(key, encoding, errors, keep, preserve_dict_class) \ @@ -415,8 +423,8 @@ def encode_dict(data, encoding=None, errors='strict', keep=False, if not keep: raise - rv[key] = value - return rv + ret[key] = value + return ret @jinja_filter('json_decode_list') # Remove this for Aluminium @@ -426,7 +434,7 @@ def encode_list(data, encoding=None, errors='strict', keep=False, ''' Encode all string values to bytes ''' - rv = [] + ret = [] for item in data: if isinstance(item, list): item = encode_list(item, encoding, errors, keep, @@ -451,8 +459,8 @@ def encode_list(data, encoding=None, errors='strict', keep=False, if not keep: raise - rv.append(item) - return rv + ret.append(item) + return ret def encode_tuple(data, encoding=None, errors='strict', keep=False, @@ -465,21 +473,21 @@ def encode_tuple(data, encoding=None, errors='strict', keep=False, @jinja_filter('exactly_n_true') -def exactly_n(l, n=1): +def exactly_n(iterable, amount=1): ''' Tests that exactly N items in an iterable are "truthy" (neither None, False, nor 0). ''' - i = iter(l) - return all(any(i) for j in range(n)) and not any(i) + i = iter(iterable) + return all(any(i) for j in range(amount)) and not any(i) @jinja_filter('exactly_one_true') -def exactly_one(l): +def exactly_one(iterable): ''' Check if only one item is not None, False, or 0 in an iterable. ''' - return exactly_n(l) + return exactly_n(iterable) def filter_by(lookup_dict, @@ -639,22 +647,23 @@ def _match(target, pattern, regex_match=False, exact_match=False): else fnmatch.fnmatch(target, pattern) def _dict_match(target, pattern, regex_match=False, exact_match=False): + ret = False wildcard = pattern.startswith('*:') if wildcard: pattern = pattern[2:] if pattern == '*': # We are just checking that the key exists - return True - elif pattern in target: + ret = True + if not ret and pattern in target: # We might want to search for a key - return True - elif subdict_match(target, - pattern, - regex_match=regex_match, - exact_match=exact_match): - return True - if wildcard: + ret = True + if not ret and subdict_match(target, + pattern, + regex_match=regex_match, + exact_match=exact_match): + ret = True + if not ret and wildcard: for key in target: if isinstance(target[key], dict): if _dict_match(target[key], @@ -674,13 +683,7 @@ def _dict_match(target, pattern, regex_match=False, exact_match=False): regex_match=regex_match, exact_match=exact_match): return True - return False - - splits = expr.split(delimiter) - num_splits = len(splits) - if num_splits == 1: - # Delimiter not present, this can't possibly be a match - return False + return ret splits = expr.split(delimiter) num_splits = len(splits) @@ -785,7 +788,7 @@ def repack_dictlist(data, for element in data: if isinstance(element, valid_non_dict): continue - elif isinstance(element, dict): + if isinstance(element, dict): if len(element) != 1: log.error( 'Invalid input for repack_dictlist: key/value pairs ' @@ -838,7 +841,7 @@ def is_list(value): @jinja_filter('is_iter') -def is_iter(y, ignore=six.string_types): +def is_iter(thing, ignore=six.string_types): ''' Test if an object is iterable, but not a string type. @@ -851,11 +854,10 @@ def is_iter(y, ignore=six.string_types): Based on https://bitbucket.org/petershinners/yter ''' - - if ignore and isinstance(y, ignore): + if ignore and isinstance(thing, ignore): return False try: - iter(y) + iter(thing) return True except TypeError: return False @@ -898,10 +900,9 @@ def is_true(value=None): # Now check for truthiness if isinstance(value, (six.integer_types, float)): return value > 0 - elif isinstance(value, six.string_types): + if isinstance(value, six.string_types): return six.text_type(value).lower() == 'true' - else: - return bool(value) + return bool(value) @jinja_filter('mysql_to_dict') @@ -925,8 +926,7 @@ def mysql_to_dict(data, key): for field in range(index): if field < 1: continue - else: - row[headers[field]] = salt.utils.stringutils.to_num(comps[field]) + row[headers[field]] = salt.utils.stringutils.to_num(comps[field]) ret[row[key]] = row else: headers = comps @@ -1017,14 +1017,6 @@ def json_query(data, expr): return jmespath.search(expr, data) -def is_iterable(data): - ''' - Helper function to determine if something is an iterable. - Strings are manually excluded. - ''' - return hasattr(data, '__iter__') and not isinstance(data, six.string_types) - - def _is_not_considered_falsey(value, ignore_types=()): ''' Helper function for filter_falsey to determine if something is not to be @@ -1049,6 +1041,7 @@ def filter_falsey(data, recurse_depth=None, ignore_types=()): or lists to also process those. Default: 0 (do not recurse) :param list ignore_types: Contains types that can be falsey but must not be filtered. Default: Only booleans are not filtered. + :return type(data) .. version-added:: Neon @@ -1067,7 +1060,7 @@ def filter_falsey(data, recurse_depth=None, ignore_types=()): for key, value in processed_elements if _is_not_considered_falsey(value, ignore_types=ignore_types) ]) - elif is_iterable(data): + if is_iter(data): processed_elements = (filter_element(value) for value in data) return type(data)([ value for value in processed_elements @@ -1163,7 +1156,7 @@ def recursive_diff( ret = {'old': ret_old, 'new': ret_new} if ret_old or ret_new else {} elif isinstance(old, set) and isinstance(new, set): ret = {'old': old - new, 'new': new - old} if old - new or new - old else {} - elif is_iterable(old) and is_iterable(new): + elif is_iter(old) and is_iter(new): # Create a list so we can edit on an index-basis. list_old = list(ret_old) list_new = list(ret_new)