From 8c168d09fb3c0a49847350116a5369402e823220 Mon Sep 17 00:00:00 2001 From: Rob White Date: Tue, 9 Feb 2021 12:44:05 +1100 Subject: [PATCH 1/5] feat(261): Adds parameter to descend into lists when scrubbing None params --- .../261-scrub-params-descend-into-lists.yml | 4 ++ plugins/__init__.py | 0 plugins/module_utils/core.py | 5 +- .../core/test_scrub_none_parameters.py | 48 +++++++++++++++++-- 4 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/261-scrub-params-descend-into-lists.yml create mode 100644 plugins/__init__.py diff --git a/changelogs/fragments/261-scrub-params-descend-into-lists.yml b/changelogs/fragments/261-scrub-params-descend-into-lists.yml new file mode 100644 index 00000000000..5034e6180a5 --- /dev/null +++ b/changelogs/fragments/261-scrub-params-descend-into-lists.yml @@ -0,0 +1,4 @@ +--- +minor_changes: + - module_utils/core - add parameter ``descend_into_lists`` to ``scrub_none_parameters`` helper function. Defaults to + True as the function has yet to be implemented anywhere. diff --git a/plugins/__init__.py b/plugins/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/plugins/module_utils/core.py b/plugins/module_utils/core.py index 349ec3b410d..a50775165f4 100644 --- a/plugins/module_utils/core.py +++ b/plugins/module_utils/core.py @@ -359,13 +359,14 @@ def get_boto3_client_method_parameters(client, method_name, required=False): return parameters -def scrub_none_parameters(parameters): +def scrub_none_parameters(parameters, descend_into_lists=True): """ Iterate over a dictionary removing any keys that have a None value Reference: https://github.com/ansible-collections/community.aws/issues/251 Credit: https://medium.com/better-programming/how-to-remove-null-none-values-from-a-dictionary-in-python-1bedf1aab5e4 + :param descend_into_lists: whether or not to descend in to lists to continue to remove None values :param parameters: parameter dict :return: parameter dict with all keys = None removed """ @@ -375,6 +376,8 @@ def scrub_none_parameters(parameters): for k, v in parameters.items(): if isinstance(v, dict): clean_parameters[k] = scrub_none_parameters(v) + elif descend_into_lists and isinstance(v, list): + clean_parameters[k] = [scrub_none_parameters(vv) if isinstance(vv, dict) else vv for vv in v] elif v is not None: clean_parameters[k] = v diff --git a/tests/unit/module_utils/core/test_scrub_none_parameters.py b/tests/unit/module_utils/core/test_scrub_none_parameters.py index 084c31028d4..20d6fea2a29 100644 --- a/tests/unit/module_utils/core/test_scrub_none_parameters.py +++ b/tests/unit/module_utils/core/test_scrub_none_parameters.py @@ -1,56 +1,94 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -from ansible_collections.amazon.aws.plugins.module_utils.core import scrub_none_parameters +#from ansible_collections.amazon.aws.plugins.module_utils.core import scrub_none_parameters +from plugins.module_utils.core import scrub_none_parameters import pytest - scrub_none_test_data = [ (dict(), + True, dict() ), (dict(param1='something'), + True, dict(param1='something') ), (dict(param1=False), + True, dict(param1=False) ), (dict(param1='something', param2='something_else'), + True, dict(param1='something', param2='something_else') ), (dict(param1='something', param2=dict()), + True, dict(param1='something', param2=dict()) ), (dict(param1='something', param2=None), + True, dict(param1='something') ), (dict(param1='something', param2=None, param3=None), + True, dict(param1='something') ), (dict(param1='something', param2=None, param3=None, param4='something_else'), + True, dict(param1='something', param4='something_else') ), (dict(param1=dict(sub_param1='something', sub_param2=None), param2=None, param3=None, param4='something_else'), + True, dict(param1=dict(sub_param1='something'), param4='something_else') ), (dict(param1=dict(sub_param1='something', sub_param2=dict(sub_sub_param1='another_thing')), param2=None, param3=None, param4='something_else'), + True, dict(param1=dict(sub_param1='something', sub_param2=dict(sub_sub_param1='another_thing')), param4='something_else') ), (dict(param1=dict(sub_param1='something', sub_param2=dict()), param2=None, param3=None, param4='something_else'), + True, dict(param1=dict(sub_param1='something', sub_param2=dict()), param4='something_else') ), (dict(param1=dict(sub_param1='something', sub_param2=False), param2=None, param3=None, param4='something_else'), + True, dict(param1=dict(sub_param1='something', sub_param2=False), param4='something_else') ), (dict(param1=None, param2=None), + True, dict() ), (dict(param1=None, param2=[]), + True, dict(param2=[]) + ), + (dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2='my_dict_nested_in_a_list_2')], param2=[]), + True, + dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2='my_dict_nested_in_a_list_2')], param2=[]) + ), + (dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2='my_dict_nested_in_a_list_2')], param2=[]), + False, + dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2='my_dict_nested_in_a_list_2')], param2=[]) + ), + (dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2=None)], param2=[]), + True, + dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1')], param2=[]) + ), + (dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2=None)], param2=[]), + False, + dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2=None)], param2=[]) + ), + (dict(param1=[dict(sub_param1=[dict(sub_sub_param1=None)], sub_param2=None)], param2=[]), + True, + dict(param1=[dict(sub_param1=[{}])], param2=[]) + ), + (dict(param1=[dict(sub_param1=[dict(sub_sub_param1=None)], sub_param2=None)], param2=None), + False, + dict(param1=[dict(sub_param1=[dict(sub_sub_param1=None)], sub_param2=None)]), ) ] -@pytest.mark.parametrize("input_params, output_params", scrub_none_test_data) -def test_scrub_none_parameters(input_params, output_params): +@pytest.mark.parametrize("input_params, descend_into_lists_flag, output_params", scrub_none_test_data) +def test_scrub_none_parameters(input_params, descend_into_lists_flag, output_params): - assert scrub_none_parameters(input_params) == output_params + assert scrub_none_parameters(input_params, descend_into_lists_flag) == output_params From 2c3b9b8103941dd8f4583cffe853c681291e8636 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 9 Mar 2021 11:21:34 +0100 Subject: [PATCH 2/5] Switch default of descend_into_lists to False to avoid a potentially breaking change --- plugins/module_utils/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/module_utils/core.py b/plugins/module_utils/core.py index a50775165f4..d6d5a997089 100644 --- a/plugins/module_utils/core.py +++ b/plugins/module_utils/core.py @@ -359,7 +359,7 @@ def get_boto3_client_method_parameters(client, method_name, required=False): return parameters -def scrub_none_parameters(parameters, descend_into_lists=True): +def scrub_none_parameters(parameters, descend_into_lists=False): """ Iterate over a dictionary removing any keys that have a None value From 3681b54425e24def27c76ac576077aba324ef517 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 9 Mar 2021 12:49:07 +0100 Subject: [PATCH 3/5] ensure descend_into_lists is passed on recursion --- plugins/module_utils/core.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/module_utils/core.py b/plugins/module_utils/core.py index d6d5a997089..b67241b8570 100644 --- a/plugins/module_utils/core.py +++ b/plugins/module_utils/core.py @@ -375,9 +375,9 @@ def scrub_none_parameters(parameters, descend_into_lists=False): for k, v in parameters.items(): if isinstance(v, dict): - clean_parameters[k] = scrub_none_parameters(v) + clean_parameters[k] = scrub_none_parameters(v, descend_into_lists=descend_into_lists) elif descend_into_lists and isinstance(v, list): - clean_parameters[k] = [scrub_none_parameters(vv) if isinstance(vv, dict) else vv for vv in v] + clean_parameters[k] = [scrub_none_parameters(vv, descend_into_lists=descend_into_lists) if isinstance(vv, dict) else vv for vv in v] elif v is not None: clean_parameters[k] = v From e638bfeb26d5a173d99439d5149fe4fb0f04b56b Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 9 Mar 2021 12:49:29 +0100 Subject: [PATCH 4/5] Update changelog --- changelogs/fragments/261-scrub-params-descend-into-lists.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/changelogs/fragments/261-scrub-params-descend-into-lists.yml b/changelogs/fragments/261-scrub-params-descend-into-lists.yml index 5034e6180a5..1116d35ed27 100644 --- a/changelogs/fragments/261-scrub-params-descend-into-lists.yml +++ b/changelogs/fragments/261-scrub-params-descend-into-lists.yml @@ -1,4 +1,3 @@ --- minor_changes: - - module_utils/core - add parameter ``descend_into_lists`` to ``scrub_none_parameters`` helper function. Defaults to - True as the function has yet to be implemented anywhere. + - module_utils/core - add parameter ``descend_into_lists`` to ``scrub_none_parameters`` helper function (https://github.com/ansible-collections/amazon.aws/pull/262). From 8c7290a326cbdd8644d24e1cd388d2d7cb945e01 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 9 Mar 2021 12:50:16 +0100 Subject: [PATCH 5/5] Update tests to ensure that the default behaviour of descend_into_lists is also tested --- .../core/test_scrub_none_parameters.py | 108 +++++++++--------- 1 file changed, 51 insertions(+), 57 deletions(-) diff --git a/tests/unit/module_utils/core/test_scrub_none_parameters.py b/tests/unit/module_utils/core/test_scrub_none_parameters.py index 20d6fea2a29..a1a1b491788 100644 --- a/tests/unit/module_utils/core/test_scrub_none_parameters.py +++ b/tests/unit/module_utils/core/test_scrub_none_parameters.py @@ -1,94 +1,88 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -#from ansible_collections.amazon.aws.plugins.module_utils.core import scrub_none_parameters -from plugins.module_utils.core import scrub_none_parameters import pytest + +from ansible_collections.amazon.aws.plugins.module_utils.core import scrub_none_parameters + scrub_none_test_data = [ - (dict(), - True, - dict() + (dict(), # Input + dict(), # Output with descend_into_lists=False + dict(), # Output with descend_into_lists=True + ), + (dict(param1=None, param2=None), + dict(), + dict(), ), (dict(param1='something'), - True, - dict(param1='something') + dict(param1='something'), + dict(param1='something'), ), (dict(param1=False), - True, - dict(param1=False) + dict(param1=False), + dict(param1=False), + ), + (dict(param1=None, param2=[]), + dict(param2=[]), + dict(param2=[]), + ), + (dict(param1=None, param2=["list_value"]), + dict(param2=["list_value"]), + dict(param2=["list_value"]), ), (dict(param1='something', param2='something_else'), - True, - dict(param1='something', param2='something_else') + dict(param1='something', param2='something_else'), + dict(param1='something', param2='something_else'), ), (dict(param1='something', param2=dict()), - True, - dict(param1='something', param2=dict()) + dict(param1='something', param2=dict()), + dict(param1='something', param2=dict()), ), (dict(param1='something', param2=None), - True, - dict(param1='something') + dict(param1='something'), + dict(param1='something'), ), (dict(param1='something', param2=None, param3=None), - True, - dict(param1='something') + dict(param1='something'), + dict(param1='something'), ), (dict(param1='something', param2=None, param3=None, param4='something_else'), - True, - dict(param1='something', param4='something_else') - ), - (dict(param1=dict(sub_param1='something', sub_param2=None), param2=None, param3=None, param4='something_else'), - True, - dict(param1=dict(sub_param1='something'), param4='something_else') + dict(param1='something', param4='something_else'), + dict(param1='something', param4='something_else'), ), (dict(param1=dict(sub_param1='something', sub_param2=dict(sub_sub_param1='another_thing')), param2=None, param3=None, param4='something_else'), - True, - dict(param1=dict(sub_param1='something', sub_param2=dict(sub_sub_param1='another_thing')), param4='something_else') + dict(param1=dict(sub_param1='something', sub_param2=dict(sub_sub_param1='another_thing')), param4='something_else'), + dict(param1=dict(sub_param1='something', sub_param2=dict(sub_sub_param1='another_thing')), param4='something_else'), ), (dict(param1=dict(sub_param1='something', sub_param2=dict()), param2=None, param3=None, param4='something_else'), - True, - dict(param1=dict(sub_param1='something', sub_param2=dict()), param4='something_else') + dict(param1=dict(sub_param1='something', sub_param2=dict()), param4='something_else'), + dict(param1=dict(sub_param1='something', sub_param2=dict()), param4='something_else'), ), (dict(param1=dict(sub_param1='something', sub_param2=False), param2=None, param3=None, param4='something_else'), - True, - dict(param1=dict(sub_param1='something', sub_param2=False), param4='something_else') - ), - (dict(param1=None, param2=None), - True, - dict() - ), - (dict(param1=None, param2=[]), - True, - dict(param2=[]) - ), - (dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2='my_dict_nested_in_a_list_2')], param2=[]), - True, - dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2='my_dict_nested_in_a_list_2')], param2=[]) + dict(param1=dict(sub_param1='something', sub_param2=False), param4='something_else'), + dict(param1=dict(sub_param1='something', sub_param2=False), param4='something_else'), ), (dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2='my_dict_nested_in_a_list_2')], param2=[]), - False, - dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2='my_dict_nested_in_a_list_2')], param2=[]) + dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2='my_dict_nested_in_a_list_2')], param2=[]), + dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2='my_dict_nested_in_a_list_2')], param2=[]), ), (dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2=None)], param2=[]), - True, - dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1')], param2=[]) - ), - (dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2=None)], param2=[]), - False, - dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2=None)], param2=[]) + dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1', sub_param2=None)], param2=[]), + dict(param1=[dict(sub_param1='my_dict_nested_in_a_list_1')], param2=[]), ), (dict(param1=[dict(sub_param1=[dict(sub_sub_param1=None)], sub_param2=None)], param2=[]), - True, - dict(param1=[dict(sub_param1=[{}])], param2=[]) + dict(param1=[dict(sub_param1=[dict(sub_sub_param1=None)], sub_param2=None)], param2=[]), + dict(param1=[dict(sub_param1=[dict()])], param2=[]), ), (dict(param1=[dict(sub_param1=[dict(sub_sub_param1=None)], sub_param2=None)], param2=None), - False, dict(param1=[dict(sub_param1=[dict(sub_sub_param1=None)], sub_param2=None)]), - ) + dict(param1=[dict(sub_param1=[dict()])]), + ), ] -@pytest.mark.parametrize("input_params, descend_into_lists_flag, output_params", scrub_none_test_data) -def test_scrub_none_parameters(input_params, descend_into_lists_flag, output_params): - - assert scrub_none_parameters(input_params, descend_into_lists_flag) == output_params +@pytest.mark.parametrize("input_params, output_params_no_descend, output_params_descend", scrub_none_test_data) +def test_scrub_none_parameters(input_params, output_params_no_descend, output_params_descend): + assert scrub_none_parameters(input_params) == output_params_no_descend + assert scrub_none_parameters(input_params, descend_into_lists=False) == output_params_no_descend + assert scrub_none_parameters(input_params, descend_into_lists=True) == output_params_descend