BIND 10 trac2861, updated. 6917e1335e35a4465b9e242a6c3e9b38c909d201 [2861] Use structure for the command
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Jul 2 08:15:05 UTC 2013
The branch, trac2861 has been updated
via 6917e1335e35a4465b9e242a6c3e9b38c909d201 (commit)
from 2e7cd3c5a4935c2957d800c9e9f29e191e9bcece (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 6917e1335e35a4465b9e242a6c3e9b38c909d201
Author: Michal 'vorner' Vaner <vorner at vorner.cz>
Date: Tue Jul 2 10:13:06 2013 +0200
[2861] Use structure for the command
Replace std::pair with our own structure and add the callback there too.
Update code to match the change. No functional change.
-----------------------------------------------------------------------
Summary of changes:
src/bin/auth/datasrc_clients_mgr.h | 31 +++++----
.../auth/tests/datasrc_clients_builder_unittest.cc | 69 ++++++++++++--------
src/bin/auth/tests/datasrc_clients_mgr_unittest.cc | 12 ++--
src/bin/auth/tests/test_datasrc_clients_mgr.cc | 4 +-
4 files changed, 71 insertions(+), 45 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/auth/datasrc_clients_mgr.h b/src/bin/auth/datasrc_clients_mgr.h
index 8c8c4d3..14dbbd1 100644
--- a/src/bin/auth/datasrc_clients_mgr.h
+++ b/src/bin/auth/datasrc_clients_mgr.h
@@ -82,12 +82,22 @@ enum CommandID {
typedef boost::function<void ()> FinishedCallback;
/// \brief The data type passed from DataSrcClientsMgr to
-/// DataSrcClientsBuilder.
+/// DataSrcClientsBuilder.
///
-/// The first element of the pair is the command ID, and the second element
-/// is its argument. If the command doesn't take an argument it should be
-/// a null pointer.
-typedef std::pair<CommandID, data::ConstElementPtr> Command;
+/// This just holds the data items together, no logic or protection
+/// is present here.
+struct Command {
+ Command(CommandID id, const data::ConstElementPtr& params,
+ const FinishedCallback& callback) :
+ id(id),
+ params(params),
+ callback(callback)
+ {}
+ CommandID id;
+ data::ConstElementPtr params;
+ FinishedCallback callback;
+};
+
} // namespace datasrc_clientmgr_internal
/// \brief Frontend to the manager object for data source clients.
@@ -336,13 +346,12 @@ private:
const datasrc_clientmgr_internal::FinishedCallback&
callback = datasrc_clientmgr_internal::FinishedCallback())
{
- (void) callback; // Temporary, remove!
// The lock will be held until the end of this method. Only
// push_back has to be protected, but we can avoid having an extra
// block this way.
typename MutexType::Locker locker(queue_mutex_);
command_queue_.push_back(
- datasrc_clientmgr_internal::Command(command, arg));
+ datasrc_clientmgr_internal::Command(command, arg, callback));
cond_.signal();
}
@@ -533,7 +542,7 @@ bool
DataSrcClientsBuilderBase<MutexType, CondVarType>::handleCommand(
const Command& command)
{
- const CommandID cid = command.first;
+ const CommandID cid = command.id;
if (cid >= NUM_COMMANDS) {
// This shouldn't happen except for a bug within this file.
isc_throw(Unexpected, "internal bug: invalid command, ID: " << cid);
@@ -544,12 +553,12 @@ DataSrcClientsBuilderBase<MutexType, CondVarType>::handleCommand(
};
LOG_DEBUG(auth_logger, DBGLVL_TRACE_BASIC,
AUTH_DATASRC_CLIENTS_BUILDER_COMMAND).arg(command_desc.at(cid));
- switch (command.first) {
+ switch (command.id) {
case RECONFIGURE:
- doReconfigure(command.second);
+ doReconfigure(command.params);
break;
case LOADZONE:
- doLoadZone(command.second);
+ doLoadZone(command.params);
break;
case SHUTDOWN:
return (false);
diff --git a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
index 8aa223b..bc41385 100644
--- a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
+++ b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
@@ -56,8 +56,8 @@ protected:
boost::shared_ptr<ConfigurableClientList> >),
builder(&command_queue, &cond, &queue_mutex, &clients_map, &map_mutex),
cond(command_queue, delayed_command_queue), rrclass(RRClass::IN()),
- shutdown_cmd(SHUTDOWN, ConstElementPtr()),
- noop_cmd(NOOP, ConstElementPtr())
+ shutdown_cmd(SHUTDOWN, ConstElementPtr(), FinishedCallback()),
+ noop_cmd(NOOP, ConstElementPtr(), FinishedCallback())
{}
void configureZones(); // used for loadzone related tests
@@ -138,7 +138,7 @@ TEST_F(DataSrcClientsBuilderTest, reconfigure) {
// the error handling
// A command structure we'll modify to send different commands
- Command reconfig_cmd(RECONFIGURE, ConstElementPtr());
+ Command reconfig_cmd(RECONFIGURE, ConstElementPtr(), FinishedCallback());
// Initially, no clients should be there
EXPECT_TRUE(clients_map->empty());
@@ -166,7 +166,7 @@ TEST_F(DataSrcClientsBuilderTest, reconfigure) {
"}"
);
- reconfig_cmd.second = good_config;
+ reconfig_cmd.params = good_config;
EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
EXPECT_EQ(1, clients_map->size());
EXPECT_EQ(1, map_mutex.lock_count);
@@ -177,7 +177,7 @@ TEST_F(DataSrcClientsBuilderTest, reconfigure) {
// If a 'bad' command argument got here, the config validation should
// have failed already, but still, the handler should return true,
// and the clients_map should not be updated.
- reconfig_cmd.second = Element::create("{ \"foo\": \"bar\" }");
+ reconfig_cmd.params = Element::create("{ \"foo\": \"bar\" }");
EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
EXPECT_EQ(working_config_clients, clients_map);
// Building failed, so map mutex should not have been locked again
@@ -185,7 +185,7 @@ TEST_F(DataSrcClientsBuilderTest, reconfigure) {
// The same for a configuration that has bad data for the type it
// specifies
- reconfig_cmd.second = bad_config;
+ reconfig_cmd.params = bad_config;
builder.handleCommand(reconfig_cmd);
EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
EXPECT_EQ(working_config_clients, clients_map);
@@ -194,21 +194,21 @@ TEST_F(DataSrcClientsBuilderTest, reconfigure) {
// The same goes for an empty parameter (it should at least be
// an empty map)
- reconfig_cmd.second = ConstElementPtr();
+ reconfig_cmd.params = ConstElementPtr();
EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
EXPECT_EQ(working_config_clients, clients_map);
EXPECT_EQ(1, map_mutex.lock_count);
// Reconfigure again with the same good clients, the result should
// be a different map than the original, but not an empty one.
- reconfig_cmd.second = good_config;
+ reconfig_cmd.params = good_config;
EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
EXPECT_NE(working_config_clients, clients_map);
EXPECT_EQ(1, clients_map->size());
EXPECT_EQ(2, map_mutex.lock_count);
// And finally, try an empty config to disable all datasource clients
- reconfig_cmd.second = Element::createMap();
+ reconfig_cmd.params = Element::createMap();
EXPECT_TRUE(builder.handleCommand(reconfig_cmd));
EXPECT_EQ(0, clients_map->size());
EXPECT_EQ(3, map_mutex.lock_count);
@@ -224,7 +224,8 @@ TEST_F(DataSrcClientsBuilderTest, shutdown) {
TEST_F(DataSrcClientsBuilderTest, badCommand) {
// out-of-range command ID
EXPECT_THROW(builder.handleCommand(Command(NUM_COMMANDS,
- ConstElementPtr())),
+ ConstElementPtr(),
+ FinishedCallback())),
isc::Unexpected);
}
@@ -308,7 +309,8 @@ TEST_F(DataSrcClientsBuilderTest, loadZone) {
const Command loadzone_cmd(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"test1.example\"}"));
+ " \"origin\": \"test1.example\"}"),
+ FinishedCallback());
EXPECT_TRUE(builder.handleCommand(loadzone_cmd));
// loadZone involves two critical sections: one for getting the zone
@@ -369,7 +371,8 @@ TEST_F(DataSrcClientsBuilderTest,
// Now send the command to reload it
const Command loadzone_cmd(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"example.org\"}"));
+ " \"origin\": \"example.org\"}"),
+ FinishedCallback());
EXPECT_TRUE(builder.handleCommand(loadzone_cmd));
// And now it should be present too.
EXPECT_EQ(ZoneFinder::SUCCESS,
@@ -380,7 +383,8 @@ TEST_F(DataSrcClientsBuilderTest,
// An error case: the zone has no configuration. (note .com here)
const Command nozone_cmd(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"example.com\"}"));
+ " \"origin\": \"example.com\"}"),
+ FinishedCallback());
EXPECT_THROW(builder.handleCommand(nozone_cmd),
TestDataSrcClientsBuilder::InternalCommandError);
// The previous zone is not hurt in any way
@@ -403,7 +407,8 @@ TEST_F(DataSrcClientsBuilderTest,
builder.handleCommand(
Command(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"example.org\"}")));
+ " \"origin\": \"example.org\"}"),
+ FinishedCallback()));
// Only one mutex was needed because there was no actual reload.
EXPECT_EQ(orig_lock_count + 1, map_mutex.lock_count);
EXPECT_EQ(orig_unlock_count + 1, map_mutex.unlock_count);
@@ -421,7 +426,8 @@ TEST_F(DataSrcClientsBuilderTest,
builder.handleCommand(
Command(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"nosuchzone.example\"}"))),
+ " \"origin\": \"nosuchzone.example\"}"),
+ FinishedCallback())),
TestDataSrcClientsBuilder::InternalCommandError);
// basically impossible case: in-memory cache is completely disabled.
@@ -441,7 +447,8 @@ TEST_F(DataSrcClientsBuilderTest,
EXPECT_THROW(builder.handleCommand(
Command(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"example.org\"}"))),
+ " \"origin\": \"example.org\"}"),
+ FinishedCallback())),
TestDataSrcClientsBuilder::InternalCommandError);
}
@@ -454,7 +461,8 @@ TEST_F(DataSrcClientsBuilderTest, loadBrokenZone) {
// there's an error in the new zone file. reload will be rejected.
const Command loadzone_cmd(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"test1.example\"}"));
+ " \"origin\": \"test1.example\"}"),
+ FinishedCallback());
EXPECT_THROW(builder.handleCommand(loadzone_cmd),
TestDataSrcClientsBuilder::InternalCommandError);
zoneChecks(clients_map, rrclass); // zone shouldn't be replaced
@@ -469,7 +477,8 @@ TEST_F(DataSrcClientsBuilderTest, loadUnreadableZone) {
TEST_DATA_BUILDDIR "/test1.zone.copied"));
const Command loadzone_cmd(LOADZONE, Element::fromJSON(
"{\"class\": \"IN\","
- " \"origin\": \"test1.example\"}"));
+ " \"origin\": \"test1.example\"}"),
+ FinishedCallback());
EXPECT_THROW(builder.handleCommand(loadzone_cmd),
TestDataSrcClientsBuilder::InternalCommandError);
zoneChecks(clients_map, rrclass); // zone shouldn't be replaced
@@ -482,7 +491,8 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneWithoutDataSrc) {
Command(LOADZONE,
Element::fromJSON(
"{\"class\": \"IN\", "
- " \"origin\": \"test1.example\"}"))),
+ " \"origin\": \"test1.example\"}"),
+ FinishedCallback())),
TestDataSrcClientsBuilder::InternalCommandError);
}
@@ -492,7 +502,8 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneInvalidParams) {
if (!isc::util::unittests::runningOnValgrind()) {
// null arg: this causes assertion failure
EXPECT_DEATH_IF_SUPPORTED({
- builder.handleCommand(Command(LOADZONE, ElementPtr()));
+ builder.handleCommand(Command(LOADZONE, ElementPtr(),
+ FinishedCallback()));
}, "");
}
@@ -501,7 +512,8 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneInvalidParams) {
Command(LOADZONE,
Element::fromJSON(
"{\"origin\": \"test1.example\","
- " \"class\": \"no_such_class\"}"))),
+ " \"class\": \"no_such_class\"}"),
+ FinishedCallback())),
InvalidRRClass);
// not a string
@@ -509,7 +521,8 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneInvalidParams) {
Command(LOADZONE,
Element::fromJSON(
"{\"origin\": \"test1.example\","
- " \"class\": 1}"))),
+ " \"class\": 1}"),
+ FinishedCallback())),
isc::data::TypeError);
// class or origin is missing: result in assertion failure
@@ -517,7 +530,8 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneInvalidParams) {
EXPECT_DEATH_IF_SUPPORTED({
builder.handleCommand(Command(LOADZONE,
Element::fromJSON(
- "{\"class\": \"IN\"}")));
+ "{\"class\": \"IN\"}"),
+ FinishedCallback()));
}, "");
}
@@ -525,12 +539,14 @@ TEST_F(DataSrcClientsBuilderTest, loadZoneInvalidParams) {
EXPECT_THROW(builder.handleCommand(
Command(LOADZONE,
Element::fromJSON(
- "{\"class\": \"IN\", \"origin\": \"...\"}"))),
+ "{\"class\": \"IN\", \"origin\": \"...\"}"),
+ FinishedCallback())),
EmptyLabel);
EXPECT_THROW(builder.handleCommand(
Command(LOADZONE,
Element::fromJSON(
- "{\"origin\": 10, \"class\": 1}"))),
+ "{\"origin\": 10, \"class\": 1}"),
+ FinishedCallback())),
isc::data::TypeError);
}
@@ -561,7 +577,8 @@ TEST_F(DataSrcClientsBuilderTest,
Command(LOADZONE,
Element::fromJSON(
"{\"origin\": \"test1.example\","
- " \"class\": \"IN\"}"))),
+ " \"class\": \"IN\"}"),
+ FinishedCallback())),
TestDataSrcClientsBuilder::InternalCommandError);
}
diff --git a/src/bin/auth/tests/datasrc_clients_mgr_unittest.cc b/src/bin/auth/tests/datasrc_clients_mgr_unittest.cc
index c37ef11..00a47a1 100644
--- a/src/bin/auth/tests/datasrc_clients_mgr_unittest.cc
+++ b/src/bin/auth/tests/datasrc_clients_mgr_unittest.cc
@@ -43,8 +43,8 @@ shutdownCheck() {
EXPECT_EQ(1, FakeDataSrcClientsBuilder::cond->signal_count);
EXPECT_EQ(1, FakeDataSrcClientsBuilder::command_queue->size());
const Command& cmd = FakeDataSrcClientsBuilder::command_queue->front();
- EXPECT_EQ(SHUTDOWN, cmd.first);
- EXPECT_FALSE(cmd.second); // no argument
+ EXPECT_EQ(SHUTDOWN, cmd.id);
+ EXPECT_FALSE(cmd.params); // no argument
// Finally, the manager should wait for the thread to terminate.
EXPECT_TRUE(FakeDataSrcClientsBuilder::thread_waited);
@@ -130,8 +130,8 @@ TEST(DataSrcClientsMgrTest, reconfigure) {
// touch or refer to the map, so it shouldn't acquire the map lock.
checkSharedMembers(1, 1, 0, 0, 1, 1);
const Command& cmd1 = FakeDataSrcClientsBuilder::command_queue->front();
- EXPECT_EQ(RECONFIGURE, cmd1.first);
- EXPECT_EQ(reconfigure_arg, cmd1.second);
+ EXPECT_EQ(RECONFIGURE, cmd1.id);
+ EXPECT_EQ(reconfigure_arg, cmd1.params);
// Non-null, but semantically invalid argument. The manager doesn't do
// this check, so it should result in the same effect.
@@ -140,8 +140,8 @@ TEST(DataSrcClientsMgrTest, reconfigure) {
mgr.reconfigure(reconfigure_arg);
checkSharedMembers(2, 2, 0, 0, 2, 1);
const Command& cmd2 = FakeDataSrcClientsBuilder::command_queue->front();
- EXPECT_EQ(RECONFIGURE, cmd2.first);
- EXPECT_EQ(reconfigure_arg, cmd2.second);
+ EXPECT_EQ(RECONFIGURE, cmd2.id);
+ EXPECT_EQ(reconfigure_arg, cmd2.params);
// Passing NULL argument is immediately rejected
EXPECT_THROW(mgr.reconfigure(ConstElementPtr()), isc::InvalidParameter);
diff --git a/src/bin/auth/tests/test_datasrc_clients_mgr.cc b/src/bin/auth/tests/test_datasrc_clients_mgr.cc
index 82937c0..fc173ef 100644
--- a/src/bin/auth/tests/test_datasrc_clients_mgr.cc
+++ b/src/bin/auth/tests/test_datasrc_clients_mgr.cc
@@ -81,9 +81,9 @@ TestDataSrcClientsMgr::reconfigureHook() {
using namespace datasrc_clientmgr_internal;
// Simply replace the local map, ignoring bogus config value.
- assert(command_queue_.front().first == RECONFIGURE);
+ assert(command_queue_.front().id == RECONFIGURE);
try {
- clients_map_ = configureDataSource(command_queue_.front().second);
+ clients_map_ = configureDataSource(command_queue_.front().params);
} catch (...) {}
}
More information about the bind10-changes
mailing list