[svn] commit: r882 - in /trunk: ./ src/bin/auth/ src/bin/bind10/ src/bin/bindctl/ src/bin/cmdctl/ src/lib/cc/cpp/ src/lib/cc/python/isc/cc/ src/lib/config/cpp/ src/lib/config/python/isc/config/ src/lib/config/testdata/ src/lib/dns/cpp/

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Feb 19 16:08:19 UTC 2010


Author: jelte
Date: Fri Feb 19 16:08:18 2010
New Revision: 882

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

Added:
    trunk/src/lib/cc/python/isc/cc/data_test.py
      - copied unchanged from r881, branches/jelte-configuration/src/lib/cc/python/isc/cc/data_test.py
    trunk/src/lib/config/cpp/module_spec.cc
      - copied unchanged from r881, branches/jelte-configuration/src/lib/config/cpp/module_spec.cc
    trunk/src/lib/config/cpp/module_spec.h
      - copied unchanged from r881, branches/jelte-configuration/src/lib/config/cpp/module_spec.h
    trunk/src/lib/config/cpp/module_spec_unittests.cc
      - copied unchanged from r881, branches/jelte-configuration/src/lib/config/cpp/module_spec_unittests.cc
    trunk/src/lib/config/python/isc/config/cfgmgr_test.py
      - copied unchanged from r881, branches/jelte-configuration/src/lib/config/python/isc/config/cfgmgr_test.py
    trunk/src/lib/config/python/isc/config/config_data.py
      - copied unchanged from r881, branches/jelte-configuration/src/lib/config/python/isc/config/config_data.py
    trunk/src/lib/config/python/isc/config/config_data_test.py
      - copied unchanged from r881, branches/jelte-configuration/src/lib/config/python/isc/config/config_data_test.py
    trunk/src/lib/config/python/isc/config/module_spec.py
      - copied unchanged from r881, branches/jelte-configuration/src/lib/config/python/isc/config/module_spec.py
    trunk/src/lib/config/python/isc/config/module_spec_test.py
      - copied unchanged from r881, branches/jelte-configuration/src/lib/config/python/isc/config/module_spec_test.py
    trunk/src/lib/config/testdata/b10-config-bad1.db
      - copied unchanged from r881, branches/jelte-configuration/src/lib/config/testdata/b10-config-bad1.db
    trunk/src/lib/config/testdata/b10-config-bad2.db
      - copied unchanged from r881, branches/jelte-configuration/src/lib/config/testdata/b10-config-bad2.db
    trunk/src/lib/config/testdata/b10-config-bad3.db
      - copied unchanged from r881, branches/jelte-configuration/src/lib/config/testdata/b10-config-bad3.db
    trunk/src/lib/config/testdata/b10-config.db
      - copied unchanged from r881, branches/jelte-configuration/src/lib/config/testdata/b10-config.db
Removed:
    trunk/src/lib/config/cpp/data_def.cc
    trunk/src/lib/config/cpp/data_def.h
    trunk/src/lib/config/cpp/data_def_unittests.cc
    trunk/src/lib/config/python/isc/config/datadefinition.py
    trunk/src/lib/config/python/isc/config/datadefinition_test.py
Modified:
    trunk/   (props changed)
    trunk/src/bin/auth/auth.spec
    trunk/src/bin/auth/auth_srv.cc
    trunk/src/bin/auth/main.cc
    trunk/src/bin/bind10/bind10.py.in
    trunk/src/bin/bind10/bob.spec
    trunk/src/bin/bindctl/bindcmd.py
    trunk/src/bin/bindctl/bindctl.in
    trunk/src/bin/bindctl/bindctl.py
    trunk/src/bin/cmdctl/b10-cmdctl.py.in
    trunk/src/lib/cc/cpp/data.cc
    trunk/src/lib/cc/cpp/data.h
    trunk/src/lib/cc/cpp/data_unittests.cc
    trunk/src/lib/cc/python/isc/cc/data.py
    trunk/src/lib/config/cpp/Makefile.am
    trunk/src/lib/config/cpp/ccsession.cc
    trunk/src/lib/config/cpp/ccsession.h
    trunk/src/lib/config/python/isc/config/__init__.py
    trunk/src/lib/config/python/isc/config/ccsession.py
    trunk/src/lib/config/python/isc/config/cfgmgr.py
    trunk/src/lib/config/python/isc/config/config_test.in
    trunk/src/lib/config/testdata/spec1.spec
    trunk/src/lib/config/testdata/spec10.spec
    trunk/src/lib/config/testdata/spec11.spec
    trunk/src/lib/config/testdata/spec12.spec
    trunk/src/lib/config/testdata/spec13.spec
    trunk/src/lib/config/testdata/spec14.spec
    trunk/src/lib/config/testdata/spec15.spec
    trunk/src/lib/config/testdata/spec16.spec
    trunk/src/lib/config/testdata/spec17.spec
    trunk/src/lib/config/testdata/spec18.spec
    trunk/src/lib/config/testdata/spec19.spec
    trunk/src/lib/config/testdata/spec2.spec
    trunk/src/lib/config/testdata/spec20.spec
    trunk/src/lib/config/testdata/spec21.spec
    trunk/src/lib/config/testdata/spec22.spec
    trunk/src/lib/config/testdata/spec23.spec
    trunk/src/lib/config/testdata/spec3.spec
    trunk/src/lib/config/testdata/spec4.spec
    trunk/src/lib/config/testdata/spec5.spec
    trunk/src/lib/config/testdata/spec6.spec
    trunk/src/lib/config/testdata/spec7.spec
    trunk/src/lib/config/testdata/spec9.spec
    trunk/src/lib/dns/cpp/   (props changed)

Modified: trunk/src/bin/auth/auth.spec
==============================================================================
--- trunk/src/bin/auth/auth.spec (original)
+++ trunk/src/bin/auth/auth.spec Fri Feb 19 16:08:18 2010
@@ -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": []
+      }
+    ]
   }
 }
 

Modified: trunk/src/bin/auth/auth_srv.cc
==============================================================================
--- trunk/src/bin/auth/auth_srv.cc (original)
+++ trunk/src/bin/auth/auth_srv.cc Fri Feb 19 16:08:18 2010
@@ -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 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 @@
         // 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);
 }

Modified: trunk/src/bin/auth/main.cc
==============================================================================
--- trunk/src/bin/auth/main.cc (original)
+++ trunk/src/bin/auth/main.cc Fri Feb 19 16:08:18 2010
@@ -64,18 +64,15 @@
 
 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 @@
         } 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;

Modified: trunk/src/bin/bind10/bind10.py.in
==============================================================================
--- trunk/src/bin/bind10/bind10.py.in (original)
+++ trunk/src/bin/bind10/bind10.py.in Fri Feb 19 16:08:18 2010
@@ -106,6 +106,7 @@
         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 @@
         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 @@
         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 @@
         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 @@
 
         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)
 

Modified: trunk/src/bin/bind10/bob.spec
==============================================================================
--- trunk/src/bin/bind10/bob.spec (original)
+++ trunk/src/bin/bind10/bob.spec Fri Feb 19 16:08:18 2010
@@ -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": []

Modified: trunk/src/bin/bindctl/bindcmd.py
==============================================================================
--- trunk/src/bin/bindctl/bindcmd.py (original)
+++ trunk/src/bin/bindctl/bindcmd.py Fri Feb 19 16:08:18 2010
@@ -31,6 +31,7 @@
 import getpass
 from hashlib import sha1
 import csv
+import ast
 
 try:
     from collections import OrderedDict
@@ -85,7 +86,7 @@
                 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 @@
         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 @@
                     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 @@
                         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:

Modified: trunk/src/bin/bindctl/bindctl.in
==============================================================================
--- trunk/src/bin/bindctl/bindctl.in (original)
+++ trunk/src/bin/bindctl/bindctl.in Fri Feb 19 16:08:18 2010
@@ -5,7 +5,7 @@
 
 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}

Modified: trunk/src/bin/bindctl/bindctl.py
==============================================================================
--- trunk/src/bin/bindctl/bindctl.py (original)
+++ trunk/src/bin/bindctl/bindctl.py Fri Feb 19 16:08:18 2010
@@ -53,6 +53,9 @@
     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 @@
     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?")
 
 

Modified: trunk/src/bin/cmdctl/b10-cmdctl.py.in
==============================================================================
--- trunk/src/bin/cmdctl/b10-cmdctl.py.in (original)
+++ trunk/src/bin/cmdctl/b10-cmdctl.py.in Fri Feb 19 16:08:18 2010
@@ -169,8 +169,8 @@
             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 @@
         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 @@
             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

Modified: trunk/src/lib/cc/cpp/data.cc
==============================================================================
--- trunk/src/lib/cc/cpp/data.cc (original)
+++ trunk/src/lib/cc/cpp/data.cc Fri Feb 19 16:08:18 2010
@@ -829,13 +829,19 @@
         (*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 @@
     // add length if needed
     //
     if (omit_length) {
-        ss << ss2.rdbuf();
-    } else {
-        
+        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;
+        }
     }
 }
 

Modified: trunk/src/lib/cc/cpp/data.h
==============================================================================
--- trunk/src/lib/cc/cpp/data.h (original)
+++ trunk/src/lib/cc/cpp/data.h Fri Feb 19 16:08:18 2010
@@ -35,7 +35,6 @@
 /// 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) :

Modified: trunk/src/lib/cc/cpp/data_unittests.cc
==============================================================================
--- trunk/src/lib/cc/cpp/data_unittests.cc (original)
+++ trunk/src/lib/cc/cpp/data_unittests.cc Fri Feb 19 16:08:18 2010
@@ -266,5 +266,13 @@
 
     //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);
+}
+

Modified: trunk/src/lib/cc/python/isc/cc/data.py
==============================================================================
--- trunk/src/lib/cc/python/isc/cc/data.py (original)
+++ trunk/src/lib/cc/python/isc/cc/data.py Fri Feb 19 16:08:18 2010
@@ -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 @@
     """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 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 @@
     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 @@
         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 @@
         # 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)

Modified: trunk/src/lib/config/cpp/Makefile.am
==============================================================================
--- trunk/src/lib/config/cpp/Makefile.am (original)
+++ trunk/src/lib/config/cpp/Makefile.am Fri Feb 19 16:08:18 2010
@@ -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)

Modified: trunk/src/lib/config/cpp/ccsession.cc
==============================================================================
--- trunk/src/lib/config/cpp/ccsession.cc (original)
+++ trunk/src/lib/config/cpp/ccsession.cc Fri Feb 19 16:08:18 2010
@@ -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 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 @@
     }
 
     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 @@
     //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 @@
         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 @@
         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 @@
     return 0;
 }
 
+}
+}

Modified: trunk/src/lib/config/cpp/ccsession.h
==============================================================================
--- trunk/src/lib/config/cpp/ccsession.h (original)
+++ trunk/src/lib/config/cpp/ccsession.h Fri Feb 19 16:08:18 2010
@@ -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 @@
      * 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:

Modified: trunk/src/lib/config/python/isc/config/__init__.py
==============================================================================
--- trunk/src/lib/config/python/isc/config/__init__.py (original)
+++ trunk/src/lib/config/python/isc/config/__init__.py Fri Feb 19 16:08:18 2010
@@ -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 *

Modified: trunk/src/lib/config/python/isc/config/ccsession.py
==============================================================================
--- trunk/src/lib/config/python/isc/config/ccsession.py (original)
+++ trunk/src/lib/config/python/isc/config/ccsession.py Fri Feb 19 16:08:18 2010
@@ -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 @@
         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()

Modified: trunk/src/lib/config/python/isc/config/cfgmgr.py
==============================================================================
--- trunk/src/lib/config/python/isc/config/cfgmgr.py (original)
+++ trunk/src/lib/config/python/isc/config/cfgmgr.py Fri Feb 19 16:08:18 2010
@@ -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 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"
-
-    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)
+        self.db_filename = data_path + os.sep + file_name
+
+    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 @@
                 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 @@
         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_commands(self, module_name, commands):
-        self.commands[module_name] = commands
-
-    def remove_commands(self, module_name):
-        del self.commands[module_name]
+    def set_module_spec(self, spec):
+        """Adds a ModuleSpec"""
+        self.module_specs[spec.get_module_name()] = spec
+
+    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 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]
-        else:
-            print("[bind-cfgd] unknown message: " + str(msg))
-            answer["result"] = [ 1, "Unknown module: " + str(msg) ]
+            # this seems wrong, might start pingpong
+            answer = isc.config.ccsession.create_answer(0)
+        else:
+            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 @@
                 self.cc.group_reply(env, answer)
             else:
                 self.running = False
-
-cm = None
-
-def signal_handler(signal, frame):
-    global cm
-    if cm:
-        cm.running = False

Modified: trunk/src/lib/config/python/isc/config/config_test.in
==============================================================================
--- trunk/src/lib/config/python/isc/config/config_test.in (original)
+++ trunk/src/lib/config/python/isc/config/config_test.in Fri Feb 19 16:08:18 2010
@@ -12,5 +12,8 @@
 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 $*

Modified: trunk/src/lib/config/testdata/spec1.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec1.spec (original)
+++ trunk/src/lib/config/testdata/spec1.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec1"
   }
 }

Modified: trunk/src/lib/config/testdata/spec10.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec10.spec (original)
+++ trunk/src/lib/config/testdata/spec10.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

Modified: trunk/src/lib/config/testdata/spec11.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec11.spec (original)
+++ trunk/src/lib/config/testdata/spec11.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

Modified: trunk/src/lib/config/testdata/spec12.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec12.spec (original)
+++ trunk/src/lib/config/testdata/spec12.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

Modified: trunk/src/lib/config/testdata/spec13.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec13.spec (original)
+++ trunk/src/lib/config/testdata/spec13.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

Modified: trunk/src/lib/config/testdata/spec14.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec14.spec (original)
+++ trunk/src/lib/config/testdata/spec14.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

Modified: trunk/src/lib/config/testdata/spec15.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec15.spec (original)
+++ trunk/src/lib/config/testdata/spec15.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

Modified: trunk/src/lib/config/testdata/spec16.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec16.spec (original)
+++ trunk/src/lib/config/testdata/spec16.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": 1
   }

Modified: trunk/src/lib/config/testdata/spec17.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec17.spec (original)
+++ trunk/src/lib/config/testdata/spec17.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "commands": [
       {

Modified: trunk/src/lib/config/testdata/spec18.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec18.spec (original)
+++ trunk/src/lib/config/testdata/spec18.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "commands": [
       {

Modified: trunk/src/lib/config/testdata/spec19.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec19.spec (original)
+++ trunk/src/lib/config/testdata/spec19.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "commands": [
       {

Modified: trunk/src/lib/config/testdata/spec2.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec2.spec (original)
+++ trunk/src/lib/config/testdata/spec2.spec Fri Feb 19 16:08:18 2010
@@ -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": []
+      }
     ]
   }
 }

Modified: trunk/src/lib/config/testdata/spec20.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec20.spec (original)
+++ trunk/src/lib/config/testdata/spec20.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "commands": [
       {

Modified: trunk/src/lib/config/testdata/spec21.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec21.spec (original)
+++ trunk/src/lib/config/testdata/spec21.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "commands": 1
   }

Modified: trunk/src/lib/config/testdata/spec22.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec22.spec (original)
+++ trunk/src/lib/config/testdata/spec22.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "value1",

Modified: trunk/src/lib/config/testdata/spec23.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec23.spec (original)
+++ trunk/src/lib/config/testdata/spec23.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "commands": [
       {

Modified: trunk/src/lib/config/testdata/spec3.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec3.spec (original)
+++ trunk/src/lib/config/testdata/spec3.spec Fri Feb 19 16:08:18 2010
@@ -1,6 +1,6 @@
 {
-  "data_specification": {
-    "module_name": "Spec2",
+  "module_spec": {
+    "module_name": "Spec3",
     "config_data": [
       {
         "item_type": "integer",

Modified: trunk/src/lib/config/testdata/spec4.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec4.spec (original)
+++ trunk/src/lib/config/testdata/spec4.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

Modified: trunk/src/lib/config/testdata/spec5.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec5.spec (original)
+++ trunk/src/lib/config/testdata/spec5.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

Modified: trunk/src/lib/config/testdata/spec6.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec6.spec (original)
+++ trunk/src/lib/config/testdata/spec6.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",

Modified: trunk/src/lib/config/testdata/spec7.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec7.spec (original)
+++ trunk/src/lib/config/testdata/spec7.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
   }
 }
 

Modified: trunk/src/lib/config/testdata/spec9.spec
==============================================================================
--- trunk/src/lib/config/testdata/spec9.spec (original)
+++ trunk/src/lib/config/testdata/spec9.spec Fri Feb 19 16:08:18 2010
@@ -1,5 +1,5 @@
 {
-  "data_specification": {
+  "module_spec": {
     "module_name": "Spec2",
     "config_data": [
       { "item_name": "item1",




More information about the bind10-changes mailing list