[5111] Added "command-processed" hook point to BaseCommandMgr

src/lib/config/base_command_mgr.cc
 - BaseCommandMgr::BaseCommandMgr() - registers command-processed hookpoint
 - BaseCommandMgr::processCommand() - invokes command-processed callouts

src/lib/config/tests/command_mgr_unittests.cc
  - Cleanup and renaming to avoid confusion
  - Added new tests to verify hookpoint callout
      TEST_F(CommandMgrTest, commandProcessedHook)
      TEST_F(CommandMgrTest, commandProcessedHookReplaceResponse)

src/lib/config/base_command_mgr.cc

@@ -7,14 +7,18 @@
 #include <cc/command_interpreter.h>
 #include <config/base_command_mgr.h>
 #include <config/config_log.h>
+#include <hooks/callout_handle.h>
+#include <hooks/hooks_manager.h>
 #include <boost/bind.hpp>
 
 using namespace isc::data;
+using namespace isc::hooks;
 
 namespace isc {
 namespace config {
 
 BaseCommandMgr::BaseCommandMgr() {
+    hook_index_command_processed_ = HooksManager::registerHook("command-processed");
     registerCommand("list-commands", boost::bind(&BaseCommandMgr::listCommandsHandler,
                                                  this, _1, _2));
 }
@@ -98,7 +102,29 @@ BaseCommandMgr::processCommand(const isc::data::ConstElementPtr& cmd) {
 
         LOG_INFO(command_logger, COMMAND_RECEIVED).arg(name);
 
-        return (handleCommand(name, arg, cmd));
+        ConstElementPtr response = handleCommand(name, arg, cmd);
+
+        // If there any callouts for command-processed hook point call them
+        if (HooksManager::calloutsPresent(hook_index_command_processed_)) {
+            // Commands are not associated with anything so there's no pre-existing
+            // callout.
+            CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle();
+
+            // Add the command name, arguments, and response to the callout context
+            callout_handle->setArgument("name", name);
+            callout_handle->setArgument("arguments", arg);
+            callout_handle->setArgument("response", response);
+
+            // Call callouts
+            HooksManager::callCallouts(hook_index_command_processed_,
+                                        *callout_handle);
+
+            // Refresh the response from the callout context in case it was modified.
+            // @todo Should we allow this?
+            callout_handle->getArgument("response", response);
+        }
+
+        return (response);
 
     } catch (const Exception& e) {
         LOG_WARN(command_logger, COMMAND_PROCESS_ERROR2).arg(e.what());

src/lib/config/base_command_mgr.h

@@ -97,6 +97,7 @@ public:
 
     /// @brief Constructor.
     ///
+    /// Registers hookpoint "command-processed"
     /// Registers "list-commands" command.
     BaseCommandMgr();
 
@@ -107,6 +108,8 @@ public:
     ///
     /// This method processes specified command. The command is specified using
     /// a single Element. See @ref BaseCommandMgr for description of its syntax.
+    /// After the command has been handled, callouts for the hook point,
+    /// "command-processed" will be invoked.
     ///
     /// @param cmd Pointer to the data element representing command in JSON
     /// format.
@@ -191,6 +194,9 @@ private:
     isc::data::ConstElementPtr
     listCommandsHandler(const std::string& name,
                         const isc::data::ConstElementPtr& params);
+
+    /// @brief Index of command-processed hook point
+    int hook_index_command_processed_;
 };
 
 } // end of namespace isc::config

src/lib/config/tests/command_mgr_unittests.cc

@@ -36,6 +36,9 @@ public:
         handler_name_ = "";
         handler_params_ = ElementPtr();
         handler_called_ = false;
+        callout_name_ = "";
+        callout_argument_names_.clear();
+        std::string processed_log_ = "";
 
         CommandMgr::instance().deregisterAll();
         CommandMgr::instance().closeCommandSocket();
@@ -90,23 +93,14 @@ public:
         return (createAnswer(123, "test error message"));
     }
 
-    /// @brief A simple command handler used from within hook library.
-    ///
-    /// @param name Command name.
-    /// @param params Command arguments.
-    static ConstElementPtr my_hook_handler(const std::string& /*name*/,
-                                           const ConstElementPtr& /*params*/) {
-        return (createAnswer(234, "text generated by hook handler"));
-    }
-
     /// @brief Test callback which stores callout name and passed arguments and
     /// which handles the command.
     ///
     /// @param callout_handle Handle passed by the hooks framework.
     /// @return Always 0.
     static int
-    control_command_receive_handle_callout(CalloutHandle& callout_handle) {
-        callout_name_ = "control_command_receive_handle";
+    hook_lib_callout(CalloutHandle& callout_handle) {
+        callout_name_ = "hook_lib_callout";
 
         ConstElementPtr command;
         callout_handle.getArgument("command", command);
@@ -124,6 +118,35 @@ public:
         return (0);
     }
 
+    /// @brief Test callback which stores callout name and passed arguments and
+    /// which handles the command.
+    ///
+    /// @param callout_handle Handle passed by the hooks framework.
+    /// @return Always 0.
+    static int
+    command_processed_callout(CalloutHandle& callout_handle) {
+        callout_name_ = "command_processed_handler";
+
+        std::string name;
+        callout_handle.getArgument("name", name);
+
+        ConstElementPtr arguments;
+        callout_handle.getArgument("arguments", arguments);
+
+        ConstElementPtr response;
+        callout_handle.getArgument("response", response);
+        std::ostringstream os;
+        os << name << ":" << arguments->str() << ":" << response->str();
+        processed_log_ = os.str();
+
+        if (name == "change-response") {
+            callout_handle.setArgument("response",
+                createAnswer(777, "replaced response text"));
+        }
+
+        return (0);
+    }
+
     /// @brief IO service used by these tests.
     IOServicePtr io_service_;
 
@@ -141,6 +164,9 @@ public:
 
     /// @brief Holds a list of arguments passed to the callout.
     static std::vector<std::string> callout_argument_names_;
+
+    /// @brief Holds the generated command process log
+    static std::string processed_log_;
 };
 
 /// Name passed to the handler (used in my_handler)
@@ -155,6 +181,9 @@ bool CommandMgrTest::handler_called_(false);
 /// Holds invoked callout name.
 std::string CommandMgrTest::callout_name_("");
 
+/// @brief Holds the generated command process log
+std::string CommandMgrTest::processed_log_;
+
 /// Holds a list of arguments passed to the callout.
 std::vector<std::string> CommandMgrTest::callout_argument_names_;
 
@@ -317,7 +346,7 @@ TEST_F(CommandMgrTest, delegateProcessCommand) {
     // Register callout so as we can check that it is called before
     // processing the command by the manager.
     HooksManager::preCalloutsLibraryHandle().registerCommandCallout(
-        "my-command", control_command_receive_handle_callout);
+        "my-command", hook_lib_callout);
 
     // Install local handler
     EXPECT_NO_THROW(CommandMgr::instance().registerCommand("my-command",
@@ -343,7 +372,7 @@ TEST_F(CommandMgrTest, delegateProcessCommand) {
     ASSERT_NO_THROW(answer_arg = parseAnswer(status_code, answer));
     EXPECT_EQ(234, status_code);
 
-    EXPECT_EQ("control_command_receive_handle", callout_name_);
+    EXPECT_EQ("hook_lib_callout", callout_name_);
 
     // Check that the appropriate arguments have been set. Include the
     // 'response' which should have been set by the callout.
@@ -358,7 +387,7 @@ TEST_F(CommandMgrTest, delegateListCommands) {
     // Register callout so as we can check that it is called before
     // processing the command by the manager.
     HooksManager::preCalloutsLibraryHandle().registerCommandCallout(
-        "my-command", control_command_receive_handle_callout);
+        "my-command", hook_lib_callout);
 
     // Create my-command-bis which is unique for the local Command Manager,
     // i.e. not supported by the hook library. This command should also
@@ -435,3 +464,79 @@ TEST_F(CommandMgrTest, unixCreateTooLong) {
     EXPECT_THROW(CommandMgr::instance().openCommandSocket(socket_info),
                  SocketError);
 }
+
+// This test verifies that a registered callout for the command-processed
+// hookpoint is invoked and passed the correct information.
+TEST_F(CommandMgrTest, commandProcessedHook) {
+    // Register callout so as we can check that it is called before
+    // processing the command by the manager.
+    HooksManager::preCalloutsLibraryHandle().registerCallout(
+        "command-processed", command_processed_callout);
+
+    // Install local handler
+    EXPECT_NO_THROW(CommandMgr::instance().registerCommand("my-command",
+                                                           my_handler));
+
+    // Now tell CommandMgr to process a command 'my-command' with the
+    // specified parameter.
+    ElementPtr my_params = Element::fromJSON("[ \"just\", \"some\", \"data\" ]");
+    ConstElementPtr command = createCommand("my-command", my_params);
+    ConstElementPtr answer;
+    ASSERT_NO_THROW(answer = CommandMgr::instance().processCommand(command));
+
+    // There should be an answer.
+    ASSERT_TRUE(answer);
+
+    // Local handler should be called
+    ASSERT_TRUE(handler_called_);
+
+    // Verify that the response came through intact
+    EXPECT_EQ("{ \"result\": 123, \"text\": \"test error message\" }",
+              answer->str());
+
+    // Make sure invoked the command-processed callout
+    EXPECT_EQ("command_processed_handler", callout_name_);
+
+    // Verify the callout could extract all the context arguments
+    EXPECT_EQ("my-command:[ \"just\", \"some\", \"data\" ]:"
+              "{ \"result\": 123, \"text\": \"test error message\" }",
+              processed_log_);
+}
+
+// This test verifies that a registered callout for the command-processed
+// hookpoint is invoked and can replace the command response content.
+TEST_F(CommandMgrTest, commandProcessedHookReplaceResponse) {
+    // Register callout so as we can check that it is called before
+    // processing the command by the manager.
+    HooksManager::preCalloutsLibraryHandle().registerCallout(
+        "command-processed", command_processed_callout);
+
+    // Install local handler
+    EXPECT_NO_THROW(CommandMgr::instance().registerCommand("my-command",
+                                                           my_handler));
+
+    // Now tell CommandMgr to process a command 'my-command' with the
+    // specified parameter.
+    ElementPtr my_params = Element::fromJSON("[ \"just\", \"some\", \"data\" ]");
+    ConstElementPtr command = createCommand("change-response", my_params);
+    ConstElementPtr answer;
+    ASSERT_NO_THROW(answer = CommandMgr::instance().processCommand(command));
+
+    // There should be an answer.
+    ASSERT_TRUE(answer);
+
+    // Local handler should not have been called, command isn't recognized
+    ASSERT_FALSE(handler_called_);
+
+    // Verify that we overrode response came
+    EXPECT_EQ("{ \"result\": 777, \"text\": \"replaced response text\" }",
+              answer->str());
+
+    // Make sure invoked the command-processed callout
+    EXPECT_EQ("command_processed_handler", callout_name_);
+
+    // Verify the callout could extract all the context arguments
+    EXPECT_EQ("change-response:[ \"just\", \"some\", \"data\" ]:"
+             "{ \"result\": 2, \"text\": \"'change-response' command not supported.\" }",
+              processed_log_);
+}