Merge branch 'trac926'

src/bin/bindctl/bindcmd.py

@@ -398,6 +398,8 @@ class BindCmdInterpreter(Cmd):
                 print("Error: " + str(dte))
             except isc.cc.data.DataNotFoundError as dnfe:
                 print("Error: " + str(dnfe))
+            except isc.cc.data.DataAlreadyPresentError as dape:
+                print("Error: " + str(dape))
             except KeyError as ke:
                 print("Error: missing " + str(ke))
         else:
@@ -634,7 +636,15 @@ class BindCmdInterpreter(Cmd):
                     # we have more data to show
                     line += "/"
                 else:
-                    line += "\t" + json.dumps(value_map['value'])
+                    # if type is named_set, don't print value if None
+                    # (it is either {} meaning empty, or None, meaning
+                    # there actually is data, but not to be shown with
+                    # the current command
+                    if value_map['type'] == 'named_set' and\
+                       value_map['value'] is None:
+                        line += "/\t"
+                    else:
+                        line += "\t" + json.dumps(value_map['value'])
                 line += "\t" + value_map['type']
                 line += "\t"
                 if value_map['default']:
@@ -649,10 +659,9 @@ class BindCmdInterpreter(Cmd):
                 data, default = self.config_data.get_value(identifier)
                 print(json.dumps(data))
         elif cmd.command == "add":
-            if 'value' in cmd.params:
-                self.config_data.add_value(identifier, cmd.params['value'])
-            else:
-                self.config_data.add_value(identifier)
+            self.config_data.add_value(identifier,
+                                       cmd.params.get('value_or_name'),
+                                       cmd.params.get('value_for_set'))
         elif cmd.command == "remove":
             if 'value' in cmd.params:
                 self.config_data.remove_value(identifier, cmd.params['value'])
@@ -679,7 +688,7 @@ class BindCmdInterpreter(Cmd):
             except isc.config.ModuleCCSessionError as mcse:
                 print(str(mcse))
         elif cmd.command == "diff":
-            print(self.config_data.get_local_changes());
+            print(self.config_data.get_local_changes())
         elif cmd.command == "go":
             self.go(identifier)
 

src/bin/bindctl/bindctl_main.py.in

@@ -50,17 +50,28 @@ def prepare_config_commands(tool):
     cmd.add_param(param)
     module.add_command(cmd)
 
-    cmd = CommandInfo(name = "add", desc = "Add an entry to configuration list. If no value is given, a default value is added.")
+    cmd = CommandInfo(name = "add", desc =
+        "Add an entry to configuration list or a named set. "
+        "When adding to a list, the command has one optional argument, "
+        "a value to add to the list. The value must be in correct JSON "
+        "and complete. When adding to a named set, it has one "
+        "mandatory parameter (the name to add), and an optional "
+        "parameter value, similar to when adding to a list. "
+        "In either case, when no value is given, an entry will be "
+        "constructed with default values.")
     param = ParamInfo(name = "identifier", type = "string", optional=True, desc = DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
-    param = ParamInfo(name = "value", type = "string", optional=True, desc = "Specifies a value to add to the list. It must be in correct JSON format and complete.")
+    param = ParamInfo(name = "value_or_name", type = "string", optional=True, desc = "Specifies a value to add to the list, or the name when adding to a named set. It must be in correct JSON format and complete.")
+    cmd.add_param(param)
+    module.add_command(cmd)
+    param = ParamInfo(name = "value_for_set", type = "string", optional=True, desc = "Specifies an optional value to add to the named map. It must be in correct JSON format and complete.")
     cmd.add_param(param)
     module.add_command(cmd)
 
-    cmd = CommandInfo(name = "remove", desc = "Remove entry from configuration list.")
+    cmd = CommandInfo(name = "remove", desc = "Remove entry from configuration list or named set.")
     param = ParamInfo(name = "identifier", type = "string", optional=True, desc = DEFAULT_IDENTIFIER_DESC)
     cmd.add_param(param)
-    param = ParamInfo(name = "value", type = "string", optional=True, desc = "Specifies a value to remove from the list. It must be in correct JSON format and complete.")
+    param = ParamInfo(name = "value", type = "string", optional=True, desc = "When identifier is a list, specifies a value to remove from the list. It must be in correct JSON format and complete. When it is a named set, specifies the name to remove.")
     cmd.add_param(param)
     module.add_command(cmd)
 

src/lib/cc/data.cc

@@ -511,6 +511,8 @@ Element::nameToType(const std::string& type_name) {
         return (Element::list);
     } else if (type_name == "map") {
         return (Element::map);
+    } else if (type_name == "named_set") {
+        return (Element::map);
     } else if (type_name == "null") {
         return (Element::null);
     } else if (type_name == "any") {

src/lib/config/module_spec.cc

@@ -67,10 +67,13 @@ check_config_item(ConstElementPtr spec) {
         check_leaf_item(spec, "list_item_spec", Element::map, true);
         check_config_item(spec->get("list_item_spec"));
     }
-    // todo: add stuff for type map
-    if (Element::nameToType(spec->get("item_type")->stringValue()) == Element::map) {
+
+    if (spec->get("item_type")->stringValue() == "map") {
         check_leaf_item(spec, "map_item_spec", Element::list, true);
         check_config_item_list(spec->get("map_item_spec"));
+    } else if (spec->get("item_type")->stringValue() == "named_set") {
+        check_leaf_item(spec, "named_set_item_spec", Element::map, true);
+        check_config_item(spec->get("named_set_item_spec"));
     }
 }
 
@@ -286,7 +289,8 @@ check_type(ConstElementPtr spec, ConstElementPtr element) {
             return (cur_item_type == "list");
             break;
         case Element::map:
-            return (cur_item_type == "map");
+            return (cur_item_type == "map" ||
+                    cur_item_type == "named_set");
             break;
     }
     return (false);
@@ -323,8 +327,20 @@ ModuleSpec::validateItem(ConstElementPtr spec, ConstElementPtr data,
         }
     }
     if (data->getType() == Element::map) {
-        if (!validateSpecList(spec->get("map_item_spec"), data, full, errors)) {
-            return (false);
+        // either a normal 'map' or a 'named set' (determined by which
+        // subspecification it has)
+        if (spec->contains("map_item_spec")) {
+            if (!validateSpecList(spec->get("map_item_spec"), data, full, errors)) {
+                return (false);
+            }
+        } else {
+            typedef std::pair<std::string, ConstElementPtr> maptype;
+
+            BOOST_FOREACH(maptype m, data->mapValue()) {
+                if (!validateItem(spec->get("named_set_item_spec"), m.second, full, errors)) {
+                    return (false);
+                }
+            }
         }
     }
     return (true);

src/lib/config/tests/module_spec_unittests.cc

@@ -211,3 +211,12 @@ TEST(ModuleSpec, CommandValidation) {
     EXPECT_EQ(errors->get(0)->stringValue(), "Type mismatch");
 
 }
+
+TEST(ModuleSpec, NamedSetValidation) {
+    ModuleSpec dd = moduleSpecFromFile(specfile("spec32.spec"));
+
+    ElementPtr errors = Element::createList();
+    EXPECT_TRUE(dataTestWithErrors(dd, "data32_1.data", errors));
+    EXPECT_FALSE(dataTest(dd, "data32_2.data"));
+    EXPECT_FALSE(dataTest(dd, "data32_3.data"));
+}

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

@@ -22,6 +22,9 @@ EXTRA_DIST += data22_7.data
 EXTRA_DIST += data22_8.data
 EXTRA_DIST += data22_9.data
 EXTRA_DIST += data22_10.data
+EXTRA_DIST += data32_1.data
+EXTRA_DIST += data32_2.data
+EXTRA_DIST += data32_3.data
 EXTRA_DIST += spec1.spec
 EXTRA_DIST += spec2.spec
 EXTRA_DIST += spec3.spec
@@ -53,3 +56,4 @@ EXTRA_DIST += spec28.spec
 EXTRA_DIST += spec29.spec
 EXTRA_DIST += spec30.spec
 EXTRA_DIST += spec31.spec
+EXTRA_DIST += spec32.spec

src/lib/config/tests/testdata/data32_1.data

@@ -0,0 +1,3 @@
+{
+    "named_set_item": { "foo": 1, "bar": 2 }
+}

src/lib/config/tests/testdata/data32_2.data

@@ -0,0 +1,3 @@
+{
+    "named_set_item": { "foo": "wrongtype", "bar": 2 }
+}

src/lib/config/tests/testdata/data32_3.data

@@ -0,0 +1,3 @@
+{
+    "named_set_item": []
+}

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

@@ -0,0 +1,19 @@
+{
+  "module_spec": {
+    "module_name": "Spec32",
+    "config_data": [
+      { "item_name": "named_set_item",
+        "item_type": "named_set",
+        "item_optional": false,
+        "item_default": { "a": 1, "b": 2 },
+        "named_set_item_spec": {
+          "item_name": "named_set_element",
+          "item_type": "integer",
+          "item_optional": false,
+          "item_default": 3
+        }
+      }
+    ]
+  }
+}
+

src/lib/python/isc/cc/data.py

@@ -22,8 +22,22 @@
 
 import json
 
-class DataNotFoundError(Exception): pass
-class DataTypeError(Exception): pass
+class DataNotFoundError(Exception):
+    """Raised if an identifier does not exist according to a spec file,
+       or if an item is addressed that is not in the current (or default)
+       config (such as a nonexistent list or map element)"""
+    pass
+
+class DataAlreadyPresentError(Exception):
+    """Raised if there is an attemt to add an element to a list or a
+       map that is already present in that list or map (i.e. if 'add'
+       is used when it should be 'set')"""
+    pass
+
+class DataTypeError(Exception):
+    """Raised if there is an attempt to set an element that is of a
+       different type than the type specified in the specification."""
+    pass
 
 def remove_identical(a, b):
     """Removes the values from dict a that are the same as in dict b.

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

@@ -312,7 +312,7 @@ class ModuleCCSession(ConfigData):
         module_spec = isc.config.module_spec_from_file(spec_file_name)
         module_cfg = ConfigData(module_spec)
         module_name = module_spec.get_module_name()
-        self._session.group_subscribe(module_name);
+        self._session.group_subscribe(module_name)
 
         # Get the current config for that module now
         seq = self._session.group_sendmsg(create_command(COMMAND_GET_CONFIG, { "module_name": module_name }), "ConfigManager")
@@ -327,7 +327,7 @@ class ModuleCCSession(ConfigData):
             rcode, value = parse_answer(answer)
             if rcode == 0:
                 if value != None and module_spec.validate_config(False, value):
-                    module_cfg.set_local_config(value);
+                    module_cfg.set_local_config(value)
                     if config_update_callback is not None:
                         config_update_callback(value, module_cfg)
 
@@ -377,7 +377,7 @@ class ModuleCCSession(ConfigData):
                         if self.get_module_spec().validate_config(False,
                                                                   value,
                                                                   errors):
-                            self.set_local_config(value);
+                            self.set_local_config(value)
                             if self._config_handler:
                                 self._config_handler(value)
                         else:
@@ -414,8 +414,8 @@ class UIModuleCCSession(MultiConfigData):
             self.set_specification(isc.config.ModuleSpec(specs[module]))
 
     def update_specs_and_config(self):
-        self.request_specifications();
-        self.request_current_config();
+        self.request_specifications()
+        self.request_current_config()
 
     def request_current_config(self):
         """Requests the current configuration from the configuration
@@ -425,47 +425,90 @@ class UIModuleCCSession(MultiConfigData):
             raise ModuleCCSessionError("Bad config version")
         self._set_current_config(config)
 
-
-    def add_value(self, identifier, value_str = None):
-        """Add a value to a configuration list. Raises a DataTypeError
-           if the value does not conform to the list_item_spec field
-           of the module config data specification. If value_str is
-           not given, we add the default as specified by the .spec
-           file."""
-        module_spec = self.find_spec_part(identifier)
-        if (type(module_spec) != dict or "list_item_spec" not in module_spec):
-            raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list")
-
+    def _add_value_to_list(self, identifier, value):
         cur_list, status = self.get_value(identifier)
         if not cur_list:
             cur_list = []
 
-        # Hmm. Do we need to check for duplicates?
-        value = None
-        if value_str is not None:
-            value = isc.cc.data.parse_value_str(value_str)
-        else:
+        if value is None:
             if "item_default" in module_spec["list_item_spec"]:
                 value = module_spec["list_item_spec"]["item_default"]
 
         if value is None:
-            raise isc.cc.data.DataNotFoundError("No value given and no default for " + str(identifier))
-            
+            raise isc.cc.data.DataNotFoundError(
+                "No value given and no default for " + str(identifier))
+
         if value not in cur_list:
             cur_list.append(value)
             self.set_value(identifier, cur_list)
+        else:
+            raise isc.cc.data.DataAlreadyPresentError(value +
+                                                      " already in "
+                                                      + identifier)
+
+    def _add_value_to_named_set(self, identifier, value, item_value):
+        if type(value) != str:
+            raise isc.cc.data.DataTypeError("Name for named_set " +
+                                            identifier +
+                                            " must be a string")
+        # fail on both None and empty string
+        if not value:
+            raise isc.cc.data.DataNotFoundError(
+                    "Need a name to add a new item to named_set " +
+                    str(identifier))
+        else:
+            cur_map, status = self.get_value(identifier)
+            if not cur_map:
+                cur_map = {}
+            if value not in cur_map:
+                cur_map[value] = item_value
+                self.set_value(identifier, cur_map)
+            else:
+                raise isc.cc.data.DataAlreadyPresentError(value +
+                                                          " already in "
+                                                          + identifier)
 
-    def remove_value(self, identifier, value_str):
-        """Remove a value from a configuration list. The value string
-           must be a string representation of the full item. Raises
-           a DataTypeError if the value at the identifier is not a list,
-           or if the given value_str does not match the list_item_spec
-           """
+    def add_value(self, identifier, value_str = None, set_value_str = None):
+        """Add a value to a configuration list. Raises a DataTypeError
+           if the value does not conform to the list_item_spec field
+           of the module config data specification. If value_str is
+           not given, we add the default as specified by the .spec
+           file. Raises a DataNotFoundError if the given identifier
+           is not specified in the specification as a map or list.
+           Raises a DataAlreadyPresentError if the specified element
+           already exists."""
         module_spec = self.find_spec_part(identifier)
-        if (type(module_spec) != dict or "list_item_spec" not in module_spec):
-            raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list")
+        if module_spec is None:
+            raise isc.cc.data.DataNotFoundError("Unknown item " + str(identifier))
+
+        # the specified element must be a list or a named_set
+        if 'list_item_spec' in module_spec:
+            value = None
+            # in lists, we might get the value with spaces, making it
+            # the third argument. In that case we interpret both as
+            # one big string meant as the value
+            if value_str is not None:
+                if set_value_str is not None:
+                    value_str += set_value_str
+                value = isc.cc.data.parse_value_str(value_str)
+            self._add_value_to_list(identifier, value)
+        elif 'named_set_item_spec' in module_spec:
+            item_name = None
+            item_value = None
+            if value_str is not None:
+                item_name =  isc.cc.data.parse_value_str(value_str)
+            if set_value_str is not None:
+                item_value = isc.cc.data.parse_value_str(set_value_str)
+            else:
+                if 'item_default' in module_spec['named_set_item_spec']:
+                    item_value = module_spec['named_set_item_spec']['item_default']
+            self._add_value_to_named_set(identifier, item_name,
+                                         item_value)
+        else:
+            raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list or a named set")
 
-        if value_str is None:
+    def _remove_value_from_list(self, identifier, value):
+        if value is None:
             # we are directly removing an list index
             id, list_indices = isc.cc.data.split_identifier_list_indices(identifier)
             if list_indices is None:
@@ -473,17 +516,52 @@ class UIModuleCCSession(MultiConfigData):
             else:
                 self.set_value(identifier, None)
         else:
-            value = isc.cc.data.parse_value_str(value_str)
-            isc.config.config_data.check_type(module_spec, [value])
             cur_list, status = self.get_value(identifier)
-            #if not cur_list:
-            #    cur_list = isc.cc.data.find_no_exc(self.config.data, identifier)
             if not cur_list:
                 cur_list = []
-            if value in cur_list:
+            elif value in cur_list:
                 cur_list.remove(value)
             self.set_value(identifier, cur_list)
 
+    def _remove_value_from_named_set(self, identifier, value):
+        if value is None:
+            raise isc.cc.data.DataNotFoundError("Need a name to remove an item from named_set " + str(identifier))
+        elif type(value) != str:
+            raise isc.cc.data.DataTypeError("Name for named_set " + identifier + " must be a string")
+        else:
+            cur_map, status = self.get_value(identifier)
+            if not cur_map:
+                cur_map = {}
+            if value in cur_map:
+                del cur_map[value]
+            else:
+                raise isc.cc.data.DataNotFoundError(value + " not found in named_set " + str(identifier))
+
+    def remove_value(self, identifier, value_str):
+        """Remove a value from a configuration list or named set.
+        The value string must be a string representation of the full
+        item. Raises a DataTypeError if the value at the identifier
+        is not a list, or if the given value_str does not match the
+        list_item_spec """
+        module_spec = self.find_spec_part(identifier)
+        if module_spec is None:
+            raise isc.cc.data.DataNotFoundError("Unknown item " + str(identifier))
+
+        value = None
+        if value_str is not None:
+            value = isc.cc.data.parse_value_str(value_str)
+
+        if 'list_item_spec' in module_spec:
+            if value is not None:
+                isc.config.config_data.check_type(module_spec['list_item_spec'], value)
+            self._remove_value_from_list(identifier, value)
+        elif 'named_set_item_spec' in module_spec:
+            self._remove_value_from_named_set(identifier, value)
+        else:
+            raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list or a named_set")
+
+
+
     def commit(self):
         """Commit all local changes, send them through b10-cmdctl to
            the configuration manager"""

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

@@ -145,6 +145,8 @@ def _find_spec_part_single(cur_spec, id_part):
             return cur_spec['list_item_spec']
         # not found
         raise isc.cc.data.DataNotFoundError(id + " not found")
+    elif type(cur_spec) == dict and 'named_set_item_spec' in cur_spec.keys():
+        return cur_spec['named_set_item_spec']
     elif type(cur_spec) == list:
         for cur_spec_item in cur_spec:
             if cur_spec_item['item_name'] == id:
@@ -191,11 +193,14 @@ def spec_name_list(spec, prefix="", recurse=False):
                     result.extend(spec_name_list(map_el['map_item_spec'], prefix + map_el['item_name'], recurse))
                 else:
                     result.append(prefix + name)
+        elif 'named_set_item_spec' in spec:
+            # we added a '/' above, but in this one case we don't want it
+            result.append(prefix[:-1])
         else:
             for name in spec:
                 result.append(prefix + name + "/")
                 if recurse:
-                    result.extend(spec_name_list(spec[name],name, recurse))
+                    result.extend(spec_name_list(spec[name], name, recurse))
     elif type(spec) == list:
         for list_el in spec:
             if 'item_name' in list_el:
@@ -207,7 +212,7 @@ def spec_name_list(spec, prefix="", recurse=False):
             else:
                 raise ConfigDataError("Bad specification")
     else:
-        raise ConfigDataError("Bad specication")
+        raise ConfigDataError("Bad specification")
     return result
 
 class ConfigData:
@@ -255,7 +260,7 @@ class ConfigData:
 
     def get_local_config(self):
         """Returns the non-default config values in a dict"""
-        return self.data;
+        return self.data
 
     def get_item_list(self, identifier = None, recurse = False):
         """Returns a list of strings containing the full identifiers of
@@ -412,7 +417,39 @@ class MultiConfigData:
                 item_id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
                 id_list = module + "/" + id_prefix + "/" + item_id
                 id_prefix += "/" + id_part
-                if list_indices is not None:
+                part_spec = find_spec_part(self._specifications[module].get_config_spec(), id_prefix)
+                if part_spec['item_type'] == 'named_set':
+                    # For named sets, the identifier is partly defined
+                    # by which values are actually present, and not
+                    # purely by the specification.
+                    # So if there is a part of the identifier left,
+                    # we need to look up the value, then see if that
+                    # contains the next part of the identifier we got
+                    if len(id_parts) == 0:
+                        if 'item_default' in part_spec:
+                            return part_spec['item_default']
+                        else:
+                            return None
+                    id_part = id_parts.pop(0)
+
+                    named_set_value, type = self.get_value(id_list)
+                    if id_part in named_set_value:
+                        if len(id_parts) > 0:
+                            # we are looking for the *default* value.
+                            # so if not present in here, we need to
+                            # lookup the one from the spec
+                            rest_of_id = "/".join(id_parts)
+                            result = isc.cc.data.find_no_exc(named_set_value[id_part], rest_of_id)
+                            if result is None:
+                                spec_part = self.find_spec_part(identifier)
+                                if 'item_default' in spec_part:
+                                    return spec_part['item_default']
+                            return result
+                        else:
+                            return named_set_value[id_part]
+                    else:
+                        return None
+                elif list_indices is not None:
                     # there's actually two kinds of default here for
                     # lists; they can have a default value (like an
                     # empty list), but their elements can  also have
@@ -449,7 +486,12 @@ class MultiConfigData:
                     
             spec = find_spec_part(self._specifications[module].get_config_spec(), id)
             if 'item_default' in spec:
-                return spec['item_default']
+                # one special case, named_set
+                if spec['item_type'] == 'named_set':
+                    print("is " + id_part + " in named set?")
+                    return spec['item_default']
+                else:
+                    return spec['item_default']
             else:
                 return None
 
@@ -493,7 +535,7 @@ class MultiConfigData:
                 spec_part_list = spec_part['list_item_spec']
                 list_value, status = self.get_value(identifier)
                 if list_value is None:
-                    raise isc.cc.data.DataNotFoundError(identifier)
+                    raise isc.cc.data.DataNotFoundError(identifier + " not found")
 
                 if type(list_value) != list:
                     # the identifier specified a single element
@@ -509,12 +551,38 @@ class MultiConfigData:
                         for i in range(len(list_value)):
                             self._append_value_item(result, spec_part_list, "%s[%d]" % (identifier, i), all)
             elif item_type == "map":
+                value, status = self.get_value(identifier)
                 # just show the specific contents of a map, we are
                 # almost never interested in just its name
                 spec_part_map = spec_part['map_item_spec']
                 self._append_value_item(result, spec_part_map, identifier, all)
+            elif item_type == "named_set":
+                value, status = self.get_value(identifier)
+
+                # show just the one entry, when either the map is empty,
+                # or when this is element is not requested specifically
+                if len(value.keys()) == 0:
+                    entry = _create_value_map_entry(identifier,
+                                                    item_type,
+                                                    {}, status)
+                    result.append(entry)
+                elif not first and not all:
+                    entry = _create_value_map_entry(identifier,
+                                                    item_type,
+                                                    None, status)
+                    result.append(entry)
+                else:
+                    spec_part_named_set = spec_part['named_set_item_spec']
+                    for entry in value:
+                        self._append_value_item(result,
+                                                spec_part_named_set,
+                                                identifier + "/" + entry,
+                                                all)
             else:
                 value, status = self.get_value(identifier)
+                if status == self.NONE and not spec_part['item_optional']:
+                    raise isc.cc.data.DataNotFoundError(identifier + " not found")
+
                 entry = _create_value_map_entry(identifier,
                                                 item_type,
                                                 value, status)
@@ -569,7 +637,7 @@ class MultiConfigData:
                     spec_part = spec_part['list_item_spec']
                 check_type(spec_part, value)
         else:
-            raise isc.cc.data.DataNotFoundError(identifier)
+            raise isc.cc.data.DataNotFoundError(identifier + " not found")
 
         # Since we do not support list diffs (yet?), we need to
         # copy the currently set list of items to _local_changes
@@ -579,15 +647,50 @@ class MultiConfigData:
         cur_id_part = '/'
         for id_part in id_parts:
             id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
+            cur_value, status = self.get_value(cur_id_part + id)
+            # Check if the value was there in the first place
+            if status == MultiConfigData.NONE and cur_id_part != "/":
+                raise isc.cc.data.DataNotFoundError(id_part +
+                                                    " not found in " +
+                                                    cur_id_part)
             if list_indices is not None:
-                cur_list, status = self.get_value(cur_id_part + id)
+                # And check if we don't set something outside of any
+                # list
+                cur_list = cur_value
+                for list_index in list_indices:
+                    if list_index >= len(cur_list):
+                        raise isc.cc.data.DataNotFoundError("No item " +
+                                  str(list_index) + " in " + id_part)
+                    else:
+                        cur_list = cur_list[list_index]
                 if status != MultiConfigData.LOCAL:
                     isc.cc.data.set(self._local_changes,
                                     cur_id_part + id,
-                                    cur_list)
+                                    cur_value)
             cur_id_part = cur_id_part + id_part + "/"
         isc.cc.data.set(self._local_changes, identifier, value)
- 
+
+    def _get_list_items(self, item_name):
+        """This method is used in get_config_item_list, to add list
+           indices and named_set names to the completion list. If
+           the given item_name is for a list or named_set, it'll
+           return a list of those (appended to item_name), otherwise
+           the list will only contain the item_name itself."""
+        spec_part = self.find_spec_part(item_name)
+        if 'item_type' in spec_part and \
+           spec_part['item_type'] == 'named_set':
+            subslash = ""
+            if spec_part['named_set_item_spec']['item_type'] == 'map' or\
+               spec_part['named_set_item_spec']['item_type'] == 'named_set':
+                subslash = "/"
+            values, status = self.get_value(item_name)
+            if len(values) > 0:
+                return [ item_name + "/" + v + subslash for v in values.keys() ]
+            else:
+                return [ item_name ]
+        else:
+            return [ item_name ]
+
     def get_config_item_list(self, identifier = None, recurse = False):
         """Returns a list of strings containing the item_names of
            the child items at the given identifier. If no identifier is
@@ -598,7 +701,11 @@ class MultiConfigData:
             if identifier.startswith("/"):
                 identifier = identifier[1:]
             spec = self.find_spec_part(identifier)
-            return spec_name_list(spec, identifier + "/", recurse)
+            spec_list = spec_name_list(spec, identifier + "/", recurse)
+            result_list = []
+            for spec_name in spec_list:
+                result_list.extend(self._get_list_items(spec_name))
+            return result_list
         else:
             if recurse:
                 id_list = []

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

@@ -229,7 +229,7 @@ def _check_item_spec(config_item):
     item_type = config_item["item_type"]
     if type(item_type) != str:
         raise ModuleSpecError("item_type in " + item_name + " is not a string: " + str(type(item_type)))
-    if item_type not in ["integer", "real", "boolean", "string", "list", "map", "any"]:
+    if item_type not in ["integer", "real", "boolean", "string", "list", "map", "named_set", "any"]:
         raise ModuleSpecError("unknown item_type in " + item_name + ": " + item_type)
     if "item_optional" in config_item:
         if type(config_item["item_optional"]) != bool:
@@ -293,6 +293,10 @@ def _validate_type(spec, value, errors):
         if errors != None:
             errors.append(str(value) + " should be a map")
         return False
+    elif data_type == "named_set" and type(value) != dict:
+        if errors != None:
+            errors.append(str(value) + " should be a map")
+        return False
     else:
         return True
 
@@ -308,8 +312,16 @@ def _validate_item(spec, full, data, errors):
                 if not _validate_item(list_spec, full, data_el, errors):
                     return False
     elif type(data) == dict:
-        if not _validate_spec_list(spec['map_item_spec'], full, data, errors):
-            return False
+        if 'map_item_spec' in spec:
+            if not _validate_spec_list(spec['map_item_spec'], full, data, errors):
+                return False
+        else:
+            named_set_spec = spec['named_set_item_spec']
+            for data_el in data.values():
+                if not _validate_type(named_set_spec, data_el, errors):
+                    return False
+                if not _validate_item(named_set_spec, full, data_el, errors):
+                    return False
     return True
 
 def _validate_spec(spec, full, data, errors):

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

@@ -695,6 +695,12 @@ class TestUIModuleCCSession(unittest.TestCase):
         fake_conn.set_get_answer('/config_data', { 'version': BIND10_CONFIG_DATA_VERSION })
         return UIModuleCCSession(fake_conn)
 
+    def create_uccs_named_set(self, fake_conn):
+        module_spec = isc.config.module_spec_from_file(self.spec_file("spec32.spec"))
+        fake_conn.set_get_answer('/module_spec', { module_spec.get_module_name(): module_spec.get_full_spec()})
+        fake_conn.set_get_answer('/config_data', { 'version': BIND10_CONFIG_DATA_VERSION })
+        return UIModuleCCSession(fake_conn)
+
     def test_init(self):
         fake_conn = fakeUIConn()
         fake_conn.set_get_answer('/module_spec', {})
@@ -715,12 +721,14 @@ class TestUIModuleCCSession(unittest.TestCase):
     def test_add_remove_value(self):
         fake_conn = fakeUIConn()
         uccs = self.create_uccs2(fake_conn)
+
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, 1, "a")
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, "no_such_item", "a")
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.add_value, "Spec2/item1", "a")
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.remove_value, 1, "a")
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.remove_value, "no_such_item", "a")
         self.assertRaises(isc.cc.data.DataNotFoundError, uccs.remove_value, "Spec2/item1", "a")
+
         self.assertEqual({}, uccs._local_changes)
         uccs.add_value("Spec2/item5", "foo")
         self.assertEqual({'Spec2': {'item5': ['a', 'b', 'foo']}}, uccs._local_changes)
@@ -730,11 +738,36 @@ class TestUIModuleCCSession(unittest.TestCase):
         uccs.remove_value("Spec2/item5", "foo")
         uccs.add_value("Spec2/item5", "foo")
         self.assertEqual({'Spec2': {'item5': ['foo']}}, uccs._local_changes)
-        uccs.add_value("Spec2/item5", "foo")
+        self.assertRaises(isc.cc.data.DataAlreadyPresentError,
+                          uccs.add_value, "Spec2/item5", "foo")
         self.assertEqual({'Spec2': {'item5': ['foo']}}, uccs._local_changes)
+        self.assertRaises(isc.cc.data.DataNotFoundError,
+                          uccs.remove_value, "Spec2/item5[123]", None)
         uccs.remove_value("Spec2/item5[0]", None)
         self.assertEqual({'Spec2': {'item5': []}}, uccs._local_changes)
 
+    def test_add_remove_value_named_set(self):
+        fake_conn = fakeUIConn()
+        uccs = self.create_uccs_named_set(fake_conn)
+        value, status = uccs.get_value("/Spec32/named_set_item")
+        self.assertEqual({'a': 1, 'b': 2}, value)
+        uccs.add_value("/Spec32/named_set_item", "foo")
+        value, status = uccs.get_value("/Spec32/named_set_item")
+        self.assertEqual({'a': 1, 'b': 2, 'foo': 3}, value)
+
+        uccs.remove_value("/Spec32/named_set_item", "a")
+        uccs.remove_value("/Spec32/named_set_item", "foo")
+        value, status = uccs.get_value("/Spec32/named_set_item")
+        self.assertEqual({'b': 2}, value)
+
+        self.assertRaises(isc.cc.data.DataNotFoundError,
+                          uccs.set_value,
+                          "/Spec32/named_set_item/no_such_item",
+                          4)
+        self.assertRaises(isc.cc.data.DataNotFoundError,
+                          uccs.remove_value, "/Spec32/named_set_item",
+                          "no_such_item")
+
     def test_commit(self):
         fake_conn = fakeUIConn()
         uccs = self.create_uccs2(fake_conn)

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

@@ -236,6 +236,7 @@ class TestConfigData(unittest.TestCase):
         value, default = self.cd.get_value("item6/value2")
         self.assertEqual(None, value)
         self.assertEqual(False, default)
+        self.assertRaises(isc.cc.data.DataNotFoundError, self.cd.get_value, "item6/no_such_item")
 
     def test_get_default_value(self):
         self.assertEqual(1, self.cd.get_default_value("item1"))
@@ -360,7 +361,7 @@ class TestMultiConfigData(unittest.TestCase):
 
     def test_get_current_config(self):
         cf = { 'module1': { 'item1': 2, 'item2': True } }
-        self.mcd._set_current_config(cf);
+        self.mcd._set_current_config(cf)
         self.assertEqual(cf, self.mcd.get_current_config())
 
     def test_get_local_changes(self):
@@ -421,6 +422,17 @@ class TestMultiConfigData(unittest.TestCase):
         value = self.mcd.get_default_value("Spec2/no_such_item/asdf")
         self.assertEqual(None, value)
 
+        module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec32.spec")
+        self.mcd.set_specification(module_spec)
+        value = self.mcd.get_default_value("Spec32/named_set_item")
+        self.assertEqual({ 'a': 1, 'b': 2}, value)
+        value = self.mcd.get_default_value("Spec32/named_set_item/a")
+        self.assertEqual(1, value)
+        value = self.mcd.get_default_value("Spec32/named_set_item/b")
+        self.assertEqual(2, value)
+        value = self.mcd.get_default_value("Spec32/named_set_item/no_such_item")
+        self.assertEqual(None, value)
+
     def test_get_value(self):
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec2.spec")
         self.mcd.set_specification(module_spec)
@@ -544,6 +556,29 @@ class TestMultiConfigData(unittest.TestCase):
         maps = self.mcd.get_value_maps("/Spec22/value9")
         self.assertEqual(expected, maps)
 
+    def test_get_value_maps_named_set(self):
+        module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec32.spec")
+        self.mcd.set_specification(module_spec)
+        maps = self.mcd.get_value_maps()
+        self.assertEqual([{'default': False, 'type': 'module',
+                           'name': 'Spec32', 'value': None,
+                           'modified': False}], maps)
+        maps = self.mcd.get_value_maps("/Spec32/named_set_item")
+        self.assertEqual([{'default': True, 'type': 'integer',
+                           'name': 'Spec32/named_set_item/a',
+                           'value': 1, 'modified': False},
+                          {'default': True, 'type': 'integer',
+                           'name': 'Spec32/named_set_item/b',
+                           'value': 2, 'modified': False}], maps)
+        maps = self.mcd.get_value_maps("/Spec32/named_set_item/a")
+        self.assertEqual([{'default': True, 'type': 'integer',
+                           'name': 'Spec32/named_set_item/a',
+                           'value': 1, 'modified': False}], maps)
+        maps = self.mcd.get_value_maps("/Spec32/named_set_item/b")
+        self.assertEqual([{'default': True, 'type': 'integer',
+                           'name': 'Spec32/named_set_item/b',
+                           'value': 2, 'modified': False}], maps)
+
     def test_set_value(self):
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec2.spec")
         self.mcd.set_specification(module_spec)
@@ -582,6 +617,24 @@ class TestMultiConfigData(unittest.TestCase):
         config_items = self.mcd.get_config_item_list("Spec2", True)
         self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
 
+    def test_get_config_item_list_named_set(self):
+        config_items = self.mcd.get_config_item_list()
+        self.assertEqual([], config_items)
+        module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec32.spec")
+        self.mcd.set_specification(module_spec)
+        config_items = self.mcd.get_config_item_list()
+        self.assertEqual(['Spec32'], config_items)
+        config_items = self.mcd.get_config_item_list(None, False)
+        self.assertEqual(['Spec32'], config_items)
+        config_items = self.mcd.get_config_item_list(None, True)
+        self.assertEqual(['Spec32/named_set_item'], config_items)
+        self.mcd.set_value('Spec32/named_set_item', { "aaaa": 4, "aabb": 5, "bbbb": 6})
+        config_items = self.mcd.get_config_item_list("/Spec32/named_set_item", True)
+        self.assertEqual(['Spec32/named_set_item/aaaa',
+                          'Spec32/named_set_item/aabb',
+                          'Spec32/named_set_item/bbbb',
+                         ], config_items)
+
 if __name__ == '__main__':
     unittest.main()
 

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

@@ -98,6 +98,9 @@ class TestModuleSpec(unittest.TestCase):
         self.assertEqual(True, self.validate_data("spec22.spec", "data22_6.data"))
         self.assertEqual(True, self.validate_data("spec22.spec", "data22_7.data"))
         self.assertEqual(False, self.validate_data("spec22.spec", "data22_8.data"))
+        self.assertEqual(True, self.validate_data("spec32.spec", "data32_1.data"))
+        self.assertEqual(False, self.validate_data("spec32.spec", "data32_2.data"))
+        self.assertEqual(False, self.validate_data("spec32.spec", "data32_3.data"))
 
     def validate_command_params(self, specfile_name, datafile_name, cmd_name):
         dd = self.read_spec_file(specfile_name);