added tests for the bad condition, and fixed them

one open potential issue; if there is a CCSessionError in the cpp version it still sends a command. Once we have logging and timeouts we should remove that answer


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

src/lib/config/ccsession.cc

@@ -327,8 +327,8 @@ ModuleCCSession::checkCommand()
         ElementPtr answer;
         try {
             std::string cmd_str = parseCommand(arg, data);
+            std::string target_module = routing->get("group")->stringValue();
             if (cmd_str == "config_update") {
-                std::string target_module = routing->get("group")->stringValue();
                 if (target_module == module_name_) {
                     answer = handleConfigUpdate(arg);
                 } else {
@@ -339,16 +339,22 @@ ModuleCCSession::checkCommand()
                     return 0;
                 }
             } else {
-                if (command_handler_) {
-                    answer = command_handler_(cmd_str, arg);
-                } else {
-                    answer = createAnswer(1, "Command given but no command handler for module");
+                if (target_module == module_name_) {
+                    if (command_handler_) {
+                        answer = command_handler_(cmd_str, arg);
+                    } else {
+                        answer = createAnswer(1, "Command given but no command handler for module");
+                    }
                 }
             }
         } catch (CCSessionError re) {
+            // TODO: Once we have logging and timeouts, we should not
+            // answer here (potential interference)
             answer = createAnswer(1, re.what());
         }
-        session_.reply(routing, answer);
+        if (!isNull(answer)) {
+            session_.reply(routing, answer);
+        }
     }
     
     return 0;

src/lib/config/tests/ccsession_unittests.cc

@@ -405,3 +405,41 @@ TEST(CCSession, remoteConfig)
     endFakeSession();
 }
 
+TEST(CCSession, ignoreRemoteConfigCommands) {
+
+    initFakeSession();
+    // client will ask for config
+    initial_messages->add(createAnswer(0, el("{  }")));
+
+    EXPECT_EQ(false, haveSubscription("Spec2", "*"));
+    ModuleCCSession mccs(ccspecfile("spec2.spec"), my_config_handler, my_command_handler);
+    EXPECT_EQ(true, haveSubscription("Spec2", "*"));
+
+    EXPECT_EQ(2, msg_queue->size());
+    ElementPtr msg;
+    std::string group, to;
+    // drop the module_spec and config commands
+    getFirstMessage(group, to);
+    getFirstMessage(group, to);
+
+    initial_messages->add(createAnswer(0, el("{  }")));
+    mccs.addRemoteConfig(ccspecfile("spec1.spec"));
+    EXPECT_EQ(1, msg_queue->size());
+    msg = getFirstMessage(group, to);
+
+    // Check if commands for the module are handled
+    addMessage(el("{ \"command\": [ \"good_command\" ] }"), "Spec2", "*");
+    int result = mccs.checkCommand();
+    EXPECT_EQ(1, msg_queue->size());
+    msg = getFirstMessage(group, to);
+    EXPECT_EQ("{ \"result\": [ 0 ] }", msg->str());
+    EXPECT_EQ(0, result);
+
+    // Check if commands for the other module are ignored
+    addMessage(el("{ \"command\": [ \"good_command\" ] }"), "Spec1", "*");
+    EXPECT_EQ(1, msg_queue->size());
+    result = mccs.checkCommand();
+    EXPECT_EQ(0, msg_queue->size());
+    
+    endFakeSession();
+}

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

@@ -183,10 +183,10 @@ class ModuleCCSession(ConfigData):
         if msg and not 'result' in msg:
             answer = None
             try:
+                module_name = env['group']
                 cmd, arg = isc.config.ccsession.parse_command(msg)
                 if cmd == COMMAND_CONFIG_UPDATE:
                     new_config = arg
-                    module_name = env['group']
                     # If the target channel was not this module
                     # it might be in the remote_module_configs
                     if module_name != self._module_name:
@@ -213,10 +213,12 @@ class ModuleCCSession(ConfigData):
                             isc.cc.data.merge(newc, new_config)
                             self.set_local_config(newc)
                 else:
-                    if self._command_handler:
-                        answer = self._command_handler(cmd, arg)
-                    else:
-                        answer = create_answer(2, self._module_name + " has no command handler")
+                    # ignore commands for 'remote' modules
+                    if module_name == self._module_name:
+                        if self._command_handler:
+                            answer = self._command_handler(cmd, arg)
+                        else:
+                            answer = create_answer(2, self._module_name + " has no command handler")
             except Exception as exc:
                 answer = create_answer(1, str(exc))
             if answer:

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

@@ -377,7 +377,34 @@ class TestModuleCCSession(unittest.TestCase):
         mccs = None
         self.assertFalse("Spec2" in fake_session.subscriptions)
         
-    
+    def test_ignore_command_remote_module(self):
+        # Create a Spec1 module and subscribe to remote config for Spec2
+        fake_session = FakeModuleCCSession()
+        mccs = self.create_session("spec1.spec", None, None, fake_session)
+        mccs.set_command_handler(self.my_command_handler_ok)
+        rmodname = mccs.add_remote_config(self.spec_file("spec2.spec"))
+
+        # remove the 'get config' from the queue
+        self.assertEqual(len(fake_session.message_queue), 1)
+        fake_session.get_message("ConfigManager")
+
+        # check if the command for the module itself is received
+        cmd = isc.config.ccsession.create_command("just_some_command", { 'foo': 'a' })
+        fake_session.group_sendmsg(cmd, 'Spec1')
+        self.assertEqual(len(fake_session.message_queue), 1)
+        mccs.check_command()
+        self.assertEqual(len(fake_session.message_queue), 1)
+        self.assertEqual({'result': [ 0 ]},
+                         fake_session.get_message('Spec1', None))
+
+        # check if the command for the other module is ignored
+        cmd = isc.config.ccsession.create_command("just_some_command", { 'foo': 'a' })
+        fake_session.group_sendmsg(cmd, 'Spec2')
+        self.assertEqual(len(fake_session.message_queue), 1)
+        mccs.check_command()
+        self.assertEqual(len(fake_session.message_queue), 0)
+        
+
 class fakeUIConn():
     def __init__(self):
         self.get_answers = {}