diff --git a/lib/dictutils.py b/lib/dictutils.py index f54d2070d2..4cc7acc8a2 100644 --- a/lib/dictutils.py +++ b/lib/dictutils.py @@ -50,33 +50,43 @@ def append_if(array, item): return False -def recursive_list_removal(inventory, purge_list): +def recursive_list_removal(base_list, purge_list): """Remove items from a list. - :param inventory: ``dict`` Dictionary representing the inventory + If base_list and purge_list resolve to the same list in memory, + only the first item will be removed due to Python's handling of mutation + during iteration. + + :param base_list: ``list`` List representing base_list entries :param purge_list: ``list`` List of items to remove """ for item in purge_list: - for _item in inventory: + for _item in base_list: if item == _item: - inventory.pop(inventory.index(item)) + base_list.pop(base_list.index(item)) +# TODO(nrb): this probably needs to be renamed, as it's not really recursive def recursive_dict_removal(inventory, purge_list): """Remove items from a dictionary. - Keyword arguments: + Only items in child dictionaries and lists are removed. + + Dictionary keys can only be deleted at the 3rd level (e.g in + inventory['top']['middle']['bottom'], only 'bottom' would be targetted by + this function) + :param inventory: ``dict`` Dictionary representing the inventory :param purge_list: ``list`` List of items to remove """ for key, value in inventory.iteritems(): if isinstance(value, dict): - for _key, _value in value.iteritems(): - if isinstance(_value, dict): + for child_key, child_value in value.iteritems(): + if isinstance(child_value, dict): for item in purge_list: - if item in _value: - del(_value[item]) - elif isinstance(_value, list): - recursive_list_removal(_value, purge_list) + if item in child_value: + del(child_value[item]) + elif isinstance(child_value, list): + recursive_list_removal(child_value, purge_list) elif isinstance(value, list): recursive_list_removal(value, purge_list) diff --git a/tests/test_dictutils.py b/tests/test_dictutils.py new file mode 100644 index 0000000000..c6c970460a --- /dev/null +++ b/tests/test_dictutils.py @@ -0,0 +1,155 @@ +#!/usr/bin/env python + +import os +from os import path +import sys +import unittest + +LIB_DIR = 'lib' + +sys.path.append(path.join(os.getcwd(), LIB_DIR)) + +import dictutils as du + + +class TestMergeDictUnit(unittest.TestCase): + def test_merging_dict(self): + base = {'key1': 'value1'} + target = {'key2': 'value2'} + + new = du.merge_dict(base, target) + + self.assertIn('key2', new.keys()) + self.assertEqual('value2', new['key2']) + + def test_base_dict_is_modified(self): + base = {'key1': 'value1'} + target = {'key2': 'value2'} + + new = du.merge_dict(base, target) + + self.assertIs(base, new) + + def test_merging_nested_dicts(self): + base = {'key1': 'value1'} + target = {'key2': {'key2.1': 'value2'}} + + new = du.merge_dict(base, target) + + self.assertIn('key2', new.keys()) + self.assertIn('key2.1', new['key2'].keys()) + + +class TestAppendIfUnit(unittest.TestCase): + def test_appending_not_present(self): + base = ['foo', 'bar'] + target = 'baz' + + retval = du.append_if(base, target) + + self.assertIn(target, base) + self.assertTrue(retval) + + def test_appending_present(self): + base = ['foo', 'bar', 'baz'] + target = 'baz' + + retval = du.append_if(base, target) + + self.assertFalse(retval) + + +class TestListRemovalUnit(unittest.TestCase): + def setUp(self): + # Can't just use a member list, since it remains changed after each + # test + self.base = ['foo', 'bar'] + + def test_removing_one_target(self): + target = ['bar'] + + du.recursive_list_removal(self.base, target) + + self.assertNotIn('bar', self.base) + + def test_removing_entire_list(self): + # Use a copy so we're not hitting the exact same object in memory. + target = list(self.base) + + du.recursive_list_removal(self.base, target) + + self.assertEqual(0, len(self.base)) + + def test_using_base_as_target(self): + target = self.base + + du.recursive_list_removal(self.base, target) + + self.assertEqual(1, len(self.base)) + self.assertEqual(1, len(target)) + self.assertIn('bar', self.base) + + def test_using_bare_string(self): + target = 'foo' + + du.recursive_list_removal(self.base, target) + + self.assertEqual(2, len(self.base)) + + +class TestDictRemovalUnit(unittest.TestCase): + def test_deleting_single_item_in_single_level_noop(self): + """The funtion only operates on nested dictionaries""" + base = {'key1': 'value1'} + target = ['value1'] + + du.recursive_dict_removal(base, target) + + self.assertEqual('value1', base['key1']) + + def test_deleting_single_item(self): + base = {'key1': {'key1.1': 'value1'}} + target = ['value1'] + + du.recursive_dict_removal(base, target) + + self.assertEqual('value1', base['key1']['key1.1']) + + def test_deleting_single_item_from_list(self): + base = {'key1': {'key1.1': ['value1']}} + target = ['value1'] + + du.recursive_dict_removal(base, target) + + self.assertEqual(0, len(base['key1']['key1.1'])) + self.assertNotIn('value1', base['key1']['key1.1']) + + def test_deleting_single_item_from_nested_list(self): + """The function only operates on the 2nd level dictionary""" + base = {'key1': {'key1.1': {'key1.1.1': ['value1']}}} + target = ['value1'] + + du.recursive_dict_removal(base, target) + + self.assertEqual(1, len(base['key1']['key1.1']['key1.1.1'])) + self.assertIn('value1', base['key1']['key1.1']['key1.1.1']) + + def test_deleting_single_item_top_level_list(self): + base = {'key1': ['value1']} + target = ['value1'] + + du.recursive_dict_removal(base, target) + + self.assertEqual(0, len(base['key1'])) + + def test_deleting_single_nested_key(self): + base = {'key1': {'key1.1': {'key1.1.1': ['value1']}}} + target = ['key1.1.1'] + + du.recursive_dict_removal(base, target) + + self.assertNotIn('key1.1.1', base['key1']['key1.1']) + + +if __name__ == '__main__': + unittest.main() diff --git a/tox.ini b/tox.ini index b78931ff43..1a9ffe7df1 100644 --- a/tox.ini +++ b/tox.ini @@ -151,6 +151,7 @@ commands = coverage erase coverage run -a {toxinidir}/tests/test_inventory.py coverage run -a {toxinidir}/tests/test_manage.py + coverage run -a {toxinidir}/tests/test_dictutils.py coverage run -a {toxinidir}/tests/test_ip.py coverage run -a {toxinidir}/tests/test_filesystem.py coverage report --show-missing --include={toxinidir}/playbooks/inventory/*,{toxinidir}/lib/*