From 97311a6c04eba447b0d1cf829cda69471dea9e81 Mon Sep 17 00:00:00 2001 From: Renat Akhmerov Date: Mon, 13 Mar 2017 19:07:47 +0700 Subject: [PATCH] Refactor methods in utils related to dicts * The previous versions of these methods were too specific for the general utils module. Now they are more generic thereby less confusing. Change-Id: Ifa8bbf0cc8a63bf1de1142dc8c52b1f8ad3958c5 --- mistral/lang/v2/actions.py | 2 +- mistral/lang/v2/workflows.py | 2 +- mistral/tests/unit/utils/test_utils.py | 23 +++++----- mistral/utils/__init__.py | 59 ++++++++++++-------------- 4 files changed, 42 insertions(+), 44 deletions(-) diff --git a/mistral/lang/v2/actions.py b/mistral/lang/v2/actions.py index 529ff6db..2d37d9d6 100644 --- a/mistral/lang/v2/actions.py +++ b/mistral/lang/v2/actions.py @@ -42,7 +42,7 @@ class ActionSpec(base.BaseSpec): self._tags = data.get('tags', []) self._base = data['base'] self._base_input = data.get('base-input', {}) - self._input = utils.get_input_dict(data.get('input', [])) + self._input = utils.get_dict_from_entries(data.get('input', [])) self._output = data.get('output') self._base, _input = self._parse_cmd_and_input(self._base) diff --git a/mistral/lang/v2/workflows.py b/mistral/lang/v2/workflows.py index 44031fcc..a197760c 100644 --- a/mistral/lang/v2/workflows.py +++ b/mistral/lang/v2/workflows.py @@ -54,7 +54,7 @@ class WorkflowSpec(base.BaseSpec): self._description = data.get('description') self._tags = data.get('tags', []) self._type = data['type'] if 'type' in data else 'direct' - self._input = utils.get_input_dict(data.get('input', [])) + self._input = utils.get_dict_from_entries(data.get('input', [])) self._output = data.get('output', {}) self._output_on_error = data.get('output-on-error', {}) self._vars = data.get('vars', {}) diff --git a/mistral/tests/unit/utils/test_utils.py b/mistral/tests/unit/utils/test_utils.py index eb0133df..bff4b8c1 100644 --- a/mistral/tests/unit/utils/test_utils.py +++ b/mistral/tests/unit/utils/test_utils.py @@ -100,25 +100,26 @@ class UtilsTest(base.BaseTest): self.assertEqual([B, C, D], list(utils.iter_subclasses(A))) - def test_get_input_dict(self): + def test_get_dict_from_entries(self): input = ['param1', {'param2': 2}] - input_dict = utils.get_input_dict(input) + input_dict = utils.get_dict_from_entries(input) self.assertIn('param1', input_dict) self.assertIn('param2', input_dict) self.assertEqual(2, input_dict.get('param2')) self.assertIs(input_dict.get('param1'), utils.NotDefined) - def test_get_input_dict_from_input_string(self): - input_string = 'param1, param2=2, param3="var3"' - input_dict = utils.get_dict_from_string(input_string) + def test_get_input_dict_from_string(self): + self.assertDictEqual( + { + 'param1': utils.NotDefined, + 'param2': 2, + 'param3': 'var3' + }, + utils.get_dict_from_string('param1, param2=2, param3="var3"') + ) - self.assertIn('param1', input_dict) - self.assertIn('param2', input_dict) - self.assertIn('param3', input_dict) - self.assertEqual(2, input_dict.get('param2')) - self.assertEqual('var3', input_dict.get('param3')) - self.assertIs(input_dict.get('param1'), utils.NotDefined) + self.assertDictEqual({}, utils.get_dict_from_string('')) def test_paramiko_to_private_key(self): self.assertRaises( diff --git a/mistral/utils/__init__.py b/mistral/utils/__init__.py index 6510ec73..00fdb5b6 100644 --- a/mistral/utils/__init__.py +++ b/mistral/utils/__init__.py @@ -343,53 +343,50 @@ class NotDefined(object): pass -def get_dict_from_string(input_string, delimiter=','): - # TODO(rakhmerov): Why is it here? This module is too generic. - if not input_string: +def get_dict_from_string(string, delimiter=','): + if not string: return {} - raw_inputs = input_string.split(delimiter) + kv_dicts = [] - inputs = [] + for kv_pair_str in string.split(delimiter): + kv_str = kv_pair_str.strip() + kv_list = kv_str.split('=') - for raw in raw_inputs: - input = raw.strip() - name_value = input.split('=') - - if len(name_value) > 1: + if len(kv_list) > 1: try: - value = json.loads(name_value[1]) + value = json.loads(kv_list[1]) except ValueError: - value = name_value[1] + value = kv_list[1] - inputs += [{name_value[0]: value}] + kv_dicts += [{kv_list[0]: value}] else: - inputs += [name_value[0]] + kv_dicts += [kv_list[0]] - return get_input_dict(inputs) + return get_dict_from_entries(kv_dicts) -def get_input_dict(inputs): - # TODO(rakhmerov): Why is it here? This module is too generic. - # TODO(rakhmerov): Move it to the spec. +def get_dict_from_entries(entries): + """Transforms a list of entries into dictionary. - """Transform input list to dictionary. - - Ensure every input param has a default value(it will be a NotDefined - object if it's not provided). + :param entries: A list of entries. + If an entry is a dictionary the method simply updates the result + dictionary with its content. + If an entry is not a dict adds {entry, NotDefined} into the result. """ - input_dict = {} - for x in inputs: - if isinstance(x, dict): - input_dict.update(x) + result = {} + + for e in entries: + if isinstance(e, dict): + result.update(e) else: - # NOTE(xylan): we put a NotDefined class here as the value of - # param without value specified, to distinguish from the valid - # values such as None, ''(empty string), etc. - input_dict[x] = NotDefined + # NOTE(xylan): we put NotDefined here as the value of + # param without value specified, to distinguish from + # the valid values such as None, ''(empty string), etc. + result[e] = NotDefined - return input_dict + return result def get_process_identifier():