[5150a] Merged review improvements

diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
index 7134bc4b3da9ff2da04cbe453163f274fa9a27eb..f19aba2a83eed151ecbf6369d0da9bf8461ca590 100644 (file)
--- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc
+++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
@@ -6,6 +6,7 @@
 
 #include <config.h>
 #include <cc/data.h>
+#include <cc/command_interpreter.h>
 #include <dhcp4/ctrl_dhcp4_srv.h>
 #include <dhcp4/dhcp4_log.h>
 #include <dhcp4/dhcp4to6_ipc.h>
@@ -148,7 +149,7 @@ ControlledDhcpv4Srv::commandConfigWriteHandler(const string&,
 ConstElementPtr
 ControlledDhcpv4Srv::commandSetConfigHandler(const string&,
                                              ConstElementPtr args) {
-    const int status_code = 1; // 1 indicates an error
+    const int status_code = CONTROL_RESULT_ERROR; // 1 indicates an error
     ConstElementPtr dhcp4;
     string message;
 
@@ -209,7 +210,7 @@ ControlledDhcpv4Srv::commandSetConfigHandler(const string&,
 ConstElementPtr
 ControlledDhcpv4Srv::commandConfigTestHandler(const string&,
                                               ConstElementPtr args) {
-    const int status_code = 1; // 1 indicates an error
+    const int status_code = CONTROL_RESULT_ERROR; // 1 indicates an error
     ConstElementPtr dhcp4;
     string message;
 
@@ -266,7 +267,7 @@ ControlledDhcpv4Srv::commandBuildReportHandler(const string&,
 ConstElementPtr
 ControlledDhcpv4Srv::commandLeasesReclaimHandler(const string&,
                                                  ConstElementPtr args) {
-    int status_code = 1;
+    int status_code = CONTROL_RESULT_ERROR;
     string message;
 
     // args must be { "remove": <bool> }

diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h
index a245e02fe3f15169badcdedb074ea3c56a87bacd..8df6045d605503010ca664b8072bc5d0f56d7c69 100644 (file)
--- a/src/bin/dhcp4/ctrl_dhcp4_srv.h
+++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h
@@ -60,9 +60,10 @@ public:
     /// in them.
     ///
     /// Currently supported commands are:
+    /// - config-reload
+    /// - config-test
     /// - shutdown
     /// - libreload
-    /// - config-reload
     /// - leases-reclaim
     /// ...
     ///
@@ -94,7 +95,7 @@ public:
     ///
     /// This is a method for checking incoming configuration.
     ///
-    /// @param new_config textual representation of the new configuration
+    /// @param new_config JSON representation of the new configuration
     ///
     /// @return status of the config check
     isc::data::ConstElementPtr

diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc
index 496634182d8ca08d6f79816befc11977d7ea9888..3fc7e3274a3c0add1f7e6eebccaece3df1b0a0f0 100644 (file)
--- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc
@@ -696,7 +696,8 @@ TEST_F(CtrlChannelDhcpv4SrvTest, set_config) {
 
     // Should fail with a syntax error
     EXPECT_EQ("{ \"result\": 1, "
-              "\"text\": \"subnet configuration failed: mandatory 'subnet' parameter is missing for a subnet being configured (<string>:20:17)\" }",
+              "\"text\": \"subnet configuration failed: mandatory 'subnet' "
+              "parameter is missing for a subnet being configured (<string>:20:17)\" }",
               response);
 
     // Check that the config was not lost
@@ -716,13 +717,13 @@ TEST_F(CtrlChannelDhcpv4SrvTest, set_config) {
         << "}\n"                      // close dhcp4
         << "}}";
 
-    /* Verify the control channel socket exists */
+    // Verify the control channel socket exists.
     ASSERT_TRUE(fileExists(socket_path_));
 
-    // Send the set-config command
+    // Send the set-config command.
     sendUnixCommand(os.str(), response);
 
-    /* Verify the control channel socket no longer exists */
+    // Verify the control channel socket no longer exists.
     EXPECT_FALSE(fileExists(socket_path_));
 
     // With no command channel, should still receive the response.
@@ -893,7 +894,8 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configTest) {
 
     // Should fail with a syntax error
     EXPECT_EQ("{ \"result\": 1, "
-              "\"text\": \"subnet configuration failed: mandatory 'subnet' parameter is missing for a subnet being configured (<string>:20:17)\" }",
+              "\"text\": \"subnet configuration failed: mandatory 'subnet' "
+              "parameter is missing for a subnet being configured (<string>:20:17)\" }",
               response);
 
     // Check that the config was not lost
@@ -912,17 +914,19 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configTest) {
         << "}\n"                      // close dhcp4
         << "}}";
 
-    /* Verify the control channel socket exists */
+    // Verify the control channel socket exists.
     ASSERT_TRUE(fileExists(socket_path_));
 
     // Send the config-test command
     sendUnixCommand(os.str(), response);
 
-    /* Verify the control channel socket still exists */
+    // Verify the control channel socket still exists.
     EXPECT_TRUE(fileExists(socket_path_));
 
     // Verify the configuration was successful.
-    EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration seems sane. Control-socket, hook-libraries, and D2 configuration were sanity checked, but not applied.\" }",
+    EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration seems sane. "
+             "Control-socket, hook-libraries, and D2 configuration were "
+             "sanity checked, but not applied.\" }",
               response);
 
     // Check that the config was not applied

diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
index 8f5b1cffc8b6f4840b5ff2948cfce0771c961bc1..c035d3fdef94c98895214fca0a8ba268937f3535 100644 (file)
--- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc
+++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc
@@ -6,6 +6,7 @@
 
 #include <config.h>
 #include <cc/data.h>
+#include <cc/command_interpreter.h>
 #include <config/command_mgr.h>
 #include <dhcp/libdhcp++.h>
 #include <dhcpsrv/cfgmgr.h>
@@ -153,7 +154,7 @@ ControlledDhcpv6Srv::commandConfigWriteHandler(const string&, ConstElementPtr ar
 ConstElementPtr
 ControlledDhcpv6Srv::commandSetConfigHandler(const string&,
                                              ConstElementPtr args) {
-    const int status_code = 1; // 1 indicates an error
+    const int status_code = CONTROL_RESULT_ERROR;
     ConstElementPtr dhcp6;
     string message;
 
@@ -201,7 +202,7 @@ ControlledDhcpv6Srv::commandSetConfigHandler(const string&,
     // the logging first in case there's a configuration failure.
     int rcode = 0;
     isc::config::parseAnswer(rcode, result);
-    if (rcode == 0) {
+    if (rcode == CONTROL_RESULT_SUCCESS) {
         CfgMgr::instance().getStagingCfg()->applyLoggingCfg();
 
         // Use new configuration.
@@ -214,7 +215,7 @@ ControlledDhcpv6Srv::commandSetConfigHandler(const string&,
 ConstElementPtr
 ControlledDhcpv6Srv::commandConfigTestHandler(const string&,
                                               ConstElementPtr args) {
-    const int status_code = 1; // 1 indicates an error
+    const int status_code = CONTROL_RESULT_ERROR; // 1 indicates an error
     ConstElementPtr dhcp6;
     string message;
 
@@ -359,8 +360,9 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) {
     ControlledDhcpv6Srv* srv = ControlledDhcpv6Srv::getInstance();
 
     if (!srv) {
-        ConstElementPtr no_srv = isc::config::createAnswer(1,
-          "Server object not initialized, can't process config.");
+        ConstElementPtr no_srv = isc::config::createAnswer(
+            CONTROL_RESULT_ERROR,
+            "Server object not initialized, can't process config.");
         return (no_srv);
     }
 

diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h
index 6037048149e2ce25ebe434bc77128bb11834787e..881efc1e68cec973e08bce92b5425d9b49f3f2b1 100644 (file)
--- a/src/bin/dhcp6/ctrl_dhcp6_srv.h
+++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h
@@ -60,10 +60,11 @@ public:
     /// in them.
     ///
     /// Currently supported commands are:
-    /// - shutdown
-    /// - libreload
     /// - config-reload
+    /// - config-test
     /// - leases-reclaim
+    /// - libreload    
+    /// - shutdown
     /// ...
     ///
     /// @note It never throws.
@@ -94,7 +95,7 @@ public:
     ///
     /// This is a method for checking incoming configuration.
     ///
-    /// @param new_config textual representation of the new configuration
+    /// @param new_config JSON representation of the new configuration
     ///
     /// @return status of the config check
     isc::data::ConstElementPtr

diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
index eebfa4ac5f7d2145c5b1e195c40d241e8bdb2392..c20fb8028cea7bf5b481b3af65d38519165bd007 100644 (file)
--- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
@@ -430,7 +430,7 @@ TEST_F(CtrlDhcpv6SrvTest, configReload) {
 }
 
 // Check that the "set-config" command will replace current configuration
-TEST_F(CtrlChannelDhcpv6SrvTest, set_config) {
+TEST_F(CtrlChannelDhcpv6SrvTest, configSet) {
     createUnixChannelServer();
 
     // Define strings to permutate the config arguments
@@ -552,13 +552,13 @@ TEST_F(CtrlChannelDhcpv6SrvTest, set_config) {
         << "}\n"                      // close dhcp6
         << "}}";
 
-    /* Verify the control channel socket exists */
+    // Verify the control channel socket exists.
     ASSERT_TRUE(fileExists(socket_path_));
 
-    // Send the set-config command
+    // Send the set-config command.
     sendUnixCommand(os.str(), response);
 
-    /* Verify the control channel socket no longer exists */
+    // Verify the control channel socket no longer exists.
     EXPECT_FALSE(fileExists(socket_path_));
 
     // With no command channel, should still receive the response.
@@ -574,7 +574,7 @@ TEST_F(CtrlChannelDhcpv6SrvTest, set_config) {
 }
 
   // Verify that the "config-test" command will do what we expect.
-TEST_F(CtrlChannelDhcpv6SrvTest, config_test) {
+TEST_F(CtrlChannelDhcpv6SrvTest, configTest) {
     createUnixChannelServer();
 
     // Define strings to permutate the config arguments
@@ -677,7 +677,8 @@ TEST_F(CtrlChannelDhcpv6SrvTest, config_test) {
 
     // Should fail with a syntax error
     EXPECT_EQ("{ \"result\": 1, "
-              "\"text\": \"subnet configuration failed: mandatory 'subnet' parameter is missing for a subnet being configured (<string>:21:17)\" }",
+              "\"text\": \"subnet configuration failed: mandatory 'subnet' parameter "
+              "is missing for a subnet being configured (<string>:21:17)\" }",
               response);
 
     // Check that the config was not lost
@@ -696,17 +697,19 @@ TEST_F(CtrlChannelDhcpv6SrvTest, config_test) {
         << "}\n"                      // close dhcp6
         << "}}";
 
-    /* Verify the control channel socket exists */
+    // Verify the control channel socket exists.
     ASSERT_TRUE(fileExists(socket_path_));
 
-    // Send the config-test command
+    // Send the config-test command.
     sendUnixCommand(os.str(), response);
 
-    /* Verify the control channel socket still exists */
+    // Verify the control channel socket still exists.
     EXPECT_TRUE(fileExists(socket_path_));
 
     // Verify the configuration was successful.
-    EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration seems sane. Control-socket, hook-libraries, and D2 configuration were sanity checked, but not applied.\" }",
+    EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration seems sane. "
+             "Control-socket, hook-libraries, and D2 configuration were "
+             "sanity checked, but not applied.\" }",
               response);
 
     // Check that the config was not applied.
@@ -717,7 +720,6 @@ TEST_F(CtrlChannelDhcpv6SrvTest, config_test) {
     CfgMgr::instance().clear();
 }
 
-
 typedef std::map<std::string, isc::data::ConstElementPtr> ElementMap;
 
 // This test checks which commands are registered by the DHCPv4 server.