Merge branch 'trac473'

src/bin/auth/statistics.h

@@ -18,6 +18,7 @@
 #define __STATISTICS_H 1
 
 #include <cc/session.h>
+#include <stdint.h>
 
 class AuthCountersImpl;
 

src/lib/config/ccsession.cc

@@ -135,8 +135,6 @@ createCommand(const std::string& command, ConstElementPtr arg) {
     return (cmd);
 }
 
-/// Returns "" and empty ElementPtr() if this does not
-/// look like a command
 std::string
 parseCommand(ConstElementPtr& arg, ConstElementPtr command) {
     if (command &&
@@ -149,7 +147,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 {
@@ -257,7 +255,7 @@ ModuleCCSession::handleConfigUpdate(ConstElementPtr new_config) {
     ElementPtr errors = Element::createList();
     if (!config_handler_) {
         answer = createAnswer(1, module_name_ + " does not have a config handler");
-    } else if (!module_specification_.validate_config(new_config, false,
+    } else if (!module_specification_.validateConfig(new_config, false,
                                                       errors)) {
         std::stringstream ss;
         ss << "Error in config validation: ";
@@ -286,6 +284,51 @@ ModuleCCSession::hasQueuedMsgs() const {
     return (session_.hasQueuedMsgs());
 }
 
+ConstElementPtr
+ModuleCCSession::checkConfigUpdateCommand(const std::string& target_module,
+                                          ConstElementPtr arg)
+{
+    if (target_module == module_name_) {
+        return (handleConfigUpdate(arg));
+    } else {
+        // ok this update is not for us, if we have this module
+        // in our remote config list, update that
+        updateRemoteConfig(target_module, arg);
+        // we're not supposed to answer to this, so return
+        return (ElementPtr());
+    }
+}
+
+ConstElementPtr
+ModuleCCSession::checkModuleCommand(const std::string& cmd_str,
+                                    const std::string& target_module,
+                                    ConstElementPtr arg) const
+{
+    if (target_module == module_name_) {
+        if (command_handler_) {
+            ElementPtr errors = Element::createList();
+            if (module_specification_.validateCommand(cmd_str,
+                                                       arg,
+                                                       errors)) {
+                return (command_handler_(cmd_str, arg));
+            } else {
+                std::stringstream ss;
+                ss << "Error in command validation: ";
+                BOOST_FOREACH(ConstElementPtr error,
+                              errors->listValue()) {
+                    ss << error->stringValue();
+                }
+                return (createAnswer(3, ss.str()));
+            }
+        } else {
+            return (createAnswer(1,
+                                 "Command given but no "
+                                 "command handler for module"));
+        }
+    }
+    return (ElementPtr());
+}
+
 int
 ModuleCCSession::checkCommand() {
     ConstElementPtr cmd, routing, data;
@@ -302,23 +345,9 @@ ModuleCCSession::checkCommand() {
             std::string cmd_str = parseCommand(arg, data);
             std::string target_module = routing->get("group")->stringValue();
             if (cmd_str == "config_update") {
-                if (target_module == module_name_) {
-                    answer = handleConfigUpdate(arg);
-                } else {
-                    // ok this update is not for us, if we have this module
-                    // in our remote config list, update that
-                    updateRemoteConfig(target_module, arg);
-                    // we're not supposed to answer to this, so return
-                    return (0);
-                }
+                answer = checkConfigUpdateCommand(target_module, arg);
             } else {
-                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");
-                    }
-                }
+                answer = checkModuleCommand(cmd_str, target_module, arg);
             }
         } catch (const CCSessionError& re) {
             // TODO: Once we have logging and timeouts, we should not

src/lib/config/ccsession.h

@@ -91,11 +91,36 @@ isc::data::ConstElementPtr createCommand(const std::string& command,
 /// \brief Parses the given command into a string containing the actual
 ///        command and an ElementPtr containing the optional argument.
 ///
+/// Raises a CCSessionError if this is not a well-formed command
+///
+/// Example code: (command_message is a ConstElementPtr that is
+/// passed here)
+/// \code
+/// ElementPtr command_message = Element::fromJSON("{ \"command\": [\"foo\", { \"bar\": 123 } ] }");
+/// try {
+///     ConstElementPtr args;
+///     std::string command_str = parseCommand(args, command_message);
+///     if (command_str == "foo") {
+///         std::cout << "The command 'foo' was given" << std::endl;
+///         if (args->contains("bar")) {
+///             std::cout << "It had argument name 'bar' set, which has"
+///                       << "value " 
+///                       << args->get("bar")->intValue();
+///         }
+///     } else {
+///         std::cout << "Unknown command '" << command_str << std::endl;
+///     }
+/// } catch (CCSessionError cse) {
+///     std::cerr << "Bad command in CC Session: "
+///     << cse.what() << std::endl;
+/// }
+/// \endcode
+/// 
 /// \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
+/// \return The command name
 std::string parseCommand(isc::data::ConstElementPtr& arg,
                          isc::data::ConstElementPtr command);
 
@@ -244,6 +269,15 @@ private:
     isc::data::ConstElementPtr handleConfigUpdate(
         isc::data::ConstElementPtr new_config);
 
+    isc::data::ConstElementPtr checkConfigUpdateCommand(
+        const std::string& target_module,
+        isc::data::ConstElementPtr arg);
+
+    isc::data::ConstElementPtr checkModuleCommand(
+        const std::string& cmd_str,
+        const std::string& target_module,
+        isc::data::ConstElementPtr arg) const;
+
     isc::data::ConstElementPtr(*config_handler_)(
         isc::data::ConstElementPtr new_config);
     isc::data::ConstElementPtr(*command_handler_)(

src/lib/config/module_spec.cc

@@ -177,17 +177,47 @@ ModuleSpec::getModuleDescription() const {
 }
 
 bool
-ModuleSpec::validate_config(ConstElementPtr data, const bool full) const {
+ModuleSpec::validateConfig(ConstElementPtr data, const bool full) const {
     ConstElementPtr spec = module_specification->find("config_data");
-    return (validate_spec_list(spec, data, full, ElementPtr()));
+    return (validateSpecList(spec, data, full, ElementPtr()));
 }
 
 bool
-ModuleSpec::validate_config(ConstElementPtr data, const bool full,
+ModuleSpec::validateCommand(const std::string& command,
+                             ConstElementPtr args,
+                             ElementPtr errors) const
+{
+    if (args->getType() != Element::map) {
+        errors->add(Element::create("args for command " +
+                                    command + " is not a map"));
+        return (false);
+    }
+
+    ConstElementPtr commands_spec = module_specification->find("commands");
+    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 (validateSpecList(cur_command->get("command_args"),
+                                       args, true, errors));
+        }
+    }
+
+    // this command is unknown
+    errors->add(Element::create("Unknown command " + command));
+    return (false);
+}
+
+bool
+ModuleSpec::validateConfig(ConstElementPtr data, const bool full,
                             ElementPtr errors) const
 {
     ConstElementPtr spec = module_specification->find("config_data");
-    return (validate_spec_list(spec, data, full, errors));
+    return (validateSpecList(spec, data, full, errors));
 }
 
 ModuleSpec
@@ -264,7 +294,7 @@ check_type(ConstElementPtr spec, ConstElementPtr element) {
 }
 
 bool
-ModuleSpec::validate_item(ConstElementPtr spec, ConstElementPtr data,
+ModuleSpec::validateItem(ConstElementPtr spec, ConstElementPtr data,
                           const bool full, ElementPtr errors) const
 {
     if (!check_type(spec, data)) {
@@ -286,14 +316,14 @@ ModuleSpec::validate_item(ConstElementPtr spec, ConstElementPtr data,
                 return (false);
             }
             if (list_spec->get("item_type")->stringValue() == "map") {
-                if (!validate_item(list_spec, list_el, full, errors)) {
+                if (!validateItem(list_spec, list_el, full, errors)) {
                     return (false);
                 }
             }
         }
     }
     if (data->getType() == Element::map) {
-        if (!validate_spec_list(spec->get("map_item_spec"), data, full, errors)) {
+        if (!validateSpecList(spec->get("map_item_spec"), data, full, errors)) {
             return (false);
         }
     }
@@ -302,7 +332,7 @@ ModuleSpec::validate_item(ConstElementPtr spec, ConstElementPtr data,
 
 // spec is a map with item_name etc, data is a map
 bool
-ModuleSpec::validate_spec(ConstElementPtr spec, ConstElementPtr data,
+ModuleSpec::validateSpec(ConstElementPtr spec, ConstElementPtr data,
                           const bool full, ElementPtr errors) const
 {
     std::string item_name = spec->get("item_name")->stringValue();
@@ -311,7 +341,7 @@ ModuleSpec::validate_spec(ConstElementPtr spec, ConstElementPtr data,
     data_el = data->get(item_name);
     
     if (data_el) {
-        if (!validate_item(spec, data_el, full, errors)) {
+        if (!validateItem(spec, data_el, full, errors)) {
             return (false);
         }
     } else {
@@ -327,13 +357,13 @@ ModuleSpec::validate_spec(ConstElementPtr spec, ConstElementPtr data,
 
 // spec is a list of maps, data is a map
 bool
-ModuleSpec::validate_spec_list(ConstElementPtr spec, ConstElementPtr data,
+ModuleSpec::validateSpecList(ConstElementPtr spec, ConstElementPtr data,
                                const bool full, ElementPtr errors) const
 {
     bool validated = true;
     std::string cur_item_name;
     BOOST_FOREACH(ConstElementPtr cur_spec_el, spec->listValue()) {
-        if (!validate_spec(cur_spec_el, data, full, errors)) {
+        if (!validateSpec(cur_spec_el, data, full, errors)) {
             validated = false;
         }
     }

src/lib/config/module_spec.h

@@ -88,23 +88,66 @@ namespace isc { namespace config {
         /// \param data The base \c Element of the data to check
         /// \return true if the data conforms to the specification,
         /// false otherwise.
-        bool validate_config(isc::data::ConstElementPtr data,
+        bool validateConfig(isc::data::ConstElementPtr data,
                              const bool full = false) const;
 
+        /// Validates the arguments for the given command
+        ///
+        /// This checks the command and argument against the
+        /// specification in the module's .spec file.
+        ///
+        /// A command is considered valid if:
+        /// - it is known (the 'command' string must have an entry in
+        ///                the specification)
+        /// - the args is a MapElement
+        /// - args contains all mandatory arguments
+        /// - args does not contain unknown arguments
+        /// - all arguments in args match their specification
+        /// If all of these are true, this function returns \c true
+        /// If not, this method returns \c false
+        ///
+        /// Example usage:
+        /// \code
+        ///     ElementPtr errors = Element::createList();
+        ///     if (module_specification_.validateCommand(cmd_str,
+        ///                                               arg,
+        ///                                               errors)) {
+        ///         std::cout << "Command is valid" << std::endl;
+        ///     } else {
+        ///         std::cout << "Command is invalid: " << std::endl;
+        ///         BOOST_FOREACH(ConstElementPtr error,
+        ///                       errors->listValue()) {
+        ///             std::cout << error->stringValue() << std::endl;
+        ///         }
+        ///     }
+        /// \endcode
+        ///
+        /// \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 validateCommand(const std::string& command,
+                              isc::data::ConstElementPtr args,
+                              isc::data::ElementPtr errors) const;
+
+
         /// errors must be of type ListElement
-        bool validate_config(isc::data::ConstElementPtr data, const bool full,
+        bool validateConfig(isc::data::ConstElementPtr data, const bool full,
                              isc::data::ElementPtr errors) const;
 
     private:
-        bool validate_item(isc::data::ConstElementPtr spec,
-                           isc::data::ConstElementPtr data,
-                           const bool full,
-                           isc::data::ElementPtr errors) const;
-        bool validate_spec(isc::data::ConstElementPtr spec,
+        bool validateItem(isc::data::ConstElementPtr spec,
+                          isc::data::ConstElementPtr data,
+                          const bool full,
+                          isc::data::ElementPtr errors) const;
+        bool validateSpec(isc::data::ConstElementPtr spec,
                            isc::data::ConstElementPtr data,
                            const bool full,
                            isc::data::ElementPtr errors) const;
-        bool validate_spec_list(isc::data::ConstElementPtr spec,
+        bool validateSpecList(isc::data::ConstElementPtr spec,
                                 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

@@ -30,7 +30,7 @@ std::string specfile(const std::string name) {
 }
 
 void
-module_spec_error(const std::string& file,
+moduleSpecError(const std::string& file,
                const std::string& error1,
                const std::string& error2 = "",
                const std::string& error3 = "")
@@ -52,7 +52,7 @@ TEST(ModuleSpec, ReadingSpecfiles) {
                               ->stringValue(), "Spec1");
     dd = moduleSpecFromFile(specfile("spec2.spec"));
     EXPECT_EQ(dd.getFullSpec()->get("config_data")->size(), 6);
-    module_spec_error("doesnotexist",
+    moduleSpecError("doesnotexist",
                    "Error opening ",
                    specfile("doesnotexist"),
                    ": No such file or directory");
@@ -81,69 +81,65 @@ TEST(ModuleSpec, ReadingSpecfiles) {
 }
 
 TEST(ModuleSpec, SpecfileItems) {
-    module_spec_error("spec3.spec",
+    moduleSpecError("spec3.spec",
                    "item_name missing in { \"item_default\": 1, \"item_optional\": false, \"item_type\": \"integer\" }");
-    module_spec_error("spec4.spec",
+    moduleSpecError("spec4.spec",
                    "item_type missing in { \"item_default\": 1, \"item_name\": \"item1\", \"item_optional\": false }");
-    module_spec_error("spec5.spec",
+    moduleSpecError("spec5.spec",
                    "item_optional missing in { \"item_default\": 1, \"item_name\": \"item1\", \"item_type\": \"integer\" }");
-    module_spec_error("spec6.spec",
+    moduleSpecError("spec6.spec",
                    "item_default missing in { \"item_name\": \"item1\", \"item_optional\": false, \"item_type\": \"integer\" }");
-    module_spec_error("spec9.spec",
+    moduleSpecError("spec9.spec",
                    "item_default not of type integer");
-    module_spec_error("spec10.spec",
+    moduleSpecError("spec10.spec",
                    "item_default not of type real");
-    module_spec_error("spec11.spec",
+    moduleSpecError("spec11.spec",
                    "item_default not of type boolean");
-    module_spec_error("spec12.spec",
+    moduleSpecError("spec12.spec",
                    "item_default not of type string");
-    module_spec_error("spec13.spec",
+    moduleSpecError("spec13.spec",
                    "item_default not of type list");
-    module_spec_error("spec14.spec",
+    moduleSpecError("spec14.spec",
                    "item_default not of type map");
-    module_spec_error("spec15.spec",
+    moduleSpecError("spec15.spec",
                    "badname is not a valid type name");
 }
 
 TEST(ModuleSpec, SpecfileConfigData) {
-    module_spec_error("spec7.spec",
+    moduleSpecError("spec7.spec",
                    "module_name missing in {  }");
-    module_spec_error("spec8.spec",
+    moduleSpecError("spec8.spec",
                    "No module_spec in specification");
-    module_spec_error("spec16.spec",
+    moduleSpecError("spec16.spec",
                    "config_data is not a list of elements");
-    module_spec_error("spec21.spec",
+    moduleSpecError("spec21.spec",
                    "commands is not a list of elements");
 }
 
 TEST(ModuleSpec, SpecfileCommands) {
-    module_spec_error("spec17.spec",
+    moduleSpecError("spec17.spec",
                    "command_name missing in { \"command_args\": [ { \"item_default\": \"\", \"item_name\": \"message\", \"item_optional\": false, \"item_type\": \"string\" } ], \"command_description\": \"Print the given message to stdout\" }");
-    module_spec_error("spec18.spec",
+    moduleSpecError("spec18.spec",
                    "command_args missing in { \"command_description\": \"Print the given message to stdout\", \"command_name\": \"print_message\" }");
-    module_spec_error("spec19.spec",
+    moduleSpecError("spec19.spec",
                    "command_args not of type list");
-    module_spec_error("spec20.spec",
+    moduleSpecError("spec20.spec",
                    "somethingbad is not a valid type name");
-/*
-    module_spec_error("spec22.spec",
-                   "");
-*/
 }
 
 bool
-data_test(const ModuleSpec& dd, const std::string& data_file_name) {
+dataTest(const ModuleSpec& dd, const std::string& data_file_name) {
     std::ifstream data_file;
 
     data_file.open(specfile(data_file_name).c_str());
     ConstElementPtr data = Element::fromJSON(data_file, data_file_name);
     data_file.close();
 
-    return (dd.validate_config(data));
+    return (dd.validateConfig(data));
 }
 
 bool
-data_test_with_errors(const ModuleSpec& dd, const std::string& data_file_name,
+dataTestWithErrors(const ModuleSpec& dd, const std::string& data_file_name,
                       ElementPtr errors)
 {
     std::ifstream data_file;
@@ -152,27 +148,64 @@ data_test_with_errors(const ModuleSpec& dd, const std::string& data_file_name,
     ConstElementPtr data = Element::fromJSON(data_file, data_file_name);
     data_file.close();
 
-    return (dd.validate_config(data, true, errors));
+    return (dd.validateConfig(data, true, errors));
 }
 
 TEST(ModuleSpec, DataValidation) {
     ModuleSpec dd = moduleSpecFromFile(specfile("spec22.spec"));
 
-    EXPECT_TRUE(data_test(dd, "data22_1.data"));
-    EXPECT_FALSE(data_test(dd, "data22_2.data"));
-    EXPECT_FALSE(data_test(dd, "data22_3.data"));
-    EXPECT_FALSE(data_test(dd, "data22_4.data"));
-    EXPECT_FALSE(data_test(dd, "data22_5.data"));
-    EXPECT_TRUE(data_test(dd, "data22_6.data"));
-    EXPECT_TRUE(data_test(dd, "data22_7.data"));
-    EXPECT_FALSE(data_test(dd, "data22_8.data"));
-    EXPECT_FALSE(data_test(dd, "data22_9.data"));
+    EXPECT_TRUE(dataTest(dd, "data22_1.data"));
+    EXPECT_FALSE(dataTest(dd, "data22_2.data"));
+    EXPECT_FALSE(dataTest(dd, "data22_3.data"));
+    EXPECT_FALSE(dataTest(dd, "data22_4.data"));
+    EXPECT_FALSE(dataTest(dd, "data22_5.data"));
+    EXPECT_TRUE(dataTest(dd, "data22_6.data"));
+    EXPECT_TRUE(dataTest(dd, "data22_7.data"));
+    EXPECT_FALSE(dataTest(dd, "data22_8.data"));
+    EXPECT_FALSE(dataTest(dd, "data22_9.data"));
 
     ElementPtr errors = Element::createList();
-    EXPECT_FALSE(data_test_with_errors(dd, "data22_8.data", errors));
+    EXPECT_FALSE(dataTestWithErrors(dd, "data22_8.data", errors));
     EXPECT_EQ("[ \"Type mismatch\" ]", errors->str());
 
     errors = Element::createList();
-    EXPECT_FALSE(data_test_with_errors(dd, "data22_9.data", errors));
+    EXPECT_FALSE(dataTestWithErrors(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.validateCommand("shutdown", arg, errors));
+    EXPECT_EQ(errors->size(), 0);
+
+    errors = Element::createList();
+    EXPECT_FALSE(dd.validateCommand("unknowncommand", arg, errors));
+    EXPECT_EQ(errors->size(), 1);
+    EXPECT_EQ(errors->get(0)->stringValue(), "Unknown command unknowncommand");
+
+    errors = Element::createList();
+    EXPECT_FALSE(dd.validateCommand("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.validateCommand("print_message", arg, errors));
+    EXPECT_EQ(errors->size(), 0);
+
+    errors = Element::createList();
+    arg = Element::fromJSON("{ \"message\": \"Hello\", \"unknown_second_arg\": 1 }");
+    EXPECT_FALSE(dd.validateCommand("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.validateCommand("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 command with an (integer) argument",
+        "command_args": [ {
+          "item_name": "number",
+          "item_type": "integer",
+          "item_optional": true,
+          "item_default": 1
+        } ]
+      }
+    ]
+  }
+}
+