merge back branches/jelte-configuration (branched at rev. 745, up to HEAD)

git-svn-id: svn://bind10.isc.org/svn/bind10/trunk@882 e5f2f494-b856-4b98-b285-d166d9295462

src/bin/auth/auth.spec

@@ -1,6 +1,41 @@
 {
-  "data_specification": {
-    "module_name": "Auth"
+  "module_spec": {
+    "module_name": "Auth",
+    "config_data": [
+      { "item_name": "default_name",
+        "item_type": "string",
+        "item_optional": False,
+        "item_default": "Hello, world!"
+      },
+      { "item_name": "zone_list",
+        "item_type": "list",
+        "item_optional": False,
+        "item_default": [],
+        "list_item_spec":
+          { "item_name": "zone_name",
+            "item_type": "string",
+            "item_optional": True,
+            "item_default": ""
+          }
+      }
+    ],
+    "commands": [
+      {
+        "command_name": "print_message",
+        "command_description": "Print the given message to stdout",
+        "command_args": [ {
+          "item_name": "message",
+          "item_type": "string",
+          "item_optional": False,
+          "item_default": ""
+        } ]
+      },
+      {
+        "command_name": "shutdown",
+        "command_description": "Shut down BIND 10",
+        "command_args": []
+      }
+    ]
   }
 }
 

src/bin/auth/auth_srv.cc

@@ -32,6 +32,7 @@
 #include <dns/rrset.h>
 #include <dns/rrttl.h>
 #include <dns/message.h>
+#include <config/ccsession.h>
 
 #include <cc/data.h>
 
@@ -46,6 +47,7 @@ using namespace std;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 using namespace isc::data;
+using namespace isc::config;
 
 AuthSrv::AuthSrv(int port) {
     int s = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
@@ -120,5 +122,7 @@ AuthSrv::updateConfig(isc::data::ElementPtr config) {
         // todo: what to do with port change. restart automatically?
         // ignore atm
     //}
-    return isc::data::Element::createFromString("{ \"result\": [0] }");
+    std::cout << "[XX] auth: new config " << config << std::endl;
+    
+    return isc::config::createAnswer(0);
 }

src/bin/auth/main.cc

@@ -64,18 +64,15 @@ my_config_handler(isc::data::ElementPtr config)
 
 isc::data::ElementPtr
 my_command_handler(isc::data::ElementPtr command) {
-    isc::data::ElementPtr answer = isc::data::Element::createFromString("{ \"result\": [0] }");
+    isc::data::ElementPtr answer = isc::config::createAnswer(0);
 
     cout << "[XX] Handle command: " << endl << command->str() << endl;
     if (command->get(0)->stringValue() == "print_message") 
     {
         cout << command->get(1)->get("message") << endl;
         /* let's add that message to our answer as well */
-        cout << "[XX] answer was: " << answer->str() << endl;
         answer->get("result")->add(command->get(1));
-        cout << "[XX] answer now: " << answer->str() << endl;
     }
-
     return answer;
 }
 
@@ -106,9 +103,9 @@ main(int argc, char* argv[]) {
         } else {
             specfile = std::string(AUTH_SPECFILE_LOCATION);
         }
-        CommandSession cs = CommandSession(specfile,
-                                           my_config_handler,
-                                           my_command_handler);
+        isc::config::ModuleCCSession cs = isc::config::ModuleCCSession(specfile,
+                                                                       my_config_handler,
+                                                                       my_command_handler);
     
         // main server loop
         fd_set fds;

src/bin/bind10/bind10.py.in

@@ -106,6 +106,7 @@ class BoB:
         self.verbose = verbose
         self.c_channel_port = c_channel_port
         self.cc_session = None
+        self.ccs = None
         self.processes = {}
         self.dead_processes = {}
         self.runnable = False
@@ -114,6 +115,8 @@ class BoB:
         if self.verbose:
             print("[XX] handling new config:")
             print(new_config)
+        answer = isc.config.ccsession.create_answer(0)
+        return answer
         # TODO
 
     def command_handler(self, command):
@@ -121,21 +124,27 @@ class BoB:
         if self.verbose:
             print("[XX] Boss got command:")
             print(command)
-        answer = None
+        answer = [ 1, "Command not implemented" ]
         if type(command) != list or len(command) == 0:
-            answer = { "result": [ 1, "bad command" ] }
+            answer = isc.config.ccsession.create_answer(1, "bad command")
         else:
             cmd = command[0]
             if cmd == "shutdown":
                 print("[XX] got shutdown command")
                 self.runnable = False
-                answer = { "result": [ 0 ] }
+                answer = isc.config.ccsession.create_answer(0)
             elif cmd == "print_message":
                 if len(command) > 1 and type(command[1]) == dict and "message" in command[1]:
                     print(command[1]["message"])
-                answer = { "result": [ 0 ] }
+                answer = isc.config.ccsession.create_answer(0)
+            elif cmd == "print_settings":
+                print("Full Config:")
+                full_config = self.ccs.get_full_config()
+                for item in full_config:
+                    print(item + ": " + str(full_config[item]))
+                answer = isc.config.ccsession.create_answer(0)
             else:
-                answer = { "result": [ 1, "Unknown command" ] }
+                answer = isc.config.ccsession.create_answer(1, "Unknown command")
         return answer
     
     def startup(self):
@@ -190,7 +199,8 @@ class BoB:
         time.sleep(1)
         if self.verbose:
             print("[XX] starting ccsession")
-        self.ccs = isc.config.CCSession(SPECFILE_LOCATION, self.config_handler, self.command_handler)
+        self.ccs = isc.config.ModuleCCSession(SPECFILE_LOCATION, self.config_handler, self.command_handler)
+        self.ccs.start()
         if self.verbose:
             print("[XX] ccsession started")
 
@@ -478,7 +488,7 @@ def main():
 
         for fd in rlist + xlist:
             if fd == ccs_fd:
-                boss_of_bind.ccs.checkCommand()
+                boss_of_bind.ccs.check_command()
             elif fd == wakeup_fd:
                 os.read(wakeup_fd, 32)
 

src/bin/bind10/bob.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Boss",
     "config_data": [
       {
@@ -7,6 +7,12 @@
         "item_type": "string",
         "item_optional": False,
         "item_default": "Hi, shane!"
+      },
+      {
+        "item_name": "some_int",
+        "item_type": "integer",
+        "item_optional": False,
+        "item_default": 1
       }
     ],
     "commands": [
@@ -21,6 +27,11 @@
         } ]
       },
       {
+        "command_name": "print_settings",
+        "command_description": "Print some_string and some_int to stdout",
+        "command_args": []
+      },
+      {
         "command_name": "shutdown",
         "command_description": "Shut down BIND 10",
         "command_args": []

src/bin/bindctl/bindcmd.py

@@ -31,6 +31,7 @@ import os, time, random, re
 import getpass
 from hashlib import sha1
 import csv
+import ast
 
 try:
     from collections import OrderedDict
@@ -85,7 +86,7 @@ class BindCmdInterpreter(Cmd):
                 return False
 
             # Get all module information from cmd-ctrld
-            self.config_data = isc.cc.data.UIConfigData(self)
+            self.config_data = isc.config.UIModuleCCSession(self)
             self.update_commands()
             self.cmdloop()
         except KeyboardInterrupt:
@@ -150,7 +151,8 @@ class BindCmdInterpreter(Cmd):
         if (len(cmd_spec) == 0):
             print('can\'t get any command specification')
         for module_name in cmd_spec.keys():
-            self.prepare_module_commands(module_name, cmd_spec[module_name])
+            if cmd_spec[module_name]:
+                self.prepare_module_commands(module_name, cmd_spec[module_name])
 
     def send_GET(self, url, body = None):
         headers = {"cookie" : self.session_id}
@@ -315,7 +317,7 @@ class BindCmdInterpreter(Cmd):
                     if cmd.module == "config":
                         # grm text has been stripped of slashes...
                         my_text = self.location + "/" + cur_line.rpartition(" ")[2]
-                        list = self.config_data.config.get_item_list(my_text.rpartition("/")[0])
+                        list = self.config_data.get_config_item_list(my_text.rpartition("/")[0])
                         hints.extend([val for val in list if val.startswith(text)])
             except CmdModuleNameFormatError:
                 if not text:
@@ -440,17 +442,28 @@ class BindCmdInterpreter(Cmd):
                         line += "(modified)"
                     print(line)
             elif cmd.command == "add":
-                self.config_data.add(identifier, cmd.params['value'])
+                self.config_data.add_value(identifier, cmd.params['value'])
             elif cmd.command == "remove":
-                self.config_data.remove(identifier, cmd.params['value'])
+                self.config_data.remove_value(identifier, cmd.params['value'])
             elif cmd.command == "set":
-                self.config_data.set(identifier, cmd.params['value'])
+                if 'identifier' not in cmd.params:
+                    print("Error: missing identifier or value")
+                else:
+                    parsed_value = None
+                    try:
+                        parsed_value = ast.literal_eval(cmd.params['value'])
+                    except Exception as exc:
+                        # ok could be an unquoted string, interpret as such
+                        parsed_value = cmd.params['value']
+                    self.config_data.set_value(identifier, parsed_value)
             elif cmd.command == "unset":
                 self.config_data.unset(identifier)
             elif cmd.command == "revert":
                 self.config_data.revert()
             elif cmd.command == "commit":
-                self.config_data.commit(self)
+                self.config_data.commit()
+            elif cmd.command == "diff":
+                print(self.config_data.get_local_changes());
             elif cmd.command == "go":
                 self.go(identifier)
         except isc.cc.data.DataTypeError as dte:

src/bin/bindctl/bindctl.in

@@ -5,7 +5,7 @@ export PYTHON_EXEC
 
 BINDCTL_PATH=@abs_top_srcdir@/src/bin/bindctl
 
-PYTHONPATH=@abs_top_srcdir@/src/lib/cc/python
+PYTHONPATH=@abs_top_builddir@/pyshared
 export PYTHONPATH
 
 cd ${BINDCTL_PATH}

src/bin/bindctl/bindctl.py

@@ -53,6 +53,9 @@ def prepare_config_commands(tool):
     cmd.add_param(param)
     module.add_command(cmd)
 
+    cmd = CommandInfo(name = "diff", desc = "Show all local changes", need_inst_param = False)
+    module.add_command(cmd)
+
     cmd = CommandInfo(name = "revert", desc = "Revert all local changes", need_inst_param = False)
     module.add_command(cmd)
 
@@ -67,12 +70,17 @@ def prepare_config_commands(tool):
     tool.add_module_info(module)
 
 if __name__ == '__main__':
-    try:
-        tool = BindCmdInterpreter("localhost:8080")
-        prepare_config_commands(tool)
-        tool.run()
-    except Exception as e:
-        print(e)
-        print("Failed to connect with b10-cmdctl module, is it running?")
+    tool = BindCmdInterpreter("localhost:8080")
+    prepare_config_commands(tool)
+    tool.run()
+# TODO: put below back, was removed to see errors
+#if __name__ == '__main__':
+    #try:
+        #tool = BindCmdInterpreter("localhost:8080")
+        #prepare_config_commands(tool)
+        #tool.run()
+    #except Exception as e:
+        #print(e)
+        #print("Failed to connect with b10-cmdctl module, is it running?")
 
 

src/bin/cmdctl/b10-cmdctl.py.in

@@ -169,8 +169,8 @@ class SecureHTTPRequestHandler(http.server.BaseHTTPRequestHandler):
             param = json.loads(post_str)
             # TODO, need return some proper return code. 
             # currently always OK.
-            reply = self.server.send_command_to_module(mod, cmd, param)
-            print('b10-cmdctl finish send message \'%s\' to module %s' % (cmd, mod))            
+        reply = self.server.send_command_to_module(mod, cmd, param)
+        print('b10-cmdctl finish send message \'%s\' to module %s' % (cmd, mod))            
 
         return rcode, reply
             
@@ -189,7 +189,7 @@ class CommandControl():
         self.config_data = self.get_config_data()
 
     def get_cmd_specification(self): 
-        return self.send_command('ConfigManager', 'get_commands')
+        return self.send_command('ConfigManager', 'get_commands_spec')
 
     def get_config_data(self):
         return self.send_command('ConfigManager', 'get_config')
@@ -199,7 +199,7 @@ class CommandControl():
             self.config_data = self.get_config_data()
 
     def get_data_specification(self):
-        return self.send_command('ConfigManager', 'get_data_spec')
+        return self.send_command('ConfigManager', 'get_module_spec')
 
     def handle_recv_msg(self):
         # Handle received message, if 'shutdown' is received, return False

src/lib/cc/cpp/data.cc

@@ -829,13 +829,19 @@ ListElement::toWire(std::stringstream& ss, int omit_length)
         (*it)->toWire(ss2, 0);
     }
 
+
     if (omit_length) {
-        ss << ss2.rdbuf();
+        stringbuf *ss2_buf = ss2.rdbuf();
+        if (ss2_buf->in_avail() > 0) {
+            ss << ss2_buf;
+        }
     } else {
         stringbuf *ss2_buf = ss2.rdbuf();
         ss2_buf->pubseekpos(0);
         ss << encode_length(ss2_buf->in_avail(), ITEM_LIST);
-        ss << ss2_buf;
+        if (ss2_buf->in_avail() > 0) {
+            ss << ss2_buf;
+        }
     }
 }
 
@@ -873,13 +879,17 @@ MapElement::toWire(std::stringstream& ss, int omit_length)
     // add length if needed
     //
     if (omit_length) {
-        ss << ss2.rdbuf();
+        stringbuf *ss2_buf = ss2.rdbuf();
+        if (ss2_buf->in_avail() > 0) {
+            ss << ss2_buf;
+        }
     } else {
-        
         stringbuf *ss2_buf = ss2.rdbuf();
         ss2_buf->pubseekpos(0);
         ss << encode_length(ss2_buf->in_avail(), ITEM_HASH);
-        ss << ss2_buf;
+        if (ss2_buf->in_avail() > 0) {
+            ss << ss2_buf;
+        }
     }
 }
 

src/lib/cc/cpp/data.h

@@ -35,7 +35,6 @@ typedef boost::shared_ptr<Element> ElementPtr;
 /// is called for an Element that has a wrong type (e.g. int_value on a
 /// ListElement)
 ///
-// todo: include types and called function in the exception
 class TypeError : public isc::Exception {
 public:
     TypeError(const char* file, size_t line, const char* what) :

src/lib/cc/cpp/data_unittests.cc

@@ -266,5 +266,13 @@ TEST(Element, to_and_from_wire) {
 
     //EXPECT_EQ("\047\0031.2", Element::create(1.2)->toWire(0));
     EXPECT_EQ("\046\0011", Element::createFromString("[ 1 ]")->toWire(1));
+
+    std::string ddef = "{\"data_specification\": {\"config_data\": [ {\"item_default\": \"Hello, world!\", \"item_name\": \"default_name\", \"item_optional\": False, \"item_type\": \"string\"}, {\"item_default\": [  ], \"item_name\": \"zone_list\", \"item_optional\": False, \"item_type\": \"list\", \"list_item_spec\": {\"item_name\": \"zone_name\", \"item_optional\": True, \"item_type\": \"string\"}} ], \"module_name\": \"Auth\"}}";
+    //std::string ddef = "{\"aaa\": 123, \"test\": [  ], \"zzz\": 123}";
+    ElementPtr ddef_el = Element::createFromString(ddef);
+    std::string ddef_wire = ddef_el->toWire();
+    ElementPtr ddef_el2 = Element::fromWire(ddef_wire);
+    std::string ddef2 = ddef_el2->str();
+    EXPECT_EQ(ddef, ddef2);
 }
 

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

@@ -1,5 +1,25 @@
-# data, data_definition, config_data, module_config_data and ui_config_data classes
-# we might want to split these up :)
+# Copyright (C) 2010  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+#
+# Helper functions for data elements as used in cc-channel and
+# configuration. There is no python equivalent for the cpp Element
+# class, since data elements are represented by native python types
+# (int, real, bool, string, list and dict respectively)
+#
+
 import ast
 
 class DataNotFoundError(Exception): pass
@@ -9,6 +29,8 @@ def merge(orig, new):
     """Merges the contents of new into orig, think recursive update()
        orig and new must both be dicts. If an element value is None in
        new it will be removed in orig."""
+    if type(orig) != dict or type(new) != dict:
+        raise DataTypeError("Not a dict in merge()")
     for kn in new.keys():
         if kn in orig:
             if new[kn]:
@@ -23,6 +45,10 @@ def merge(orig, new):
 
 def find(element, identifier):
     """Returns the subelement in the given data element, raises DataNotFoundError if not found"""
+    if type(identifier) != str or (type(element) != dict and identifier != ""):
+        raise DataTypeError("identifier in merge() is not a string")
+    if type(identifier) != str or (type(element) != dict and identifier != ""):
+        raise DataTypeError("element in merge() is not a dict")
     id_parts = identifier.split("/")
     id_parts[:] = (value for value in id_parts if value != "")
     cur_el = element
@@ -34,6 +60,17 @@ def find(element, identifier):
     return cur_el
 
 def set(element, identifier, value):
+    """Sets the value at the element specified by identifier to value.
+       If the value is None, it is removed from the dict. If element
+       is not a dict, or if the identifier points to something that is
+       not, a DataTypeError is raised. The element is updated inline,
+       so if the original needs to be kept, you must make a copy before
+       calling set(). The updated base element is returned (so that
+       el.set().set().set() is possible)"""
+    if type(element) != dict:
+        raise DataTypeError("element in set() is not a dict")
+    if type(identifier) != str:
+        raise DataTypeError("identifier in set() is not a string")
     id_parts = identifier.split("/")
     id_parts[:] = (value for value in id_parts if value != "")
     cur_el = element
@@ -41,112 +78,49 @@ def set(element, identifier, value):
         if id in cur_el.keys():
             cur_el = cur_el[id]
         else:
-            cur_el[id] = {}
-            cur_el = cur_el[id]
-    cur_el[id_parts[-1]] = value
+            if value:
+                cur_el[id] = {}
+                cur_el = cur_el[id]
+            else:
+                # set to none, and parent el not found, return
+                return element
+    # value can be an empty list or dict, so check for None eplicitely
+    if value != None:
+        cur_el[id_parts[-1]] = value
+    elif id_parts[-1] in cur_el:
+        del cur_el[id_parts[-1]]
     return element
 
 def unset(element, identifier):
-    id_parts = identifier.split("/")
-    id_parts[:] = (value for value in id_parts if value != "")
-    cur_el = element
-    for id in id_parts[:-1]:
-        if id in cur_el.keys():
-            cur_el = cur_el[id]
-        else:
-            cur_el[id] = {}
-            cur_el = cur_el[id]
-    cur_el[id_parts[-1]] = None
-    return element
+    """Removes the element at the given identifier if it exists. Raises
+       a DataTypeError if element is not a dict or if identifier is not
+       a string. Returns the base element."""
+    # perhaps we can simply do with set none, and remove this whole
+    # function
+    return set(element, identifier, None)
 
 def find_no_exc(element, identifier):
-    """Returns the subelement in the given data element, returns None if not found"""
+    """Returns the subelement in the given data element, returns None
+       if not found, or if an error occurred (i.e. this function should
+       never raise an exception)"""
+    if type(identifier) != str:
+        return None
     id_parts = identifier.split("/")
     id_parts[:] = (value for value in id_parts if value != "")
     cur_el = element
     for id in id_parts:
-        if type(cur_el) == dict and id in cur_el.keys():
+        if (type(cur_el) == dict and id in cur_el.keys()) or id=="":
             cur_el = cur_el[id]
         else:
             return None
     return cur_el
 
-def find_spec(element, identifier):
-    """find the data definition for the given identifier
-       returns either a map with 'item_name' etc, or a list of those"""
-    id_parts = identifier.split("/")
-    id_parts[:] = (value for value in id_parts if value != "")
-    cur_el = element
-    for id in id_parts:
-        if type(cur_el) == dict and id in cur_el.keys():
-            cur_el = cur_el[id]
-        elif type(cur_el) == dict and 'item_name' in cur_el.keys() and cur_el['item_name'] == id:
-            pass
-        elif type(cur_el) == list:
-            found = False
-            for cur_el_item in cur_el:
-                if cur_el_item['item_name'] == id and 'item_default' in cur_el_item.keys():
-                    cur_el = cur_el_item
-                    found = True
-            if not found:
-                raise DataNotFoundError(id + " in " + str(cur_el))
-        else:
-            raise DataNotFoundError(id + " in " + str(cur_el))
-    return cur_el
-
-def check_type(specification, value):
-    """Returns true if the value is of the correct type given the
-       specification"""
-    if type(specification) == list:
-        data_type = "list"
-    else:
-        data_type = specification['item_type']
-
-    if data_type == "integer" and type(value) != int:
-        raise DataTypeError(str(value) + " should be an integer")
-    elif data_type == "real" and type(value) != double:
-        raise DataTypeError(str(value) + " should be a real")
-    elif data_type == "boolean" and type(value) != boolean:
-        raise DataTypeError(str(value) + " should be a boolean")
-    elif data_type == "string" and type(value) != str:
-        raise DataTypeError(str(value) + " should be a string")
-    elif data_type == "list":
-        if type(value) != list:
-            raise DataTypeError(str(value) + " should be a list, not a " + str(value.__class__.__name__))
-        else:
-            # todo: check subtypes etc
-            for element in value:
-                check_type(specification['list_item_spec'], element)
-    elif data_type == "map" and type(value) != dict:
-        # todo: check subtypes etc
-        raise DataTypeError(str(value) + " should be a map")
-
-def spec_name_list(spec, prefix="", recurse=False):
-    """Returns a full list of all possible item identifiers in the
-       specification (part)"""
-    result = []
-    if prefix != "" and not prefix.endswith("/"):
-        prefix += "/"
-    if type(spec) == dict:
-        for name in spec:
-            result.append(prefix + name + "/")
-            if 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:
-                if list_el['item_type'] == dict:
-                    if recurse:
-                        result.extend(spec_name_list(list_el['map_item_spec'], prefix + list_el['item_name'], recurse))
-                else:
-                    name = list_el['item_name']
-                    if list_el['item_type'] in ["list", "map"]:
-                        name += "/"
-                    result.append(name)
-
-    return result
-
 def parse_value_str(value_str):
+    """Parses the given string to a native python object. If the
+       string cannot be parsed, it is returned. If it is not a string,
+       None is returned"""
+    if type(value_str) != str:
+        return None
     try:
         return ast.literal_eval(value_str)
     except ValueError as ve:
@@ -156,181 +130,3 @@ def parse_value_str(value_str):
         # simply return the string itself
         return value_str
 
-class ConfigData:
-    def __init__(self, specification):
-        self.specification = specification
-        self.data = {}
-
-    def get_item_list(self, identifier = None):
-        if identifier:
-            spec = find_spec(self.specification, identifier)
-            return spec_name_list(spec, identifier + "/")
-        return spec_name_list(self.specification)
-
-    def get_value(self, identifier):
-        """Returns a tuple where the first item is the value at the
-           given identifier, and the second item is a bool which is
-           true if the value is an unset default"""
-        value = find_no_exc(self.data, identifier)
-        if value:
-            return value, False
-        spec = find_spec(self.specification, identifier)
-        if spec and 'item_default' in spec:
-            return spec['item_default'], True
-        return None, False
-
-class UIConfigData():
-    def __init__(self, conn, name = ''):
-        self.module_name = name
-        data_spec = self.get_data_specification(conn)
-        self.config = ConfigData(data_spec)
-        self.get_config_data(conn)
-        self.config_changes = {}
-    
-    def get_config_data(self, conn):
-        self.config.data = conn.send_GET('/config_data') 
-
-    def send_changes(self, conn):
-        conn.send_POST('/ConfigManager/set_config', self.config_changes)
-        # Get latest config data
-        self.get_config_data(conn)
-        self.config_changes = {}
-    
-    def get_data_specification(self, conn):
-        return conn.send_GET('/config_spec') 
-
-    def set(self, identifier, value):
-        # check against definition
-        spec = find_spec(identifier)
-        check_type(spec, value)
-        set(self.config_changes, identifier, value)
-
-    def get_value(self, identifier):
-        """Returns a three-tuple, where the first item is the value
-           (or None), the second is a boolean specifying whether
-           the value is the default value, and the third is a boolean
-           specifying whether the value is an uncommitted change"""
-        value = find_no_exc(self.config_changes, identifier)
-        if value:
-            return value, False, True
-        value, default = self.config.get_value(identifier)
-        if value:
-            return value, default, False
-        return None, False, False
-
-    def get_value_map_single(self, identifier, entry):
-        """Returns a single entry for a value_map, where the value is
-           not a part of a bigger map"""
-        result_part = {}
-        result_part['name'] = entry['item_name']
-        result_part['type'] = entry['item_type']
-        value, default, modified = self.get_value(identifier)
-        # should we check type and only set int, double, bool and string here?
-        result_part['value'] = value
-        result_part['default'] = default
-        result_part['modified'] = modified
-        return result_part
-
-    def get_value_map(self, identifier, entry):
-        """Returns a single entry for a value_map, where the value is
-           a part of a bigger map"""
-        result_part = {}
-        result_part['name'] = entry['item_name']
-        result_part['type'] = entry['item_type']
-        value, default, modified = self.get_value(identifier + "/" + entry['item_name'])
-        # should we check type and only set int, double, bool and string here?
-        result_part['value'] = value
-        result_part['default'] = default
-        result_part['modified'] = modified
-        return result_part
-
-    def get_value_maps(self, identifier = None):
-        """Returns a list of maps, containing the following values:
-           name: name of the entry (string)
-           type: string containing the type of the value (or 'module')
-           value: value of the entry if it is a string, int, double or bool
-           modified: true if the value is a local change
-           default: true if the value has been changed
-           Throws DataNotFoundError if the identifier is bad
-        """
-        spec = find_spec(self.config.specification, identifier)
-        result = []
-        if type(spec) == dict:
-            # either the top-level list of modules or a spec map
-            if 'item_name' in spec:
-                result_part = self.get_value_map_single(identifier, spec)
-                if result_part['type'] == "list":
-                    values = self.get_value(identifier)[0]
-                    if values:
-                        for value in values:
-                            result_part2 = {}
-                            li_spec = spec['list_item_spec']
-                            result_part2['name'] = li_spec['item_name']
-                            result_part2['value'] = value
-                            result_part2['type'] = li_spec['item_type']
-                            result_part2['default'] = False
-                            result_part2['modified'] = False
-                            result.append(result_part2)
-                else:
-                    result.append(result_part)
-                
-            else:
-                for name in spec:
-                    result_part = {}
-                    result_part['name'] = name
-                    result_part['type'] = "module"
-                    result_part['value'] = None
-                    result_part['default'] = False
-                    result_part['modified'] = False
-                    result.append(result_part)
-        elif type(spec) == list:
-            for entry in spec:
-                if type(entry) == dict and 'item_name' in entry:
-                    result.append(self.get_value_map(identifier, entry))
-        return result
-
-    def add(self, identifier, value_str):
-        data_spec = find_spec(self.config.specification, identifier)
-        if (type(data_spec) != dict or "list_item_spec" not in data_spec):
-            raise DataTypeError(identifier + " is not a list")
-        value = parse_value_str(value_str)
-        check_type(data_spec, [value])
-        cur_list = find_no_exc(self.config_changes, identifier)
-        if not cur_list:
-            cur_list = find_no_exc(self.config.data, identifier)
-        if not cur_list:
-            cur_list = []
-        if value not in cur_list:
-            cur_list.append(value)
-        set(self.config_changes, identifier, cur_list)
-
-    def remove(self, identifier, value_str):
-        data_spec = find_spec(self.config.specification, identifier)
-        if (type(data_spec) != dict or "list_item_spec" not in data_spec):
-            raise DataTypeError(identifier + " is not a list")
-        value = parse_value_str(value_str)
-        check_type(data_spec, [value])
-        cur_list = find_no_exc(self.config_changes, identifier)
-        if not cur_list:
-            cur_list = find_no_exc(self.config.data, identifier)
-        if not cur_list:
-            cur_list = []
-        if value in cur_list:
-            cur_list.remove(value)
-        set(self.config_changes, identifier, cur_list)
-
-    def set(self, identifier, value_str):
-        data_spec = find_spec(self.config.specification, identifier)
-        value = parse_value_str(value_str)
-        check_type(data_spec, value)
-        set(self.config_changes, identifier, value)
-
-    def unset(self, identifier):
-        # todo: check whether the value is optional?
-        unset(self.config_changes, identifier)
-
-    def revert(self):
-        self.config_changes = {}
-
-    def commit(self, conn):
-        self.send_changes(conn)

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

@@ -0,0 +1,106 @@
+# Copyright (C) 2010  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+#
+# Tests for the functions in data.py
+#
+
+import unittest
+import os
+import data
+
+class TestData(unittest.TestCase):
+    def test_merge(self):
+        d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2 } }
+        d2 = { 'a': None, 'c': { 'd': None, 'e': 3, 'f': [ 1 ] } }
+        d12 = { 'b': 1, 'c': { 'e': 3, 'f': [ 1 ] } }
+        m12 = d1
+        data.merge(m12, d2)
+        self.assertEqual(d12, m12)
+        self.assertRaises(data.DataTypeError, data.merge, d1, "a")
+        self.assertRaises(data.DataTypeError, data.merge, 1, d2)
+        self.assertRaises(data.DataTypeError, data.merge, None, None)
+
+    def test_find(self):
+        d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2, 'more': { 'data': 'here' } } }
+        self.assertEqual(data.find(d1, ''), d1)
+        self.assertEqual(data.find(d1, 'a'), 'a')
+        self.assertEqual(data.find(d1, 'c/e'), 2)
+        self.assertEqual(data.find(d1, 'c/more/'), { 'data': 'here' })
+        self.assertEqual(data.find(d1, 'c/more/data'), 'here')
+        self.assertRaises(data.DataNotFoundError, data.find, d1, 'c/f')
+        self.assertRaises(data.DataNotFoundError, data.find, d1, 'f')
+        self.assertRaises(data.DataTypeError, data.find, d1, 1)
+        self.assertRaises(data.DataTypeError, data.find, None, 1)
+        self.assertRaises(data.DataTypeError, data.find, "123", "123")
+        self.assertEqual(data.find("123", ""), "123")
+        
+    def test_set(self):
+        d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2 } }
+        d12 = { 'b': 1, 'c': { 'e': 3, 'f': [ 1 ] } }
+        data.set(d1, 'a', None)
+        data.set(d1, 'c/d', None)
+        data.set(d1, 'c/e/', 3)
+        data.set(d1, 'c/f', [ 1 ] )
+        self.assertEqual(d1, d12)
+        self.assertRaises(data.DataTypeError, data.set, d1, 1, 2)
+        self.assertRaises(data.DataTypeError, data.set, 1, "", 2)
+        d3 = {}
+        e3 = data.set(d3, "does/not/exist", 123)
+        self.assertEqual(d3,
+                         { 'does': { 'not': { 'exist': 123 } } })
+        self.assertEqual(e3,
+                         { 'does': { 'not': { 'exist': 123 } } })
+
+    def test_unset(self):
+        d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2 } }
+        data.unset(d1, 'a')
+        data.unset(d1, 'c/d')
+        data.unset(d1, 'does/not/exist')
+        self.assertEqual(d1, { 'b': 1, 'c': { 'e': 2 } })
+        
+    def test_find_no_exc(self):
+        d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2, 'more': { 'data': 'here' } } }
+        self.assertEqual(data.find_no_exc(d1, ''), d1)
+        self.assertEqual(data.find_no_exc(d1, 'a'), 'a')
+        self.assertEqual(data.find_no_exc(d1, 'c/e'), 2)
+        self.assertEqual(data.find_no_exc(d1, 'c/more/'), { 'data': 'here' })
+        self.assertEqual(data.find_no_exc(d1, 'c/more/data'), 'here')
+        self.assertEqual(data.find_no_exc(d1, 'c/f'), None)
+        self.assertEqual(data.find_no_exc(d1, 'f'), None)
+        self.assertEqual(data.find_no_exc(d1, 1), None)
+        self.assertEqual(data.find_no_exc(d1, 'more/data/here'), None)
+        self.assertEqual(data.find_no_exc(None, 1), None)
+        self.assertEqual(data.find_no_exc("123", ""), "123")
+        self.assertEqual(data.find_no_exc("123", ""), "123")
+        
+    def test_parse_value_str(self):
+        self.assertEqual(data.parse_value_str("1"), 1)
+        self.assertEqual(data.parse_value_str("True"), True)
+        self.assertEqual(data.parse_value_str("None"), None)
+        self.assertEqual(data.parse_value_str("1.1"), 1.1)
+        self.assertEqual(data.parse_value_str("[]"), [])
+        self.assertEqual(data.parse_value_str("[ 1, None, 'asdf' ]"), [ 1, None, "asdf" ])
+        self.assertEqual(data.parse_value_str("{}"), {})
+        self.assertEqual(data.parse_value_str("{ 'a': 'b', 'c': 1 }"), { 'a': 'b', 'c': 1 })
+        self.assertEqual(data.parse_value_str("[ a c"), "[ a c")
+
+if __name__ == '__main__':
+    #if not 'CONFIG_TESTDATA_PATH' in os.environ:
+    #    print("You need to set the environment variable CONFIG_TESTDATA_PATH to point to the directory containing the test data files")
+    #    exit(1)
+    unittest.main()
+
+

src/lib/config/cpp/Makefile.am

@@ -1,14 +1,14 @@
 AM_CPPFLAGS = -I$(top_builddir)/include -I$(top_srcdir)/ext -Wall -Werror
 
 lib_LIBRARIES = libcfgclient.a
-libcfgclient_a_SOURCES = data_def.h data_def.cc ccsession.cc ccsession.h
+libcfgclient_a_SOURCES = module_spec.h module_spec.cc ccsession.cc ccsession.h
 
 CLEANFILES = *.gcno *.gcda
 
 TESTS =
 if HAVE_GTEST
 TESTS += run_unittests
-run_unittests_SOURCES = data_def_unittests.cc run_unittests.cc
+run_unittests_SOURCES = module_spec_unittests.cc run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
 run_unittests_LDADD = libcfgclient.a $(GTEST_LDADD)

src/lib/config/cpp/ccsession.cc

@@ -34,8 +34,9 @@
 #include <boost/foreach.hpp>
 
 #include <cc/data.h>
-#include <data_def.h>
+#include <module_spec.h>
 #include <cc/session.h>
+#include <exceptions/exceptions.h>
 
 //#include "common.h"
 #include "ccsession.h"
@@ -45,12 +46,65 @@ using namespace std;
 
 using isc::data::Element;
 using isc::data::ElementPtr;
-using isc::data::DataDefinition;
 using isc::data::ParseError;
-using isc::data::DataDefinitionError;
+
+namespace isc {
+namespace config {
+
+/// Creates a standard config/command protocol answer message
+ElementPtr
+createAnswer(const int rcode)
+{
+    ElementPtr answer = Element::createFromString("{\"result\": [] }");
+    ElementPtr answer_content = answer->get("result");
+    answer_content->add(Element::create(rcode));
+    return answer;
+}
+
+ElementPtr
+createAnswer(const int rcode, const ElementPtr arg)
+{
+    ElementPtr answer = Element::createFromString("{\"result\": [] }");
+    ElementPtr answer_content = answer->get("result");
+    answer_content->add(Element::create(rcode));
+    answer_content->add(arg);
+    return answer;
+}
+
+ElementPtr
+createAnswer(const int rcode, const std::string& arg)
+{
+    ElementPtr answer = Element::createFromString("{\"result\": [] }");
+    ElementPtr answer_content = answer->get("result");
+    answer_content->add(Element::create(rcode));
+    answer_content->add(Element::create(arg));
+    return answer;
+}
+
+ElementPtr
+parseAnswer(int &rcode, const ElementPtr msg)
+{
+    if (!msg->contains("result")) {
+        // TODO: raise CCSessionError exception
+        dns_throw(CCSessionError, "No result in answer message");
+    } else {
+        ElementPtr result = msg->get("result");
+        if (result->get(0)->getType() != Element::integer) {
+            dns_throw(CCSessionError, "First element of result is not an rcode in answer message");
+        } else if (result->get(0)->intValue() != 0 && result->get(1)->getType() != Element::string) {
+            dns_throw(CCSessionError, "Rcode in answer message is non-zero, but other argument is not a StringElement");
+        }
+        rcode = result->get(0)->intValue();
+        if (result->size() > 1) {
+            return result->get(1);
+        } else {
+            return ElementPtr();
+        }
+    }
+}
 
 void
-CommandSession::read_data_definition(const std::string& filename) {
+ModuleCCSession::read_module_specification(const std::string& filename) {
     std::ifstream file;
 
     // this file should be declared in a @something@ directive
@@ -61,27 +115,27 @@ CommandSession::read_data_definition(const std::string& filename) {
     }
 
     try {
-        data_definition_ = DataDefinition(file, true);
+        module_specification_ = moduleSpecFromFile(file, true);
     } catch (ParseError pe) {
-        cout << "Error parsing definition file: " << pe.what() << endl;
+        cout << "Error parsing module specification file: " << pe.what() << endl;
         exit(1);
-    } catch (DataDefinitionError dde) {
-        cout << "Error reading definition file: " << dde.what() << endl;
+    } catch (ModuleSpecError dde) {
+        cout << "Error reading module specification file: " << dde.what() << endl;
         exit(1);
     }
     file.close();
 }
 
-CommandSession::CommandSession(std::string spec_file_name,
+ModuleCCSession::ModuleCCSession(std::string spec_file_name,
                                isc::data::ElementPtr(*config_handler)(isc::data::ElementPtr new_config),
                                isc::data::ElementPtr(*command_handler)(isc::data::ElementPtr command)
                               ) throw (isc::cc::SessionError):
     session_(isc::cc::Session())
 {
-    read_data_definition(spec_file_name);
+    read_module_specification(spec_file_name);
     sleep(1);
 
-    module_name_ = data_definition_.getDefinition()->get("data_specification")->get("module_name")->stringValue();
+    module_name_ = module_specification_.getFullSpec()->get("module_spec")->get("module_name")->stringValue();
     config_handler_ = config_handler;
     command_handler_ = command_handler;
 
@@ -96,7 +150,7 @@ CommandSession::CommandSession(std::string spec_file_name,
     //session_.subscribe("Boss", "*");
     //session_.subscribe("statistics", "*");
     // send the data specification
-    session_.group_sendmsg(data_definition_.getDefinition(), "ConfigManager");
+    session_.group_sendmsg(module_specification_.getFullSpec(), "ConfigManager");
     session_.group_recvmsg(env, answer, false);
     
     // get any stored configuration from the manager
@@ -105,24 +159,43 @@ CommandSession::CommandSession(std::string spec_file_name,
         session_.group_sendmsg(cmd, "ConfigManager");
         session_.group_recvmsg(env, answer, false);
         cout << "[XX] got config: " << endl << answer->str() << endl;
-        if (answer->contains("result") &&
-            answer->get("result")->get(0)->intValue() == 0 &&
-            answer->get("result")->size() > 1) {
-            config_handler(answer->get("result")->get(1));
-        } else {
-            cout << "[XX] no result in answer" << endl;
+        int rcode;
+        ElementPtr new_config = parseAnswer(rcode, answer);
+        handleConfigUpdate(new_config);
+    }
+}
+
+/// Validates the new config values, if they are correct,
+/// call the config handler
+/// If that results in success, store the new config
+ElementPtr
+ModuleCCSession::handleConfigUpdate(ElementPtr new_config)
+{
+    ElementPtr answer;
+    if (!config_handler_) {
+        answer = createAnswer(1, module_name_ + " does not have a config handler");
+    } else if (!module_specification_.validate_config(new_config)) {
+        answer = createAnswer(2, "Error in config validation");
+    } else {
+        // handle config update
+        answer = config_handler_(new_config);
+        int rcode;
+        parseAnswer(rcode, answer);
+        if (rcode == 0) {
+            config_ = new_config;
         }
     }
+    return answer;
 }
 
 int
-CommandSession::getSocket()
+ModuleCCSession::getSocket()
 {
     return (session_.getSocket());
 }
 
 int
-CommandSession::check_command()
+ModuleCCSession::check_command()
 {
     cout << "[XX] check for command" << endl;
     ElementPtr cmd, routing, data;
@@ -135,12 +208,8 @@ CommandSession::check_command()
         cout << "[XX] got something!" << endl << data->str() << endl;
         ElementPtr answer;
         if (data->contains("config_update")) {
-            if (config_handler_) {
-                // handle config update
-                answer = config_handler_(data->get("config_update"));
-            } else {
-                answer = Element::createFromString("{ \"result\": [0] }");
-            }
+            ElementPtr new_config = data->get("config_update");
+            answer = handleConfigUpdate(new_config);
         }
         if (data->contains("command")) {
             if (command_handler_) {
@@ -155,3 +224,5 @@ CommandSession::check_command()
     return 0;
 }
 
+}
+}

src/lib/config/cpp/ccsession.h

@@ -19,24 +19,39 @@
 
 #include <string>
 
-#include <config/data_def.h>
+#include <config/module_spec.h>
 #include <cc/session.h>
 #include <cc/data.h>
 
-class CommandSession {
+namespace isc {
+namespace config {
+
+///
+/// \brief A standard cc session exception that is thrown if a function
+/// is there is a problem with one of the messages
+///
+// todo: include types and called function in the exception
+class CCSessionError : public isc::Exception {
+public:
+    CCSessionError(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
+
+class ModuleCCSession {
 public:
     /**
      * Initialize a config/command session
      * @param module_name: The name of this module. This is not a
      *                     reference because we expect static strings
      *                     to be passed here.
-     * @param spec_file_name: The name of the file containing the data
-     *                        definition.
+     * @param spec_file_name: The name of the file containing the
+     *                        module specification.
      */
-    CommandSession(std::string spec_file_name,
-                   isc::data::ElementPtr(*config_handler)(isc::data::ElementPtr new_config) = NULL,
-                   isc::data::ElementPtr(*command_handler)(isc::data::ElementPtr command) = NULL
-                  ) throw (isc::cc::SessionError);
+    ModuleCCSession(std::string spec_file_name,
+                    isc::data::ElementPtr(*config_handler)(isc::data::ElementPtr new_config) = NULL,
+                    isc::data::ElementPtr(*command_handler)(isc::data::ElementPtr command) = NULL
+                    ) throw (isc::cc::SessionError);
     int getSocket();
 
     /**
@@ -69,19 +84,26 @@ public:
      * This protocol is very likely to change.
      */
     void set_command_handler(isc::data::ElementPtr(*command_handler)(isc::data::ElementPtr command)) { command_handler_ = command_handler; };
-    
+
 private:
-    void read_data_definition(const std::string& filename);
+    void read_module_specification(const std::string& filename);
     
     std::string module_name_;
     isc::cc::Session session_;
-    isc::data::DataDefinition data_definition_;
+    ModuleSpec module_specification_;
     isc::data::ElementPtr config_;
+    ElementPtr handleConfigUpdate(ElementPtr new_config);
 
     isc::data::ElementPtr(*config_handler_)(isc::data::ElementPtr new_config);
     isc::data::ElementPtr(*command_handler_)(isc::data::ElementPtr command);
 };
 
+ElementPtr createAnswer(const int rcode);
+ElementPtr createAnswer(const int rcode, const ElementPtr arg);
+ElementPtr createAnswer(const int rcode, const std::string& arg);
+
+}
+}
 #endif // __CCSESSION_H
 
 // Local Variables:

src/lib/config/cpp/data_def.h

@@ -1,88 +0,0 @@
-#ifndef _DATA_DEF_H
-#define _DATA_DEF_H 1
-
-#include <cc/data.h>
-
-#include <sstream>
-
-namespace isc { namespace data {
-
-    ///
-    /// A standard DataDefinition exception that is thrown when a
-    /// specification is not in the correct form.
-    ///
-    /// TODO: use jinmei's exception class as a base and not c_str in
-    /// what() there
-    class DataDefinitionError : public std::exception {
-    public:
-        DataDefinitionError(std::string m = "Data definition is invalid") : msg(m) {}
-        ~DataDefinitionError() throw() {}
-        const char* what() const throw() { return msg.c_str(); }
-    private:
-        std::string msg;
-    };
-
-    ///
-    /// The \c DataDefinition class holds a data specification.
-    /// Each module should have a .spec file containing the specification
-    /// for configuration and commands for that module.
-    /// This class holds that specification, and provides a function to
-    /// validate a set of data, to see whether it conforms to the given
-    /// specification
-    ///
-    /// The form of the specification is described in doc/ (TODO)
-    ///
-    class DataDefinition {
-    public:
-        explicit DataDefinition() {};
-        /// Create a \c DataDefinition instance with the given data as
-        /// the specification
-        /// \param e The Element containing the data specification
-        explicit DataDefinition(ElementPtr e) : definition(e) {};
-
-        /// Creates a \c DataDefinition instance from the contents
-        /// of the file given by file_name.
-        /// If check is true, and the definition is not of the correct
-        /// form, a DataDefinitionError is thrown. If the file could
-        /// not be parse, a ParseError is thrown.
-        /// \param file_name The file to be opened and parsed
-        /// \param check If true, the data definition in the file is
-        /// checked to be of the correct form
-        DataDefinition(const std::string& file_name, const bool check = true)
-                       throw(ParseError, DataDefinitionError);
-
-        // todo: make check default false, or leave out option completely and always check?
-        /// Creates a \c DataDefinition instance from the given input
-        /// stream that contains the contents of a .spec file.
-        /// If check is true, and the definition is not of
-        /// the correct form, a DataDefinitionError is thrown. If the
-        /// file could not be parsed, a ParseError is thrown.
-        /// \param in The std::istream containing the .spec file data
-        /// \param check If true, the data definition is checked to be
-        /// of the correct form
-        explicit DataDefinition(std::istream& in, const bool check = true)
-                                throw(ParseError, DataDefinitionError);
-
-        /// Returns the base \c element of the data definition contained
-        /// by this instance
-        /// \return ElementPtr Shared pointer to the data definition
-        const ElementPtr getDefinition() { return definition; };
-        // returns true if the given element conforms to this data
-        // definition scheme
-        /// Validates the given data for this specification.
-        /// \param data The base \c Element of the data to check
-        /// \return true if the data conforms to the specification,
-        /// false otherwise.
-        bool validate(const ElementPtr data);
-
-    private:
-        bool validate_item(const ElementPtr spec, const ElementPtr data);
-        bool validate_spec(const ElementPtr spec, const ElementPtr data);
-        bool validate_spec_list(const ElementPtr spec, const ElementPtr data);
-
-        ElementPtr definition;
-    };
-
-} }
-
-#endif // _DATA_DEF_H

src/lib/config/cpp/data_def.cc

@@ -1,5 +1,19 @@
+// Copyright (C) 2010  Internet Systems Consortium.
+//
+// Permission to use, copy, modify, and distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+// DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+// INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+// FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+// NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+// WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-#include "data_def.h"
+#include "module_spec.h"
 
 #include <sstream>
 #include <iostream>
@@ -8,9 +22,14 @@
 
 #include <boost/foreach.hpp>
 
-// todo: add more context to thrown DataDefinitionErrors?
+// todo: add more context to thrown ModuleSpecErrors?
+
+namespace isc {
+namespace config {
 
-using namespace isc::data;
+//
+// static functions
+//
 
 // todo: is there a direct way to get a std::string from an enum label?
 static std::string
@@ -51,7 +70,7 @@ getType_value(const std::string& type_name) {
     } else if (type_name == "any") {
         return Element::any;
     } else {
-        throw DataDefinitionError(type_name + " is not a valid type name");
+        throw ModuleSpecError(type_name + " is not a valid type name");
     }
 }
 
@@ -62,13 +81,13 @@ check_leaf_item(const ElementPtr& spec, const std::string& name, Element::types
         if (spec->get(name)->getType() == type) {
             return;
         } else {
-            throw DataDefinitionError(name + " not of type " + getType_string(type));
+            throw ModuleSpecError(name + " not of type " + getType_string(type));
         }
     } else if (mandatory) {
         // todo: want parent item name, and perhaps some info about location
         // in list? or just catch and throw new...
         // or make this part non-throwing and check return value...
-        throw DataDefinitionError(name + " missing in " + spec->str());
+        throw ModuleSpecError(name + " missing in " + spec->str());
     }
 }
 
@@ -84,7 +103,7 @@ check_config_item(const ElementPtr& spec) {
                     !spec->get("item_optional")->boolValue()
                    );
 
-    // if list, check the list definition
+    // if list, check the list specification
     if (getType_value(spec->get("item_type")->stringValue()) == Element::list) {
         check_leaf_item(spec, "list_item_spec", Element::map, true);
         check_config_item(spec->get("list_item_spec"));
@@ -99,7 +118,7 @@ check_config_item(const ElementPtr& spec) {
 static void
 check_config_item_list(const ElementPtr& spec) {
     if (spec->getType() != Element::list) {
-        throw DataDefinitionError("config_data is not a list of elements");
+        throw ModuleSpecError("config_data is not a list of elements");
     }
     BOOST_FOREACH(ElementPtr item, spec->listValue()) {
         check_config_item(item);
@@ -116,7 +135,7 @@ check_command(const ElementPtr& spec) {
 static void
 check_command_list(const ElementPtr& spec) {
     if (spec->getType() != Element::list) {
-        throw DataDefinitionError("commands is not a list of elements");
+        throw ModuleSpecError("commands is not a list of elements");
     }
     BOOST_FOREACH(ElementPtr item, spec->listValue()) {
         check_command(item);
@@ -136,45 +155,95 @@ check_data_specification(const ElementPtr& spec) {
     }
 }
 
-// checks whether the given element is a valid data definition
-// throws a DataDefinitionError if the specification is bad
+// checks whether the given element is a valid module specification
+// throws a ModuleSpecError if the specification is bad
 static void
-check_definition(const ElementPtr& def)
+check_module_specification(const ElementPtr& def)
+{
+    if (!def->contains("module_spec")) {
+        throw ModuleSpecError("Data specification does not contain module_spec element");
+    } else {
+        check_data_specification(def->get("module_spec"));
+    }
+}
+
+//
+// Public functions
+//
+
+ModuleSpec::ModuleSpec(ElementPtr module_spec_element,
+                       const bool check)
+                       throw(ModuleSpecError)
+                       
+{
+    module_specification = module_spec_element;
+    if (check) {
+        check_module_specification(module_specification);
+    }
+}
+
+const ElementPtr
+ModuleSpec::getCommandsSpec()
+{
+    if (module_specification->contains("commands")) {
+        return module_specification->get("commands");
+    } else {
+        return ElementPtr();
+    }
+}
+
+const ElementPtr
+ModuleSpec::getConfigSpec()
 {
-    if (!def->contains("data_specification")) {
-        throw DataDefinitionError("Data specification does not contain data_specification element");
+    if (module_specification->contains("config_data")) {
+        return module_specification->get("config_data");
     } else {
-        check_data_specification(def->get("data_specification"));
+        return ElementPtr();
     }
 }
 
-DataDefinition::DataDefinition(const std::string& file_name,
-                               const bool check)
-                               throw(ParseError, DataDefinitionError) {
+const std::string
+ModuleSpec::getModuleName()
+{
+    return module_specification->get("module_name")->stringValue();
+}
+
+bool
+ModuleSpec::validate_config(const ElementPtr data)
+{
+    ElementPtr spec = module_specification->find("module_spec/config_data");
+    return validate_spec_list(spec, data);
+}
+
+ModuleSpec
+moduleSpecFromFile(const std::string& file_name, const bool check)
+                   throw(ParseError, ModuleSpecError)
+{
     std::ifstream file;
 
     file.open(file_name.c_str());
     if (!file) {
         std::stringstream errs;
         errs << "Error opening " << file_name << ": " << strerror(errno);
-        throw DataDefinitionError(errs.str());
+        throw ModuleSpecError(errs.str());
     }
 
-    definition = Element::createFromString(file, file_name);
-    if (check) {
-        check_definition(definition);
-    }
+    ElementPtr module_spec_element = Element::createFromString(file, file_name);
+    return ModuleSpec(module_spec_element, check);
 }
 
-DataDefinition::DataDefinition(std::istream& in, const bool check)
-                               throw(ParseError, DataDefinitionError) {
-    definition = Element::createFromString(in);
-    // make sure the whole structure is complete and valid
-    if (check) {
-        check_definition(definition);
-    }
+ModuleSpec
+moduleSpecFromFile(std::ifstream& in, const bool check)
+                   throw(ParseError, ModuleSpecError) {
+    ElementPtr module_spec_element = Element::createFromString(in);
+    return ModuleSpec(module_spec_element, check);
 }
 
+
+//
+// private functions
+//
+
 //
 // helper functions for validation
 //
@@ -210,7 +279,7 @@ check_type(ElementPtr spec, ElementPtr element)
 }
 
 bool
-DataDefinition::validate_item(const ElementPtr spec, const ElementPtr data) {
+ModuleSpec::validate_item(const ElementPtr spec, const ElementPtr data) {
     if (!check_type(spec, data)) {
         // we should do some proper error feedback here
         // std::cout << "type mismatch; not " << spec->get("item_type") << ": " << data << std::endl;
@@ -240,7 +309,7 @@ DataDefinition::validate_item(const ElementPtr spec, const ElementPtr data) {
 
 // spec is a map with item_name etc, data is a map
 bool
-DataDefinition::validate_spec(const ElementPtr spec, const ElementPtr data) {
+ModuleSpec::validate_spec(const ElementPtr spec, const ElementPtr data) {
     std::string item_name = spec->get("item_name")->stringValue();
     bool optional = spec->get("item_optional")->boolValue();
     ElementPtr data_el;
@@ -260,7 +329,7 @@ DataDefinition::validate_spec(const ElementPtr spec, const ElementPtr data) {
 
 // spec is a list of maps, data is a map
 bool
-DataDefinition::validate_spec_list(const ElementPtr spec, const ElementPtr data) {
+ModuleSpec::validate_spec_list(const ElementPtr spec, const ElementPtr data) {
     ElementPtr cur_data_el;
     std::string cur_item_name;
     BOOST_FOREACH(ElementPtr cur_spec_el, spec->listValue()) {
@@ -271,12 +340,5 @@ DataDefinition::validate_spec_list(const ElementPtr spec, const ElementPtr data)
     return true;
 }
 
-// TODO
-// this function does *not* check if the specification is in correct
-// form, we should do that in the constructor
-bool
-DataDefinition::validate(const ElementPtr data) {
-    ElementPtr spec = definition->find("data_specification/config_data");
-    return validate_spec_list(spec, data);
 }
-
+}

src/lib/config/cpp/module_spec.h

@@ -0,0 +1,121 @@
+// Copyright (C) 2010  Internet Systems Consortium.
+//
+// Permission to use, copy, modify, and distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+// DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+// INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+// FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+// NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+// WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef _MODULE_SPEC_H
+#define _MODULE_SPEC_H 1
+
+#include <cc/data.h>
+
+#include <sstream>
+
+using namespace isc::data;
+
+namespace isc { namespace config {
+
+    ///
+    /// A standard ModuleSpec exception that is thrown when a
+    /// specification is not in the correct form.
+    ///
+    /// TODO: use jinmei's exception class as a base and not c_str in
+    /// what() there
+    class ModuleSpecError : public std::exception {
+    public:
+        ModuleSpecError(std::string m = "Module specification is invalid") : msg(m) {}
+        ~ModuleSpecError() throw() {}
+        const char* what() const throw() { return msg.c_str(); }
+    private:
+        std::string msg;
+    };
+
+    ///
+    /// The \c ModuleSpec class holds a data specification.
+    /// Each module should have a .spec file containing the specification
+    /// for configuration and commands for that module.
+    /// This class holds that specification, and provides a function to
+    /// validate a set of data, to see whether it conforms to the given
+    /// specification
+    ///
+    /// The form of the specification is described in doc/ (TODO)
+    ///
+    class ModuleSpec {
+    public:
+        explicit ModuleSpec() {};
+        /// Create a \c ModuleSpec instance with the given data as
+        /// the specification
+        /// \param e The Element containing the data specification
+        explicit ModuleSpec(ElementPtr e, const bool check = true)
+                            throw(ModuleSpecError);
+
+        /// Returns the commands part of the specification as an
+        /// ElementPtr, returns an empty ElementPtr if there is none
+        /// \return ElementPtr Shared pointer to the commands
+        ///                    part of the specification
+        const ElementPtr getCommandsSpec();
+
+        /// Returns the configuration part of the specification as an
+        /// ElementPtr
+        /// \return ElementPtr Shared pointer to the configuration
+        ///                    part of the specification
+        const ElementPtr getConfigSpec();
+
+        /// Returns the full module specification as an ElementPtr
+        /// \return ElementPtr Shared pointer to the specification
+        const ElementPtr getFullSpec() { return module_specification; };
+
+        /// Returns the module name as specified by the specification
+        const std::string getModuleName();
+        
+        // returns true if the given element conforms to this data
+        // configuration specification
+        /// Validates the given configuration data for this specification.
+        /// \param data The base \c Element of the data to check
+        /// \return true if the data conforms to the specification,
+        /// false otherwise.
+        bool validate_config(const ElementPtr data);
+
+    private:
+        bool validate_item(const ElementPtr spec, const ElementPtr data);
+        bool validate_spec(const ElementPtr spec, const ElementPtr data);
+        bool validate_spec_list(const ElementPtr spec, const ElementPtr data);
+
+        ElementPtr module_specification;
+    };
+
+    /// Creates a \c ModuleSpec instance from the contents
+    /// of the file given by file_name.
+    /// If check is true, and the module specification is not of
+    /// the correct form, a ModuleSpecError is thrown. If the file
+    /// could not be parse, a ParseError is thrown.
+    /// \param file_name The file to be opened and parsed
+    /// \param check If true, the module specification in the file
+    /// is checked to be of the correct form
+    ModuleSpec
+    moduleSpecFromFile(const std::string& file_name, const bool check = true)
+                       throw(ParseError, ModuleSpecError);
+
+    /// Creates a \c ModuleSpec instance from the given input
+    /// stream that contains the contents of a .spec file.
+    /// If check is true, and the module specification is not of
+    /// the correct form, a ModuleSpecError is thrown. If the
+    /// file could not be parsed, a ParseError is thrown.
+    /// \param in The std::istream containing the .spec file data
+    /// \param check If true, the module specification is checked
+    /// to be of the correct form
+    ModuleSpec
+    moduleSpecFromFile(std::ifstream& in, const bool check = true)
+                       throw(ParseError, ModuleSpecError);
+} }
+
+#endif // _DATA_DEF_H

src/lib/config/cpp/data_def_unittests.cc

@@ -16,111 +16,112 @@
 
 #include <gtest/gtest.h>
 
-#include <data_def.h>
+#include <module_spec.h>
 
 #include <fstream>
 
 #include "data_def_unittests_config.h"
 
 using namespace isc::data;
+using namespace isc::config;
 
 std::string specfile(const std::string name) {
     return std::string(TEST_DATA_PATH) + "/" + name;
 }
 
 void
-data_def_error(const std::string& file,
+module_spec_error(const std::string& file,
                const std::string& error1,
                const std::string& error2 = "",
                const std::string& error3 = "")
 {
-    EXPECT_THROW(DataDefinition(specfile(file)), DataDefinitionError);
+    EXPECT_THROW(moduleSpecFromFile(specfile(file)), ModuleSpecError);
     try {
-        DataDefinition dd = DataDefinition(specfile(file));
-    } catch (DataDefinitionError dde) {
+        ModuleSpec dd = moduleSpecFromFile(specfile(file));
+    } catch (ModuleSpecError dde) {
         std::string ddew = dde.what();
         EXPECT_EQ(error1 + error2 + error3, ddew);
     }
 }
 
-TEST(DataDefinition, ReadingSpecfiles) {
+TEST(ModuleSpec, ReadingSpecfiles) {
     // Tests whether we can open specfiles and if we get the
     // right parse errors
-    DataDefinition dd = DataDefinition(specfile("spec1.spec"));
-    EXPECT_EQ(dd.getDefinition()->get("data_specification")
+    ModuleSpec dd = moduleSpecFromFile(specfile("spec1.spec"));
+    EXPECT_EQ(dd.getFullSpec()->get("module_spec")
                                 ->get("module_name")
                                 ->stringValue(), "Spec1");
-    dd = DataDefinition(specfile("spec2.spec"));
-    EXPECT_EQ(dd.getDefinition()->get("data_specification")
+    dd = moduleSpecFromFile(specfile("spec2.spec"));
+    EXPECT_EQ(dd.getFullSpec()->get("module_spec")
                                 ->get("config_data")->size(), 6);
-    data_def_error("doesnotexist",
+    module_spec_error("doesnotexist",
                    "Error opening ",
                    specfile("doesnotexist"),
                    ": No such file or directory");
 
     std::ifstream file;
     file.open(specfile("spec1.spec").c_str());
-    dd = DataDefinition(file);
-    EXPECT_EQ(dd.getDefinition()->get("data_specification")
+    dd = moduleSpecFromFile(file);
+    EXPECT_EQ(dd.getFullSpec()->get("module_spec")
                                 ->get("module_name")
                                 ->stringValue(), "Spec1");
 }
 
-TEST(DataDefinition, SpecfileItems) {
-    data_def_error("spec3.spec",
+TEST(ModuleSpec, SpecfileItems) {
+    module_spec_error("spec3.spec",
                    "item_name missing in {\"item_default\": 1, \"item_optional\": False, \"item_type\": \"integer\"}");
-    data_def_error("spec4.spec",
+    module_spec_error("spec4.spec",
                    "item_type missing in {\"item_default\": 1, \"item_name\": \"item1\", \"item_optional\": False}");
-    data_def_error("spec5.spec",
+    module_spec_error("spec5.spec",
                    "item_optional missing in {\"item_default\": 1, \"item_name\": \"item1\", \"item_type\": \"integer\"}");
-    data_def_error("spec6.spec",
+    module_spec_error("spec6.spec",
                    "item_default missing in {\"item_name\": \"item1\", \"item_optional\": False, \"item_type\": \"integer\"}");
-    data_def_error("spec9.spec",
+    module_spec_error("spec9.spec",
                    "item_default not of type integer");
-    data_def_error("spec10.spec",
+    module_spec_error("spec10.spec",
                    "item_default not of type real");
-    data_def_error("spec11.spec",
+    module_spec_error("spec11.spec",
                    "item_default not of type boolean");
-    data_def_error("spec12.spec",
+    module_spec_error("spec12.spec",
                    "item_default not of type string");
-    data_def_error("spec13.spec",
+    module_spec_error("spec13.spec",
                    "item_default not of type list");
-    data_def_error("spec14.spec",
+    module_spec_error("spec14.spec",
                    "item_default not of type map");
-    data_def_error("spec15.spec",
+    module_spec_error("spec15.spec",
                    "badname is not a valid type name");
 }
 
-TEST(DataDefinition, SpecfileConfigData)
+TEST(ModuleSpec, SpecfileConfigData)
 {
-    data_def_error("spec7.spec",
+    module_spec_error("spec7.spec",
                    "module_name missing in {}");
-    data_def_error("spec8.spec",
-                   "Data specification does not contain data_specification element");
-    data_def_error("spec16.spec",
+    module_spec_error("spec8.spec",
+                   "Data specification does not contain module_spec element");
+    module_spec_error("spec16.spec",
                    "config_data is not a list of elements");
-    data_def_error("spec21.spec",
+    module_spec_error("spec21.spec",
                    "commands is not a list of elements");
 }
 
-TEST(DataDefinition, SpecfileCommands)
+TEST(ModuleSpec, SpecfileCommands)
 {
-    data_def_error("spec17.spec",
+    module_spec_error("spec17.spec",
                    "command_name missing in {\"command_args\": [ {\"item_default\": \"\", \"item_name\": \"message\", \"item_optional\": False, \"item_type\": \"string\"} ], \"command_description\": \"Print the given message to stdout\"}");
-    data_def_error("spec18.spec",
+    module_spec_error("spec18.spec",
                    "command_args missing in {\"command_description\": \"Print the given message to stdout\", \"command_name\": \"print_message\"}");
-    data_def_error("spec19.spec",
+    module_spec_error("spec19.spec",
                    "command_args not of type list");
-    data_def_error("spec20.spec",
+    module_spec_error("spec20.spec",
                    "somethingbad is not a valid type name");
 /*
-    data_def_error("spec22.spec",
+    module_spec_error("spec22.spec",
                    "");
 */
 }
 
 bool
-data_test(DataDefinition dd, const std::string& data_file_name)
+data_test(ModuleSpec dd, const std::string& data_file_name)
 {
     std::ifstream data_file;
 
@@ -128,11 +129,11 @@ data_test(DataDefinition dd, const std::string& data_file_name)
     ElementPtr data = Element::createFromString(data_file, data_file_name);
     data_file.close();
 
-    return dd.validate(data);
+    return dd.validate_config(data);
 }
 
-TEST(DataDefinition, DataValidation) {
-    DataDefinition dd = DataDefinition(specfile("spec22.spec"));
+TEST(ModuleSpec, DataValidation) {
+    ModuleSpec dd = moduleSpecFromFile(specfile("spec22.spec"));
 
     EXPECT_TRUE(data_test(dd, "data22_1.data"));
     EXPECT_FALSE(data_test(dd, "data22_2.data"));

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

@@ -1,2 +1,3 @@
 from isc.config.ccsession import *
-from isc.config.datadefinition import *
+from isc.config.config_data import *
+from isc.config.module_spec import *

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

@@ -21,14 +21,81 @@
 
 # modeled after ccsession.h/cc 'protocol' changes here need to be
 # made there as well
+"""Classes and functions for handling configuration and commands
+
+   This module provides the ModuleCCSession and UICCSession classes,
+   as well as a set of utility functions to create and parse messages
+   related to commands and configuration
+
+   Modules should use the ModuleCCSession class to connect to the
+   configuration manager, and receive updates and commands from
+   other modules.
+
+   Configuration user interfaces should use the UICCSession to connect
+   to b10-cmdctl, and receive and send configuration and commands
+   through that to the configuration manager.
+"""
 
 from isc.cc import Session
+from isc.config.config_data import ConfigData, MultiConfigData
 import isc
 
-class CCSession:
+class ModuleCCSessionError(Exception): pass
+
+def parse_answer(msg):
+    """Returns a tuple (rcode, value), where value depends on the
+       command that was called. If rcode != 0, value is a string
+       containing an error message"""
+    if 'result' not in msg:
+        raise ModuleCCSessionError("answer message does not contain 'result' element")
+    elif type(msg['result']) != list:
+        raise ModuleCCSessionError("wrong result type in answer message")
+    elif len(msg['result']) < 1:
+        raise ModuleCCSessionError("empty result list in answer message")
+    elif type(msg['result'][0]) != int:
+        raise ModuleCCSessionError("wrong rcode type in answer message")
+    else:
+        if len(msg['result']) > 1:
+            return msg['result'][0], msg['result'][1]
+        else:
+            return msg['result'][0], None
+
+def create_answer(rcode, arg = None):
+    """Creates an answer packet for config&commands. rcode must be an
+       integer. If rcode == 0, arg is an optional value that depends
+       on what the command or option was. If rcode != 0, arg must be
+       a string containing an error message"""
+    if type(rcode) != int:
+        raise ModuleCCSessionError("rcode in create_answer() must be an integer")
+    if rcode != 0 and type(arg) != str:
+        raise ModuleCCSessionError("arg in create_answer for rcode != 0 must be a string describing the error")
+    if arg != None:
+        return { 'result': [ rcode, arg ] }
+    else:
+        return { 'result': [ rcode ] }
+
+class ModuleCCSession(ConfigData):
+    """This class maintains a connection to the command channel, as
+       well as configuration options for modules. The module provides
+       a specification file that contains the module name, configuration
+       options, and commands. It also gives the ModuleCCSession two callback
+       functions, one to call when there is a direct command to the
+       module, and one to update the configuration run-time. These
+       callbacks are called when 'check_command' is called on the
+       ModuleCCSession"""
+       
     def __init__(self, spec_file_name, config_handler, command_handler):
-        self._data_definition = isc.config.DataDefinition(spec_file_name)
-        self._module_name = self._data_definition.get_module_name()
+        """Initialize a ModuleCCSession. This does *NOT* send the
+           specification and request the configuration yet. Use start()
+           for that once the ModuleCCSession has been initialized.
+           specfile_name is the path to the specification file
+           config_handler and command_handler are callback functions,
+           see set_config_handler and set_command_handler for more
+           information on their signatures."""
+        module_spec = isc.config.module_spec_from_file(spec_file_name)
+        ConfigData.__init__(self, module_spec)
+        
+        self._module_name = module_spec.get_module_name()
         
         self.set_config_handler(config_handler)
         self.set_command_handler(command_handler)
@@ -36,65 +103,162 @@ class CCSession:
         self._session = Session()
         self._session.group_subscribe(self._module_name, "*")
 
+    def start(self):
+        """Send the specification for this module to the configuration
+           manager, and request the current non-default configuration.
+           The config_handler will be called with that configuration"""
         self.__send_spec()
-        self.__get_full_config()
+        self.__request_config()
 
     def get_socket(self):
-        """Returns the socket from the command channel session"""
+        """Returns the socket from the command channel session. This can
+           be used in select() loops to see if there is anything on the
+           channel. This is not strictly necessary as long as
+           check_command is called periodically."""
         return self._session._socket
     
     def get_session(self):
         """Returns the command-channel session that is used, so the
-           application can use it directly"""
+           application can use it directly."""
         return self._session
 
     def close(self):
+        """Close the session to the command channel"""
         self._session.close()
 
     def check_command(self):
-        """Check whether there is a command on the channel.
-           Call the command callback function if so"""
+        """Check whether there is a command or configuration update
+           on the channel. Call the corresponding callback function if
+           there is."""
         msg, env = self._session.group_recvmsg(False)
-        answer = None
+        # should we default to an answer? success-by-default? unhandled error?
         if msg:
-            if "config_update" in msg and self._config_handler:
-                self._config_handler(msg["config_update"])
-                answer = { "result": [ 0 ] }
-            if "command" in msg and self._command_handler:
-                answer = self._command_handler(msg["command"])
-        if answer:
-            self._session.group_reply(env, answer)
-
+            answer = None
+            try:
+                if "config_update" in msg:
+                    new_config = msg["config_update"]
+                    errors = []
+                    if not self._config_handler:
+                        answer = create_answer(2, self._module_name + " has no config handler")
+                    elif not self.get_module_spec().validate_config(False, new_config, errors):
+                        answer = create_answer(1, " ".join(errors))
+                    else:
+                        answer = self._config_handler(msg["config_update"])
+                if "command" in msg and self._command_handler:
+                    answer = self._command_handler(msg["command"])
+            except Exception as exc:
+                answer = create_answer(1, str(exc))
+            if answer:
+                self._session.group_reply(env, answer)
     
     def set_config_handler(self, config_handler):
         """Set the config handler for this module. The handler is a
            function that takes the full configuration and handles it.
-           It should return either { "result": [ 0 ] } or
-           { "result": [ <error_number>, "error message" ] }"""
+           It should return an answer created with create_answer()"""
         self._config_handler = config_handler
         # should we run this right now since we've changed the handler?
 
     def set_command_handler(self, command_handler):
         """Set the command handler for this module. The handler is a
            function that takes a command as defined in the .spec file
-           and return either { "result": [ 0, (result) ] } or
-           { "result": [ <error_number>. "error message" ] }"""
+           and return an answer created with create_answer()"""
         self._command_handler = command_handler
 
     def __send_spec(self):
         """Sends the data specification to the configuration manager"""
-        self._session.group_sendmsg(self._data_definition.get_definition(), "ConfigManager")
+        self._session.group_sendmsg({ "module_spec": self.get_module_spec().get_full_spec() }, "ConfigManager")
         answer, env = self._session.group_recvmsg(False)
         
-    def __get_full_config(self):
+    def __request_config(self):
         """Asks the configuration manager for the current configuration, and call the config handler if set"""
         self._session.group_sendmsg({ "command": [ "get_config", { "module_name": self._module_name } ] }, "ConfigManager")
         answer, env = self._session.group_recvmsg(False)
-        if "result" in answer:
-            if answer["result"][0] == 0 and len(answer["result"]) > 1:
-                new_config = answer["result"][1]
-                if self._data_definition.validate(new_config):
-                    self._config = new_config;
-                    if self._config_handler:
-                        self._config_handler(answer["result"])
-    
+        rcode, value = parse_answer(answer)
+        if rcode == 0:
+            if value != None and self.get_module_spec().validate_config(False, value):
+                self.set_local_config(value);
+                if self._config_handler:
+                    self._config_handler(value)
+        else:
+            # log error
+            print("Error requesting configuration: " + value)
+
+class UIModuleCCSession(MultiConfigData):
+    """This class is used in a configuration user interface. It contains
+       specific functions for getting, displaying, and sending
+       configuration settings through the b10-cmdctl module."""
+    def __init__(self, conn):
+        """Initialize a UIModuleCCSession. The conn object that is
+           passed must have send_GET and send_POST functions"""
+        MultiConfigData.__init__(self)
+        self._conn = conn
+        self.request_specifications()
+        self.request_current_config()
+
+    def request_specifications(self):
+        """Request the module specifications from b10-cmdctl"""
+        # this step should be unnecessary but is the current way cmdctl returns stuff
+        # so changes are needed there to make this clean (we need a command to simply get the
+        # full specs for everything, including commands etc, not separate gets for that)
+        specs = self._conn.send_GET('/config_spec')
+        commands = self._conn.send_GET('/commands')
+        for module in specs.keys():
+            cur_spec = { 'module_name': module }
+            if module in specs and specs[module]:
+                cur_spec['config_data'] = specs[module]
+            if module in commands and commands[module]:
+                cur_spec['commands'] = commands[module]
+            
+            self.set_specification(isc.config.ModuleSpec(cur_spec))
+
+    def request_current_config(self):
+        """Requests the current configuration from the configuration
+           manager through b10-cmdctl, and stores those as CURRENT"""
+        config = self._conn.send_GET('/config_data')
+        if 'version' not in config or config['version'] != 1:
+            raise Exception("Bad config version")
+        self._set_current_config(config)
+
+    def add_value(self, identifier, value_str):
+        """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"""
+        module_spec = self.find_spec_part(identifier)
+        if (type(module_spec) != dict or "list_item_spec" not in module_spec):
+            raise DataTypeError(identifier + " is not a list")
+        value = isc.cc.data.parse_value_str(value_str)
+        cur_list, status = self.get_value(identifier)
+        if not cur_list:
+            cur_list = []
+        if value not in cur_list:
+            cur_list.append(value)
+        self.set_value(identifier, cur_list)
+
+    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
+           """
+        module_spec = self.find_spec_part(identifier)
+        if (type(module_spec) != dict or "list_item_spec" not in module_spec):
+            raise DataTypeError(identifier + " is not a list")
+        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:
+            cur_list.remove(value)
+        self.set_value(identifier, cur_list)
+
+    def commit(self):
+        """Commit all local changes, send them through b10-cmdctl to
+           the configuration manager"""
+        if self.get_local_changes():
+            self._conn.send_POST('/ConfigManager/set_config', self.get_local_changes())
+            # todo: check result
+            self.request_current_config()
+            self.clear_local_changes()

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

@@ -1,3 +1,24 @@
+# Copyright (C) 2010  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+"""This is the BIND 10 configuration manager, run by b10-cfgmgr.
+
+   It stores the system configuration, and sends updates of the
+   configuration to the modules that need them.
+"""
+
 import isc
 import signal
 import ast
@@ -5,21 +26,42 @@ import pprint
 import os
 from isc.cc import data
 
+class ConfigManagerDataReadError(Exception):
+    """This exception is thrown when there is an error while reading
+       the current configuration on startup."""
+    pass
+
+class ConfigManagerDataEmpty(Exception):
+    """This exception is thrown when the currently stored configuration
+       is not found, or appears empty."""
+    pass
+
 class ConfigManagerData:
+    """This class hold the actual configuration information, and
+       reads it from and writes it to persistent storage"""
+
     CONFIG_VERSION = 1
 
-    def __init__(self, data_path):
+    def __init__(self, data_path, file_name = "b10-config.db"):
+        """Initialize the data for the configuration manager, and
+           set the version and path for the data store. Initializing
+           this does not yet read the database, a call to
+           read_from_file is needed for that."""
         self.data = {}
         self.data['version'] = ConfigManagerData.CONFIG_VERSION
         self.data_path = data_path
-        self.db_filename = data_path + "/b10-config.db"
+        self.db_filename = data_path + os.sep + file_name
 
-    def set_data_definition(self, module_name, module_data_definition):
-        self.zones[module_name] = module_data_definition
-        self.data_definitions[module_name] = module_data_definition
-
-    def read_from_file(data_path):
-        config = ConfigManagerData(data_path)
+    def read_from_file(data_path, file_name = "b10-config.db"):
+        """Read the current configuration found in the file at
+           data_path. If the file does not exist, a
+           ConfigManagerDataEmpty exception is raised. If there is a
+           parse error, or if the data in the file has the wrong
+           version, a ConfigManagerDataReadError is raised. In the first
+           case, it is probably safe to log and ignore. In the case of
+           the second exception, the best way is probably to report the
+           error and stop loading the system."""
+        config = ConfigManagerData(data_path, file_name)
         try:
             file = open(config.db_filename, 'r')
             file_config = ast.literal_eval(file.read())
@@ -27,19 +69,20 @@ class ConfigManagerData:
                 file_config['version'] == ConfigManagerData.CONFIG_VERSION:
                 config.data = file_config
             else:
-                # of course we can put in a migration path here for old data
-                print("[bind-cfgd] Old version of data found, starting with empty configuration")
+                # We can put in a migration path here for old data
+                raise ConfigManagerDataReadError("[bind-cfgd] Old version of data found")
             file.close()
         except IOError as ioe:
-            print("No config file found, starting with empty config")
-        except EOFError as eofe:
-            print("Config file empty, starting with empty config")
+            raise ConfigManagerDataEmpty("No config file found")
         except:
-            print("Config file unreadable, starting with empty config")
+            raise ConfigManagerDataReadError("Config file unreadable")
 
         return config
         
-    def write_to_file(self):
+    def write_to_file(self, output_file_name = None):
+        """Writes the current configuration data to a file. If
+           output_file_name is not specified, the file used in
+           read_from_file is used."""
         try:
             tmp_filename = self.db_filename + ".tmp"
             file = open(tmp_filename, 'w');
@@ -52,141 +95,235 @@ class ConfigManagerData:
         except IOError as ioe:
             print("Unable to write config file; configuration not stored")
 
+    def __eq__(self, other):
+        """Returns True if the data contained is equal. data_path and
+           db_filename may be different."""
+        if type(other) != type(self):
+            return False
+        return self.data == other.data
+
 class ConfigManager:
-    def __init__(self, data_path):
-        self.commands = {}
-        self.data_definitions = {}
+    """Creates a configuration manager. The data_path is the path
+       to the directory containing the b10-config.db file.
+       If session is set, this will be used as the communication
+       channel session. If not, a new session will be created.
+       The ability to specify a custom session is for testing purposes
+       and should not be needed for normal usage."""
+    def __init__(self, data_path, session = None):
+        """Initialize the configuration manager. The data_path string
+           is the path to the directory where the configuration is
+           stored (in <data_path>/b10-config.db). Session is an optional
+           cc-channel session. If this is not given, a new one is
+           created"""
         self.data_path = data_path
+        self.module_specs = {}
         self.config = ConfigManagerData(data_path)
-        self.cc = isc.cc.Session()
+        if session:
+            self.cc = session
+        else:
+            self.cc = isc.cc.Session()
         self.cc.group_subscribe("ConfigManager")
         self.cc.group_subscribe("Boss", "ConfigManager")
         self.running = False
 
     def notify_boss(self):
+        """Notifies the Boss module that the Config Manager is running"""
         self.cc.group_sendmsg({"running": "configmanager"}, "Boss")
 
-    def set_config(self, module_name, data_specification):
-        self.data_definitions[module_name] = data_specification
-        
-    def remove_config(self, module_name):
-        self.data_definitions[module_name]
+    def set_module_spec(self, spec):
+        """Adds a ModuleSpec"""
+        self.module_specs[spec.get_module_name()] = spec
 
-    def set_commands(self, module_name, commands):
-        self.commands[module_name] = commands
+    def remove_module_spec(self, module_name):
+        """Removes the full ModuleSpec for the given module_name.
+           Does nothing if the module was not present."""
+        if module_name in self.module_specs:
+            del self.module_specs[module_name]
 
-    def remove_commands(self, module_name):
-        del self.commands[module_name]
+    def get_module_spec(self, module_name):
+        """Returns the full ModuleSpec for the module with the given
+           module_name"""
+        if module_name in self.module_specs:
+            return self.module_specs[module_name]
+
+    def get_config_spec(self, name = None):
+        """Returns a dict containing 'module_name': config_spec for
+           all modules. If name is specified, only that module will
+           be included"""
+        config_data = {}
+        if name:
+            if name in self.module_specs:
+                config_data[name] = self.module_specs[name].get_data
+        else:
+            for module_name in self.module_specs.keys():
+                config_data[module_name] = self.module_specs[module_name].get_config_spec()
+        return config_data
+
+    def get_commands_spec(self, name = None):
+        """Returns a dict containing 'module_name': commands_spec for
+           all modules. If name is specified, only that module will
+           be included"""
+        commands = {}
+        if name:
+            if name in self.module_specs:
+                commands[name] = self.module_specs[name].get_commands_spec
+        else:
+            for module_name in self.module_specs.keys():
+                commands[module_name] = self.module_specs[module_name].get_commands_spec()
+        return commands
 
     def read_config(self):
-        print("Reading config")
-        self.config = ConfigManagerData.read_from_file(self.data_path)
+        """Read the current configuration from the b10-config.db file
+           at the path specificied at init()"""
+        try:
+            self.config = ConfigManagerData.read_from_file(self.data_path)
+        except ConfigManagerDataEmpty:
+            # ok, just start with an empty config
+            self.config = ConfigManagerData(self.data_path)
         
     def write_config(self):
-        print("Writing config")
+        """Write the current configuration to the b10-config.db file
+           at the path specificied at init()"""
         self.config.write_to_file()
 
+    def _handle_get_module_spec(self, cmd):
+        """Private function that handles the 'get_module_spec' command"""
+        answer = {}
+        if len(cmd) > 1:
+            if type(cmd[1]) == dict:
+                if 'module_name' in cmd[1] and cmd[1]['module_name'] != '':
+                    module_name = cmd[1]['module_name']
+                    answer = isc.config.ccsession.create_answer(0, self.get_config_spec(module_name))
+                else:
+                    answer = isc.config.ccsession.create_answer(1, "Bad module_name in get_module_spec command")
+            else:
+                answer = isc.config.ccsession.create_answer(1, "Bad get_module_spec command, argument not a dict")
+        else:
+            answer = isc.config.ccsession.create_answer(0, self.get_config_spec())
+        return answer
+
+    def _handle_get_config(self, cmd):
+        """Private function that handles the 'get_config' command"""
+        answer = {}
+        if len(cmd) > 1:
+            if type(cmd[1]) == dict:
+                if 'module_name' in cmd[1] and cmd[1]['module_name'] != '':
+                    module_name = cmd[1]['module_name']
+                    try:
+                        answer = isc.config.ccsession.create_answer(0, data.find(self.config.data, module_name))
+                    except data.DataNotFoundError as dnfe:
+                        # no data is ok, that means we have nothing that
+                        # deviates from default values
+                        answer = isc.config.ccsession.create_answer(0, {})
+                else:
+                    answer = isc.config.ccsession.create_answer(1, "Bad module_name in get_config command")
+            else:
+                answer = isc.config.ccsession.create_answer(1, "Bad get_config command, argument not a dict")
+        else:
+            answer = isc.config.ccsession.create_answer(0, self.config.data)
+        return answer
+
+    def _handle_set_config(self, cmd):
+        """Private function that handles the 'set_config' command"""
+        answer = None
+        if len(cmd) == 3:
+            # todo: use api (and check the data against the definition?)
+            module_name = cmd[1]
+            conf_part = data.find_no_exc(self.config.data, module_name)
+            if conf_part:
+                data.merge(conf_part, cmd[2])
+                self.cc.group_sendmsg({ "config_update": conf_part }, module_name)
+                answer, env = self.cc.group_recvmsg(False)
+            else:
+                conf_part = data.set(self.config.data, module_name, {})
+                data.merge(conf_part[module_name], cmd[2])
+                # send out changed info
+                self.cc.group_sendmsg({ "config_update": conf_part[module_name] }, module_name)
+                # replace 'our' answer with that of the module
+                answer, env = self.cc.group_recvmsg(False)
+            rcode, val = isc.config.ccsession.parse_answer(answer)
+            if rcode == 0:
+                self.write_config()
+        elif len(cmd) == 2:
+            # todo: use api (and check the data against the definition?)
+            data.merge(self.config.data, cmd[1])
+            # send out changed info
+            got_error = False
+            err_list = []
+            for module in self.config.data:
+                if module != "version":
+                    self.cc.group_sendmsg({ "config_update": self.config.data[module] }, module)
+                    answer, env = self.cc.group_recvmsg(False)
+                    rcode, val = isc.config.ccsession.parse_answer(answer)
+                    if rcode != 0:
+                        got_error = True
+                        err_list.append(val)
+            if not got_error:
+                self.write_config()
+                answer = isc.config.ccsession.create_answer(0)
+            else:
+                # TODO rollback changes that did get through?
+                # feed back *all* errors?
+                answer = isc.config.ccsession.create_answer(1, " ".join(err_list))
+        else:
+            answer = isc.config.ccsession.create_answer(1, "Wrong number of arguments")
+        if not answer:
+            answer = isc.config.ccsession.create_answer(1, "Error handling set_config command")
+            
+        return answer
+
+    def _handle_module_spec(self, spec):
+        """Private function that handles the 'module_spec' command"""
+        # todo: validate? (no direct access to spec as
+        # todo: use ModuleSpec class
+        # todo: error checking (like keyerrors)
+        answer = {}
+        self.set_module_spec(spec)
+        
+        # We should make one general 'spec update for module' that
+        # passes both specification and commands at once
+        self.cc.group_sendmsg({ "specification_update": [ spec.get_module_name(), spec.get_config_spec() ] }, "Cmd-Ctrld")
+        self.cc.group_sendmsg({ "commands_update": [ spec.get_module_name(), spec.get_commands_spec() ] }, "Cmd-Ctrld")
+        answer = isc.config.ccsession.create_answer(0)
+        return answer
+
     def handle_msg(self, msg):
-        """return answer message"""
+        """Handle a command from the cc channel to the configuration manager"""
         answer = {}
         if "command" in msg:
             cmd = msg["command"]
             try:
-                if cmd[0] == "get_commands":
-                    print("[XX] bind-cfgd got get_commands");
-                    print("[XX] sending:")
-                    print(self.commands)
-                    answer["result"] = [ 0, self.commands ]
-                elif cmd[0] == "get_data_spec":
-                    if len(cmd) > 1 and cmd[1]['module_name'] != '':
-                        module_name = cmd[1]['module_name']
-                        try:
-                            answer["result"] = [0, self.data_definitions[module_name]]
-                        except KeyError as ke:
-                            answer["result"] = [1, "No specification for module " + module_name]
-                    else:
-                        answer["result"] = [0, self.data_definitions]
+                if cmd[0] == "get_commands_spec":
+                    answer = isc.config.ccsession.create_answer(0, self.get_commands_spec())
+                elif cmd[0] == "get_module_spec":
+                    answer = self._handle_get_module_spec(cmd)
                 elif cmd[0] == "get_config":
-                    # we may not have any configuration here
-                    conf_part = None
-                    print("[XX] bind-cfgd got command:")
-                    print(cmd)
-                    if len(cmd) > 1:
-                        try:
-                            conf_part = data.find(self.config.data, cmd[1]['module_name'])
-                        except data.DataNotFoundError as dnfe:
-                            pass
-                    else:
-                        conf_part = self.config.data
-                    if conf_part:
-                        answer["result"] = [ 0, conf_part ]
-                    else:
-                        answer["result"] = [ 0 ]
-
+                    answer = self._handle_get_config(cmd)
                 elif cmd[0] == "set_config":
-                    if len(cmd) == 3:
-                        # todo: use api (and check types?)
-                        if cmd[1] != "":
-                            conf_part = data.find_no_exc(self.config.data, cmd[1])
-                            if not conf_part:
-                                conf_part = data.set(self.config.data, cmd[1], "")
-                        else:
-                            conf_part = self.config.data
-                        data.merge(conf_part, cmd[2])
-                        print("[XX bind-cfgd] new config (part):")
-                        print(conf_part)
-                        # send out changed info
-                        self.cc.group_sendmsg({ "config_update": conf_part }, cmd[1])
-                        self.write_config()
-                        answer["result"] = [ 0 ]
-                    elif len(cmd) == 2:
-                        print("[XX bind-cfgd] old config:")
-                        print(self.config.data)
-                        print("[XX bind-cfgd] updating with:")
-                        print(cmd[1])
-                        # TODO: ask module to check the config first...
-                        data.merge(self.config.data, cmd[1])
-                        print("[XX bind-cfgd] new config:")
-                        print(self.config.data)
-                        # send out changed info
-                        for module in self.config.data:
-                            self.cc.group_sendmsg({ "config_update": self.config.data[module] }, module)
-                        self.write_config()
-                        answer["result"] = [ 0 ]
-                    else:
-                        answer["result"] = [ 1, "Wrong number of arguments" ]
-
+                    answer = self._handle_set_config(cmd)
                 elif cmd[0] == "shutdown":
                     print("[bind-cfgd] Received shutdown command")
                     self.running = False
+                    answer = isc.config.ccsession.create_answer(0)
                 else:
-                    print("[bind-cfgd] unknown command: " + str(cmd))
-                    answer["result"] = [ 1, "Unknown command: " + str(cmd) ]
+                    answer = isc.config.ccsession.create_answer(1, "Unknown command: " + str(cmd))
             except IndexError as ie:
-                print("[bind-cfgd] missing argument")
-                answer["result"] = [ 1, "Missing argument in command: " + str(ie) ]
+                answer = isc.config.ccsession.create_answer(1, "Missing argument in command: " + str(ie))
                 raise ie
-        elif "data_specification" in msg:
-            # todo: validate? (no direct access to spec as
-            # todo: use DataDefinition class?
-            print("[XX] bind-cfgd got specification:")
-            print(msg["data_specification"])
-            spec = msg["data_specification"]
-            if "config_data" in spec:
-                self.set_config(spec["module_name"], spec["config_data"])
-                self.cc.group_sendmsg({ "specification_update": [ spec["module_name"], spec["config_data"] ] }, "Cmd-Ctrld")
-            if "commands" in spec:
-                self.set_commands(spec["module_name"], spec["commands"])
-                self.cc.group_sendmsg({ "commands_update": [ spec["module_name"], spec["commands"] ] }, "Cmd-Ctrld")
-            answer["result"] = [ 0 ]
+        elif "module_spec" in msg:
+            try:
+                answer = self._handle_module_spec(isc.config.ModuleSpec(msg["module_spec"]))
+            except isc.config.ModuleSpecError as dde:
+                answer = isc.config.ccsession.create_answer(1, "Error in data definition: " + str(dde))
         elif 'result' in msg:
-            answer['result'] = [0]
+            # this seems wrong, might start pingpong
+            answer = isc.config.ccsession.create_answer(0)
         else:
-            print("[bind-cfgd] unknown message: " + str(msg))
-            answer["result"] = [ 1, "Unknown module: " + str(msg) ]
+            answer = isc.config.ccsession.create_answer(1, "Unknown message format: " + str(msg))
         return answer
         
     def run(self):
+        """Runs the configuration manager."""
         self.running = True
         while (self.running):
             msg, env = self.cc.group_recvmsg(False)
@@ -195,10 +332,3 @@ class ConfigManager:
                 self.cc.group_reply(env, answer)
             else:
                 self.running = False
-
-cm = None
-
-def signal_handler(signal, frame):
-    global cm
-    if cm:
-        cm.running = False

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

@@ -0,0 +1,284 @@
+# Copyright (C) 2010  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+#
+# Tests for the configuration manager module
+#
+
+import unittest
+import os
+from isc.config.cfgmgr import *
+
+class TestConfigManagerData(unittest.TestCase):
+    def setUp(self):
+        self.data_path = os.environ['CONFIG_TESTDATA_PATH']
+        self.config_manager_data = ConfigManagerData(self.data_path)
+        self.assert_(self.config_manager_data)
+
+    def test_init(self):
+        self.assertEqual(self.config_manager_data.data['version'],
+                         ConfigManagerData.CONFIG_VERSION)
+        self.assertEqual(self.config_manager_data.data_path,
+                         self.data_path)
+        self.assertEqual(self.config_manager_data.db_filename,
+                         self.data_path + os.sep + "b10-config.db")
+
+    def test_read_from_file(self):
+        ConfigManagerData.read_from_file(self.data_path)
+        self.assertRaises(ConfigManagerDataEmpty,
+                          ConfigManagerData.read_from_file,
+                          "doesnotexist")
+        self.assertRaises(ConfigManagerDataReadError,
+                          ConfigManagerData.read_from_file,
+                          self.data_path, "b10-config-bad1.db")
+        self.assertRaises(ConfigManagerDataReadError,
+                          ConfigManagerData.read_from_file,
+                          self.data_path, "b10-config-bad2.db")
+        self.assertRaises(ConfigManagerDataReadError,
+                          ConfigManagerData.read_from_file,
+                          self.data_path, "b10-config-bad3.db")
+
+    def test_write_to_file(self):
+        output_file_name = "b10-config-write-test";
+        self.config_manager_data.write_to_file(output_file_name)
+        new_config = ConfigManagerData(self.data_path, output_file_name)
+        self.assertEqual(self.config_manager_data, new_config)
+
+    def test_equality(self):
+        # tests the __eq__ function. Equality is only defined
+        # by equality of the .data element. If data_path or db_filename
+        # are different, but the contents are the same, it's still
+        # considered equal
+        cfd1 = ConfigManagerData(self.data_path)
+        cfd2 = ConfigManagerData(self.data_path)
+        self.assertEqual(cfd1, cfd2)
+        cfd2.data_path = "some/unknown/path"
+        self.assertEqual(cfd1, cfd2)
+        cfd2.db_filename = "bad_file.name"
+        self.assertEqual(cfd1, cfd2)
+        cfd2.data['test'] = { 'a': [ 1, 2, 3]}
+        self.assertNotEqual(cfd1, cfd2)
+        
+#
+# We can probably use a more general version of this
+#
+class FakeModuleCCSession:
+    def __init__(self):
+        self.subscriptions = {}
+        # each entry is of the form [ channel, instance, message ]
+        self.message_queue = []
+
+    def group_subscribe(self, group_name, instance_name = None):
+        if not group_name in self.subscriptions:
+            self.subscriptions[group_name] = []
+        if instance_name:
+            self.subscriptions[group_name].append(instance_name)
+            
+
+    def has_subscription(self, group_name, instance_name = None):
+        if group_name in self.subscriptions:
+            if instance_name:
+                return instance_name in self.subscriptions[group_name]
+            else:
+                return True
+        else:
+            return False
+
+    def group_sendmsg(self, msg, channel, target = None):
+        self.message_queue.append([ channel, target, msg ])
+
+    def group_recvmsg(self, blocking):
+        for qm in self.message_queue:
+            if qm[0] in self.subscriptions and (qm[1] == None or qm[1] in self.subscriptions[qm[0]]):
+                self.message_queue.remove(qm)
+                return qm[2], {}
+        return None, None
+
+    def get_message(self, channel, target = None):
+        for qm in self.message_queue:
+            if qm[0] == channel and qm[1] == target:
+                self.message_queue.remove(qm)
+                return qm[2]
+        return None
+        
+
+class TestConfigManager(unittest.TestCase):
+
+    def setUp(self):
+        self.data_path = os.environ['CONFIG_TESTDATA_PATH']
+        self.fake_session = FakeModuleCCSession()
+        self.cm = ConfigManager(self.data_path, self.fake_session)
+        self.name = "TestModule"
+        self.spec = isc.config.module_spec_from_file(self.data_path + os.sep + "/spec2.spec")
+    
+    def test_init(self):
+        self.assert_(self.cm.module_specs == {})
+        self.assert_(self.cm.data_path == self.data_path)
+        self.assert_(self.cm.config != None)
+        self.assert_(self.fake_session.has_subscription("ConfigManager"))
+        self.assert_(self.fake_session.has_subscription("Boss", "ConfigManager"))
+        self.assertFalse(self.cm.running)
+
+    def test_notify_boss(self):
+        self.cm.notify_boss()
+        msg = self.fake_session.get_message("Boss", None)
+        self.assert_(msg)
+        # this one is actually wrong, but 'current status quo'
+        self.assertEqual(msg, {"running": "configmanager"})
+
+    def test_set_module_spec(self):
+        module_spec = isc.config.module_spec.module_spec_from_file(self.data_path + os.sep + "spec1.spec")
+        self.assert_(module_spec.get_module_name() not in self.cm.module_specs)
+        self.cm.set_module_spec(module_spec)
+        self.assert_(module_spec.get_module_name() in self.cm.module_specs)
+
+    def test_remove_module_spec(self):
+        module_spec = isc.config.module_spec.module_spec_from_file(self.data_path + os.sep + "spec1.spec")
+        self.assert_(module_spec.get_module_name() not in self.cm.module_specs)
+        self.cm.set_module_spec(module_spec)
+        self.assert_(module_spec.get_module_name() in self.cm.module_specs)
+        self.cm.remove_module_spec(module_spec.get_module_name())
+        self.assert_(module_spec.get_module_name() not in self.cm.module_specs)
+
+    def test_get_module_spec(self):
+        module_spec = isc.config.module_spec.module_spec_from_file(self.data_path + os.sep + "spec1.spec")
+        self.assert_(module_spec.get_module_name() not in self.cm.module_specs)
+        self.cm.set_module_spec(module_spec)
+        self.assert_(module_spec.get_module_name() in self.cm.module_specs)
+        module_spec2 = self.cm.get_module_spec(module_spec.get_module_name())
+        self.assertEqual(module_spec, module_spec2)
+
+    def test_get_config_spec(self):
+        config_spec = self.cm.get_config_spec()
+        self.assertEqual(config_spec, {})
+        module_spec = isc.config.module_spec.module_spec_from_file(self.data_path + os.sep + "spec1.spec")
+        self.assert_(module_spec.get_module_name() not in self.cm.module_specs)
+        self.cm.set_module_spec(module_spec)
+        self.assert_(module_spec.get_module_name() in self.cm.module_specs)
+        config_spec = self.cm.get_config_spec()
+        self.assertEqual(config_spec, { 'Spec1': None })
+        self.cm.remove_module_spec('Spec1')
+        module_spec = isc.config.module_spec.module_spec_from_file(self.data_path + os.sep + "spec2.spec")
+        self.assert_(module_spec.get_module_name() not in self.cm.module_specs)
+        self.cm.set_module_spec(module_spec)
+        self.assert_(module_spec.get_module_name() in self.cm.module_specs)
+        config_spec = self.cm.get_config_spec()
+        self.assertEqual(config_spec['Spec2'], module_spec.get_config_spec())
+    
+    def test_get_commands_spec(self):
+        commands_spec = self.cm.get_commands_spec()
+        self.assertEqual(commands_spec, {})
+        module_spec = isc.config.module_spec.module_spec_from_file(self.data_path + os.sep + "spec1.spec")
+        self.assert_(module_spec.get_module_name() not in self.cm.module_specs)
+        self.cm.set_module_spec(module_spec)
+        self.assert_(module_spec.get_module_name() in self.cm.module_specs)
+        commands_spec = self.cm.get_commands_spec()
+        self.assertEqual(commands_spec, { 'Spec1': None })
+        self.cm.remove_module_spec('Spec1')
+        module_spec = isc.config.module_spec.module_spec_from_file(self.data_path + os.sep + "spec2.spec")
+        self.assert_(module_spec.get_module_name() not in self.cm.module_specs)
+        self.cm.set_module_spec(module_spec)
+        self.assert_(module_spec.get_module_name() in self.cm.module_specs)
+        commands_spec = self.cm.get_commands_spec()
+        self.assertEqual(commands_spec['Spec2'], module_spec.get_commands_spec())
+
+    def test_read_config(self):
+        self.assertEqual(self.cm.config.data, {'version': 1})
+        self.cm.read_config()
+        self.assertEqual(self.cm.config.data, {'TestModule': {'test': 124}, 'version': 1})
+
+    def test_write_config(self):
+        # tested in ConfigManagerData tests
+        pass
+    
+    def _handle_msg_helper(self, msg, expected_answer):
+        answer = self.cm.handle_msg(msg)
+        self.assertEqual(expected_answer, answer)
+
+    def test_handle_msg(self):
+        self._handle_msg_helper({}, { 'result': [ 1, 'Unknown message format: {}']})
+        self._handle_msg_helper("", { 'result': [ 1, 'Unknown message format: ']})
+        self._handle_msg_helper({ "command": [ "badcommand" ] }, { 'result': [ 1, "Unknown command: ['badcommand']"]})
+        self._handle_msg_helper({ "command": [ "get_commands_spec" ] }, { 'result': [ 0, {} ]})
+        self._handle_msg_helper({ "command": [ "get_module_spec" ] }, { 'result': [ 0, {} ]})
+        #self._handle_msg_helper({ "command": [ "get_module_spec", { "module_name": "nosuchmodule" } ] },
+        #                        {'result': [1, 'No specification for module nosuchmodule']})
+        self._handle_msg_helper({ "command": [ "get_module_spec", 1 ] },
+                                {'result': [1, 'Bad get_module_spec command, argument not a dict']})
+        self._handle_msg_helper({ "command": [ "get_module_spec", { } ] },
+                                {'result': [1, 'Bad module_name in get_module_spec command']})
+        self._handle_msg_helper({ "command": [ "get_config" ] }, { 'result': [ 0, { 'version': 1} ]})
+        self._handle_msg_helper({ "command": [ "get_config", { "module_name": "nosuchmodule" } ] },
+                                {'result': [0, {}]})
+        self._handle_msg_helper({ "command": [ "get_config", 1 ] },
+                                {'result': [1, 'Bad get_config command, argument not a dict']})
+        self._handle_msg_helper({ "command": [ "get_config", { } ] },
+                                {'result': [1, 'Bad module_name in get_config command']})
+        self._handle_msg_helper({ "command": [ "set_config" ] },
+                                {'result': [1, 'Wrong number of arguments']})
+        self._handle_msg_helper({ "command": [ "set_config", {} ] },
+                                {'result': [0]})
+        self.assertEqual(len(self.fake_session.message_queue), 0)
+
+        # the targets of some of these tests expect specific answers, put
+        # those in our fake msgq first.
+        my_ok_answer = { 'result': [ 0 ] }
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self._handle_msg_helper({ "command": [ "set_config", self.name, { "test": 123 } ] },
+                                my_ok_answer)
+        self.assertEqual(len(self.fake_session.message_queue), 1)
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.assertEqual({'config_update': {'test': 123}},
+                         self.fake_session.get_message(self.name, None))
+        self._handle_msg_helper({ "command": [ "set_config", self.name, { "test": 124 } ] },
+                                {'result': [0]})
+
+        #print(self.fake_session.message_queue)
+        self.assertEqual(len(self.fake_session.message_queue), 1)
+        self.assertEqual({'config_update': {'test': 124}},
+                         self.fake_session.get_message(self.name, None))
+        self.assertEqual({'version': 1, 'TestModule': {'test': 124}}, self.cm.config.data)
+        self._handle_msg_helper({ "module_spec": 
+                                  self.spec.get_full_spec()
+                                },
+                                {'result': [0]})
+        self._handle_msg_helper({ "module_spec": 
+                                  { 'foo': 1 }
+                                },
+                                {'result': [1, 'Error in data definition: no module_name in module_spec']})
+        self._handle_msg_helper({ "command": [ "get_module_spec" ] }, { 'result': [ 0, { self.spec.get_module_name(): self.spec.get_config_spec() } ]})
+        self._handle_msg_helper({ "command": [ "get_commands_spec" ] }, { 'result': [ 0, { self.spec.get_module_name(): self.spec.get_commands_spec() } ]})
+        # re-add this once we have new way to propagate spec changes (1 instead of the current 2 messages)
+        #self.assertEqual(len(self.fake_session.message_queue), 2)
+        # the name here is actually wrong (and hardcoded), but needed in the current version
+        # TODO: fix that
+        #self.assertEqual({'specification_update': [ self.name, self.spec ] },
+        #                 self.fake_session.get_message("Cmd-Ctrld", None))
+        #self.assertEqual({'commands_update': [ self.name, self.commands ] },
+        #                 self.fake_session.get_message("Cmd-Ctrld", None))
+        
+        
+
+    def test_run(self):
+        pass
+
+
+if __name__ == '__main__':
+    if not 'CONFIG_TESTDATA_PATH' in os.environ:
+        print("You need to set the environment variable CONFIG_TESTDATA_PATH to point to the directory containing the test data files")
+        exit(1)
+    unittest.main()
+

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

@@ -0,0 +1,375 @@
+# Copyright (C) 2010  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+"""
+Classes to store configuration data and module specifications
+
+Used by the config manager, (python) modules, and UI's (those last
+two through the classes in ccsession)
+"""
+
+import isc.cc.data
+import isc.config.module_spec
+
+class ConfigDataError(Exception): pass
+
+def check_type(spec_part, value):
+    """Returns true if the value is of the correct type given the
+       specification part relevant for the value. spec_part can be
+       retrieved with find_spec()"""
+    if type(spec_part) == list:
+        data_type = "list"
+    else:
+        data_type = spec_part['item_type']
+
+    if data_type == "integer" and type(value) != int:
+        raise isc.cc.data.DataTypeError(str(value) + " is not an integer")
+    elif data_type == "real" and type(value) != double:
+        raise isc.cc.data.DataTypeError(str(value) + " is not a real")
+    elif data_type == "boolean" and type(value) != boolean:
+        raise isc.cc.data.DataTypeError(str(value) + " is not a boolean")
+    elif data_type == "string" and type(value) != str:
+        raise isc.cc.data.DataTypeError(str(value) + " is not a string")
+    elif data_type == "list":
+        if type(value) != list:
+            raise isc.cc.data.DataTypeError(str(value) + " is not a list")
+        else:
+            for element in value:
+                check_type(spec_part['list_item_spec'], element)
+    elif data_type == "map" and type(value) != dict:
+        # todo: check types of map contents too
+        raise isc.cc.data.DataTypeError(str(value) + " is not a map")
+
+def find_spec(element, identifier):
+    """find the data definition for the given identifier
+       returns either a map with 'item_name' etc, or a list of those"""
+    if identifier == "":
+        return element
+    id_parts = identifier.split("/")
+    id_parts[:] = (value for value in id_parts if value != "")
+    cur_el = element
+    for id in id_parts:
+        if type(cur_el) == dict and id in cur_el.keys():
+            cur_el = cur_el[id]
+        elif type(cur_el) == dict and 'item_name' in cur_el.keys() and cur_el['item_name'] == id:
+            pass
+        elif type(cur_el) == list:
+            found = False
+            for cur_el_item in cur_el:
+                if cur_el_item['item_name'] == id and 'item_default' in cur_el_item.keys():
+                    cur_el = cur_el_item
+                    found = True
+            if not found:
+                raise isc.cc.data.DataNotFoundError(id + " in " + str(cur_el))
+        else:
+            raise isc.cc.data.DataNotFoundError(id + " in " + str(cur_el))
+    return cur_el
+
+def spec_name_list(spec, prefix="", recurse=False):
+    """Returns a full list of all possible item identifiers in the
+       specification (part)"""
+    result = []
+    if prefix != "" and not prefix.endswith("/"):
+        prefix += "/"
+    if type(spec) == dict:
+        for name in spec:
+            result.append(prefix + name + "/")
+            if 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:
+                if list_el['item_type'] == dict:
+                    if recurse:
+                        result.extend(spec_name_list(list_el['map_item_spec'], prefix + list_el['item_name'], recurse))
+                else:
+                    name = list_el['item_name']
+                    if list_el['item_type'] in ["list", "map"]:
+                        name += "/"
+                    result.append(name)
+    return result
+
+
+class ConfigData:
+    """This class stores the module specs and the current non-default
+       config values. It provides functions to get the actual value or
+       the default value if no non-default value has been set"""
+   
+    def __init__(self, specification):
+        """Initialize a ConfigData instance. If specification is not
+           of type ModuleSpec, a ConfigDataError is raised."""
+        if type(specification) != isc.config.ModuleSpec:
+            raise ConfigDataError("specification is of type " + str(type(specification)) + ", not ModuleSpec")
+        self.specification = specification
+        self.data = {}
+
+    def get_value(self, identifier):
+        """Returns a tuple where the first item is the value at the
+           given identifier, and the second item is a bool which is
+           true if the value is an unset default"""
+        value = isc.cc.data.find_no_exc(self.data, identifier)
+        if value:
+            return value, False
+        spec = find_spec(self.specification.get_config_spec(), identifier)
+        if spec and 'item_default' in spec:
+            return spec['item_default'], True
+        return None, False
+
+    def get_module_spec(self):
+        """Returns the ModuleSpec object associated with this ConfigData"""
+        return self.specification
+
+    def set_local_config(self, data):
+        """Set the non-default config values, as passed by cfgmgr"""
+        self.data = data
+
+    def get_local_config(self):
+        """Returns the non-default config values in a dict"""
+        return self.data;
+
+    def get_item_list(self, identifier = None, recurse = False):
+        """Returns a list of strings containing the full identifiers of
+           all 'sub'options at the given identifier. If recurse is True,
+           it will also add all identifiers of all children, if any"""
+        if identifier:
+            spec = find_spec(self.specification.get_config_spec(), identifier, recurse)
+            return spec_name_list(spec, identifier + "/")
+        return spec_name_list(self.specification.get_config_spec(), "", recurse)
+
+    def get_full_config(self):
+        """Returns a dict containing identifier: value elements, for
+           all configuration options for this module. If there is
+           a local setting, that will be used. Otherwise the value
+           will be the default as specified by the module specification.
+           If there is no default and no local setting, the value will
+           be None"""
+        items = self.get_item_list(None, True)
+        result = {}
+        for item in items:
+            value, default = self.get_value(item)
+            result[item] = value
+        return result
+
+class MultiConfigData:
+    """This class stores the module specs, current non-default
+       configuration values and 'local' (uncommitted) changes for
+       multiple modules"""
+    LOCAL   = 1
+    CURRENT = 2
+    DEFAULT = 3
+    NONE    = 4
+    
+    def __init__(self):
+        self._specifications = {}
+        self._current_config = {}
+        self._local_changes = {}
+
+    def set_specification(self, spec):
+        """Add or update a ModuleSpec"""
+        if type(spec) != isc.config.ModuleSpec:
+            raise Exception("not a datadef")
+        self._specifications[spec.get_module_name()] = spec
+
+    def get_module_spec(self, module):
+        """Returns the ModuleSpec for the module with the given name"""
+        if module in self._specifications:
+            return self._specifications[module]
+        else:
+            return None
+
+    def find_spec_part(self, identifier):
+        """Returns the specification for the item at the given
+           identifier, or None if not found. The first part of the
+           identifier (up to the first /) is interpreted as the module
+           name."""
+        if identifier[0] == '/':
+            identifier = identifier[1:]
+        module, sep, id = identifier.partition("/")
+        try:
+            return find_spec(self._specifications[module].get_config_spec(), id)
+        except isc.cc.data.DataNotFoundError as dnfe:
+            return None
+
+    # this function should only be called by __request_config
+    def _set_current_config(self, config):
+        """Replace the full current config values."""
+        self._current_config = config
+
+    def get_current_config(self):
+        """Returns the current configuration as it is known by the
+           configuration manager. It is a dict where the first level is
+           the module name, and the value is the config values for
+           that module"""
+        return self._current_config
+        
+    def get_local_changes(self):
+        """Returns the local config changes, i.e. those that have not
+           been committed yet and are not known by the configuration
+           manager or the modules."""
+        return self._local_changes
+
+    def clear_local_changes(self):
+        """Reverts all local changes"""
+        self._local_changes = {}
+
+    def get_local_value(self, identifier):
+        """Returns a specific local (uncommitted) configuration value,
+           as specified by the identifier. If the local changes do not
+           contain a new setting for this identifier, or if the
+           identifier cannot be found, None is returned. See
+           get_value() for a general way to find a configuration value
+           """
+        return isc.cc.data.find_no_exc(self._local_changes, identifier)
+        
+    def get_current_value(self, identifier):
+        """Returns the current non-default value as known by the
+           configuration manager, or None if it is not set.
+           See get_value() for a general way to find a configuration
+           value
+        """
+        return isc.cc.data.find_no_exc(self._current_config, identifier)
+        
+    def get_default_value(self, identifier):
+        """Returns the default value for the given identifier as
+           specified by the module specification, or None if there is
+           no default or the identifier could not be found.
+           See get_value() for a general way to find a configuration
+           value
+        """
+        if identifier[0] == '/':
+            identifier = identifier[1:]
+        module, sep, id = identifier.partition("/")
+        try:
+            spec = find_spec(self._specifications[module].get_config_spec(), id)
+            if 'item_default' in spec:
+                return spec['item_default']
+            else:
+                return None
+        except isc.cc.data.DataNotFoundError as dnfe:
+            return None
+
+    def get_value(self, identifier):
+        """Returns a tuple containing value,status.
+           The value contains the configuration value for the given
+           identifier. The status reports where this value came from;
+           it is one of: LOCAL, CURRENT, DEFAULT or NONE, corresponding
+           (local change, current setting, default as specified by the
+           specification, or not found at all)."""
+        value = self.get_local_value(identifier)
+        if value:
+            return value, self.LOCAL
+        value = self.get_current_value(identifier)
+        if value:
+            return value, self.CURRENT
+        value = self.get_default_value(identifier)
+        if value:
+            return value, self.DEFAULT
+        return None, self.NONE
+
+    def get_value_maps(self, identifier = None):
+        """Returns a list of dicts, containing the following values:
+           name: name of the entry (string)
+           type: string containing the type of the value (or 'module')
+           value: value of the entry if it is a string, int, double or bool
+           modified: true if the value is a local change
+           default: true if the value has been changed
+           TODO: use the consts for those last ones
+           Throws DataNotFoundError if the identifier is bad
+        """
+        result = []
+        if not identifier:
+            # No identifier, so we need the list of current modules
+            for module in self._specifications.keys():
+                entry = {}
+                entry['name'] = module
+                entry['type'] = 'module'
+                entry['value'] = None
+                entry['modified'] = False
+                entry['default'] = False
+                result.append(entry)
+        else:
+            if identifier[0] == '/':
+                identifier = identifier[1:]
+            module, sep, id = identifier.partition('/')
+            spec = self.get_module_spec(module)
+            if spec:
+                spec_part = find_spec(spec.get_config_spec(), id)
+                print(spec_part)
+                if type(spec_part) == list:
+                    for item in spec_part:
+                        entry = {}
+                        entry['name'] = item['item_name']
+                        entry['type'] = item['item_type']
+                        value, status = self.get_value("/" + identifier + "/" + item['item_name'])
+                        entry['value'] = value
+                        if status == self.LOCAL:
+                            entry['modified'] = True
+                        else:
+                            entry['modified'] = False
+                        if status == self.DEFAULT:
+                            entry['default'] = False
+                        else:
+                            entry['default'] = False
+                        result.append(entry)
+                else:
+                    item = spec_part
+                    if item['item_type'] == 'list':
+                        li_spec = item['list_item_spec']
+                        l, status =  self.get_value("/" + identifier)
+                        if l:
+                            for value in l:
+                                result_part2 = {}
+                                result_part2['name'] = li_spec['item_name']
+                                result_part2['value'] = value
+                                result_part2['type'] = li_spec['item_type']
+                                result_part2['default'] = False
+                                result_part2['modified'] = False
+                                result.append(result_part2)
+                    else:
+                        entry = {}
+                        entry['name'] = item['item_name']
+                        entry['type'] = item['item_type']
+                        #value, status = self.get_value("/" + identifier + "/" + item['item_name'])
+                        value, status = self.get_value("/" + identifier)
+                        entry['value'] = value
+                        if status == self.LOCAL:
+                            entry['modified'] = True
+                        else:
+                            entry['modified'] = False
+                        if status == self.DEFAULT:
+                            entry['default'] = False
+                        else:
+                            entry['default'] = False
+                        result.append(entry)
+            #print(spec)
+        return result
+
+    def set_value(self, identifier, value):
+        """Set the local value at the given identifier to value"""
+        spec_part = self.find_spec_part(identifier)
+        check_type(spec_part, value)
+        isc.cc.data.set(self._local_changes, identifier, value)
+ 
+    def get_config_item_list(self, identifier = None):
+        """Returns a list of strings containing the item_names of
+           the child items at the given identifier. If no identifier is
+           specified, returns a list of module names. The first part of
+           the identifier (up to the first /) is interpreted as the
+           module name"""
+        if identifier:
+            spec = self.find_spec_part(identifier)
+            return spec_name_list(spec, identifier + "/")
+        else:
+            return self._specifications.keys()

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

@@ -0,0 +1,41 @@
+# Copyright (C) 2010  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+#
+# Tests for the ConfigData and MultiConfigData classes
+#
+
+import unittest
+import os
+from isc.config.config_data import *
+from isc.config.module_spec import *
+
+class TestConfigData(unittest.TestCase):
+    def setUp(self):
+        if 'CONFIG_TESTDATA_PATH' in os.environ:
+            self.data_path = os.environ['CONFIG_TESTDATA_PATH']
+        else:
+            self.data_path = "../../../testdata"
+
+    def test_module_spec_from_file(self):
+        spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec1.spec")
+        cd = ConfigData(spec)
+        self.assertEqual(cd.specification, spec)
+        self.assertEqual(cd.data, {})
+        self.assertRaises(ConfigDataError, ConfigData, 1)
+
+if __name__ == '__main__':
+    unittest.main()
+

src/lib/config/python/isc/config/config_test.in

@@ -12,5 +12,8 @@ CONFIG_TESTDATA_PATH=@abs_top_srcdir@/src/lib/config/testdata
 export CONFIG_TESTDATA_PATH
 
 cd ${BIND10_PATH}
-exec ${PYTHON_EXEC} -O ${CONFIG_PATH}/datadefinition_test.py $*
+${PYTHON_EXEC} -O ${CONFIG_PATH}/config_data_test.py $*
 
+${PYTHON_EXEC} -O ${CONFIG_PATH}/module_spec_test.py $*
+
+${PYTHON_EXEC} -O ${CONFIG_PATH}/cfgmgr_test.py $*

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

@@ -1,255 +0,0 @@
-# Copyright (C) 2009  Internet Systems Consortium.
-#
-# Permission to use, copy, modify, and distribute this software for any
-# purpose with or without fee is hereby granted, provided that the above
-# copyright notice and this permission notice appear in all copies.
-#
-# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
-# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
-# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
-# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
-# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
-# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
-# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
-# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
-
-#
-# This class holds the data definition and validates data agains that
-# definition. It is the python equivalent of data_def.h
-#
-import ast
-
-import isc.cc.data
-
-# file objects are passed around as _io.TextIOWrapper objects
-# import that so we can check those types
-
-class DataDefinitionError(Exception):
-    pass
-
-class DataDefinition:
-    def __init__(self, spec_file, check = True):
-        if hasattr(spec_file, 'read'):
-            self._data_spec = self.__read_data_spec_file(spec_file)
-        elif type(spec_file) == str:
-            file = open(spec_file)
-            self._data_spec = self.__read_data_spec_file(file)
-            file.close()
-        else:
-            raise DataDefinitionError("Not a str or file-like object")
-
-    def validate(self, data, errors = None):
-        """Check whether the given piece of data conforms to this
-           data definition. If so, it returns True. If not, it will
-           return false. If errors is given, and is an array, a string
-           describing the error will be appended to it. The current
-           version stops as soon as there is one error so this list
-           will not be exhaustive."""
-        data_def = self.get_definition()
-        if 'data_specification' not in data_def:
-            if errors:
-                errors.append("Data definition has no data_specification element")
-            return False
-        data_def = data_def['data_specification']
-        if 'config_data' not in data_def:
-            if errors:
-                errors.append("The is no config_data for this specification")
-            return False
-        errors = []
-        return _validate_spec_list(data_def['config_data'], data, errors)
-
-    def __read_data_spec_file(self, file, check = True):
-        """Reads the data spec from the given file object.
-           If check is True, check whether it is of the correct form.
-           If it is not, an DataDefinitionError exception is raised"""
-        if not hasattr(file, 'read'):
-            raise DataDefinitionError("Not a file-like object:" + str(type(file)))
-        str = file.read(-1)
-        # TODO catch error here and reraise as a less ugly exception
-        data_spec = ast.literal_eval(str)
-        if check:
-            # TODO
-            _check(data_spec)
-            pass
-        return data_spec
-
-    def get_definition(self):
-        return self._data_spec
-
-    def get_module_name(self):
-        return self._data_spec["data_specification"]["module_name"]
-
-def _check(data_spec):
-    """Checks the full specification. This is a dict that contains the
-       element "data_specification", which is in itself a dict that
-       must contain at least a "module_name" (string) and optionally
-       a "config_data" and a "commands" element, both of which are lists
-       of dicts. Raises a DataDefinitionError if there is a problem."""
-    if type(data_spec) != dict:
-        raise DataDefinitionError("data specification not a dict")
-    if "data_specification" not in data_spec:
-        raise DataDefinitionError("no data_specification element in specification")
-    data_spec = data_spec["data_specification"]
-    if "module_name" not in data_spec:
-        raise DataDefinitionError("no module_name in data_specification")
-    if "config_data" in data_spec:
-        _check_config_spec(data_spec["config_data"])
-    if "commands" in data_spec:
-        _check_command_spec(data_spec["commands"])
-
-def _check_config_spec(config_data):
-    # config data is a list of items represented by dicts that contain
-    # things like "item_name", depending on the type they can have
-    # specific subitems
-    """Checks a list that contains the configuration part of the
-       specification. Raises a DataDefinitionError if there is a
-       problem."""
-    if type(config_data) != list:
-        raise DataDefinitionError("config_data is not a list of items")
-    for config_item in config_data:
-        _check_item_spec(config_item)
-
-def _check_command_spec(commands):
-    """Checks the list that contains a set of commands. Raises a
-       DataDefinitionError is there is an error"""
-    if type(commands) != list:
-        raise DataDefinitionError("commands is not a list of commands")
-    for command in commands:
-        if type(command) != dict:
-            raise DataDefinitionError("command in commands list is not a dict")
-        if "command_name" not in command:
-            raise DataDefinitionError("no command_name in command item")
-        command_name = command["command_name"]
-        if type(command_name) != str:
-            raise DataDefinitionError("command_name not a string: " + str(type(command_name)))
-        if "command_description" in command:
-            if type(command["command_description"]) != str:
-                raise DataDefinitionError("command_description not a string in " + command_name)
-        if "command_args" in command:
-            if type(command["command_args"]) != list:
-                raise DataDefinitionError("command_args is not a list in " + command_name)
-            for command_arg in command["command_args"]:
-                if type(command_arg) != dict:
-                    raise DataDefinitionError("command argument not a dict in " + command_name)
-                _check_item_spec(command_arg)
-        else:
-            raise DataDefinitionError("command_args missing in " + command_name)
-    pass
-
-def _check_item_spec(config_item):
-    """Checks the dict that defines one config item
-       (i.e. containing "item_name", "item_type", etc.
-       Raises a DataDefinitionError if there is an error"""
-    if type(config_item) != dict:
-        raise DataDefinitionError("item spec not a dict")
-    if "item_name" not in config_item:
-        raise DataDefinitionError("no item_name in config item")
-    if type(config_item["item_name"]) != str:
-        raise DataDefinitionError("item_name is not a string: " + str(config_item["item_name"]))
-    item_name = config_item["item_name"]
-    if "item_type" not in config_item:
-        raise DataDefinitionError("no item_type in config item")
-    item_type = config_item["item_type"]
-    if type(item_type) != str:
-        raise DataDefinitionError("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"]:
-        raise DataDefinitionError("unknown item_type in " + item_name + ": " + item_type)
-    if "item_optional" in config_item:
-        if type(config_item["item_optional"]) != bool:
-            raise DataDefinitionError("item_default in " + item_name + " is not a boolean")
-        if not config_item["item_optional"] and "item_default" not in config_item:
-            raise DataDefinitionError("no default value for non-optional item " + item_name)
-    else:
-        raise DataDefinitionError("item_optional not in item " + item_name)
-    if "item_default" in config_item:
-        item_default = config_item["item_default"]
-        if (item_type == "integer" and type(item_default) != int) or \
-           (item_type == "real" and type(item_default) != float) or \
-           (item_type == "boolean" and type(item_default) != bool) or \
-           (item_type == "string" and type(item_default) != str) or \
-           (item_type == "list" and type(item_default) != list) or \
-           (item_type == "map" and type(item_default) != dict):
-            raise DataDefinitionError("Wrong type for item_default in " + item_name)
-    # TODO: once we have check_type, run the item default through that with the list|map_item_spec
-    if item_type == "list":
-        if "list_item_spec" not in config_item:
-            raise DataDefinitionError("no list_item_spec in list item " + item_name)
-        if type(config_item["list_item_spec"]) != dict:
-            raise DataDefinitionError("list_item_spec in " + item_name + " is not a dict")
-        _check_item_spec(config_item["list_item_spec"])
-    if item_type == "map":
-        if "map_item_spec" not in config_item:
-            raise DataDefinitionError("no map_item_sepc in map item " + item_name)
-        if type(config_item["map_item_spec"]) != list:
-            raise DataDefinitionError("map_item_spec in " + item_name + " is not a list")
-        for map_item in config_item["map_item_spec"]:
-            if type(map_item) != dict:
-                raise DataDefinitionError("map_item_spec element is not a dict")
-            _check_item_spec(map_item)
-
-
-def _validate_type(spec, value, errors):
-    """Returns true if the value is of the correct type given the
-       specification"""
-    data_type = spec['item_type']
-    if data_type == "integer" and type(value) != int:
-        if errors:
-            errors.append(str(value) + " should be an integer")
-        return False
-    elif data_type == "real" and type(value) != float:
-        if errors:
-            errors.append(str(value) + " should be a real")
-        return False
-    elif data_type == "boolean" and type(value) != bool:
-        if errors:
-            errors.append(str(value) + " should be a boolean")
-        return False
-    elif data_type == "string" and type(value) != str:
-        if errors:
-            errors.append(str(value) + " should be a string")
-        return False
-    elif data_type == "list" and type(value) != list:
-        if errors:
-            errors.append(str(value) + " should be a list, not a " + str(value.__class__.__name__))
-        return False
-    elif data_type == "map" and type(value) != dict:
-        if errors:
-            errors.append(str(value) + " should be a map")
-        return False
-    else:
-        return True
-
-def _validate_item(spec, data, errors):
-    if not _validate_type(spec, data, errors):
-        return False
-    elif type(data) == list:
-        list_spec = spec['list_item_spec']
-        for data_el in data:
-            if not _validate_type(list_spec, data_el, errors):
-                return False
-            if list_spec['item_type'] == "map":
-                if not _validate_item(list_spec, data_el, errors):
-                    return False
-    elif type(data) == dict:
-        if not _validate_spec_list(spec['map_item_spec'], data, errors):
-            return False
-    return True
-
-def _validate_spec(spec, data, errors):
-    item_name = spec['item_name']
-    item_optional = spec['item_optional']
-
-    if item_name in data:
-        return _validate_item(spec, data[item_name], errors)
-    elif not item_optional:
-        if errors:
-            errors.append("non-optional item " + item_name + " missing")
-        return False
-    else:
-        return True
-
-def _validate_spec_list(data_spec, data, errors):
-    for spec_item in data_spec:
-        if not _validate_spec(spec_item, data, errors):
-            return False
-    return True

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

@@ -1,92 +0,0 @@
-# Copyright (C) 2009  Internet Systems Consortium.
-#
-# Permission to use, copy, modify, and distribute this software for any
-# purpose with or without fee is hereby granted, provided that the above
-# copyright notice and this permission notice appear in all copies.
-#
-# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
-# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
-# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
-# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
-# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
-# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
-# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
-# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
-
-#
-# Tests for the datadefinition module
-#
-
-import unittest
-import os
-from isc.config import DataDefinition, DataDefinitionError
-import isc.cc.data
-
-class TestDataDefinition(unittest.TestCase):
-
-    def setUp(self):
-        self.assert_('CONFIG_TESTDATA_PATH' in os.environ)
-        self.data_path = os.environ['CONFIG_TESTDATA_PATH']
-
-    def spec_file(self, filename):
-        return(self.data_path + os.sep + filename)
-
-    def read_spec_file(self, filename):
-        return DataDefinition(self.spec_file(filename))
-
-    def spec1(self, dd):
-        data_def = dd.get_definition()
-        self.assert_('data_specification' in data_def)
-        data_spec = data_def['data_specification']
-        self.assert_('module_name' in data_spec)
-        self.assertEqual(data_spec['module_name'], "Spec1")
-        
-    def test_open_file_name(self):
-        dd = DataDefinition(self.spec_file("spec1.spec"))
-        self.spec1(dd)
-
-    def test_open_file_obj(self):
-        file1 = open(self.spec_file("spec1.spec"))
-        dd = DataDefinition(file1)
-        self.spec1(dd)
-
-    def test_bad_specfiles(self):
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec3.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec4.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec5.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec6.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec7.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec8.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec9.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec10.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec11.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec12.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec13.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec14.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec15.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec16.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec17.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec18.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec19.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec20.spec")
-        self.assertRaises(DataDefinitionError, self.read_spec_file, "spec21.spec")
-
-    def validate_data(self, specfile_name, datafile_name):
-        dd = DataDefinition(self.spec_file(specfile_name));
-        data_file = open(self.spec_file(datafile_name))
-        data_str = data_file.read()
-        data = isc.cc.data.parse_value_str(data_str)
-        return dd.validate(data)
-        
-    def test_data_validation(self):
-        self.assertEqual(True, self.validate_data("spec22.spec", "data22_1.data"))
-        self.assertEqual(False, self.validate_data("spec22.spec", "data22_2.data"))
-        self.assertEqual(False, self.validate_data("spec22.spec", "data22_3.data"))
-        self.assertEqual(False, self.validate_data("spec22.spec", "data22_4.data"))
-        self.assertEqual(False, self.validate_data("spec22.spec", "data22_5.data"))
-        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"))
-
-if __name__ == '__main__':
-    unittest.main()

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

@@ -0,0 +1,280 @@
+# Copyright (C) 2009  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+"""Module Specifications
+
+   A module specification holds the information about what configuration
+   a module can have, and what commands it understands. It provides
+   functions to read it from a .spec file, and to validate a given
+   set of data against the specification
+"""
+
+import ast
+
+import isc.cc.data
+
+# file objects are passed around as _io.TextIOWrapper objects
+# import that so we can check those types
+
+class ModuleSpecError(Exception):
+    """This exception is raised it the ModuleSpec fails to initialize
+       or if there is a failure or parse error reading the specification
+       file"""
+    pass
+
+def module_spec_from_file(spec_file, check = True):
+    """Returns a ModuleSpec object defined by the file at spec_file.
+       If check is True, the contents are verified. If there is an error
+       in those contents, a ModuleSpecError is raised."""
+    module_spec = None
+    if hasattr(spec_file, 'read'):
+        module_spec = ast.literal_eval(spec_file.read(-1))
+    elif type(spec_file) == str:
+        file = open(spec_file)
+        module_spec = ast.literal_eval(file.read(-1))
+        file.close()
+    else:
+        raise ModuleSpecError("spec_file not a str or file-like object")
+    if 'module_spec' not in module_spec:
+        raise ModuleSpecError("Data definition has no module_spec element")
+        
+    return ModuleSpec(module_spec['module_spec'], check)
+
+class ModuleSpec:
+    def __init__(self, module_spec, check = True):
+        """Initializes a ModuleSpec object from the specification in
+           the given module_spec (which must be a dict). If check is
+           True, the contents are verified. Raises a ModuleSpec error
+           if there is something wrong with the contents of the dict"""
+        if type(module_spec) != dict:
+            raise ModuleSpecError("module_spec is of type " + str(type(module_spec)) + ", not dict")
+        if check:
+            _check(module_spec)
+        self._module_spec = module_spec
+
+    def validate_config(self, full, data, errors = None):
+        """Check whether the given piece of data conforms to this
+           data definition. If so, it returns True. If not, it will
+           return false. If errors is given, and is an array, a string
+           describing the error will be appended to it. The current
+           version stops as soon as there is one error so this list
+           will not be exhaustive. If 'full' is true, it also errors on
+           non-optional missing values. Set this to False if you want to
+           validate only a part of a configuration tree (like a list of
+           non-default values)"""
+        data_def = self.get_config_spec()
+        return _validate_spec_list(data_def, full, data, errors)
+
+
+    def get_module_name(self):
+        """Returns a string containing the name of the module as
+           specified by the specification given at __init__"""
+        return self._module_spec['module_name']
+
+    def get_full_spec(self):
+        """Returns a dict representation of the full module specification"""
+        return self._module_spec
+
+    def get_config_spec(self):
+        """Returns a dict representation of the configuration data part
+           of the specification, or None if there is none."""
+        if 'config_data' in self._module_spec:
+            return self._module_spec['config_data']
+        else:
+            return None
+    
+    def get_commands_spec(self):
+        """Returns a dict representation of the commands part of the
+           specification, or None if there is none."""
+        if 'commands' in self._module_spec:
+            return self._module_spec['commands']
+        else:
+            return None
+    
+    def __str__(self):
+        """Returns a string representation of the full specification"""
+        return self._module_spec.__str__()
+
+def _check(module_spec):
+    """Checks the full specification. This is a dict that contains the
+       element "module_spec", which is in itself a dict that
+       must contain at least a "module_name" (string) and optionally
+       a "config_data" and a "commands" element, both of which are lists
+       of dicts. Raises a ModuleSpecError if there is a problem."""
+    if type(module_spec) != dict:
+        raise ModuleSpecError("data specification not a dict")
+    if "module_name" not in module_spec:
+        raise ModuleSpecError("no module_name in module_spec")
+    if "config_data" in module_spec:
+        _check_config_spec(module_spec["config_data"])
+    if "commands" in module_spec:
+        _check_command_spec(module_spec["commands"])
+
+def _check_config_spec(config_data):
+    # config data is a list of items represented by dicts that contain
+    # things like "item_name", depending on the type they can have
+    # specific subitems
+    """Checks a list that contains the configuration part of the
+       specification. Raises a ModuleSpecError if there is a
+       problem."""
+    if type(config_data) != list:
+        raise ModuleSpecError("config_data is of type " + str(type(config_data)) + ", not a list of items")
+    for config_item in config_data:
+        _check_item_spec(config_item)
+
+def _check_command_spec(commands):
+    """Checks the list that contains a set of commands. Raises a
+       ModuleSpecError is there is an error"""
+    if type(commands) != list:
+        raise ModuleSpecError("commands is not a list of commands")
+    for command in commands:
+        if type(command) != dict:
+            raise ModuleSpecError("command in commands list is not a dict")
+        if "command_name" not in command:
+            raise ModuleSpecError("no command_name in command item")
+        command_name = command["command_name"]
+        if type(command_name) != str:
+            raise ModuleSpecError("command_name not a string: " + str(type(command_name)))
+        if "command_description" in command:
+            if type(command["command_description"]) != str:
+                raise ModuleSpecError("command_description not a string in " + command_name)
+        if "command_args" in command:
+            if type(command["command_args"]) != list:
+                raise ModuleSpecError("command_args is not a list in " + command_name)
+            for command_arg in command["command_args"]:
+                if type(command_arg) != dict:
+                    raise ModuleSpecError("command argument not a dict in " + command_name)
+                _check_item_spec(command_arg)
+        else:
+            raise ModuleSpecError("command_args missing in " + command_name)
+    pass
+
+def _check_item_spec(config_item):
+    """Checks the dict that defines one config item
+       (i.e. containing "item_name", "item_type", etc.
+       Raises a ModuleSpecError if there is an error"""
+    if type(config_item) != dict:
+        raise ModuleSpecError("item spec not a dict")
+    if "item_name" not in config_item:
+        raise ModuleSpecError("no item_name in config item")
+    if type(config_item["item_name"]) != str:
+        raise ModuleSpecError("item_name is not a string: " + str(config_item["item_name"]))
+    item_name = config_item["item_name"]
+    if "item_type" not in config_item:
+        raise ModuleSpecError("no item_type in 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"]:
+        raise ModuleSpecError("unknown item_type in " + item_name + ": " + item_type)
+    if "item_optional" in config_item:
+        if type(config_item["item_optional"]) != bool:
+            raise ModuleSpecError("item_default in " + item_name + " is not a boolean")
+        if not config_item["item_optional"] and "item_default" not in config_item:
+            raise ModuleSpecError("no default value for non-optional item " + item_name)
+    else:
+        raise ModuleSpecError("item_optional not in item " + item_name)
+    if "item_default" in config_item:
+        item_default = config_item["item_default"]
+        if (item_type == "integer" and type(item_default) != int) or \
+           (item_type == "real" and type(item_default) != float) or \
+           (item_type == "boolean" and type(item_default) != bool) or \
+           (item_type == "string" and type(item_default) != str) or \
+           (item_type == "list" and type(item_default) != list) or \
+           (item_type == "map" and type(item_default) != dict):
+            raise ModuleSpecError("Wrong type for item_default in " + item_name)
+    # TODO: once we have check_type, run the item default through that with the list|map_item_spec
+    if item_type == "list":
+        if "list_item_spec" not in config_item:
+            raise ModuleSpecError("no list_item_spec in list item " + item_name)
+        if type(config_item["list_item_spec"]) != dict:
+            raise ModuleSpecError("list_item_spec in " + item_name + " is not a dict")
+        _check_item_spec(config_item["list_item_spec"])
+    if item_type == "map":
+        if "map_item_spec" not in config_item:
+            raise ModuleSpecError("no map_item_sepc in map item " + item_name)
+        if type(config_item["map_item_spec"]) != list:
+            raise ModuleSpecError("map_item_spec in " + item_name + " is not a list")
+        for map_item in config_item["map_item_spec"]:
+            if type(map_item) != dict:
+                raise ModuleSpecError("map_item_spec element is not a dict")
+            _check_item_spec(map_item)
+
+
+def _validate_type(spec, value, errors):
+    """Returns true if the value is of the correct type given the
+       specification"""
+    data_type = spec['item_type']
+    if data_type == "integer" and type(value) != int:
+        if errors != None:
+            errors.append(str(value) + " should be an integer")
+        return False
+    elif data_type == "real" and type(value) != float:
+        if errors != None:
+            errors.append(str(value) + " should be a real")
+        return False
+    elif data_type == "boolean" and type(value) != bool:
+        if errors != None:
+            errors.append(str(value) + " should be a boolean")
+        return False
+    elif data_type == "string" and type(value) != str:
+        if errors != None:
+            errors.append(str(value) + " should be a string")
+        return False
+    elif data_type == "list" and type(value) != list:
+        if errors != None:
+            errors.append(str(value) + " should be a list, not a " + str(value.__class__.__name__))
+        return False
+    elif data_type == "map" and type(value) != dict:
+        if errors != None:
+            errors.append(str(value) + " should be a map")
+        return False
+    else:
+        return True
+
+def _validate_item(spec, full, data, errors):
+    if not _validate_type(spec, data, errors):
+        return False
+    elif type(data) == list:
+        list_spec = spec['list_item_spec']
+        for data_el in data:
+            if not _validate_type(list_spec, data_el, errors):
+                return False
+            if list_spec['item_type'] == "map":
+                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
+    return True
+
+def _validate_spec(spec, full, data, errors):
+    item_name = spec['item_name']
+    item_optional = spec['item_optional']
+
+    if item_name in data:
+        return _validate_item(spec, full, data[item_name], errors)
+    elif full and not item_optional:
+        if errors != None:
+            errors.append("non-optional item " + item_name + " missing")
+        return False
+    else:
+        return True
+
+def _validate_spec_list(module_spec, full, data, errors):
+    for spec_item in module_spec:
+        if not _validate_spec(spec_item, full, data, errors):
+            return False
+    return True

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

@@ -0,0 +1,92 @@
+# Copyright (C) 2009  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+#
+# Tests for the module_spec module
+#
+
+import unittest
+import os
+from isc.config import ModuleSpec, ModuleSpecError
+import isc.cc.data
+
+class TestModuleSpec(unittest.TestCase):
+
+    def setUp(self):
+        if 'CONFIG_TESTDATA_PATH' in os.environ:
+            self.data_path = os.environ['CONFIG_TESTDATA_PATH']
+        else:
+            self.data_path = "../../../testdata"
+
+    def spec_file(self, filename):
+        return(self.data_path + os.sep + filename)
+
+    def read_spec_file(self, filename):
+        return isc.config.module_spec_from_file(self.spec_file(filename))
+
+    def spec1(self, dd):
+        module_spec = dd.get_full_spec()
+        self.assert_('module_name' in module_spec)
+        self.assertEqual(module_spec['module_name'], "Spec1")
+        
+    def test_open_file_name(self):
+        dd = self.read_spec_file("spec1.spec")
+        self.spec1(dd)
+
+    def test_open_file_obj(self):
+        file1 = open(self.spec_file("spec1.spec"))
+        dd = isc.config.module_spec_from_file(file1)
+        self.spec1(dd)
+
+    def test_bad_specfiles(self):
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec3.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec4.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec5.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec6.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec7.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec8.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec9.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec10.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec11.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec12.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec13.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec14.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec15.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec16.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec17.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec18.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec19.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec20.spec")
+        self.assertRaises(ModuleSpecError, self.read_spec_file, "spec21.spec")
+
+    def validate_data(self, specfile_name, datafile_name):
+        dd = self.read_spec_file(specfile_name);
+        data_file = open(self.spec_file(datafile_name))
+        data_str = data_file.read()
+        data = isc.cc.data.parse_value_str(data_str)
+        return dd.validate_config(True, data)
+        
+    def test_data_validation(self):
+        self.assertEqual(True, self.validate_data("spec22.spec", "data22_1.data"))
+        self.assertEqual(False, self.validate_data("spec22.spec", "data22_2.data"))
+        self.assertEqual(False, self.validate_data("spec22.spec", "data22_3.data"))
+        self.assertEqual(False, self.validate_data("spec22.spec", "data22_4.data"))
+        self.assertEqual(False, self.validate_data("spec22.spec", "data22_5.data"))
+        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"))
+
+if __name__ == '__main__':
+    unittest.main()

src/lib/config/testdata/b10-config-bad1.db

@@ -0,0 +1 @@
+{'version': 0}

src/lib/config/testdata/b10-config-bad2.db

@@ -0,0 +1 @@
+{'version':

src/lib/config/testdata/b10-config.db

@@ -0,0 +1 @@
+{'version': 1}

src/lib/config/testdata/spec1.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec1"
   }
 }

src/lib/config/testdata/spec10.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

src/lib/config/testdata/spec11.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

src/lib/config/testdata/spec12.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

src/lib/config/testdata/spec13.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

src/lib/config/testdata/spec14.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

src/lib/config/testdata/spec15.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

src/lib/config/testdata/spec16.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": 1
   }

src/lib/config/testdata/spec17.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "commands": [
       {

src/lib/config/testdata/spec18.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "commands": [
       {

src/lib/config/testdata/spec19.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "commands": [
       {

src/lib/config/testdata/spec2.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",
@@ -49,6 +49,23 @@
           }
         ]
       }
+    ],
+    "commands": [
+      {
+        "command_name": "print_message",
+        "command_description": "Print the given message to stdout",
+        "command_args": [ {
+          "item_name": "message",
+          "item_type": "string",
+          "item_optional": False,
+          "item_default": ""
+        } ]
+      },
+      {
+        "command_name": "shutdown",
+        "command_description": "Shut down BIND 10",
+        "command_args": []
+      }
     ]
   }
 }

src/lib/config/testdata/spec20.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "commands": [
       {

src/lib/config/testdata/spec21.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "commands": 1
   }

src/lib/config/testdata/spec22.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "value1",

src/lib/config/testdata/spec23.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "commands": [
       {

src/lib/config/testdata/spec3.spec

@@ -1,6 +1,6 @@
 {
-  "data_specification": {
-    "module_name": "Spec2",
+  "module_spec": {
+    "module_name": "Spec3",
     "config_data": [
       {
         "item_type": "integer",

src/lib/config/testdata/spec4.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

src/lib/config/testdata/spec5.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

src/lib/config/testdata/spec6.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

src/lib/config/testdata/spec7.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
   }
 }
 

src/lib/config/testdata/spec9.spec

@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",