From: Thomas Markwalder Date: Wed, 30 Aug 2017 14:27:55 +0000 (-0400) Subject: [5111] Added "command-processed" hook point to BaseCommandMgr X-Git-Tag: trac5073a_base~1^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=349989ba16cb2abc1b6e59a284661aa0726135f0;p=thirdparty%2Fkea.git [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) --- diff --git a/src/lib/config/base_command_mgr.cc b/src/lib/config/base_command_mgr.cc index 88b6a290d0..9d5c46e547 100644 --- a/src/lib/config/base_command_mgr.cc +++ b/src/lib/config/base_command_mgr.cc @@ -7,14 +7,18 @@ #include #include #include +#include +#include #include 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()); diff --git a/src/lib/config/base_command_mgr.h b/src/lib/config/base_command_mgr.h index 19bbae3248..80ab7d696a 100644 --- a/src/lib/config/base_command_mgr.h +++ b/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 diff --git a/src/lib/config/tests/command_mgr_unittests.cc b/src/lib/config/tests/command_mgr_unittests.cc index 59682cd84f..fdc3341154 100644 --- a/src/lib/config/tests/command_mgr_unittests.cc +++ b/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 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 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_); +}