[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 bc9027d679e489acd277a946639e0de49d9032f3..d3a44a310798d642673ca0e37955dee03a914d87 100644 (file)
--- 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<AuthCommand>(createAuthCommand(command_id))->exec(server,
-                                                                             args);
+        return (scoped_ptr<AuthCommand>(
+                    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 aaa4b4fd0420d9e63e765d44b92d84b0f8c21521..20efea41461054b752d81fa46ac37fbf528abd10 100644 (file)
--- 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<isc::data::ConstElementPtr>(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 82fdc6d071376ca9dcfba67d09c35a4bd74a29dc..ffc1d803a4fd304c6e2e8bded1fb43d6bec744a6 100644 (file)
--- 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 7e923856c3d40e65fc5306200e8ee0c8e62890c0..a44f04f42791d1338f3306155bf9e484d06e1c26 100644 (file)
--- 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.