From fbfee315087139abf5bd1ba2c4f9fade746c41a0 Mon Sep 17 00:00:00 2001 From: Michal Novak Date: Wed, 4 Oct 2023 12:32:01 +0200 Subject: [PATCH] minor refactorings and fixes Signed-off-by: Michal Novak --- gnmi-tools.xml | 16 ++++++------ gnmi-tools.yang | 2 +- src/confd_gnmi_common.py | 42 +++++++++++++++---------------- tests/client_server_test_base.py | 39 ++++++++++++++++++++-------- tests/test_client_server_confd.py | 6 ++--- 5 files changed, 60 insertions(+), 45 deletions(-) diff --git a/gnmi-tools.xml b/gnmi-tools.xml index 61c42e4..f6b3f80 100644 --- a/gnmi-tools.xml +++ b/gnmi-tools.xml @@ -34,25 +34,25 @@ 23 - + n1 t1 In-Service - - + + 1010/0/AD-2-RX opticalTransport In-Service - - + + 1010/0/[AD 4-11]-1-RX opticalTransport In-Service - - + + ab[cd opticalTransport In-Service - + diff --git a/gnmi-tools.yang b/gnmi-tools.yang index 7421a47..f02e3a1 100644 --- a/gnmi-tools.yang +++ b/gnmi-tools.yang @@ -33,7 +33,7 @@ module gnmi-tools { uses combo; } - list double-list { + list double-key-list { key "name type"; leaf name {type string;} leaf type {type string;} diff --git a/src/confd_gnmi_common.py b/src/confd_gnmi_common.py index 68a8a9c..d36bac5 100644 --- a/src/confd_gnmi_common.py +++ b/src/confd_gnmi_common.py @@ -143,33 +143,31 @@ def split_gnmi_path(xpath_string: str, prefix_len: int) -> Tuple[str, str]: def _make_string_path(gnmi_path=None, gnmi_prefix=None, quote_val=False, xpath=False) -> str: """ - Create string path from gnmi_path and gnmi_prefix - :param gnmi_path: - :param gnmi_prefix: - :param quote_val: - :param xpath: - :return: + Create a string path from gnmi_path and gnmi_prefix + :param gnmi_path: The gnmi_path object + :param gnmi_prefix: The gnmi_prefix object + :param quote_val: Whether to quote the values in the path + :param xpath: Whether to use xpath formatting for the keys + :return: The string representation of the path """ log.debug("==> gnmi_path=%s gnmi_prefix=%s quote_val=%s xpath=%s", gnmi_path, gnmi_prefix, quote_val, xpath) def make_path(gnmi_path): - path = "" - for e in gnmi_path.elem: - path += "/" + e.name - len_items = len(e.key.items()) - if len_items: - if not xpath: path += '{' - for index, (k, v) in enumerate(e.key.items()): - val = v if not quote_val else "\"{}\"".format(v) - path += "[{}={}]".format(k, val) if xpath else "{}".format( - val) - if not xpath and index < len_items - 1: path += " " - if not xpath: path += '}' - - if path == "": - path = "/" - return path + path_elements = [e.name + elem_keys(e) for e in gnmi_path.elem] + return "/" + "/".join(path_elements) + + valstr = '"{}"' if quote_val else '{}' + + def elem_keys(elem): + if not elem.key: + return "" + if xpath: + return ''.join( + f'[{k}={valstr.format(v)}]' for k, v in elem.key.items()) + else: + return '{' + ' '.join( + valstr.format(v) for v in elem.key.values()) + '}' path_str = "" if gnmi_prefix is not None and len(gnmi_prefix.elem) > 0: diff --git a/tests/client_server_test_base.py b/tests/client_server_test_base.py index d5f1f07..6f52d8a 100644 --- a/tests/client_server_test_base.py +++ b/tests/client_server_test_base.py @@ -43,7 +43,7 @@ def _setup(self): self.leaves = ["name", "type", "enabled"] self.leaf_paths_str = [f"interface[name={{}}if_{{}}]/{leaf}" for leaf in self.leaves] self.list_paths_str = ["interface[name={}if_{}]", "interface", "ietf-interfaces:interfaces{}" ] - self.gnmi_tools_leaf_paths_str_val = [ + self.leaf_paths_str_for_gnmi_tools = [ ("/top/empty-leaf", [None]), ("/top/down/str-leaf", "test3"), ("/top/down/int-leaf", 44), @@ -52,12 +52,12 @@ def _setup(self): ("/top-pres/down/str-leaf", "test4"), ("/top-pres/down/int-leaf", 10), ("/top-pres/pres", {}), - ("/double-list[type=t1][name=n1]/admin-state", "In-Service"), - ("/double-list[name=1010/0/AD-2-RX]/admin-state", "In-Service", + ("/double-key-list[type=t1][name=n1]/admin-state", "In-Service"), + ("/double-key-list[name=1010/0/AD-2-RX]/admin-state", "In-Service", {"check_path": False}), - ("/double-list[name=1010/0/AD-2-RX][type=opticalTransport]/admin-state", "In-Service"), - ("/double-list[name=\"1010/0/[AD 4-11]-1-RX\"][type=opticalTransport]/admin-state", "In-Service"), - ("/double-list[name=\"ab[cd\"][type=opticalTransport]/admin-state", "In-Service") + ("/double-key-list[name=1010/0/AD-2-RX][type=opticalTransport]/admin-state", "In-Service"), + ("/double-key-list[name=\"1010/0/[AD 4-11]-1-RX\"][type=opticalTransport]/admin-state", "In-Service"), + ("/double-key-list[name=\"ab[cd\"][type=opticalTransport]/admin-state", "In-Service") ] @staticmethod @@ -90,14 +90,31 @@ def capability_supported(cap): @staticmethod def assert_update(update, path_val): + """ + Asserts that the update matches the expected path and value. + + Args: + update (Update): The update object to be checked. + path_val (tuple): A tuple containing the expected path and value and optional options map + + Raises: + AssertionError: If the update does not match the expected path and value. + """ + # Check if the path should be validated check_path = True - #do we have options attribute? - if len(path_val) >= 3: - if "check_path" in path_val[2]: - check_path = path_val[2]["check_path"] + + # Check if the options attribute map has the "check_path" key + if len(path_val) >= 3 and "check_path" in path_val[2]: + check_path = path_val[2]["check_path"] + + # Validate the path if required if check_path: - assert (update.path == path_val[0]) + assert update.path == path_val[0] + + # Parse the json_ietf_val attribute of the update object json_value = json.loads(update.val.json_ietf_val) + + # Assert that the parsed json value matches the expected value assert json_value == path_val[1] @staticmethod diff --git a/tests/test_client_server_confd.py b/tests/test_client_server_confd.py index fe3944e..4105507 100644 --- a/tests/test_client_server_confd.py +++ b/tests/test_client_server_confd.py @@ -79,7 +79,7 @@ def _test_gnmi_tools_get_subscribe_gnmi_tools(self, is_subscribe=False, leaf_paths_val = [ (GrpcBase.mk_gnmi_if_path(p[0]), p[1]) if len(p) == 2 else ( GrpcBase.mk_gnmi_if_path(p[0]), p[1], p[2]) for p in - self.gnmi_tools_leaf_paths_str_val] + self.leaf_paths_str_for_gnmi_tools] if is_subscribe: verify_response_updates = self.verify_sub_sub_response_updates @@ -89,9 +89,9 @@ def _test_gnmi_tools_get_subscribe_gnmi_tools(self, is_subscribe=False, kwargs["read_count"] = read_count kwargs["sample_interval"] = sample_interval kwargs["allow_aggregation"] = allow_aggregation - # TODO temporarily disabled subscription tests for double lists + # disabled subscription tests for double key lists as not handled by verify_sub_sub_response_updates if not allow_aggregation: - leaf_paths_val = [ p for p in leaf_paths_val if not "double-list" in p[0].elem[0].name] + leaf_paths_val = [ p for p in leaf_paths_val if not "double-key-list" in p[0].elem[0].name] else: verify_response_updates = self.verify_get_response_updates kwargs["datatype"] = datatype