BIND 10 trac3086, updated. 92adc1fb42e8966816f96b4e6521bd2a18cead1f [3086] Addressed review comments
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Sep 3 17:43:44 UTC 2013
The branch, trac3086 has been updated
via 92adc1fb42e8966816f96b4e6521bd2a18cead1f (commit)
from c542421c3e40b6417862a07a3e0b5d3fb46f89f9 (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 92adc1fb42e8966816f96b4e6521bd2a18cead1f
Author: Thomas Markwalder <tmark at isc.org>
Date: Tue Sep 3 13:41:18 2013 -0400
[3086] Addressed review comments
Added virtual method for validating state handler map, added additional unit tests and commentary.
-----------------------------------------------------------------------
Summary of changes:
src/bin/d2/d2_update_mgr.cc | 8 +-
src/bin/d2/nc_trans.cc | 26 ++-
src/bin/d2/nc_trans.h | 73 +++++---
src/bin/d2/tests/nc_trans_unittests.cc | 295 ++++++++++++++++++++++++++------
4 files changed, 304 insertions(+), 98 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/d2/d2_update_mgr.cc b/src/bin/d2/d2_update_mgr.cc
index c498050..b88b415 100644
--- a/src/bin/d2/d2_update_mgr.cc
+++ b/src/bin/d2/d2_update_mgr.cc
@@ -77,7 +77,7 @@ D2UpdateMgr::checkFinishedTransactions() {
// NOTE: One must use postfix increments of the iterator on the calls
// to erase. This replaces the old iterator which becomes invalid by the
// erase with a the next valid iterator. Prefix incrementing will not
- // work.
+ // work.
TransactionList::iterator it = transaction_list_.begin();
while (it != transaction_list_.end()) {
NameChangeTransactionPtr trans = (*it).second;
@@ -108,12 +108,6 @@ void D2UpdateMgr::pickNextJob() {
if (!hasTransaction(found_ncr->getDhcid())) {
queue_mgr_->dequeueAt(index);
makeTransaction(found_ncr);
-
-#if 0
- // this will run it up to its first IO
- trans->startTransaction();
-#endif
-
return;
}
}
diff --git a/src/bin/d2/nc_trans.cc b/src/bin/d2/nc_trans.cc
index c8f0ec8..ae106ef 100644
--- a/src/bin/d2/nc_trans.cc
+++ b/src/bin/d2/nc_trans.cc
@@ -37,7 +37,6 @@ const int NameChangeTransaction::NO_MORE_SERVERS_EVT;
const int NameChangeTransaction::IO_COMPLETED_EVT;
const int NameChangeTransaction::UPDATE_OK_EVT;
const int NameChangeTransaction::UPDATE_FAILED_EVT;
-const int NameChangeTransaction::CANCEL_TRANSACTION_EVT;
const int NameChangeTransaction::ALL_DONE_EVT;
const int NameChangeTransaction::DERIVED_EVENTS;
@@ -80,18 +79,16 @@ NameChangeTransaction::startTransaction() {
// Initialize the state handler map first.
initStateHandlerMap();
+ // Test validity of the handler map. This provides an opportunity to
+ // sanity check the map prior to attempting to execute the model.
+ verifyStateHandlerMap();
+
// Set the current state to READY and enter the run loop.
setState(READY_ST);
runStateModel(START_TRANSACTION_EVT);
}
void
-NameChangeTransaction::cancelTransaction() {
- //@todo It is up to the deriving state model to handle this event.
- runStateModel(CANCEL_TRANSACTION_EVT);
-}
-
-void
NameChangeTransaction::operator()(DNSClient::Status status) {
// Stow the completion status and re-enter the run loop with the event
// set to indicate IO completed.
@@ -166,11 +163,6 @@ NameChangeTransaction::setDnsUpdateStatus(const DNSClient::Status& status) {
}
void
-NameChangeTransaction::setDnsUpdateResponse(D2UpdateMessagePtr& response) {
- dns_update_response_ = response;
-}
-
-void
NameChangeTransaction::setForwardChangeCompleted(const bool value) {
forward_change_completed_ = value;
}
@@ -206,7 +198,11 @@ NameChangeTransaction::getReverseDomain() {
}
void
-NameChangeTransaction::initServerSelection(DdnsDomainPtr& domain) {
+NameChangeTransaction::initServerSelection(const DdnsDomainPtr& domain) {
+ if (!domain) {
+ isc_throw(NameChangeTransactionError,
+ "initServerSelection called with an empty domain");
+ }
current_server_list_ = domain->getServers();
next_server_pos_ = 0;
current_server_.reset();
@@ -219,8 +215,8 @@ NameChangeTransaction::selectNextServer() {
current_server_ = (*current_server_list_)[next_server_pos_];
dns_update_response_.reset(new
D2UpdateMessage(D2UpdateMessage::INBOUND));
- // @todo Prototype is set on DNSClient constructor. We need
- // to progate a configruation value downward, probably starting
+ // @todo Protocol is set on DNSClient constructor. We need
+ // to propagate a configuration value downward, probably starting
// at global, then domain, then server
// Once that is supported we need to add it here.
dns_client_.reset(new DNSClient(dns_update_response_ , this,
diff --git a/src/bin/d2/nc_trans.h b/src/bin/d2/nc_trans.h
index 7cbf947..f71bc7f 100644
--- a/src/bin/d2/nc_trans.h
+++ b/src/bin/d2/nc_trans.h
@@ -57,7 +57,7 @@ typedef std::map<unsigned int, StateHandler> StateHandlerMap;
/// of DNS server(s) needed. It is responsible for knowing what conversations
/// it must have with which servers and in the order necessary to fulfill the
/// request. Upon fulfillment of the request, the transaction's work is complete
-/// it is destroyed.
+/// and it is destroyed.
///
/// Fulfillment of the request is carried out through the performance of the
/// transaction's state model. Using a state driven implementation accounts
@@ -121,10 +121,22 @@ public:
static const int NEW_ST = 0;
/// @brief State from which a transaction is started.
static const int READY_ST = 1;
- /// @brief State in which forward DNS server selection is done.
+ /// @brief State in which forward DNS server selection is done.
+ ///
+ /// Within this state, the actual selection of the next forward server
+ /// to use is conducted. Upon conclusion of this state the next server
+ /// is either selected or it should transition out with NO_MORE_SERVERS_EVT
+ /// event.
static const int SELECTING_FWD_SERVER_ST = 2;
+
/// @brief State in which reverse DNS server selection is done.
+ ///
+ /// Within this state, the actual selection of the next reverse server
+ /// to use is conducted. Upon conclusion of this state the next server
+ /// is either selected or it should transition out with NO_MORE_SERVERS_EVT
+ /// event.
static const int SELECTING_REV_SERVER_ST = 3;
+
/// @brief Final state, all work has been performed.
static const int DONE_ST = 4;
@@ -163,11 +175,8 @@ public:
/// the failure is given by the DNSClient return status and the response
/// packet (if one was received).
static const int UPDATE_FAILED_EVT = 8;
- /// @brief Issued when the transaction should be cancelled.
- /// @todo - still on the fence about this one.
- static const int CANCEL_TRANSACTION_EVT = 9;
/// @brief Issued when the state model has no more work left to do.
- static const int ALL_DONE_EVT = 10;
+ static const int ALL_DONE_EVT = 9;
/// @define Value at which custom events in a derived class should begin.
static const int DERIVED_EVENTS = 100;
@@ -182,7 +191,7 @@ public:
/// @param forward_domain is the domain to use for forward DNS updates
/// @param reverse_domain is the domain to use for reverse DNS updates
///
- /// @throw NameChangeTransaction error if given an null request,
+ /// @throw NameChangeTransactionError if given an null request,
/// if forward change is enabled but forward domain is null, if
/// reverse change is enabled but reverse domain is null.
NameChangeTransaction(isc::asiolink::IOService& io_service,
@@ -190,6 +199,7 @@ public:
DdnsDomainPtr& forward_domain,
DdnsDomainPtr& reverse_domain);
+ /// @brief Destructor
virtual ~NameChangeTransaction();
/// @brief Begins execution of the transaction.
@@ -200,9 +210,6 @@ public:
/// parameter of START_TRANSACTION_EVT.
void startTransaction();
- /// @todo - Not sure about this yet.
- void cancelTransaction();
-
/// @brief Serves as the DNSClient IO completion event handler.
///
/// This is the implementation of the method inherited by our derivation
@@ -225,11 +232,39 @@ protected:
/// Implementations should use the addToMap() method add entries to
/// the map.
/// @todo This method should be pure virtual but until there are
- /// derivations for the update manager to use we will provide an
+ /// derivations for the update manager to use, we will provide a
/// temporary empty, implementation. If we make it pure virtual now
/// D2UpdateManager will not compile.
virtual void initStateHandlerMap() {};
+
+ /// @brief Validates the contents of the state handler map.
+ ///
+ /// This method is invoked immediately after initStateHandlerMap and
+ /// provides an opportunity for derivations to verify that the map
+ /// is correct. If the map is determined to be invalid this method
+ /// should throw a NameChangeTransactionError.
+ ///
+ /// The simplest implementation would include a call to getStateHandler,
+ /// for each state the derivation supports. For example, a implementation
+ /// which included three states, READY_ST, DO_WORK_ST, and DONE_ST could
+ /// implement this function as follows:
+ ///
+ /// @code
+ /// void verifyStateHandlerMap() {
+ /// getStateHandler(READY_ST);
+ /// getStateHandler(DO_WORK_ST);
+ /// getStateHandler(DONE_ST);
+ /// }
+ /// @endcode
+ ///
+ /// @todo This method should be pure virtual but until there are
+ /// derivations for the update manager to use, we will provide a
+ /// temporary empty, implementation. If we make it pure virtual now
+ /// D2UpdateManager will not compile.
+ /// @throw NameChangeTransactionError if the map is invalid.
+ virtual void verifyStateHandlerMap() {};
+
/// @brief Adds an entry to the state handler map.
///
/// This method attempts to add an entry to the handler map which maps
@@ -257,7 +292,7 @@ protected:
///
/// @throw NameChangeTransactionError if the map already contains an entry
/// for the given state.
- void addToMap(unsigned int idx, StateHandler handler);
+ void addToMap(unsigned int state, StateHandler handler);
/// @brief Processes events through the state model
///
@@ -345,7 +380,7 @@ protected:
///
/// @param domain is the domain from which server selection is to be
/// conducted.
- void initServerSelection(DdnsDomainPtr& domain);
+ void initServerSelection(const DdnsDomainPtr& domain);
/// @brief Selects the next server in the current server list.
///
@@ -387,7 +422,7 @@ public:
///
/// This is the current status of the NameChangeRequest, not to
/// be confused with the state of the transaction. Once the transaction
- /// is reached it's conclusion, the request will end up with a final
+ /// is reached its conclusion, the request will end up with a final
/// status.
///
/// @return A dhcp_ddns::NameChangeStatus representing the current
@@ -396,16 +431,14 @@ public:
/// @brief Fetches the forward DdnsDomain.
///
- /// This value is only meaningful if the request calls for a forward change.
- ///
- /// @return A pointer reference to the forward DdnsDomain
+ /// @return A pointer reference to the forward DdnsDomain. If the
+ /// the request does not include a forward change, the pointer will empty.
DdnsDomainPtr& getForwardDomain();
/// @brief Fetches the reverse DdnsDomain.
///
- /// This value is only meaningful if the request calls for a reverse change.
- ///
- /// @return A pointer reference to the reverse DdnsDomain
+ /// @return A pointer reference to the reverse DdnsDomain. If the
+ /// the request does not include a reverse change, the pointer will empty.
DdnsDomainPtr& getReverseDomain();
/// @brief Fetches the transaction's current state.
diff --git a/src/bin/d2/tests/nc_trans_unittests.cc b/src/bin/d2/tests/nc_trans_unittests.cc
index 9f524a4..777c19f 100644
--- a/src/bin/d2/tests/nc_trans_unittests.cc
+++ b/src/bin/d2/tests/nc_trans_unittests.cc
@@ -27,7 +27,7 @@ namespace {
/// @brief Test derivation of NameChangeTransaction for exercising state
/// model mechanics.
///
-/// This class faciliates testing by making non-public methods accessible so
+/// This class facilitates testing by making non-public methods accessible so
/// they can be invoked directly in test routines. It implements a very
/// rudimentary state model, sufficient to test the state model mechanics
/// supplied by the base class.
@@ -35,7 +35,10 @@ class NameChangeStub : public NameChangeTransaction {
public:
// NameChangeStub states
- static const int DO_WORK_ST = DERIVED_STATES + 1;
+ static const int DUMMY_ST = DERIVED_STATES + 1;
+
+ static const int DO_WORK_ST = DERIVED_STATES + 2;
+
// NameChangeStub events
static const int START_WORK_EVT = DERIVED_EVENTS + 1;
@@ -48,17 +51,29 @@ public:
DdnsDomainPtr forward_domain,
DdnsDomainPtr reverse_domain)
: NameChangeTransaction(io_service, ncr, forward_domain,
- reverse_domain) {
+ reverse_domain), dummy_called_(false) {
}
/// @brief Destructor
virtual ~NameChangeStub() {
}
+ bool getDummyCalled() {
+ return (dummy_called_);
+ }
+
+ void clearDummyCalled() {
+ dummy_called_ = false;
+ }
+
+ void dummyHandler() {
+ dummy_called_ = true;
+ }
+
/// @brief State handler for the READY_ST.
///
/// Serves as the starting state handler, it consumes the
- /// START_TRANSACTION_EVT "transitioing" to the state, DO_WORK_ST and
+ /// START_TRANSACTION_EVT "transitioning" to the state, DO_WORK_ST and
/// sets the next event to START_WORK_EVT.
void readyHandler() {
switch(getNextEvent()) {
@@ -76,7 +91,7 @@ public:
/// @brief State handler for the DO_WORK_ST.
///
/// Simulates a state that starts some form of asynchronous work.
- /// When next event is START_WROK_EVT it sets the status to pending
+ /// When next event is START_WORK_EVT it sets the status to pending
/// and signals the state model must "wait" for an event by setting
/// next event to NOP_EVT.
///
@@ -122,13 +137,19 @@ public:
addToMap(READY_ST,
boost::bind(&NameChangeStub::readyHandler, this));
- addToMap(DO_WORK_ST,
+ addToMap(DO_WORK_ST,
boost::bind(&NameChangeStub::doWorkHandler, this));
- addToMap(DONE_ST,
+ addToMap(DONE_ST,
boost::bind(&NameChangeStub::doneHandler, this));
}
+ void verifyStateHandlerMap() {
+ getStateHandler(READY_ST);
+ getStateHandler(DO_WORK_ST);
+ getStateHandler(DONE_ST);
+ }
+
// Expose the protected methods to be tested.
using NameChangeTransaction::addToMap;
using NameChangeTransaction::getStateHandler;
@@ -140,6 +161,15 @@ public:
using NameChangeTransaction::selectNextServer;
using NameChangeTransaction::getCurrentServer;
using NameChangeTransaction::getDNSClient;
+ using NameChangeTransaction::setNcrStatus;
+ using NameChangeTransaction::setDnsUpdateStatus;
+ using NameChangeTransaction::getDnsUpdateResponse;
+ using NameChangeTransaction::getForwardChangeCompleted;
+ using NameChangeTransaction::getReverseChangeCompleted;
+ using NameChangeTransaction::setForwardChangeCompleted;
+ using NameChangeTransaction::setReverseChangeCompleted;
+
+ bool dummy_called_;
};
const int NameChangeStub::DO_WORK_ST;
@@ -155,6 +185,8 @@ typedef boost::shared_ptr<NameChangeStub> NameChangeStubPtr;
class NameChangeTransactionTest : public ::testing::Test {
public:
isc::asiolink::IOService io_service_;
+ DdnsDomainPtr forward_domain_;
+ DdnsDomainPtr reverse_domain_;
virtual ~NameChangeTransactionTest() {
}
@@ -167,7 +199,7 @@ public:
" \"change_type\" : 0 , "
" \"forward_change\" : true , "
" \"reverse_change\" : true , "
- " \"fqdn\" : \"walah.walah.org.\" , "
+ " \"fqdn\" : \"example.com.\" , "
" \"ip_address\" : \"192.168.2.1\" , "
" \"dhcid\" : \"0102030405060708\" , "
" \"lease_expires_on\" : \"20130121132405\" , "
@@ -179,25 +211,22 @@ public:
DnsServerInfoStoragePtr servers(new DnsServerInfoStorage());
DnsServerInfoPtr server;
- DdnsDomainPtr forward_domain;
- DdnsDomainPtr reverse_domain;
-
ncr = dhcp_ddns::NameChangeRequest::fromJSON(msg_str);
// make forward server list
- server.reset(new DnsServerInfo("forward.server.org",
+ server.reset(new DnsServerInfo("forward.example.com",
isc::asiolink::IOAddress("1.1.1.1")));
servers->push_back(server);
- forward_domain.reset(new DdnsDomain("*", "", servers));
+ forward_domain_.reset(new DdnsDomain("*", "", servers));
// make reverse server list
servers->clear();
- server.reset(new DnsServerInfo("reverse.server.org",
+ server.reset(new DnsServerInfo("reverse.example.com",
isc::asiolink::IOAddress("2.2.2.2")));
servers->push_back(server);
- reverse_domain.reset(new DdnsDomain("*", "", servers));
+ reverse_domain_.reset(new DdnsDomain("*", "", servers));
return (NameChangeStubPtr(new NameChangeStub(io_service_, ncr,
- forward_domain, reverse_domain)));
+ forward_domain_, reverse_domain_)));
}
@@ -219,7 +248,7 @@ TEST(NameChangeTransaction, construction) {
" \"change_type\" : 0 , "
" \"forward_change\" : true , "
" \"reverse_change\" : true , "
- " \"fqdn\" : \"walah.walah.org.\" , "
+ " \"fqdn\" : \"example.com.\" , "
" \"ip_address\" : \"192.168.2.1\" , "
" \"dhcid\" : \"0102030405060708\" , "
" \"lease_expires_on\" : \"20130121132405\" , "
@@ -260,87 +289,209 @@ TEST(NameChangeTransaction, construction) {
forward_domain, reverse_domain));
// Verify that an empty forward domain is allowed when the requests does
- // include a forward change.
+ // not include a forward change.
ncr->setForwardChange(false);
+ ncr->setReverseChange(true);
EXPECT_NO_THROW(NameChangeTransaction(io_service, ncr,
empty_domain, reverse_domain));
// Verify that an empty reverse domain is allowed when the requests does
- // include a reverse change.
+ // not include a reverse change.
+ ncr->setForwardChange(true);
ncr->setReverseChange(false);
EXPECT_NO_THROW(NameChangeTransaction(io_service, ncr,
- empty_domain, empty_domain));
+ forward_domain, empty_domain));
}
-/// @brief Test the basic mechanics of state model execution.
-/// It first verifies basic state handle map fucntionality, and then
-/// runs the NameChangeStub state model through from start to finish.
-TEST_F(NameChangeTransactionTest, stateModelTest) {
+/// @brief General testing of member accessors.
+/// Most if not all of these are also tested as a byproduct off larger tests.
+TEST_F(NameChangeTransactionTest, accessors) {
NameChangeStubPtr name_change;
ASSERT_NO_THROW(name_change = makeCannedTransaction());
- // Verify that getStateHandler will throw when, handler map is empty.
+ // Verify that fetching the NameChangeRequest works.
+ dhcp_ddns::NameChangeRequestPtr ncr = name_change->getNcr();
+ ASSERT_TRUE(ncr);
+
+ // Verify that getTransactionKey works.
+ EXPECT_EQ(ncr->getDhcid(), name_change->getTransactionKey());
+
+ // Verify that NcrStatus can be set and retrieved.
+ EXPECT_NO_THROW(name_change->setNcrStatus(dhcp_ddns::ST_FAILED));
+ EXPECT_EQ(dhcp_ddns::ST_FAILED, ncr->getStatus());
+
+ // Verify that the forward domain can be retrieved.
+ ASSERT_TRUE(name_change->getForwardDomain());
+ EXPECT_EQ(forward_domain_, name_change->getForwardDomain());
+
+ // Verify that the reverse domain can be retrieved.
+ ASSERT_TRUE(name_change->getReverseDomain());
+ EXPECT_EQ(reverse_domain_, name_change->getReverseDomain());
+
+ // Neither of these have direct setters, but are tested under server
+ // selection.
+ EXPECT_FALSE(name_change->getDNSClient());
+ EXPECT_FALSE(name_change->getCurrentServer());
+
+ // Previous state should be set by setState.
+ EXPECT_NO_THROW(name_change->setState(NameChangeTransaction::READY_ST));
+ EXPECT_NO_THROW(name_change->setState(NameChangeStub::DO_WORK_ST));
+ EXPECT_EQ(NameChangeTransaction::READY_ST, name_change->getPrevState());
+ EXPECT_EQ(NameChangeStub::DO_WORK_ST, name_change->getState());
+
+ // Last event should be set by setNextEvent.
+ EXPECT_NO_THROW(name_change->setNextEvent(NameChangeStub::
+ START_WORK_EVT));
+ EXPECT_NO_THROW(name_change->setNextEvent(NameChangeTransaction::
+ IO_COMPLETED_EVT));
+ EXPECT_EQ(NameChangeStub::START_WORK_EVT, name_change->getLastEvent());
+ EXPECT_EQ(NameChangeTransaction::IO_COMPLETED_EVT,
+ name_change->getNextEvent());
+
+ // Verify that DNS update status can be set and retrieved.
+ EXPECT_NO_THROW(name_change->setDnsUpdateStatus(DNSClient::TIMEOUT));
+ EXPECT_EQ(DNSClient::TIMEOUT, name_change->getDnsUpdateStatus());
+
+ // Verify that the DNS update response can be retrieved.
+ EXPECT_FALSE(name_change->getDnsUpdateResponse());
+
+ // Verify that the forward change complete flag can be set and fetched.
+ EXPECT_NO_THROW(name_change->setForwardChangeCompleted(true));
+ EXPECT_TRUE(name_change->getForwardChangeCompleted());
+
+ // Verify that the reverse change complete flag can be set and fetched.
+ EXPECT_NO_THROW(name_change->setReverseChangeCompleted(true));
+ EXPECT_TRUE(name_change->getReverseChangeCompleted());
+}
+
+/// @brief Tests the fundamental methods used for state handler mapping.
+/// Verifies the ability to search for and add entries in the state handler map.
+TEST_F(NameChangeTransactionTest, basicStateMapping) {
+ NameChangeStubPtr name_change;
+ ASSERT_NO_THROW(name_change = makeCannedTransaction());
+
+ // Verify that getStateHandler will throw when, state cannot be found.
EXPECT_THROW(name_change->getStateHandler(NameChangeTransaction::READY_ST),
NameChangeTransactionError);
// Verify that we can add a handler to the map.
ASSERT_NO_THROW(name_change->addToMap(NameChangeTransaction::READY_ST,
- boost::bind(&NameChangeStub::readyHandler,
- name_change.get())));
+ boost::bind(&NameChangeStub::
+ dummyHandler,
+ name_change.get())));
// Verify that we can find the handler by its state.
- EXPECT_NO_THROW(name_change->getStateHandler(NameChangeTransaction::
+ StateHandler retreived_handler;
+ EXPECT_NO_THROW(retreived_handler =
+ name_change->getStateHandler(NameChangeTransaction::
READY_ST));
+ // Verify that retrieved handler executes the correct method.
+ name_change->clearDummyCalled();
+
+ ASSERT_NO_THROW((retreived_handler)());
+ EXPECT_TRUE(name_change->getDummyCalled());
+
// Verify that we cannot add a duplicate.
EXPECT_THROW(name_change->addToMap(NameChangeTransaction::READY_ST,
- boost::bind(&NameChangeStub::readyHandler,
- name_change.get())),
+ boost::bind(&NameChangeStub::
+ readyHandler,
+ name_change.get())),
NameChangeTransactionError);
// Verify that we can still find the handler by its state.
EXPECT_NO_THROW(name_change->getStateHandler(NameChangeTransaction::
READY_ST));
+}
-
- // Get a fresh transaction.
+/// @brief Tests state map initialization and validation.
+/// This tests the basic concept of state map initialization and verification
+/// by manually invoking the map methods normally called by startTransaction.
+TEST_F(NameChangeTransactionTest, stubStateMapInit) {
+ NameChangeStubPtr name_change;
ASSERT_NO_THROW(name_change = makeCannedTransaction());
- // Manually call checkHandlerMap to ensure our test map populates.
- // This is the method startTranscation invokes.
+ // Verify that the map validation throws prior to the map being
+ // initialized.
+ ASSERT_THROW(name_change->verifyStateHandlerMap(),
+ NameChangeTransactionError);
+
+ // Call initStateHandlerMap to initialize the state map.
ASSERT_NO_THROW(name_change->initStateHandlerMap());
- // Verify that we can find all the handlers by their state.
- EXPECT_NO_THROW(name_change->getStateHandler(NameChangeTransaction::
- READY_ST));
- EXPECT_NO_THROW(name_change->getStateHandler(NameChangeStub::DO_WORK_ST));
- EXPECT_NO_THROW(name_change->getStateHandler(NameChangeTransaction::
- DONE_ST));
+ // Verify that the map validation succeeds now that the map is initialized.
+ ASSERT_NO_THROW(name_change->verifyStateHandlerMap());
+}
+/// @brief Tests that invalid states are handled gracefully.
+/// This test verifies that attempting to execute a state which has no handler
+/// results in a failed transaction.
+TEST_F(NameChangeTransactionTest, invalidState) {
+ NameChangeStubPtr name_change;
+ ASSERT_NO_THROW(name_change = makeCannedTransaction());
- // Default value for state is NEW_ST. Attempting to run the model
- // with an invalid state will result in status of ST_FAILED.
+ // Verfiy that to running the model with a state that has no handler,
+ // will result in failed transaction (status of ST_FAILED).
+ // First, verify state is NEW_ST and that NEW_ST has no handler.
+ // that the transaction failed:
ASSERT_EQ(NameChangeTransaction::NEW_ST, name_change->getState());
+ ASSERT_THROW(name_change->getStateHandler(NameChangeTransaction::NEW_ST),
+ NameChangeTransactionError);
+
+ // Now call runStateModel() which should not throw.
EXPECT_NO_THROW(name_change->runStateModel(NameChangeTransaction::
START_TRANSACTION_EVT));
+ // Verify that the transaction has failed.
EXPECT_EQ(dhcp_ddns::ST_FAILED, name_change->getNcrStatus());
+}
- // Get a fresh transaction.
+/// @brief Tests that invalid events are handled gracefully.
+/// This test verifies that submitting an invalid event to the state machine
+/// results in a failed transaction.
+TEST_F(NameChangeTransactionTest, invalidEvent) {
+ NameChangeStubPtr name_change;
ASSERT_NO_THROW(name_change = makeCannedTransaction());
- // Launch the transaction properly by calling startTranscation.
- // Verify that this transitions through to state of DO_WORK_ST,
- // last event is START_WORK_EVT, next event is NOP_EVT, and
- // NCR status is ST_PENDING.
+ // First, lets execute the state model to a known valid point, by
+ // calling startTransaction.
ASSERT_NO_THROW(name_change->startTransaction());
+ // Verify we are in the state of DO_WORK_ST.
+ EXPECT_EQ(NameChangeStub::DO_WORK_ST, name_change->getState());
+
+ // Verity that submitting an invalid event to a valid state, results
+ // in a failed transaction without a throw (Current state is DO_WORK_ST,
+ // during which START_TRANSACTION_EVT, is invalid).
+ EXPECT_NO_THROW(name_change->runStateModel(NameChangeTransaction::
+ START_TRANSACTION_EVT));
+
+ // Verify that the transaction has failed.
+ EXPECT_EQ(dhcp_ddns::ST_FAILED, name_change->getNcrStatus());
+}
+
+/// @brief Test the basic mechanics of state model execution.
+/// This test exercises the ability to execute state model from state to
+/// finish, including the handling of a asynchronous IO operation.
+TEST_F(NameChangeTransactionTest, stateModelTest) {
+ NameChangeStubPtr name_change;
+ ASSERT_NO_THROW(name_change = makeCannedTransaction());
+
+ // Launch the transaction by calling startTransaction. The state model
+ // should run up until the "IO" operation is initiated in DO_WORK_ST.
+
+ ASSERT_NO_THROW(name_change->startTransaction());
+
+ // Verify that we are now in state of DO_WORK_ST, the last event was
+ // START_WORK_EVT, the next event is NOP_EVT, and NCR status is ST_PENDING.
EXPECT_EQ(NameChangeStub::DO_WORK_ST, name_change->getState());
EXPECT_EQ(NameChangeStub::START_WORK_EVT, name_change->getLastEvent());
EXPECT_EQ(NameChangeTransaction::NOP_EVT, name_change->getNextEvent());
EXPECT_EQ(dhcp_ddns::ST_PENDING, name_change->getNcrStatus());
- // Simulate completion of DNSClient exchange by invoking the callback.
+ // Simulate completion of DNSClient exchange by invoking the callback, as
+ // DNSClient would. This should cause the state model to progress through
+ // completion.
EXPECT_NO_THROW((*name_change)(DNSClient::SUCCESS));
// Verify that the state model has progressed through to completion:
@@ -351,12 +502,17 @@ TEST_F(NameChangeTransactionTest, stateModelTest) {
EXPECT_EQ(NameChangeTransaction::NOP_EVT, name_change->getNextEvent());
}
-/// @brief Tests server selection methods
+/// @brief Tests server selection methods.
+/// Each transaction has a list of one or more servers for each DNS direction
+/// it is required to update. The transaction must be able to start at the
+/// beginning of a server list and cycle through them one at time, as needed,
+/// when a DNS exchange fails due to an IO error. This test verifies the
+/// ability to iteratively select a server from the list as the current server.
TEST_F(NameChangeTransactionTest, serverSelectionTest) {
NameChangeStubPtr name_change;
ASSERT_NO_THROW(name_change = makeCannedTransaction());
- // Verify that the forward domain and its servers can be retrieved.
+ // Verify that the forward domain and its list of servers can be retrieved.
DdnsDomainPtr& domain = name_change->getForwardDomain();
ASSERT_TRUE(domain);
DnsServerInfoStoragePtr servers = domain->getServers();
@@ -366,72 +522,99 @@ TEST_F(NameChangeTransactionTest, serverSelectionTest) {
int num_servers = servers->size();
ASSERT_TRUE(num_servers > 0);
+ // Verify that we can initialize server selection. This "resets" the
+ // selection process to start over using the list of servers in the
+ // given domain.
ASSERT_NO_THROW(name_change->initServerSelection(domain));
+ // The server selection process determines the current server,
+ // instantiates a new DNSClient, and a DNS response message buffer.
+ // We need to save the values before each selection, so we can verify
+ // they are correct after each selection.
+ DnsServerInfoPtr prev_server = name_change->getCurrentServer();
DNSClientPtr prev_client = name_change->getDNSClient();
D2UpdateMessagePtr prev_response = name_change->getDnsUpdateResponse();
- DnsServerInfoPtr prev_server = name_change->getCurrentServer();
// Iteratively select through the list of servers.
int passes = 0;
while (name_change->selectNextServer()) {
+ // Get the new values after the selection has been made.
DnsServerInfoPtr server = name_change->getCurrentServer();
DNSClientPtr client = name_change->getDNSClient();
D2UpdateMessagePtr response = name_change->getDnsUpdateResponse();
+
+ // Verify that the new values are not empty.
EXPECT_TRUE(server);
EXPECT_TRUE(client);
EXPECT_TRUE(response);
+ // Verify that the new values are indeed new.
EXPECT_NE(server, prev_server);
EXPECT_NE(client, prev_client);
EXPECT_NE(response, prev_response);
+ // Remember the selected values for the next pass.
prev_server = server;
prev_client = client;
prev_response = response;
- passes++;
+ ++passes;
}
- // Verify that the numer of passes made equal the number of servers.
+ // Verify that the number of passes made equal the number of servers.
EXPECT_EQ (passes, num_servers);
// Repeat the same test using the reverse domain.
+ // Verify that the reverse domain and its list of servers can be retrieved.
domain = name_change->getReverseDomain();
ASSERT_TRUE(domain);
-
servers = domain->getServers();
ASSERT_TRUE(servers);
+ // Get the number of entries in the server list.
num_servers = servers->size();
ASSERT_TRUE(num_servers > 0);
+ // Verify that we can initialize server selection. This "resets" the
+ // selection process to start over using the list of servers in the
+ // given domain.
ASSERT_NO_THROW(name_change->initServerSelection(domain));
+ // The server selection process determines the current server,
+ // instantiates a new DNSClient, and a DNS response message buffer.
+ // We need to save the values before each selection, so we can verify
+ // they are correct after each selection.
+ prev_server = name_change->getCurrentServer();
prev_client = name_change->getDNSClient();
prev_response = name_change->getDnsUpdateResponse();
- prev_server = name_change->getCurrentServer();
+ // Iteratively select through the list of servers.
passes = 0;
while (name_change->selectNextServer()) {
+ // Get the new values after the selection has been made.
DnsServerInfoPtr server = name_change->getCurrentServer();
DNSClientPtr client = name_change->getDNSClient();
D2UpdateMessagePtr response = name_change->getDnsUpdateResponse();
+
+ // Verify that the new values are not empty.
EXPECT_TRUE(server);
EXPECT_TRUE(client);
EXPECT_TRUE(response);
+ // Verify that the new values are indeed new.
EXPECT_NE(server, prev_server);
EXPECT_NE(client, prev_client);
EXPECT_NE(response, prev_response);
+ // Remember the selected values for the next pass.
prev_server = server;
prev_client = client;
prev_response = response;
- passes++;
+ ++passes;
}
+ // Verify that the number of passes made equal the number of servers.
EXPECT_EQ (passes, num_servers);
}
More information about the bind10-changes
mailing list