From: Francis Dupont Date: Tue, 21 Mar 2017 18:50:07 +0000 (+0100) Subject: [5150a] Merged review improvements X-Git-Tag: trac5196_base~21^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5b8d7823cd0be44791974ccc956e79cb17de2459;p=thirdparty%2Fkea.git [5150a] Merged review improvements --- diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 7134bc4b3d..f19aba2a83 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -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": } diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index a245e02fe3..8df6045d60 100644 --- 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 496634182d..3fc7e3274a 100644 --- 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 (:20:17)\" }", + "\"text\": \"subnet configuration failed: mandatory 'subnet' " + "parameter is missing for a subnet being configured (: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 (:20:17)\" }", + "\"text\": \"subnet configuration failed: mandatory 'subnet' " + "parameter is missing for a subnet being configured (: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 8f5b1cffc8..c035d3fdef 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -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 6037048149..881efc1e68 100644 --- 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 eebfa4ac5f..c20fb8028c 100644 --- 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 (:21:17)\" }", + "\"text\": \"subnet configuration failed: mandatory 'subnet' parameter " + "is missing for a subnet being configured (: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 ElementMap; // This test checks which commands are registered by the DHCPv4 server.