From: Tomek Mrugalski Date: Wed, 1 Mar 2017 23:58:50 +0000 (+0100) Subject: [5134_rebase] Changes after review: X-Git-Tag: trac5137_base~3^2~5 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a33995021bbbcf4715bf7090c0a345796fdef0c2;p=thirdparty%2Fkea.git [5134_rebase] Changes after review: - moved 2 functions from .h to .cc file - added explicit copy constructor - renamed {get,set}{Host,Port} to {get,set}Http{Host,Port} --- diff --git a/src/bin/agent/ctrl_agent_cfg_mgr.cc b/src/bin/agent/ctrl_agent_cfg_mgr.cc index 49039fd3ec..4550a8c7be 100644 --- a/src/bin/agent/ctrl_agent_cfg_mgr.cc +++ b/src/bin/agent/ctrl_agent_cfg_mgr.cc @@ -20,9 +20,21 @@ namespace agent { CtrlAgentCfgContext::CtrlAgentCfgContext() :http_host_(""), http_port_(0) { +} + +CtrlAgentCfgContext::CtrlAgentCfgContext(const CtrlAgentCfgContext& orig) + : DCfgContextBase(),http_host_(orig.http_host_), http_port_(orig.http_port_), + libraries_(orig.libraries_) { + // We're copying pointers here only. The underlying data will be shared by + // old and new context. That's how shared pointers work and I see no reason + // why it would be different in this particular here. + ctrl_sockets_[TYPE_D2] = orig.ctrl_sockets_[TYPE_D2]; + ctrl_sockets_[TYPE_DHCP4] = orig.ctrl_sockets_[TYPE_DHCP4]; + ctrl_sockets_[TYPE_DHCP6] = orig.ctrl_sockets_[TYPE_DHCP6]; } + CtrlAgentCfgMgr::CtrlAgentCfgMgr() : DCfgMgrBase(DCfgContextBasePtr(new CtrlAgentCfgContext())) { } @@ -37,8 +49,8 @@ CtrlAgentCfgMgr::getConfigSummary(const uint32_t /*selection*/) { // First print the http stuff. std::ostringstream s; - s << "listening on " << ctx->getHost() << ", port " << ctx->getPort() - << ", control sockets: "; + s << "listening on " << ctx->getHttpHost() << ", port " + << ctx->getHttpPort() << ", control sockets: "; // Then print the control-sockets bool socks = false; @@ -55,7 +67,9 @@ CtrlAgentCfgMgr::getConfigSummary(const uint32_t /*selection*/) { socks = true; } if (!socks) { - // That's weird + // That's uncommon, but correct scenario. CA can respond to some + // commands on its own. Further down the road we will possibly get the + // capability to tell CA to start other servers. s << "none"; } @@ -127,5 +141,23 @@ CtrlAgentCfgMgr::parse(isc::data::ConstElementPtr config_set, bool check_only) { return (answer); } +const data::ConstElementPtr +CtrlAgentCfgContext::getControlSocketInfo(ServerType type) const { + if (type > MAX_TYPE_SUPPORTED) { + isc_throw(BadValue, "Invalid server type"); + } + return (ctrl_sockets_[static_cast(type)]); +} + +void +CtrlAgentCfgContext::setControlSocketInfo(const isc::data::ConstElementPtr& control_socket, + ServerType type) { + if (type > MAX_TYPE_SUPPORTED) { + isc_throw(BadValue, "Invalid server type"); + } + ctrl_sockets_[static_cast(type)] = control_socket; +} + + } // namespace isc::agent } // namespace isc diff --git a/src/bin/agent/ctrl_agent_cfg_mgr.h b/src/bin/agent/ctrl_agent_cfg_mgr.h index 165b9ad697..8424478a65 100644 --- a/src/bin/agent/ctrl_agent_cfg_mgr.h +++ b/src/bin/agent/ctrl_agent_cfg_mgr.h @@ -51,32 +51,31 @@ public: /// @brief Returns information about control socket /// + /// This method returns Element tree structure that describes the control + /// socket (or null pointer if the socket is not defined for a particular + /// server type). This information is expected to be compatible with + /// data passed to @ref isc::config::CommandMgr::openCommandSocket. + /// /// @param type type of the server being controlled - /// @return pointer to the Element that holds control-socket map - const data::ConstElementPtr getControlSocketInfo(ServerType type) const { - if (type > MAX_TYPE_SUPPORTED) { - isc_throw(BadValue, "Invalid server type"); - } - return (ctrl_sockets_[static_cast(type)]); - } + /// @return pointer to the Element that holds control-socket map (or NULL) + const data::ConstElementPtr getControlSocketInfo(ServerType type) const; /// @brief Sets information about the control socket /// + /// This method stores Element tree structure that describes the control + /// socket. This information is expected to be compatible with + /// data passed to @ref isc::config::CommandMgr::openCommandSocket. + /// /// @param control_socket Element that holds control-socket map /// @param type type of the server being controlled void setControlSocketInfo(const isc::data::ConstElementPtr& control_socket, - ServerType type) { - if (type > MAX_TYPE_SUPPORTED) { - isc_throw(BadValue, "Invalid server type"); - } - ctrl_sockets_[static_cast(type)] = control_socket; - } + ServerType type); /// @brief Sets http-host parameter /// /// @param host Hostname or IP address where the agent's HTTP service /// will be available. - void setHost(const std::string& host) { + void setHttpHost(const std::string& host) { http_host_ = host; } @@ -84,19 +83,19 @@ public: /// /// @return Hostname or IP address where the agent's HTTP service is /// available. - std::string getHost() const { + std::string getHttpHost() const { return (http_host_); } /// @brief Sets http port /// /// @param port sets the TCP port the HTTP server will listen on - void setPort(const uint16_t port) { + void setHttpPort(const uint16_t port) { http_port_ = port; } /// @brief Returns the TCP post the HTTP server will listen on - uint16_t getPort() const { + uint16_t getHttpPort() const { return (http_port_); } @@ -115,6 +114,15 @@ public: private: + + /// @brief Private copy constructor + /// + /// It is private to forbid anyone outside of this class to make copies. + /// The only legal way to copy a context is to call @ref clone(). + /// + /// @param orig the original context to copy from + CtrlAgentCfgContext(const CtrlAgentCfgContext& orig); + /// @brief Private assignment operator to avoid potential for slicing. /// /// @param rhs Context to be assigned. diff --git a/src/bin/agent/simple_parser.cc b/src/bin/agent/simple_parser.cc index a1cc067028..9b9ec5bb75 100644 --- a/src/bin/agent/simple_parser.cc +++ b/src/bin/agent/simple_parser.cc @@ -82,8 +82,8 @@ AgentSimpleParser::parse(CtrlAgentCfgContextPtr ctx, isc::data::ConstElementPtr bool check_only) { // Let's get the HTTP parameters first. - ctx->setHost(SimpleParser::getString(config, "http-host")); - ctx->setPort(SimpleParser::getIntType(config, "http-port")); + ctx->setHttpHost(SimpleParser::getString(config, "http-host")); + ctx->setHttpPort(SimpleParser::getIntType(config, "http-port")); // Control sockets are second. ConstElementPtr ctrl_sockets = config->get("control-sockets"); diff --git a/src/bin/agent/tests/ctrl_agent_cfg_mgr_unittest.cc b/src/bin/agent/tests/ctrl_agent_cfg_mgr_unittest.cc index 7f64c65b10..f1d33807fd 100644 --- a/src/bin/agent/tests/ctrl_agent_cfg_mgr_unittest.cc +++ b/src/bin/agent/tests/ctrl_agent_cfg_mgr_unittest.cc @@ -8,13 +8,16 @@ #include #include #include +#include #include +#include #include #include using namespace isc::agent; using namespace isc::data; using namespace isc::hooks; +using namespace isc::process; namespace { @@ -54,11 +57,11 @@ TEST(CtrlAgentCfgMgr, contextHttpParams) { CtrlAgentCfgContext ctx; // Check http parameters - ctx.setPort(12345); - EXPECT_EQ(12345, ctx.getPort()); + ctx.setHttpPort(12345); + EXPECT_EQ(12345, ctx.getHttpPort()); - ctx.setHost("alnitak"); - EXPECT_EQ("alnitak", ctx.getHost()); + ctx.setHttpHost("alnitak"); + EXPECT_EQ("alnitak", ctx.getHttpHost()); } // Tests if context can store and retrieve control socket information. @@ -101,6 +104,57 @@ TEST(CtrlAgentCfgMgr, contextSocketInfo) { EXPECT_EQ(socket3, ctx.getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4)); } +// Tests if copied context retains all parameters. +TEST(CtrlAgentCfgMgr, contextSocketInfoCopy) { + + CtrlAgentCfgContext ctx; + + ConstElementPtr socket1 = Element::fromJSON("{ \"socket-type\": \"unix\",\n" + " \"socket-name\": \"socket1\" }"); + ConstElementPtr socket2 = Element::fromJSON("{ \"socket-type\": \"unix\",\n" + " \"socket-name\": \"socket2\" }"); + ConstElementPtr socket3 = Element::fromJSON("{ \"socket-type\": \"unix\",\n" + " \"socket-name\": \"socket3\" }"); + // Ok, now set the control sockets + EXPECT_NO_THROW(ctx.setControlSocketInfo(socket1, CtrlAgentCfgContext::TYPE_D2)); + EXPECT_NO_THROW(ctx.setControlSocketInfo(socket2, CtrlAgentCfgContext::TYPE_DHCP4)); + EXPECT_NO_THROW(ctx.setControlSocketInfo(socket3, CtrlAgentCfgContext::TYPE_DHCP6)); + + EXPECT_NO_THROW(ctx.setHttpPort(12345)); + EXPECT_NO_THROW(ctx.setHttpHost("bellatrix")); + + HookLibsCollection libs; + string exp_name("testlib1.so"); + ConstElementPtr exp_param(new StringElement("myparam")); + libs.push_back(make_pair(exp_name, exp_param)); + ctx.setLibraries(libs); + + // Make a copy. + DCfgContextBasePtr copy_base(ctx.clone()); + CtrlAgentCfgContextPtr copy = boost::dynamic_pointer_cast(copy_base); + ASSERT_TRUE(copy); + + // Now check the values returned + EXPECT_EQ(12345, copy->getHttpPort()); + EXPECT_EQ("bellatrix", copy->getHttpHost()); + + // Check socket info + ASSERT_TRUE(copy->getControlSocketInfo(CtrlAgentCfgContext::TYPE_D2)); + ASSERT_TRUE(copy->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4)); + ASSERT_TRUE(copy->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6)); + EXPECT_EQ(socket1->str(), copy->getControlSocketInfo(CtrlAgentCfgContext::TYPE_D2)->str()); + EXPECT_EQ(socket2->str(), copy->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP4)->str()); + EXPECT_EQ(socket3->str(), copy->getControlSocketInfo(CtrlAgentCfgContext::TYPE_DHCP6)->str()); + + // Check hook libs + HookLibsCollection libs2 = copy->getLibraries(); + ASSERT_EQ(1, libs2.size()); + EXPECT_EQ(exp_name, libs2[0].first); + ASSERT_TRUE(libs2[0].second); + EXPECT_EQ(exp_param->str(), libs2[0].second->str()); +} + + // Tests if the context can store and retrieve hook libs information. TEST(CtrlAgentCfgMgr, contextHookParams) { CtrlAgentCfgContext ctx; @@ -228,8 +282,8 @@ TEST_F(AgentParserTest, configParseHttpOnly) { CtrlAgentCfgContextPtr ctx = cfg_mgr_.getCtrlAgentCfgContext(); ASSERT_TRUE(ctx); - EXPECT_EQ("betelguese", ctx->getHost()); - EXPECT_EQ(8001, ctx->getPort()); + EXPECT_EQ("betelguese", ctx->getHttpHost()); + EXPECT_EQ(8001, ctx->getHttpPort()); } // Tests if a single socket can be configured. BTW this test also checks