minor refactorings and fixes

Signed-off-by: Michal Novak <[email protected]>

gnmi-tools.xml

@@ -34,25 +34,25 @@
                 <int-leaf>23</int-leaf>
             </down>
         </top-list>
-        <double-list>
+        <double-key-list>
             <name>n1</name>
             <type>t1</type>
             <admin-state>In-Service</admin-state>
-        </double-list>
-        <double-list>
+        </double-key-list>
+        <double-key-list>
             <name>1010/0/AD-2-RX</name>
             <type>opticalTransport</type>
             <admin-state>In-Service</admin-state>
-        </double-list>
-        <double-list>
+        </double-key-list>
+        <double-key-list>
             <name>1010/0/[AD 4-11]-1-RX</name>
             <type>opticalTransport</type>
             <admin-state>In-Service</admin-state>
-        </double-list>
-        <double-list>
+        </double-key-list>
+        <double-key-list>
             <name>ab[cd</name>
             <type>opticalTransport</type>
             <admin-state>In-Service</admin-state>
-        </double-list>
+        </double-key-list>
     </gnmi-tools>
 </config>

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;}

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:

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

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