From 1181950bfabd5aaaf2fc69941ad9afeff70176d5 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Tue, 29 Nov 2022 11:36:01 -0500 Subject: [PATCH] Allow for substituting YAML value to global config variables (#23743) * Allow for subsituting YAML value to config variables * Address PR comments * Fix conflict after master merge --- .../python/chip/yaml/constraints.py | 31 ++++--- .../python/chip/yaml/format_converter.py | 88 ++++++++++++++++--- src/controller/python/chip/yaml/parser.py | 19 ++-- .../unit_tests/test_yaml_format_converter.py | 71 ++++++++++++++- 4 files changed, 174 insertions(+), 35 deletions(-) diff --git a/src/controller/python/chip/yaml/constraints.py b/src/controller/python/chip/yaml/constraints.py index 933d74fdf4a562..49ac8c99153eff 100644 --- a/src/controller/python/chip/yaml/constraints.py +++ b/src/controller/python/chip/yaml/constraints.py @@ -36,7 +36,7 @@ def is_met(self, response) -> bool: class _LoadableConstraint(BaseConstraint): '''Constraints where value might be stored in VariableStorage needing runtime load.''' - def __init__(self, value, field_type, variable_storage: VariableStorage): + def __init__(self, value, field_type, variable_storage: VariableStorage, config_values: dict): self._variable_storage = variable_storage # When not none _indirect_value_key is binding a name to the constraint value, and the # actual value can only be looked-up dynamically, which is why this is a key name. @@ -50,8 +50,8 @@ def __init__(self, value, field_type, variable_storage: VariableStorage): if isinstance(value, str) and self._variable_storage.is_key_saved(value): self._indirect_value_key = value else: - self._value = Converter.convert_yaml_type( - value, field_type) + self._value = Converter.parse_and_convert_yaml_value( + value, field_type, config_values) def get_value(self): '''Gets the current value of the constraint. @@ -112,8 +112,9 @@ def is_met(self, response) -> bool: class _ConstraintMinValue(_LoadableConstraint): - def __init__(self, min_value, field_type, variable_storage: VariableStorage): - super().__init__(min_value, field_type, variable_storage) + def __init__(self, min_value, field_type, variable_storage: VariableStorage, + config_values: dict): + super().__init__(min_value, field_type, variable_storage, config_values) def is_met(self, response) -> bool: min_value = self.get_value() @@ -121,8 +122,9 @@ def is_met(self, response) -> bool: class _ConstraintMaxValue(_LoadableConstraint): - def __init__(self, max_value, field_type, variable_storage: VariableStorage): - super().__init__(max_value, field_type, variable_storage) + def __init__(self, max_value, field_type, variable_storage: VariableStorage, + config_values: dict): + super().__init__(max_value, field_type, variable_storage, config_values) def is_met(self, response) -> bool: max_value = self.get_value() @@ -162,16 +164,17 @@ def is_met(self, response) -> bool: class _ConstraintNotValue(_LoadableConstraint): - def __init__(self, not_value, field_type, variable_storage: VariableStorage): - super().__init__(not_value, field_type, variable_storage) + def __init__(self, not_value, field_type, variable_storage: VariableStorage, + config_values: dict): + super().__init__(not_value, field_type, variable_storage, config_values) def is_met(self, response) -> bool: not_value = self.get_value() return response != not_value -def get_constraints(constraints, field_type, - variable_storage: VariableStorage) -> list[BaseConstraint]: +def get_constraints(constraints, field_type, variable_storage: VariableStorage, + config_values: dict) -> list[BaseConstraint]: _constraints = [] if 'hasValue' in constraints: _constraints.append(_ConstraintHasValue(constraints.get('hasValue'))) @@ -193,11 +196,11 @@ def get_constraints(constraints, field_type, if 'minValue' in constraints: _constraints.append(_ConstraintMinValue( - constraints.get('minValue'), field_type, variable_storage)) + constraints.get('minValue'), field_type, variable_storage, config_values)) if 'maxValue' in constraints: _constraints.append(_ConstraintMaxValue( - constraints.get('maxValue'), field_type, variable_storage)) + constraints.get('maxValue'), field_type, variable_storage, config_values)) if 'contains' in constraints: _constraints.append(_ConstraintContains(constraints.get('contains'))) @@ -213,6 +216,6 @@ def get_constraints(constraints, field_type, if 'notValue' in constraints: _constraints.append(_ConstraintNotValue( - constraints.get('notValue'), field_type, variable_storage)) + constraints.get('notValue'), field_type, variable_storage, config_values)) return _constraints diff --git a/src/controller/python/chip/yaml/format_converter.py b/src/controller/python/chip/yaml/format_converter.py index f6402c59706058..fc3c5a1b873cfc 100644 --- a/src/controller/python/chip/yaml/format_converter.py +++ b/src/controller/python/chip/yaml/format_converter.py @@ -23,8 +23,44 @@ import binascii +def substitute_in_config_variables(field_value, config_values: dict): + ''' Substitutes values that are config variables. + + YAML values can contain a string of a configuration variable name. In these instances we + substitute the configuration variable name with the actual value. + + For examples see unittest src/controller/python/test/unit_tests/test_yaml_format_converter.py + + # TODO This should also substitue any saveAs values as well as perform any required + # evaluations. + + Args: + 'field_value': Value as extracted from YAML. + 'config_values': Dictionary of global configuration variables. + Returns: + Value with all global configuration variables substituted with the real value. + ''' + if isinstance(field_value, dict): + return {key: substitute_in_config_variables( + field_value[key], config_values) for key in field_value} + if isinstance(field_value, list): + return [substitute_in_config_variables(item, config_values) for item in field_value] + if isinstance(field_value, str) and field_value in config_values: + config_value = config_values[field_value] + if isinstance(config_value, dict) and 'defaultValue' in config_value: + # TODO currently we don't validate that if config_value['type'] is provided + # that the type does in fact match our expectation. + return config_value['defaultValue'] + return config_values[field_value] + + return field_value + + def convert_yaml_octet_string_to_bytes(s: str) -> bytes: - """Convert YAML octet string body to bytes, handling any c-style hex escapes (e.g. \x5a) and hex: prefix""" + '''Convert YAML octet string body to bytes. + + Included handling any c-style hex escapes (e.g. \x5a) and 'hex:' prefix. + ''' # Step 1: handle explicit "hex:" prefix if s.startswith('hex:'): return binascii.unhexlify(s[4:]) @@ -60,14 +96,21 @@ def convert_name_value_pair_to_dict(arg_values): return ret_value -def convert_yaml_type(field_value, field_type, use_from_dict=False): - ''' Converts yaml value to expected type. +def convert_yaml_type(field_value, field_type, inline_cast_dict_to_struct): + ''' Converts yaml value to provided pythonic type. + + The YAML representation when converted to a dictionary does not line up to + the python type data model for the various command/attribute/event object + types. This function converts 'field_value' to the appropriate provided + 'field_type'. - The YAML representation when converted to a Python dictionary does not - quite line up in terms of type (see each of the specific if branches - below for the rationale for the necessary fix-ups). This function does - a fix-up given a field value (as present in the YAML) and its matching - cluster object type and returns it. + Args: + 'field_value': Value as extracted from yaml + 'field_type': Pythonic command/attribute/event object type that we + are converting value to. + 'inline_cast_dict_to_struct': If true, for any dictionary 'field_value' + types provided we will do a convertion to the corresponding data + model class in `field_type` by doing field_type.FromDict(...). ''' origin = typing.get_origin(field_type) @@ -110,8 +153,8 @@ def convert_yaml_type(field_value, field_type, use_from_dict=False): f'Did not find field "{item}" in {str(field_type)}') from None return_field_value[field_descriptor.Label] = convert_yaml_type( - field_value[item], field_descriptor.Type, use_from_dict) - if use_from_dict: + field_value[item], field_descriptor.Type, inline_cast_dict_to_struct) + if inline_cast_dict_to_struct: return field_type.FromDict(return_field_value) return return_field_value elif(type(field_value) is float): @@ -122,7 +165,8 @@ def convert_yaml_type(field_value, field_type, use_from_dict=False): # The field type passed in is the type of the list element and not list[T]. for idx, item in enumerate(field_value): - field_value[idx] = convert_yaml_type(item, list_element_type, use_from_dict) + field_value[idx] = convert_yaml_type(item, list_element_type, + inline_cast_dict_to_struct) return field_value # YAML conversion treats all numbers as ints. Convert to a uint type if the schema # type indicates so. @@ -139,3 +183,25 @@ def convert_yaml_type(field_value, field_type, use_from_dict=False): # By default, just return the field_value casted to field_type. else: return field_type(field_value) + + +def parse_and_convert_yaml_value(field_value, field_type, config_values: dict, + inline_cast_dict_to_struct: bool = False): + ''' Parse and converts YAML type + + Parsing the YAML value means performing required substitutions and evaluations. Parsing is + then followed by converting from the YAML type done using yaml.safe_load() to the type used in + the various command/attribute/event object data model types. + + Args: + 'field_value': Value as extracted from yaml to be parsed + 'field_type': Pythonic command/attribute/event object type that we + are converting value to. + 'config_values': Dictionary of global configuration variables. + 'inline_cast_dict_to_struct': If true, for any dictionary 'field_value' + types provided we will do an inline convertion to the corresponding + struct in `field_type` by doing field_type.FromDict(...). + ''' + field_value_with_config_variables = substitute_in_config_variables(field_value, config_values) + return convert_yaml_type(field_value_with_config_variables, field_type, + inline_cast_dict_to_struct) diff --git a/src/controller/python/chip/yaml/parser.py b/src/controller/python/chip/yaml/parser.py index a3f38f7514fde0..b70a743147063d 100644 --- a/src/controller/python/chip/yaml/parser.py +++ b/src/controller/python/chip/yaml/parser.py @@ -69,8 +69,8 @@ def __init__(self, value, response_type, context: _ExecutionContext): if isinstance(value, str) and self._variable_storage.is_key_saved(value): self._load_expected_response_in_verify = value else: - self._expected_response = Converter.convert_yaml_type( - value, response_type, use_from_dict=True) + self._expected_response = Converter.parse_and_convert_yaml_value( + value, response_type, context.config_values, inline_cast_dict_to_struct=True) def verify(self, response): if (self._expected_response_type is None): @@ -145,8 +145,8 @@ def __init__(self, item: dict, cluster: str, context: _ExecutionContext): request_data_as_dict = Converter.convert_name_value_pair_to_dict(args) try: - request_data = Converter.convert_yaml_type( - request_data_as_dict, type(command_object)) + request_data = Converter.parse_and_convert_yaml_value( + request_data_as_dict, type(command_object), context.config_values) except ValueError: raise ParsingError('Could not covert yaml type') @@ -166,8 +166,8 @@ def __init__(self, item: dict, cluster: str, context: _ExecutionContext): expected_response_args = self._expected_raw_response['values'] expected_response_data_as_dict = Converter.convert_name_value_pair_to_dict( expected_response_args) - expected_response_data = Converter.convert_yaml_type( - expected_response_data_as_dict, expected_command) + expected_response_data = Converter.parse_and_convert_yaml_value( + expected_response_data_as_dict, expected_command, context.config_values) self._expected_response_object = expected_command.FromDict(expected_response_data) def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): @@ -260,7 +260,8 @@ def __init__(self, item: dict, cluster: str, context: _ExecutionContext): if constraints: self._constraints = get_constraints(constraints, self._request_object.attribute_type.Type, - context.variable_storage) + context.variable_storage, + context.config_values) def run_action(self, dev_ctrl: ChipDeviceCtrl, endpoint: int, node_id: int): try: @@ -326,8 +327,8 @@ def __init__(self, item: dict, cluster: str, context: _ExecutionContext): if (item.get('arguments')): args = item['arguments']['value'] try: - request_data = Converter.convert_yaml_type( - args, attribute.attribute_type.Type) + request_data = Converter.parse_and_convert_yaml_value( + args, attribute.attribute_type.Type, context.config_values) except ValueError: raise ParsingError('Could not covert yaml type') diff --git a/src/controller/python/test/unit_tests/test_yaml_format_converter.py b/src/controller/python/test/unit_tests/test_yaml_format_converter.py index fa46705be65ed9..05f58d20b59418 100644 --- a/src/controller/python/test/unit_tests/test_yaml_format_converter.py +++ b/src/controller/python/test/unit_tests/test_yaml_format_converter.py @@ -15,7 +15,7 @@ # limitations under the License. # -from chip.yaml.format_converter import convert_yaml_octet_string_to_bytes +from chip.yaml.format_converter import convert_yaml_octet_string_to_bytes, substitute_in_config_variables from binascii import unhexlify import unittest @@ -44,6 +44,75 @@ def test_common_cases(self): convert_yaml_octet_string_to_bytes("hex:aa5") +class TestSubstitueInConfigVariables(unittest.TestCase): + + def setUp(self): + self.common_config = { + 'arg1': { + 'defaultValue': 1 + }, + 'arg2': { + 'defaultValue': 2 + }, + 'no_explicit_default': 3 + } + + def test_basic_substitution(self): + self.assertEqual(substitute_in_config_variables('arg1', self.common_config), 1) + self.assertEqual(substitute_in_config_variables('arg2', self.common_config), 2) + self.assertEqual(substitute_in_config_variables('arg3', self.common_config), 'arg3') + self.assertEqual(substitute_in_config_variables('no_explicit_default', self.common_config), 3) + + def test_basis_dict_substitution(self): + basic_dict = { + 'arg1': 'arg1', + 'arg2': 'arg2', + 'arg3': 'arg3', + 'no_explicit_default': 'no_explicit_default', + } + expected_dict = { + 'arg1': 1, + 'arg2': 2, + 'arg3': 'arg3', + 'no_explicit_default': 3, + } + self.assertEqual(substitute_in_config_variables(basic_dict, self.common_config), expected_dict) + + def test_basis_list_substitution(self): + basic_list = ['arg1', 'arg2', 'arg3', 'no_explicit_default'] + expected_list = [1, 2, 'arg3', 3] + self.assertEqual(substitute_in_config_variables(basic_list, self.common_config), expected_list) + + def test_complex_nested_type(self): + complex_nested_type = { + 'arg1': ['arg1', 'arg2', 'arg3', 'no_explicit_default'], + 'arg2': 'arg22', + 'arg3': { + 'no_explicit_default': 'no_explicit_default', + 'arg2': 'arg2', + 'another_dict': { + 'arg1': ['arg1', 'arg1', 'arg1', 'no_explicit_default'], + }, + 'another_list': ['arg1', 'arg2', 'arg3', 'no_explicit_default'] + }, + 'no_explicit_default': 'no_explicit_default', + } + expected_result = { + 'arg1': [1, 2, 'arg3', 3], + 'arg2': 'arg22', + 'arg3': { + 'no_explicit_default': 3, + 'arg2': 2, + 'another_dict': { + 'arg1': [1, 1, 1, 3], + }, + 'another_list': [1, 2, 'arg3', 3] + }, + 'no_explicit_default': 3, + } + self.assertEqual(substitute_in_config_variables(complex_nested_type, self.common_config), expected_result) + + def main(): unittest.main()