add validate_command to module_spec API

call this check before applying commands


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

src/lib/config/ccsession.cc

@@ -149,7 +149,7 @@ parseCommand(ConstElementPtr& arg, ConstElementPtr command) {
             if (cmd->size() > 1) {
                 arg = cmd->get(1);
             } else {
-                arg = ElementPtr();
+                arg = Element::createMap();
             }
             return (cmd->get(0)->stringValue());
         } else {
@@ -314,7 +314,17 @@ ModuleCCSession::checkCommand() {
             } else {
                 if (target_module == module_name_) {
                     if (command_handler_) {
-                        answer = command_handler_(cmd_str, arg);
+                        ElementPtr errors = Element::createList();
+                        if (module_specification_.validate_command(cmd_str, arg, errors)) {
+                            answer = command_handler_(cmd_str, arg);
+                        } else {
+                            std::stringstream ss;
+                            ss << "Error in command validation: ";
+                            BOOST_FOREACH(ConstElementPtr error, errors->listValue()) {
+                                ss << error->stringValue();
+                            }
+                            answer = createAnswer(3, ss.str());
+                        }
                     } else {
                         answer = createAnswer(1, "Command given but no command handler for module");
                     }

src/lib/config/ccsession.h

@@ -92,7 +92,7 @@ isc::data::ConstElementPtr createCommand(const std::string& command,
 ///        command and an ElementPtr containing the optional argument.
 ///
 /// \param arg This value will be set to the ElementPtr pointing to
-///        the argument, or to an empty ElementPtr if there was none.
+///        the argument, or to an empty Map (ElementPtr) if there was none.
 /// \param command The command message containing the command (as made
 ///        by createCommand()
 /// \return The command string

src/lib/config/module_spec.cc

@@ -183,6 +183,34 @@ ModuleSpec::validate_config(ConstElementPtr data, const bool full) const {
 }
 
 bool
+ModuleSpec::validate_command(const std::string& command,
+                             ConstElementPtr args,
+                             ElementPtr errors) {
+    ConstElementPtr commands_spec = module_specification->find("commands");
+
+    if (args->getType() != Element::map) {
+        errors->add(Element::create("args for command " + command + " is not a map"));
+        return (false);
+    }
+
+    if (!commands_spec) {
+        // there are no commands according to the spec.
+        errors->add(Element::create("The given module has no commands"));
+        return (false);
+    }
+
+    BOOST_FOREACH(ConstElementPtr cur_command, commands_spec->listValue()) {
+        if (cur_command->get("command_name")->stringValue() == command) {
+            return (validate_spec_list(cur_command->get("command_args"), args, true, errors));
+        }
+    }
+
+    // this command is unknown
+    errors->add(Element::create("Unknown command " + command));
+    return (false);
+}
+
+bool
 ModuleSpec::validate_config(ConstElementPtr data, const bool full,
                             ElementPtr errors) const
 {

src/lib/config/module_spec.h

@@ -91,6 +91,20 @@ namespace isc { namespace config {
         bool validate_config(isc::data::ConstElementPtr data,
                              const bool full = false) const;
 
+        /// Validates the arguments for the given command
+        ///
+        /// \param command The command to validate the arguments for
+        /// \param args A dict containing the command parameters
+        /// \param errors An ElementPtr pointing to a ListElement. Any
+        ///               errors that are found are added as
+        ///               StringElements to this list
+        /// \return true if the command is known and the parameters are correct
+        ///         false otherwise
+        bool validate_command(const std::string& command,
+                              isc::data::ConstElementPtr args,
+                              isc::data::ElementPtr errors);
+
+
         /// errors must be of type ListElement
         bool validate_config(isc::data::ConstElementPtr data, const bool full,
                              isc::data::ElementPtr errors) const;

src/lib/config/tests/ccsession_unittests.cc

@@ -137,7 +137,7 @@ TEST_F(CCSessionTest, parseCommand) {
 
     cmd = parseCommand(arg, el("{ \"command\": [ \"my_command\" ] }"));
     EXPECT_EQ("my_command", cmd);
-    EXPECT_TRUE(isNull(arg));
+    EXPECT_EQ(*arg, *Element::createMap());
 
     cmd = parseCommand(arg, el("{ \"command\": [ \"my_command\", 1 ] }"));
     EXPECT_EQ("my_command", cmd);
@@ -193,8 +193,8 @@ ConstElementPtr my_command_handler(const std::string& command,
     if (command == "good_command") {
         return (createAnswer());
     } else if (command == "command_with_arg") {
-        if (arg) {
-            if (arg->getType() == Element::integer) {
+        if (arg->contains("number")) {
+            if (arg->get("number")->getType() == Element::integer) {
                 return (createAnswer(0, el("2")));
             } else {
                 return (createAnswer(1, "arg bad type"));
@@ -235,10 +235,10 @@ TEST_F(CCSessionTest, checkCommand) {
     // client will ask for config
     session.getMessages()->add(createAnswer(0, el("{}")));
 
-    EXPECT_FALSE(session.haveSubscription("Spec2", "*"));
-    ModuleCCSession mccs(ccspecfile("spec2.spec"), session, my_config_handler,
+    EXPECT_FALSE(session.haveSubscription("Spec29", "*"));
+    ModuleCCSession mccs(ccspecfile("spec29.spec"), session, my_config_handler,
                          my_command_handler);
-    EXPECT_TRUE(session.haveSubscription("Spec2", "*"));
+    EXPECT_TRUE(session.haveSubscription("Spec29", "*"));
 
     EXPECT_EQ(2, session.getMsgQueue()->size());
     ConstElementPtr msg;
@@ -252,19 +252,19 @@ TEST_F(CCSessionTest, checkCommand) {
     EXPECT_EQ(0, result);
 
     // not a command, should be ignored
-    session.addMessage(el("1"), "Spec2", "*");
+    session.addMessage(el("1"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(0, result);
-
-    session.addMessage(el("{ \"command\": [ \"good_command\" ] }"), "Spec2",
+    session.addMessage(el("{ \"command\": [ \"good_command\" ] }"), "Spec29",
                        "*");
+
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
     EXPECT_EQ("{ \"result\": [ 0 ] }", msg->str());
     EXPECT_EQ(0, result);
 
-    session.addMessage(el("{ \"command\": \"bad_command\" }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": \"bad_command\" }"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
@@ -272,37 +272,37 @@ TEST_F(CCSessionTest, checkCommand) {
     EXPECT_EQ(0, result);
 
     session.addMessage(el("{ \"command\": [ \"bad_command\" ] }"),
-                       "Spec2", "*");
+                       "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
     EXPECT_EQ("{ \"result\": [ 1, \"bad command\" ] }", msg->str());
     EXPECT_EQ(0, result);
 
-    session.addMessage(el("{ \"command\": [ \"command_with_arg\", 1 ] }"),
-                       "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"command_with_arg\", {\"number\": 1} ] }"),
+                       "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
     EXPECT_EQ("{ \"result\": [ 0, 2 ] }", msg->str());
     EXPECT_EQ(0, result);
 
-    session.addMessage(el("{ \"command\": [ \"command_with_arg\" ] }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"command_with_arg\" ] }"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
     EXPECT_EQ("{ \"result\": [ 1, \"arg missing\" ] }", msg->str());
     EXPECT_EQ(0, result);
 
-    session.addMessage(el("{ \"command\": [ \"command_with_arg\", \"asdf\" ] }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"command_with_arg\", \"asdf\" ] }"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
-    EXPECT_EQ("{ \"result\": [ 1, \"arg bad type\" ] }", msg->str());
+    EXPECT_EQ("{ \"result\": [ 3, \"Error in command validation: args for command command_with_arg is not a map\" ] }", msg->str());
     EXPECT_EQ(0, result);
 
     mccs.setCommandHandler(NULL);
-    session.addMessage(el("{ \"command\": [ \"whatever\" ] }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"whatever\" ] }"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
@@ -310,7 +310,7 @@ TEST_F(CCSessionTest, checkCommand) {
     EXPECT_EQ(0, result);
 
     EXPECT_EQ(1, mccs.getValue("item1")->intValue());
-    session.addMessage(el("{ \"command\": [ \"config_update\", { \"item1\": 2 } ] }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"config_update\", { \"item1\": 2 } ] }"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
@@ -318,7 +318,7 @@ TEST_F(CCSessionTest, checkCommand) {
     EXPECT_EQ(0, result);
     EXPECT_EQ(2, mccs.getValue("item1")->intValue());
 
-    session.addMessage(el("{ \"command\": [ \"config_update\", { \"item1\": \"asdf\" } ] }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"config_update\", { \"item1\": \"asdf\" } ] }"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
@@ -326,7 +326,7 @@ TEST_F(CCSessionTest, checkCommand) {
     EXPECT_EQ(0, result);
     EXPECT_EQ(2, mccs.getValue("item1")->intValue());
 
-    session.addMessage(el("{ \"command\": [ \"config_update\", { \"item1\": 5 } ] }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"config_update\", { \"item1\": 5 } ] }"), "Spec29", "*");
     result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);
@@ -387,9 +387,9 @@ TEST_F(CCSessionTest, ignoreRemoteConfigCommands) {
     // client will ask for config
     session.getMessages()->add(createAnswer(0, el("{  }")));
 
-    EXPECT_FALSE(session.haveSubscription("Spec2", "*"));
-    ModuleCCSession mccs(ccspecfile("spec2.spec"), session, my_config_handler, my_command_handler);
-    EXPECT_TRUE(session.haveSubscription("Spec2", "*"));
+    EXPECT_FALSE(session.haveSubscription("Spec29", "*"));
+    ModuleCCSession mccs(ccspecfile("spec29.spec"), session, my_config_handler, my_command_handler);
+    EXPECT_TRUE(session.haveSubscription("Spec29", "*"));
 
     EXPECT_EQ(2, session.getMsgQueue()->size());
     ConstElementPtr msg;
@@ -404,7 +404,7 @@ TEST_F(CCSessionTest, ignoreRemoteConfigCommands) {
     msg = session.getFirstMessage(group, to);
 
     // Check if commands for the module are handled
-    session.addMessage(el("{ \"command\": [ \"good_command\" ] }"), "Spec2", "*");
+    session.addMessage(el("{ \"command\": [ \"good_command\" ] }"), "Spec29", "*");
     int result = mccs.checkCommand();
     EXPECT_EQ(1, session.getMsgQueue()->size());
     msg = session.getFirstMessage(group, to);

src/lib/config/tests/module_spec_unittests.cc

@@ -176,3 +176,40 @@ TEST(ModuleSpec, DataValidation) {
     EXPECT_FALSE(data_test_with_errors(dd, "data22_9.data", errors));
     EXPECT_EQ("[ \"Unknown item value_does_not_exist\" ]", errors->str());
 }
+
+TEST(ModuleSpec, CommandValidation) {
+    ModuleSpec dd = moduleSpecFromFile(specfile("spec2.spec"));
+    ConstElementPtr arg = Element::fromJSON("{}");
+    ElementPtr errors = Element::createList();
+
+    EXPECT_TRUE(dd.validate_command("shutdown", arg, errors));
+    EXPECT_EQ(errors->size(), 0);
+
+    errors = Element::createList();
+    EXPECT_FALSE(dd.validate_command("unknowncommand", arg, errors));
+    EXPECT_EQ(errors->size(), 1);
+    EXPECT_EQ(errors->get(0)->stringValue(), "Unknown command unknowncommand");
+
+    errors = Element::createList();
+    EXPECT_FALSE(dd.validate_command("print_message", arg, errors));
+    EXPECT_EQ(errors->size(), 1);
+    EXPECT_EQ(errors->get(0)->stringValue(), "Non-optional value missing");
+
+    errors = Element::createList();
+    arg = Element::fromJSON("{ \"message\": \"Hello\" }");
+    EXPECT_TRUE(dd.validate_command("print_message", arg, errors));
+    EXPECT_EQ(errors->size(), 0);
+
+    errors = Element::createList();
+    arg = Element::fromJSON("{ \"message\": \"Hello\", \"unknown_second_arg\": 1 }");
+    EXPECT_FALSE(dd.validate_command("print_message", arg, errors));
+    EXPECT_EQ(errors->size(), 1);
+    EXPECT_EQ(errors->get(0)->stringValue(), "Unknown item unknown_second_arg");
+
+    errors = Element::createList();
+    arg = Element::fromJSON("{ \"message\": 1 }");
+    EXPECT_FALSE(dd.validate_command("print_message", arg, errors));
+    EXPECT_EQ(errors->size(), 1);
+    EXPECT_EQ(errors->get(0)->stringValue(), "Type mismatch");
+
+}

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

@@ -0,0 +1,35 @@
+{
+  "module_spec": {
+    "module_name": "Spec29",
+    "config_data": [
+      { "item_name": "item1",
+        "item_type": "integer",
+        "item_optional": false,
+        "item_default": 1
+      }
+    ],
+    "commands": [
+      {
+        "command_name": "good_command",
+        "command_description": "A good command",
+        "command_args": []
+      },
+      {
+        "command_name": "bad_command",
+        "command_description": "A bad command",
+        "command_args": []
+      },
+      {
+        "command_name": "command_with_arg",
+        "command_description": "A bad command",
+        "command_args": [ {
+          "item_name": "number",
+          "item_type": "integer",
+          "item_optional": true,
+          "item_default": 1
+        } ]
+      }
+    ]
+  }
+}
+