Commit 2d85cd23 authored by Kazunori Fujiwara's avatar Kazunori Fujiwara
Browse files

[2138] update from jinmei's review

[2138] max number of columns (79)
[2138] recovered TEST_F(AuthConfigTest, exceptionGuarantee)
       to use server.setListenAddresses().
parent 3cd1c728
......@@ -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);
}
......@@ -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());
}
......
......@@ -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_);
}
......
......@@ -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.
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment