fix setting of config data (refactored the 'set new config for all modules' into calling 'set new config for module X' for each dataset along the way)

git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac427@3985 e5f2f494-b856-4b98-b285-d166d9295462

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

@@ -270,16 +270,15 @@ class ConfigManager:
         else:
             return ccsession.create_answer(0, self.config.data)
 
-    def _handle_set_config_module(self, cmd):
+    def _handle_set_config_module(self, module_name, cmd):
         # the answer comes (or does not come) from the relevant module
         # so we need a variable to see if we got it
         answer = None
         # todo: use api (and check the data against the definition?)
         old_data = copy.deepcopy(self.config.data)
-        module_name = cmd[0]
         conf_part = data.find_no_exc(self.config.data, module_name)
         if conf_part:
-            data.merge(conf_part, cmd[1])
+            data.merge(conf_part, cmd)
             update_cmd = ccsession.create_command(ccsession.COMMAND_CONFIG_UPDATE,
                                                   conf_part)
             seq = self.cc.group_sendmsg(update_cmd, module_name)
@@ -289,7 +288,7 @@ class ConfigManager:
                 answer = ccsession.create_answer(1, "Timeout waiting for answer from " + module_name)
         else:
             conf_part = data.set(self.config.data, module_name, {})
-            data.merge(conf_part[module_name], cmd[1])
+            data.merge(conf_part[module_name], cmd)
             # send out changed info
             update_cmd = ccsession.create_command(ccsession.COMMAND_CONFIG_UPDATE,
                                                   conf_part[module_name])
@@ -309,29 +308,21 @@ class ConfigManager:
 
     def _handle_set_config_all(self, cmd):
         old_data = copy.deepcopy(self.config.data)
-        data.merge(self.config.data, cmd[0])
-        # send out changed info
         got_error = False
         err_list = []
-        for module in self.config.data:
-            if module != "version" and \
-               (module not in old_data or self.config.data[module] != old_data[module]):
-                update_cmd = ccsession.create_command(ccsession.COMMAND_CONFIG_UPDATE,
-                                                      self.config.data[module])
-                seq = self.cc.group_sendmsg(update_cmd, module)
-                try:
-                    answer, env = self.cc.group_recvmsg(False, seq)
-                    if answer == None:
-                        got_error = True
-                        err_list.append("No answer message from " + module)
-                    else:
-                        rcode, val = ccsession.parse_answer(answer)
-                        if rcode != 0:
-                            got_error = True
-                            err_list.append(val)
-                except isc.cc.SessionTimeout:
+        # The format of the command is a dict with module->newconfig
+        # sets, so we simply call set_config_module for each of those
+        for module in cmd:
+            if module != "version":
+                answer = self._handle_set_config_module(module, cmd[module])
+                if answer == None:
                     got_error = True
-                    err_list.append("CC Timeout waiting on answer message from " + module)
+                    err_list.append("No answer message from " + module)
+                else:
+                    rcode, val = ccsession.parse_answer(answer)
+                    if rcode != 0:
+                        got_error = True
+                        err_list.append(val)
         if not got_error:
             self.write_config()
             return ccsession.create_answer(0)
@@ -343,12 +334,13 @@ class ConfigManager:
     def _handle_set_config(self, cmd):
         """Private function that handles the 'set_config' command"""
         answer = None
+
         if cmd == None:
             return ccsession.create_answer(1, "Wrong number of arguments")
         if len(cmd) == 2:
-            answer = self._handle_set_config_module(cmd)
+            answer = self._handle_set_config_module(cmd[0], cmd[1])
         elif len(cmd) == 1:
-            answer = self._handle_set_config_all(cmd)
+            answer = self._handle_set_config_all(cmd[0])
         else:
             answer = ccsession.create_answer(1, "Wrong number of arguments")
         if not answer:

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

@@ -288,6 +288,66 @@ class TestConfigManager(unittest.TestCase):
                                 },
                                 {'result': [0]})
 
+    def test_set_config_all(self):
+        my_ok_answer = { 'result': [ 0 ] }
+
+        self.assertEqual({"version": 2}, self.cm.config.data)
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.cm._handle_set_config_all({"test": { "value1": 123 }})
+        self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
+                          "test": { "value1": 123 }
+                         }, self.cm.config.data)
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.cm._handle_set_config_all({"test": { "value1": 124 }})
+        self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
+                          "test": { "value1": 124 }
+                         }, self.cm.config.data)
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.cm._handle_set_config_all({"test": { "value2": True }})
+        self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
+                          "test": { "value1": 124,
+                                    "value2": True
+                                  }
+                         }, self.cm.config.data)
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.cm._handle_set_config_all({"test": { "value3": [ 1, 2, 3 ] }})
+        self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
+                          "test": { "value1": 124,
+                                    "value2": True,
+                                    "value3": [ 1, 2, 3 ]
+                                  }
+                         }, self.cm.config.data)
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.cm._handle_set_config_all({"test": { "value2": False }})
+        self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
+                          "test": { "value1": 124,
+                                    "value2": False,
+                                    "value3": [ 1, 2, 3 ]
+                                  }
+                         }, self.cm.config.data)
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.cm._handle_set_config_all({"test": { "value1": None }})
+        self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
+                          "test": { "value2": False,
+                                    "value3": [ 1, 2, 3 ]
+                                  }
+                         }, self.cm.config.data)
+
+        self.fake_session.group_sendmsg(my_ok_answer, "ConfigManager")
+        self.cm._handle_set_config_all({"test": { "value3": [ 1 ] }})
+        self.assertEqual({"version": config_data.BIND10_CONFIG_DATA_VERSION,
+                          "test": { "value2": False,
+                                    "value3": [ 1 ]
+                                  }
+                         }, self.cm.config.data)
+
+
     def test_run(self):
         self.fake_session.group_sendmsg({ "command": [ "get_commands_spec" ] }, "ConfigManager")
         self.fake_session.group_sendmsg({ "command": [ "shutdown" ] }, "ConfigManager")