From: Kazunori Fujiwara Date: Tue, 21 Aug 2012 08:49:44 +0000 (+0900) Subject: [2138] update from jinmei's review X-Git-Tag: trac2351_base~129^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=2d85cd23b8a9e37bf365471016843110337c757d;p=thirdparty%2Fkea.git [2138] update from jinmei's review [2138] max number of columns (79) [2138] recovered TEST_F(AuthConfigTest, exceptionGuarantee) to use server.setListenAddresses(). --- diff --git a/src/bin/auth/command.cc b/src/bin/auth/command.cc index bc9027d679..d3a44a3107 100644 --- a/src/bin/auth/command.cc +++ b/src/bin/auth/command.cc @@ -100,14 +100,16 @@ public: /// /// \param server The \c AuthSrv object on which the command is executed. /// \param args Command specific argument. - /// \return command result data in JSON format. - virtual ConstElementPtr exec(AuthSrv& server, isc::data::ConstElementPtr args) = 0; + /// \return command result using createAnswer(). + virtual ConstElementPtr exec(AuthSrv& server, + isc::data::ConstElementPtr args) = 0; }; // Handle the "shutdown" command. class ShutdownCommand : public AuthCommand { public: - virtual ConstElementPtr exec(AuthSrv& server, isc::data::ConstElementPtr args) { + virtual ConstElementPtr exec(AuthSrv& server, + isc::data::ConstElementPtr args) { // Is the pid argument provided? if (args && args->contains("pid")) { // If it is, we check it is the same as our PID @@ -135,7 +137,8 @@ public: // Handle the "getstats" command. The argument is a list. class GetStatsCommand : public AuthCommand { public: - virtual ConstElementPtr exec(AuthSrv& server, isc::data::ConstElementPtr) { + virtual ConstElementPtr exec(AuthSrv& server, + isc::data::ConstElementPtr) { LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_RECEIVED_GETSTATS); return (createAnswer(0, server.getStatistics())); } @@ -143,7 +146,8 @@ public: class StartDDNSForwarderCommand : public AuthCommand { public: - virtual ConstElementPtr exec(AuthSrv& server, isc::data::ConstElementPtr) { + virtual ConstElementPtr exec(AuthSrv& server, + isc::data::ConstElementPtr) { server.createDDNSForwarder(); return (createAnswer()); } @@ -151,7 +155,8 @@ public: class StopDDNSForwarderCommand : public AuthCommand { public: - virtual ConstElementPtr exec(AuthSrv& server, isc::data::ConstElementPtr) { + virtual ConstElementPtr exec(AuthSrv& server, + isc::data::ConstElementPtr) { server.destroyDDNSForwarder(); return (createAnswer()); } @@ -160,7 +165,8 @@ public: // Handle the "loadzone" command. class LoadZoneCommand : public AuthCommand { public: - virtual ConstElementPtr exec(AuthSrv& server, isc::data::ConstElementPtr args) { + virtual ConstElementPtr exec(AuthSrv& server, + isc::data::ConstElementPtr args) { if (args == NULL) { isc_throw(AuthCommandError, "Null argument"); } @@ -240,17 +246,13 @@ ConstElementPtr execAuthServerCommand(AuthSrv& server, const string& command_id, ConstElementPtr args) { - ConstElementPtr value; - LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_RECEIVED_COMMAND).arg(command_id); try { - value = scoped_ptr(createAuthCommand(command_id))->exec(server, - args); + return (scoped_ptr( + createAuthCommand(command_id))->exec(server, args)); } catch (const isc::Exception& ex) { LOG_ERROR(auth_logger, AUTH_COMMAND_FAILED).arg(command_id) .arg(ex.what()); return (createAnswer(1, ex.what())); } - - return (value); } diff --git a/src/bin/auth/statistics.cc b/src/bin/auth/statistics.cc index aaa4b4fd04..20efea4146 100644 --- a/src/bin/auth/statistics.cc +++ b/src/bin/auth/statistics.cc @@ -105,12 +105,10 @@ AuthCountersImpl::inc(const std::string& zone, isc::data::ElementPtr AuthCountersImpl::getStatistics() const { std::stringstream statistics_string; - statistics_string - << "{ \"queries.udp\": " - << server_counter_.get(AuthCounters::SERVER_UDP_QUERY) - << ", \"queries.tcp\": " - << server_counter_.get( - AuthCounters::SERVER_TCP_QUERY); + statistics_string << "{ \"queries.udp\": " + << server_counter_.get(AuthCounters::SERVER_UDP_QUERY) + << ", \"queries.tcp\": " + << server_counter_.get(AuthCounters::SERVER_TCP_QUERY); // Insert non 0 Opcode counters. for (int i = 0; i < NUM_OPCODES; ++i) { const Counter::Type counter = opcode_counter_.get(i); @@ -143,7 +141,9 @@ AuthCountersImpl::getStatistics() const { isc::data::Element::fromJSON(statistics_string); // validate the statistics data before send if (validator_) { - if (!validator_(statistics_element)) { + if (!validator_( + static_cast(statistics_element))) + { LOG_ERROR(auth_logger, AUTH_INVALID_STATISTICS_DATA); return (isc::data::ElementPtr()); } diff --git a/src/bin/auth/tests/command_unittest.cc b/src/bin/auth/tests/command_unittest.cc index 82fdc6d071..ffc1d803a4 100644 --- a/src/bin/auth/tests/command_unittest.cc +++ b/src/bin/auth/tests/command_unittest.cc @@ -419,7 +419,7 @@ TEST_F(AuthCommandTest, getStats) { result_ = execAuthServerCommand(server_, "getstats", ConstElementPtr()); parseAnswer(rcode_, result_); - // Just check some message has been sent. Detailed tests specific to + // Just check some message has been received. Detailed tests specific to // statistics are done in its own tests. EXPECT_EQ(0, rcode_); } diff --git a/src/bin/auth/tests/config_unittest.cc b/src/bin/auth/tests/config_unittest.cc index 7e923856c3..a44f04f427 100644 --- a/src/bin/auth/tests/config_unittest.cc +++ b/src/bin/auth/tests/config_unittest.cc @@ -79,6 +79,29 @@ TEST_F(AuthConfigTest, versionConfig) { Element::fromJSON("{\"version\": 0}"))); } +TEST_F(AuthConfigTest, exceptionGuarantee) { + using namespace isc::server_common::portconfig; + AddressList a, b, c; + a.push_back(AddressPair("127.0.0.1", 53210)); + server.setListenAddresses(a); + b = server.getListenAddresses(); + EXPECT_EQ(a.size(), b.size()); + EXPECT_EQ(a.at(0).first, b.at(0).first); + EXPECT_EQ(a.at(0).second, b.at(0).second); + // This configuration contains an invalid item, which will trigger + // an exception. + EXPECT_THROW(configureAuthServer( + server, + Element::fromJSON( + "{ \"no_such_config_var\": 1}")), + AuthConfigError); + // The server state shouldn't change + c = server.getListenAddresses(); + EXPECT_EQ(a.size(), c.size()); + EXPECT_EQ(a.at(0).first, c.at(0).first); + EXPECT_EQ(a.at(0).second, c.at(0).second); +} + TEST_F(AuthConfigTest, badConfig) { // These should normally not happen, but should be handled to avoid // an unexpected crash due to a bug of the caller.