Merge branch 'trac967'

src/lib/config/tests/testdata/Makefile.am

@@ -51,3 +51,5 @@ EXTRA_DIST += spec26.spec
 EXTRA_DIST += spec27.spec
 EXTRA_DIST += spec28.spec
 EXTRA_DIST += spec29.spec
+EXTRA_DIST += spec30.spec
+EXTRA_DIST += spec31.spec

src/lib/config/tests/testdata/spec30.spec

@@ -0,0 +1,45 @@
+{
+    "module_spec": {
+        "module_name": "lists",
+        "module_description": "Logging options",
+        "config_data": [
+            {
+                "item_name": "first_list_items",
+                "item_type": "list",
+                "item_optional": false,
+                "item_default": [],
+                "list_item_spec": {
+                  "item_name": "first_list_item",
+                  "item_type": "map",
+                  "item_optional": false,
+                  "item_default": {},
+                  "map_item_spec": [
+                  {  "item_name": "foo",
+                     "item_type": "string",
+                     "item_optional": false,
+                     "item_default": "foo"
+                  },
+                  { "item_name": "second_list_items",
+                    "item_type": "list",
+                    "item_optional": false,
+                    "item_default": [],
+                    "list_item_spec": {
+                      "item_name": "second_list_item",
+                      "item_type": "map",
+                      "item_optional": false,
+                      "item_default": {},
+                      "map_item_spec": [
+                      { "item_name": "final_element",
+                        "item_type": "string",
+                        "item_optional": false,
+                        "item_default": "hello"
+                      }
+                      ]
+                    }
+                  }
+                  ]
+                }
+            }
+        ]
+    }
+}

src/lib/config/tests/testdata/spec31.spec

@@ -0,0 +1,63 @@
+{
+    "module_spec": {
+        "module_name": "lists",
+        "module_description": "Logging options",
+        "config_data": [
+            {
+                "item_name": "first_list_items",
+                "item_type": "list",
+                "item_optional": false,
+                "item_default": [],
+                "list_item_spec": {
+                  "item_name": "first_list_item",
+                  "item_type": "map",
+                  "item_optional": false,
+                  "item_default": {},
+                  "map_item_spec": [
+                  {  "item_name": "foo",
+                     "item_type": "string",
+                     "item_optional": false,
+                     "item_default": "foo"
+                  },
+                  { "item_name": "second_list_items",
+                    "item_type": "list",
+                    "item_optional": false,
+                    "item_default": [],
+                    "list_item_spec": {
+                      "item_name": "second_list_item",
+                      "item_type": "map",
+                      "item_optional": false,
+                      "item_default": {},
+                      "map_item_spec": [
+                      { "item_name": "map_element",
+                        "item_type": "map",
+                        "item_optional": false,
+                        "item_default": {},
+                        "map_item_spec": [
+                        { "item_name": "list1",
+                          "item_type": "list",
+                          "item_optional": false,
+                          "item_default": [],
+                          "list_item_spec":
+                          { "item_name": "list2",
+                            "item_type": "list",
+                            "item_optional": false,
+                            "item_default": [],
+                            "list_item_spec":
+                            { "item_name": "number",
+                              "item_type": "integer",
+                              "item_optional": false,
+                              "item_default": 1
+                            }
+                          }
+                        }]
+                      }
+                      ]
+                    }
+                  }
+                  ]
+                }
+            }
+        ]
+    }
+}

src/lib/python/isc/config/config_data.py

@@ -108,6 +108,52 @@ def convert_type(spec_part, value):
     except TypeError as err:
         raise isc.cc.data.DataTypeError(str(err))
 
+def _get_map_or_list(spec_part):
+    """Returns the list or map specification if this is a list or a
+       map specification part. If not, returns the given spec_part
+       itself"""
+    if "map_item_spec" in spec_part:
+        return spec_part["map_item_spec"]
+    elif "list_item_spec" in spec_part:
+        return spec_part["list_item_spec"]
+    else:
+        return spec_part
+
+def _find_spec_part_single(cur_spec, id_part):
+    """Find the spec part for the given (partial) name. This partial
+       name does not contain separators ('/'), and the specification
+       part should be a direct child of the given specification part.
+       id_part may contain list selectors, which will be ignored.
+       Returns the child part.
+       Raises DataNotFoundError if it was not found."""
+    # strip list selector part
+    # don't need it for the spec part, so just drop it
+    id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
+
+    # The specification we want a sub-part for should be either a
+    # list or a map, which is internally represented by a dict with
+    # an element 'map_item_spec', a dict with an element 'list_item_spec',
+    # or a list (when it is the 'main' config_data element of a module).
+    if type(cur_spec) == dict and 'map_item_spec' in cur_spec.keys():
+        for cur_spec_item in cur_spec['map_item_spec']:
+            if cur_spec_item['item_name'] == id:
+                return cur_spec_item
+        # not found
+        raise isc.cc.data.DataNotFoundError(id + " not found")
+    elif type(cur_spec) == dict and 'list_item_spec' in cur_spec.keys():
+        if cur_spec['item_name'] == id:
+            return cur_spec['list_item_spec']
+        # not found
+        raise isc.cc.data.DataNotFoundError(id + " not found")
+    elif type(cur_spec) == list:
+        for cur_spec_item in cur_spec:
+            if cur_spec_item['item_name'] == id:
+                return cur_spec_item
+        # not found
+        raise isc.cc.data.DataNotFoundError(id + " not found")
+    else:
+        raise isc.cc.data.DataNotFoundError("Not a correct config specification")
+
 def find_spec_part(element, identifier):
     """find the data definition for the given identifier
        returns either a map with 'item_name' etc, or a list of those"""
@@ -117,38 +163,15 @@ def find_spec_part(element, identifier):
     id_parts[:] = (value for value in id_parts if value != "")
     cur_el = element
 
-    for id_part in id_parts:
-        # strip list selector part
-        # don't need it for the spec part, so just drop it
-        id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
-        # is this part still needed? (see below)
-        if type(cur_el) == dict and 'map_item_spec' in cur_el.keys():
-            found = False
-            for cur_el_item in cur_el['map_item_spec']:
-                if cur_el_item['item_name'] == id:
-                    cur_el = cur_el_item
-                    found = True
-            if not found:
-                raise isc.cc.data.DataNotFoundError(id + " not found")
-        elif type(cur_el) == dict and 'list_item_spec' in cur_el.keys():
-            cur_el = cur_el['list_item_spec']
-        elif type(cur_el) == list:
-            found = False
-            for cur_el_item in cur_el:
-                if cur_el_item['item_name'] == id:
-                    cur_el = cur_el_item
-                    # if we need to go further, we may need to 'skip' a step here
-                    # but not if we're done
-                    if id_parts[-1] != id_part and type(cur_el) == dict:
-                        if "map_item_spec" in cur_el:
-                            cur_el = cur_el["map_item_spec"]
-                        elif "list_item_spec" in cur_el:
-                            cur_el = cur_el["list_item_spec"]
-                    found = True
-            if not found:
-                raise isc.cc.data.DataNotFoundError(id + " not found")
-        else:
-            raise isc.cc.data.DataNotFoundError("Not a correct config specification")
+    # up to the last element, if the result is a map or a list,
+    # we want its subspecification (i.e. list_item_spec or
+    # map_item_spec). For the last element in the identifier we
+    # always want the 'full' spec of the item
+    for id_part in id_parts[:-1]:
+        cur_el = _find_spec_part_single(cur_el, id_part)
+        cur_el = _get_map_or_list(cur_el)
+
+    cur_el = _find_spec_part_single(cur_el, id_parts[-1])
     return cur_el
 
 def spec_name_list(spec, prefix="", recurse=False):

src/lib/python/isc/config/tests/config_data_test.py

@@ -329,6 +329,35 @@ class TestMultiConfigData(unittest.TestCase):
         spec_part = self.mcd.find_spec_part("Spec2/item1")
         self.assertEqual({'item_name': 'item1', 'item_type': 'integer', 'item_optional': False, 'item_default': 1, }, spec_part)
 
+    def test_find_spec_part_nested(self):
+        module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec30.spec")
+        self.mcd.set_specification(module_spec)
+        spec_part = self.mcd.find_spec_part("/lists/first_list_items[0]/second_list_items[1]/final_element")
+        self.assertEqual({'item_name': 'final_element', 'item_type': 'string', 'item_default': 'hello', 'item_optional': False}, spec_part)
+        spec_part = self.mcd.find_spec_part("/BAD_NAME/first_list_items[0]/second_list_items[1]/final_element")
+        self.assertEqual(None, spec_part)
+
+    def test_find_spec_part_nested2(self):
+        module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec31.spec")
+        self.mcd.set_specification(module_spec)
+        spec_part = self.mcd.find_spec_part("/lists/first_list_items[0]/second_list_items[1]/map_element/list1[1]/list2[2]")
+        self.assertEqual({"item_name": "number", "item_type": "integer", "item_optional": False, "item_default": 1}, spec_part)
+
+        spec_part = self.mcd.find_spec_part("/DOESNOTEXIST")
+        self.assertEqual(None, spec_part)
+        spec_part = self.mcd.find_spec_part("/lists/DOESNOTEXIST")
+        self.assertEqual(None, spec_part)
+        spec_part = self.mcd.find_spec_part("/lists/first_list_items[0]/DOESNOTEXIST")
+        self.assertEqual(None, spec_part)
+        spec_part = self.mcd.find_spec_part("/lists/first_list_items[0]/second_list_items[1]/DOESNOTEXIST")
+        self.assertEqual(None, spec_part)
+        spec_part = self.mcd.find_spec_part("/lists/first_list_items[0]/second_list_items[1]/map_element/DOESNOTEXIST")
+        self.assertEqual(None, spec_part)
+        spec_part = self.mcd.find_spec_part("/lists/first_list_items[0]/second_list_items[1]/map_element/list1[1]/DOESNOTEXIST")
+        self.assertEqual(None, spec_part)
+        spec_part = self.mcd.find_spec_part("/lists/first_list_items[0]/second_list_items[1]/map_element/list1[1]/list2[1]/DOESNOTEXIST")
+        self.assertEqual(None, spec_part)
+
     def test_get_current_config(self):
         cf = { 'module1': { 'item1': 2, 'item2': True } }
         self.mcd._set_current_config(cf);