From 4944ac4fd36d830dcf1974310880edb0384f3088 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Wed, 23 Nov 2022 20:13:11 +0000 Subject: [PATCH 1/3] Allow for subsituting YAML value to config variables --- .../python/chip/yaml/format_converter.py | 93 ++++++++++++++++--- src/controller/python/chip/yaml/parser.py | 20 ++-- 2 files changed, 92 insertions(+), 21 deletions(-) diff --git a/src/controller/python/chip/yaml/format_converter.py b/src/controller/python/chip/yaml/format_converter.py index f6402c59706058..54c0e8ff071d84 100644 --- a/src/controller/python/chip/yaml/format_converter.py +++ b/src/controller/python/chip/yaml/format_converter.py @@ -23,8 +23,49 @@ 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. + + # 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 (type(field_value) is dict): + return_values = {} + for key in field_value: + return_values[key] = _substitute_in_config_variables(field_value[key], config_values) + + return return_values + elif(type(field_value) is list): + return_values = [] + for item in field_value: + return_values.append(_substitute_in_config_variables(item, config_values)) + return return_values + elif 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'] + else: + 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 +101,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 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. + 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'. + + 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 an inline convertion to the corresponding + struct in `field_type` by doing field_type.FromDict(...). ''' origin = typing.get_origin(field_type) @@ -110,8 +158,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 +170,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 +188,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 + than 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 8fc5cd55ec66cd..2330c04d164b74 100644 --- a/src/controller/python/chip/yaml/parser.py +++ b/src/controller/python/chip/yaml/parser.py @@ -67,8 +67,8 @@ def __init__(self, value, field_type, context: _ExecutionContext): 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, context.config_values) def get_value(self): '''Gets the current value of the constraint. @@ -173,8 +173,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): @@ -249,8 +249,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') @@ -270,8 +270,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): @@ -430,8 +430,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') From 0bda4ea125b85e69ddbbd689571ed0d19562c0d8 Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Thu, 24 Nov 2022 14:53:58 +0000 Subject: [PATCH 2/3] Address PR comments --- .../python/chip/yaml/format_converter.py | 33 ++++----- .../unit_tests/test_yaml_format_converter.py | 71 ++++++++++++++++++- 2 files changed, 84 insertions(+), 20 deletions(-) diff --git a/src/controller/python/chip/yaml/format_converter.py b/src/controller/python/chip/yaml/format_converter.py index 54c0e8ff071d84..fc3c5a1b873cfc 100644 --- a/src/controller/python/chip/yaml/format_converter.py +++ b/src/controller/python/chip/yaml/format_converter.py @@ -23,12 +23,14 @@ import binascii -def _substitute_in_config_variables(field_value, config_values: dict): +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. @@ -38,25 +40,18 @@ def _substitute_in_config_variables(field_value, config_values: dict): Returns: Value with all global configuration variables substituted with the real value. ''' - if (type(field_value) is dict): - return_values = {} - for key in field_value: - return_values[key] = _substitute_in_config_variables(field_value[key], config_values) - - return return_values - elif(type(field_value) is list): - return_values = [] - for item in field_value: - return_values.append(_substitute_in_config_variables(item, config_values)) - return return_values - elif isinstance(field_value, str) and field_value in config_values: + 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'] - else: - return config_values[field_value] + return config_values[field_value] return field_value @@ -114,8 +109,8 @@ def convert_yaml_type(field_value, field_type, inline_cast_dict_to_struct): '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 an inline convertion to the corresponding - struct in `field_type` by doing field_type.FromDict(...). + 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) @@ -195,7 +190,7 @@ def parse_and_convert_yaml_value(field_value, field_type, config_values: dict, ''' Parse and converts YAML type Parsing the YAML value means performing required substitutions and evaluations. Parsing is - than followed by converting from the YAML type done using yaml.safe_load() to the type used in + 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: @@ -207,6 +202,6 @@ def parse_and_convert_yaml_value(field_value, field_type, config_values: dict, 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) + 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/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() From 09368585dc9216f620901e31b0de3d2975bc2c1f Mon Sep 17 00:00:00 2001 From: Terence Hampson Date: Thu, 24 Nov 2022 21:26:38 +0000 Subject: [PATCH 3/3] Fix conflict after master merge --- .../python/chip/yaml/constraints.py | 31 ++++++++++--------- src/controller/python/chip/yaml/parser.py | 3 +- 2 files changed, 19 insertions(+), 15 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/parser.py b/src/controller/python/chip/yaml/parser.py index 3a4ce71b6b54a3..b70a743147063d 100644 --- a/src/controller/python/chip/yaml/parser.py +++ b/src/controller/python/chip/yaml/parser.py @@ -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: