Browse Source

[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}
Tomek Mrugalski 8 years ago
parent
commit
a33995021b

+ 35 - 3
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<uint8_t>(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<uint8_t>(type)] = control_socket;
+}
+
+
 } // namespace isc::agent
 } // namespace isc

+ 25 - 17
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<uint8_t>(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<uint8_t>(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.

+ 2 - 2
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<uint16_t>(config, "http-port"));
+    ctx->setHttpHost(SimpleParser::getString(config, "http-host"));
+    ctx->setHttpPort(SimpleParser::getIntType<uint16_t>(config, "http-port"));
 
     // Control sockets are second.
     ConstElementPtr ctrl_sockets = config->get("control-sockets");

+ 60 - 6
src/bin/agent/tests/ctrl_agent_cfg_mgr_unittest.cc

@@ -8,13 +8,16 @@
 #include <agent/ctrl_agent_cfg_mgr.h>
 #include <agent/parser_context.h>
 #include <process/testutils/d_test_stubs.h>
+#include <process/d_cfg_mgr.h>
 #include <agent/tests/test_libraries.h>
+#include <hooks/libinfo.h>
 #include <boost/scoped_ptr.hpp>
 #include <gtest/gtest.h>
 
 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<CtrlAgentCfgContext>(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