Browse Source

3087 Addressed review comments

Good deal of commentary clean up, rolled back IOService reference
changes to DNSClient, and most significantly added unit tests
for NameChangeTransaction::sendUpdate.  These tests involve an
actual message exchange.
Thomas Markwalder 11 years ago
parent
commit
fe99fae754

+ 41 - 35
src/bin/d2/d2_messages.mes

@@ -190,7 +190,7 @@ indicate a network connectivity or system resource issue.
 
 % DHCP_DDNS_QUEUE_MGR_RESUME_ERROR application could not restart the queue manager, reason: %1
 This is an error message indicating that DHCP_DDNS's Queue Manager could not
-be restarted after stopping due to an a full receive queue.  This means that
+be restarted after stopping due to a full receive queue.  This means that
 the application cannot receive requests. This is most likely due to DHCP_DDNS
 configuration parameters referring to resources such as an IP address or port,
 that is no longer unavailable.  DHCP_DDNS will attempt to restart the queue
@@ -260,60 +260,66 @@ of this update did not succeed. This is a programmatic error and should be
 reported.
 
 % DHCP_DDNS_FORWARD_ADD_REJECTED DNS Server, %1, rejected a DNS update request to add the address mapping for FQDN, %2, with an RCODE: %3
-This is an error message issued when an udpate was rejected by the DNS server itwas sent to for the reason given by the RCODE. The rcode values are defined in
-RFC 2136.
+This is an error message issued when an update was rejected by the DNS server
+it was sent to for the reason given by the RCODE. The rcode values are defined
+in RFC 2136.
 
-% DHCP_DDNS_FORWARD_ADD_IO_ERROR while attempting a request to add a forward address mapping to DNS server %1 for FQDN %2, DHCP_DDNS encountered an IO error
-This is an error message issued when a communication error occurs while DHCP_DDNS is carrying out a forward address update.  The application will retry against
-the same server or others as appropriate.
+% DHCP_DDNS_FORWARD_ADD_IO_ERROR DHCP_DDNS encountered an IO error sending a forward mapping add for FQDN %1 to DNS server %2
+This is an error message issued when a communication error occurs while
+DHCP_DDNS is carrying out a forward address update.  The application will
+retry against the same server or others as appropriate.
 
-% DHCP_DDNS_FORWARD_ADD_RESP_CORRUPT while attempting a request to add a forward address mapping to DNS server, %1, for FQDN, %2, DHCP_DDNS received a corrupt response
+% DHCP_DDNS_FORWARD_ADD_RESP_CORRUPT DHCP_DDNS received a corrupt response from the DNS server, %1, while adding forward address mapping for FQDN, %2
 This is an error message issued when the response received by DHCP_DDNS, to a
-update request to add a forward address mapping,  is mangled or mal-formed.
+update request to add a forward address mapping,  is mangled or malformed.
 The application will retry against the same server or others as appropriate.
 
-% DHCP_DDNS_FORWARD_ADD_UNKNOWN_FAILURE while attempting a request to add a forward address mapping to DNS server %1 for FQDN %2, DHCP_DDNS encountered an unexepected error.
-This is an error message issued when a unexpected error condition error occurs
-while DHCP_DDNS is carrying out a forward address update.  The request will be
-be aborted.  This is most likely a programmatic issue and should be reported.
+% DHCP_DDNS_FORWARD_ADD_BAD_DNSCLIENT_STATUS DHCP_DDNS received an unknown DNSClient status: %1, while adding a forward address mapping for FQDN %2 to DNS server %3
+This is an error message issued when DNSClient returns an unrecognized status
+while DHCP_DDNS was adding a forward address mapping.  The request will be
+aborted.  This is most likely a programmatic issue and should be reported.
 
 % DHCP_DDNS_FORWARD_REPLACE_REJECTED DNS Server, %1, rejected a DNS update request to replace the address mapping for FQDN, %2, with an RCODE: %3
-This is an error message issued when an udpate was rejected by the DNS server itwas sent to for the reason given by the RCODE. The rcode values are defined in
-RFC 2136.
+This is an error message issued when an update was rejected by the DNS server
+it was sent to for the reason given by the RCODE. The rcode values are defined
+in RFC 2136.
 
-% DHCP_DDNS_FORWARD_REPLACE_IO_ERROR while attempting a request to replace a forward address mapping to DNS server %1 for FQDN %2, DHCP_DDNS encountered an IO error
-This is an error message issued when a communication error occurs while DHCP_DDNS is carrying out a forward address update.  The application will retry against
-the same server or others as appropriate.
+% DHCP_DDNS_FORWARD_REPLACE_IO_ERROR DHCP_DDNS encountered an IO error sending a forward mapping replace for FQDN %1 to DNS server %2
+This is an error message issued when a communication error occurs while
+DHCP_DDNS is carrying out a forward address update.  The application will
+retry against the same server or others as appropriate.
 
-% DHCP_DDNS_FORWARD_REPLACE_RESP_CORRUPT while attempting a request to replace a forward address mapping to DNS server, %1, for FQDN, %2, DHCP_DDNS received a corrupt response
+% DHCP_DDNS_FORWARD_REPLACE_RESP_CORRUPT DHCP_DDNS received a corrupt response from the DNS server, %1, while replacing forward address mapping for FQDN, %2
 This is an error message issued when the response received by DHCP_DDNS, to a
-update request to replace a forward address mapping,  is mangled or mal-formed.
+update request to replace a forward address mapping,  is mangled or malformed.
 The application will retry against the same server or others as appropriate.
 
-% DHCP_DDNS_FORWARD_REPLACE_UNKNOWN_FAILURE while attempting a request to replace a forward address mapping to DNS server %1 for FQDN %2, DHCP_DDNS encountered an unexepected error.
-This is an error message issued when a unexpected error condition error occurs
-while DHCP_DDNS is carrying out a forward address update.  The request will be
-be aborted.  This is most likely a programmatic issue and should be reported.
+% DHCP_DDNS_FORWARD_REPLACE_BAD_DNSCLIENT_STATUS DHCP_DDNS received an unknown DNSClient status: %1, while replacing forward address mapping for FQDN %2 to DNS server %3
+This is an error message issued when DNSClient returns an unrecognized status
+while DHCP_DDNS was replacing a forward address mapping.  The request will be
+aborted.  This is most likely a programmatic issue and should be reported.
 
 % DHCP_DDNS_REVERSE_REPLACE_REJECTED DNS Server, %1, rejected a DNS update request to replace the reverse mapping for FQDN, %2, with an RCODE: %3
-This is an error message issued when an udpate was rejected by the DNS server itwas sent to for the reason given by the RCODE. The rcode values are defined in
-RFC 2136.
+This is an error message issued when an update was rejected by the DNS server
+it was sent to for the reason given by the RCODE. The rcode values are defined
+in RFC 2136.
 
-% DHCP_DDNS_REVERSE_REPLACE_IO_ERROR while attempting a request to replace a reverse mapping to DNS server %1 for FQDN %2, DHCP_DDNS encountered an IO error
-This is an error message issued when a communication error occurs while DHCP_DDNS is carrying out a reverse address update.  The application will retry against
-the same server or others as appropriate.
+% DHCP_DDNS_REVERSE_REPLACE_IO_ERROR DHCP_DDNS encountered an IO error sending a reverse mapping replacement for FQDN %1 to DNS server %2
+This is an error message issued when a communication error occurs while
+DHCP_DDNS is carrying out a reverse address update.  The application will
+retry against the same server or others as appropriate.
 
-% DHCP_DDNS_REVERSE_REPLACE_RESP_CORRUPT while attempting a request to replace a reverse mapping to DNS server, %1, for FQDN, %2, DHCP_DDNS received a corrupt response
+% DHCP_DDNS_REVERSE_REPLACE_RESP_CORRUPT DHCP_DDNS received a corrupt response from the DNS server, %1, while replacing reverse address mapping for FQDN, %2
 This is an error message issued when the response received by DHCP_DDNS, to a
-update request to replace a reverse address,  is mangled or mal-formed.
+update request to replace a reverse address,  is mangled or malformed.
 The application will retry against the same server or others as appropriate.
 
-% DHCP_DDNS_REVERSE_REPLACE_UNKNOWN_FAILURE while attempting a request to replace a reverse mapping to DNS server %1 for FQDN %2, DHCP_DDNS encountered an unexepected error.
-This is an error message issued when a unexpected error condition error occurs
-while DHCP_DDNS is carrying out a reverse address update.  The request will be
-be aborted.  This is most likely a programmatic issue and should be reported.
+% DHCP_DDNS_REVERSE_REPLACE_BAD_DNSCLIENT_STATUS DHCP_DDNS received an unknown DNSClient status: %1, while replacing reverse address mapping for FQDN %2 to DNS server %3
+This is an error message issued when DNSClient returns an unrecognized status
+while DHCP_DDNS was replacing a reverse address mapping.  The request will be
+aborted.  This is most likely a programmatic issue and should be reported.
 
-% DHCP_DDNS_TRANS_SEND_EROR application encountered an unexpected error while attempting to send an DNS update: %1
+% DHCP_DDNS_TRANS_SEND_ERROR application encountered an unexpected error while attempting to send a DNS update: %1
 This is error message issued when the application is able to construct an update
 message but the attempt to send it suffered a unexpected error. This is most
 likely a programmatic error, rather than a communications issue. Some or all

+ 7 - 11
src/bin/d2/dns_client.cc

@@ -65,7 +65,7 @@ public:
     virtual void operator()(asiodns::IOFetch::Result result);
 
     // Starts asynchronous DNS Update.
-    void doUpdate(IOServicePtr& io_service,
+    void doUpdate(asiolink::IOService& io_service,
                   const asiolink::IOAddress& ns_addr,
                   const uint16_t ns_port,
                   D2UpdateMessage& update,
@@ -162,7 +162,7 @@ DNSClientImpl::getStatus(const asiodns::IOFetch::Result result) {
 }
 
 void
-DNSClientImpl::doUpdate(IOServicePtr& io_service,
+DNSClientImpl::doUpdate(asiolink::IOService& io_service,
                         const IOAddress& ns_addr,
                         const uint16_t ns_port,
                         D2UpdateMessage& update,
@@ -189,11 +189,12 @@ DNSClientImpl::doUpdate(IOServicePtr& io_service,
     // Timeout value is explicitly cast to the int type to avoid warnings about
     // overflows when doing implicit cast. It should have been checked by the
     // caller that the unsigned timeout value will fit into int.
-    IOFetch io_fetch(IOFetch::UDP, *io_service, msg_buf, ns_addr, ns_port,
+    IOFetch io_fetch(IOFetch::UDP, io_service, msg_buf, ns_addr, ns_port,
                      in_buf_, this, static_cast<int>(wait));
+
     // Post the task to the task queue in the IO service. Caller will actually
     // run these tasks by executing IOService::run.
-    io_service->post(io_fetch);
+    io_service.post(io_fetch);
 }
 
 
@@ -213,7 +214,7 @@ DNSClient::getMaxTimeout() {
 }
 
 void
-DNSClient::doUpdate(IOServicePtr&,
+DNSClient::doUpdate(asiolink::IOService&,
                     const IOAddress&,
                     const uint16_t,
                     D2UpdateMessage&,
@@ -224,16 +225,11 @@ DNSClient::doUpdate(IOServicePtr&,
 }
 
 void
-DNSClient::doUpdate(IOServicePtr& io_service,
+DNSClient::doUpdate(asiolink::IOService& io_service,
                     const IOAddress& ns_addr,
                     const uint16_t ns_port,
                     D2UpdateMessage& update,
                     const unsigned int wait) {
-    if (!io_service) {
-        isc_throw(isc::BadValue, 
-                  "DNSClient::doUpdate: IOService cannot be null");
-    }
-
     // The underlying implementation which we use to send DNS Updates uses
     // signed integers for timeout. If we want to avoid overflows we need to
     // respect this limitation here.

+ 3 - 3
src/bin/d2/dns_client.h

@@ -16,8 +16,8 @@
 #define DNS_CLIENT_H
 
 #include <d2/d2_update_message.h>
-#include <d2/d2_asio.h>
 
+#include <asiolink/io_service.h>
 #include <util/buffer.h>
 
 #include <asiodns/io_fetch.h>
@@ -151,7 +151,7 @@ public:
     ///
     /// @todo Implement TSIG Support. Currently any attempt to call this
     /// function will result in exception.
-    void doUpdate(IOServicePtr& io_service,
+    void doUpdate(asiolink::IOService& io_service,
                   const asiolink::IOAddress& ns_addr,
                   const uint16_t ns_port,
                   D2UpdateMessage& update,
@@ -176,7 +176,7 @@ public:
     /// @param wait A timeout (in seconds) for the response. If a response is
     /// not received within the timeout, exchange is interrupted. This value
     /// must not exceed maximal value for 'int' data type.
-    void doUpdate(IOServicePtr& io_service,
+    void doUpdate(asiolink::IOService& io_service,
                   const asiolink::IOAddress& ns_addr,
                   const uint16_t ns_port,
                   D2UpdateMessage& update,

+ 20 - 15
src/bin/d2/nc_add.cc

@@ -217,8 +217,8 @@ NameAddTransaction::addingFwdAddrsHandler() {
             // @note For now we treat OTHER as an IO error like TIMEOUT. It
             // is not entirely clear if this is accurate.
             LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_ADD_IO_ERROR)
-                      .arg(getCurrentServer()->getIpAddress())
-                      .arg(getNcr()->getFqdn());
+                      .arg(getNcr()->getFqdn())
+                      .arg(getCurrentServer()->getIpAddress());
 
             retryTransition(SELECTING_FWD_SERVER_ST);
             break;
@@ -227,8 +227,8 @@ NameAddTransaction::addingFwdAddrsHandler() {
             // A response was received but was corrupt. Retry it like an IO
             // error.
             LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_ADD_RESP_CORRUPT)
-                      .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->getIpAddress())
+                      .arg(getNcr()->getFqdn());
 
             retryTransition(SELECTING_FWD_SERVER_ST);
             break;
@@ -236,7 +236,8 @@ NameAddTransaction::addingFwdAddrsHandler() {
         default:
             // Any other value and we will fail this transaction, something
             // bigger is wrong.
-            LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_ADD_UNKNOWN_FAILURE)
+            LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_ADD_BAD_DNSCLIENT_STATUS)
+                      .arg(getDnsUpdateStatus())
                       .arg(getNcr()->getFqdn())
                       .arg(getCurrentServer()->getIpAddress());
 
@@ -317,8 +318,8 @@ NameAddTransaction::replacingFwdAddrsHandler() {
             // @note For now we treat OTHER as an IO error like TIMEOUT. It
             // is not entirely clear if this is accurate.
             LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_REPLACE_IO_ERROR)
-                      .arg(getCurrentServer()->getIpAddress())
-                      .arg(getNcr()->getFqdn());
+                      .arg(getNcr()->getFqdn())
+                      .arg(getCurrentServer()->getIpAddress());
 
             // If we are out of retries on this server, we go back and start
             // all over on a new server.
@@ -329,8 +330,8 @@ NameAddTransaction::replacingFwdAddrsHandler() {
             // A response was received but was corrupt. Retry it like an IO
             // error.
             LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_REPLACE_RESP_CORRUPT)
-                      .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->getIpAddress())
+                      .arg(getNcr()->getFqdn());
 
             // If we are out of retries on this server, we go back and start
             // all over on a new server.
@@ -340,7 +341,9 @@ NameAddTransaction::replacingFwdAddrsHandler() {
         default:
             // Any other value and we will fail this transaction, something
             // bigger is wrong.
-            LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_REPLACE_UNKNOWN_FAILURE)
+            LOG_ERROR(dctl_logger,
+                      DHCP_DDNS_FORWARD_REPLACE_BAD_DNSCLIENT_STATUS)
+                      .arg(getDnsUpdateStatus())
                       .arg(getNcr()->getFqdn())
                       .arg(getCurrentServer()->getIpAddress());
 
@@ -438,8 +441,8 @@ NameAddTransaction::replacingRevPtrsHandler() {
             // @note For now we treat OTHER as an IO error like TIMEOUT. It
             // is not entirely clear if this is accurate.
             LOG_ERROR(dctl_logger, DHCP_DDNS_REVERSE_REPLACE_IO_ERROR)
-                      .arg(getCurrentServer()->getIpAddress())
-                      .arg(getNcr()->getFqdn());
+                      .arg(getNcr()->getFqdn())
+                      .arg(getCurrentServer()->getIpAddress());
 
             // If we are out of retries on this server, we go back and start
             // all over on a new server.
@@ -450,8 +453,8 @@ NameAddTransaction::replacingRevPtrsHandler() {
             // A response was received but was corrupt. Retry it like an IO
             // error.
             LOG_ERROR(dctl_logger, DHCP_DDNS_REVERSE_REPLACE_RESP_CORRUPT)
-                      .arg(getNcr()->getFqdn())
-                      .arg(getCurrentServer()->getIpAddress());
+                      .arg(getCurrentServer()->getIpAddress())
+                      .arg(getNcr()->getFqdn());
 
             // If we are out of retries on this server, we go back and start
             // all over on a new server.
@@ -461,7 +464,9 @@ NameAddTransaction::replacingRevPtrsHandler() {
         default:
             // Any other value and we will fail this transaction, something
             // bigger is wrong.
-            LOG_ERROR(dctl_logger, DHCP_DDNS_REVERSE_REPLACE_UNKNOWN_FAILURE)
+            LOG_ERROR(dctl_logger,
+                      DHCP_DDNS_REVERSE_REPLACE_BAD_DNSCLIENT_STATUS)
+                      .arg(getDnsUpdateStatus())
                       .arg(getNcr()->getFqdn())
                       .arg(getCurrentServer()->getIpAddress());
 

+ 32 - 7
src/bin/d2/nc_add.h

@@ -129,12 +129,16 @@ protected:
     /// @throw StateModelError if an event value is undefined.
     virtual void verifyStates();
 
-    /// @brief State handler for R
+    /// @brief State handler for READY_ST.
     ///
     /// Entered from:
     /// - INIT_ST with next event of START_EVT
     ///
-    /// Servers as the starting state handler.
+    /// The READY_ST is the state the model transitions into when the inherited
+    /// method, startTransaction() is invoked.  This handler, therefore, as the
+    /// is the entry point into the state model execuition.h  Its primary task
+    /// is to determine whether to start with a forward DNS change or a
+    /// reverse DNS change.
     ///
     /// Transitions to:
     /// - SELECTING_FWD_SERVER_ST with next event of SERVER_SELECT_ST if request
@@ -144,7 +148,7 @@ protected:
     /// includes only a reverse change.
     ///
     /// @throw NameAddTransactionError if upon entry next event is not
-    /// START_EVT.EADY_ST.
+    /// START_EVT.READY_ST.
     void readyHandler();
 
     /// @brief State handler for SELECTING_FWD_SERVER_ST.
@@ -209,7 +213,14 @@ protected:
     /// schedules an asynchronous send via sendUpdate(), and returns.  Note
     /// that sendUpdate will post NOP_EVT as next event.
     ///
-    /// If the handler is invoked with a next event of IO_COMPELTED_EVT, then
+    /// Posting the NOP_EVT will cause runModel() to suspend execution of
+    /// the state model thus affecting a "wait" for the update IO to complete.
+    /// Update completion occurs via the DNSClient callback operator() method
+    /// inherited from NameChangeTransaction.  When invoked this callback will
+    /// post a next event of IO_COMPLETED_EVT and then invoke runModel which
+    /// resumes execution of the state model.
+    ///
+    /// When the handler is invoked with a next event of IO_COMPELTED_EVT,
     /// the DNS update status is checked and acted upon accordingly:
     ///
     /// Transitions to:
@@ -239,7 +250,7 @@ protected:
     /// SERVER_SELECTED_EVT or IO_COMPLETE_EVT.
     void addingFwdAddrsHandler();
 
-    /// @brief State handler for .
+    /// @brief State handler for REPLACING_FWD_ADDRS_ST.
     ///
     /// Entered from:
     /// - ADDING_FWD_ADDRS_ST with next event of FQDN_IN_USE_EVT
@@ -252,7 +263,14 @@ protected:
     /// via sendUpdate(), and returns.  Note that sendUpdate will post NOP_EVT
     /// as the next event.
     ///
-    /// If the handler is invoked with a next event of IO_COMPELTED_EVT, then
+    /// Posting the NOP_EVT will cause runModel() to suspend execution of
+    /// the state model thus affecting a "wait" for the update IO to complete.
+    /// Update completion occurs via the DNSClient callback operator() method
+    /// inherited from NameChangeTransaction.  When invoked this callback will
+    /// post a next event of IO_COMPLETED_EVT and then invoke runModel which
+    /// resumes execution of the state model.
+    ///
+    /// When the handler is invoked with a next event of IO_COMPELTED_EVT,
     /// the DNS update status is checked and acted upon accordingly:
     ///
     /// Transitions to:
@@ -296,7 +314,14 @@ protected:
     /// add request, schedules an asynchronous send via sendUpdate(), and
     /// returns.  Note that sendUpdate will post NOP_EVT as next event.
     ///
-    /// If the handler is invoked with a next event of IO_COMPELTED_EVT, then
+    /// Posting the NOP_EVT will cause runModel() to suspend execution of
+    /// the state model thus affecting a "wait" for the update IO to complete.
+    /// Update completion occurs via the DNSClient callback operator() method
+    /// inherited from NameChangeTransaction.  When invoked this callback will
+    /// post a next event of IO_COMPLETED_EVT and then invoke runModel which
+    /// resumes execution of the state model.
+    ///
+    /// When the handler is invoked with a next event of IO_COMPELTED_EVT,
     /// the DNS update status is checked and acted upon accordingly:
     ///
     /// Transitions to:

+ 6 - 8
src/bin/d2/nc_trans.cc

@@ -102,7 +102,7 @@ NameChangeTransaction::sendUpdate(bool /* use_tsig_ */) {
 
         // @todo time out should ultimately be configurable, down to
         // server level?
-        dns_client_->doUpdate(io_service_, current_server_->getIpAddress(),
+        dns_client_->doUpdate(*io_service_, current_server_->getIpAddress(),
                               current_server_->getPort(), *dns_update_request_,
                               DNS_UPDATE_DEFAULT_TIMEOUT);
 
@@ -116,11 +116,9 @@ NameChangeTransaction::sendUpdate(bool /* use_tsig_ */) {
         // mechansisms and manifested as an unsuccessful IO statu in the
         // DNSClient callback.  Any problem here most likely means the request
         // is corrupt in some way and cannot be completed, therefore we will
-        // log it, mark it as failed, and set next event to NOP_EVT.
-        LOG_ERROR(dctl_logger, DHCP_DDNS_TRANS_SEND_EROR).arg(ex.what());
-        setNcrStatus(dhcp_ddns::ST_FAILED);
-        // @todo is this right?
-        postNextEvent(NOP_EVT);
+        // log it and transition it to failure.
+        LOG_ERROR(dctl_logger, DHCP_DDNS_TRANS_SEND_ERROR).arg(ex.what());
+        transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
     }
 }
 
@@ -183,7 +181,7 @@ NameChangeTransaction::onModelFailure(const std::string& explanation) {
 }
 
 void
-NameChangeTransaction::retryTransition(int server_sel_state) {
+NameChangeTransaction::retryTransition(const int server_sel_state) {
     if (update_attempts_ < MAX_UPDATE_TRIES_PER_SERVER) {
         // Re-enter the current state with same server selected.
         transition(getCurrState(), SERVER_SELECTED_EVT);
@@ -230,7 +228,7 @@ NameChangeTransaction::setReverseChangeCompleted(const bool value) {
 }
 
 void
-NameChangeTransaction::setUpdateAttempts(size_t value) {
+NameChangeTransaction::setUpdateAttempts(const size_t value) {
     update_attempts_ = value;
 }
 

+ 31 - 6
src/bin/d2/nc_trans.h

@@ -197,7 +197,15 @@ public:
     virtual void operator()(DNSClient::Status status);
 
 protected:
-    /// @todo
+    /// @brief Send the update request to the current server.
+    ///
+    /// This method increments the update attempt count and then passes the
+    /// current update request to the DNSClient instance to be sent to the
+    /// currently selected server.  Since the send is asynchronous, the method
+    /// posts NOP_EVT as the next event and then returns.
+    ///
+    /// If an exception occurs it will be logged and and the transaction will
+    /// be failed.
     virtual void sendUpdate(bool use_tsig_ = false);
 
     /// @brief Adds events defined by NameChangeTransaction to the event set.
@@ -257,8 +265,19 @@ protected:
     /// @param explanation is text detailing the error
     virtual void onModelFailure(const std::string& explanation);
 
-    /// @todo
-    void retryTransition(int server_sel_state);
+    /// @brief Determines the state and next event based on update attempts.
+    ///
+    /// This method will post a next event of SERVER_SELECTED_EVT to the
+    /// current state if the number of udpate attempts has not reached the
+    /// maximum allowed.
+    ///
+    /// If the maximum number of attempts has been reached, it will transition
+    /// to the given state with a next event of SERVER_IO_ERROR_EVT.
+    ///
+    /// @param server_sel_state  State to transition to if maximum attempts
+    /// have been tried.
+    ///
+    void retryTransition(const int server_sel_state);
 
     /// @brief Sets the update request packet to the given packet.
     ///
@@ -278,7 +297,7 @@ protected:
     /// @param response is the new response packet to assign.
     void setDnsUpdateResponse(D2UpdateMessagePtr& response);
 
-    /// @brief Destroys the current update respons packet.
+    /// @brief Destroys the current update response packet.
     void clearDnsUpdateResponse();
 
     /// @brief Sets the forward change completion flag to the given value.
@@ -333,7 +352,13 @@ protected:
     /// @brief Sets the update attempt count to the given value.
     ///
     /// @param value is the new value to assign.
-    void setUpdateAttempts(size_t value);
+    void setUpdateAttempts(const size_t value);
+
+    /// @todo
+    const IOServicePtr& getIOService() {
+        return (io_service_);
+    }
+
 
 public:
     /// @brief Fetches the NameChangeRequest for this transaction.
@@ -465,7 +490,7 @@ private:
     /// list, which may be beyond the end of the list.
     size_t next_server_pos_;
 
-    // @todo
+    /// @brief Number of transmit attempts for the current request.
     size_t update_attempts_;
 };
 

+ 5 - 1
src/bin/d2/tests/d2_queue_mgr_unittests.cc

@@ -78,8 +78,12 @@ const long TEST_TIMEOUT = 5 * 1000;
 
 /// @brief Tests that construction with max queue size of zero is not allowed.
 TEST(D2QueueMgrBasicTest, construction1) {
-    IOServicePtr io_service(new isc::asiolink::IOService());
+    IOServicePtr io_service;
+
+    // Verify that constructing with null IOServicePtr is not allowed.
+    EXPECT_THROW((D2QueueMgr(io_service)), D2QueueMgrError);
 
+    io_service.reset(new isc::asiolink::IOService());
     // Verify that constructing with max queue size of zero is not allowed.
     EXPECT_THROW(D2QueueMgr(io_service, 0), D2QueueMgrError);
 }

+ 6 - 0
src/bin/d2/tests/d2_update_mgr_unittests.cc

@@ -181,6 +181,12 @@ TEST(D2UpdateMgr, construction) {
 
     ASSERT_NO_THROW(cfg_mgr.reset(new D2CfgMgr()));
 
+    // Verify that constructor fails with invalid io_service.
+    io_service.reset();
+    EXPECT_THROW(D2UpdateMgr(queue_mgr, cfg_mgr, io_service),
+                 D2UpdateMgrError);
+    io_service.reset(new isc::asiolink::IOService());
+
     // Verify that max transactions cannot be zero.
     EXPECT_THROW(D2UpdateMgr(queue_mgr, cfg_mgr, io_service, 0),
                  D2UpdateMgrError);

+ 9 - 9
src/bin/d2/tests/dns_client_unittests.cc

@@ -61,7 +61,7 @@ const long TEST_TIMEOUT = 5 * 1000;
 // timeout is hit. This will result in test failure.
 class DNSClientTest : public virtual ::testing::Test, DNSClient::Callback {
 public:
-    IOServicePtr service_;
+    IOService service_;
     D2UpdateMessagePtr response_;
     DNSClient::Status status_;
     uint8_t receive_buffer_[MAX_SIZE];
@@ -79,11 +79,11 @@ public:
     // in case when response from the server is not received. Tests output would
     // become messy if such errors were logged.
     DNSClientTest()
-        : service_(new isc::asiolink::IOService()),
+        : service_(),
           status_(DNSClient::SUCCESS),
           corrupt_response_(false),
           expect_response_(true),
-          test_timer_(*service_) {
+          test_timer_(service_) {
         asiodns::logger.setSeverity(isc::log::INFO);
         response_.reset(new D2UpdateMessage(D2UpdateMessage::INBOUND));
         dns_client_.reset(new DNSClient(response_, this));
@@ -108,7 +108,7 @@ public:
     // @param status A status code returned by DNSClient.
     virtual void operator()(DNSClient::Status status) {
         status_ = status;
-        service_->stop();
+        service_.stop();
 
         if (expect_response_) {
             if (!corrupt_response_) {
@@ -139,7 +139,7 @@ public:
     //
     // This callback stops all running (hanging) tasks on IO service.
     void testTimeoutHandler() {
-        service_->stop();
+        service_.stop();
         FAIL() << "Test timeout hit.";
     }
 
@@ -261,7 +261,7 @@ public:
 
         // Set the response wait time to 0 so as our test is not hanging. This
         // should cause instant timeout.
-        const int timeout = 0;
+        const int timeout = 500;
         // The doUpdate() function starts asynchronous message exchange with DNS
         // server. When message exchange is done or timeout occurs, the
         // completion callback will be triggered. The doUpdate function returns
@@ -271,7 +271,7 @@ public:
 
         // This starts the execution of tasks posted to IOService. run() blocks
         // until stop() is called in the completion callback function.
-        service_->run();
+        service_.run();
 
     }
 
@@ -295,7 +295,7 @@ public:
         // responses. The reuse address option is set so as both sockets can
         // use the same address. This new socket is bound to the test address
         // and port, where requests will be sent.
-        udp::socket udp_socket(service_->get_io_service(), asio::ip::udp::v4());
+        udp::socket udp_socket(service_.get_io_service(), asio::ip::udp::v4());
         udp_socket.set_option(socket_base::reuse_address(true));
         udp_socket.bind(udp::endpoint(address::from_string(TEST_ADDRESS),
                                       TEST_PORT));
@@ -334,7 +334,7 @@ public:
 
         // Kick of the message exchange by actually running the scheduled
         // "send" and "receive" operations.
-        service_->run();
+        service_.run();
 
         udp_socket.close();
 

File diff suppressed because it is too large
+ 377 - 323
src/bin/d2/tests/nc_add_unittests.cc


+ 491 - 29
src/bin/d2/tests/nc_trans_unittests.cc

@@ -12,7 +12,18 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <asiolink/interval_timer.h>
 #include <d2/nc_trans.h>
+#include <dns/opcode.h>
+#include <dns/messagerenderer.h>
+#include <log/logger_support.h>
+#include <log/macros.h>
+#include <util/buffer.h>
+
+#include <asio/ip/udp.hpp>
+#include <asio/socket_base.hpp>
+#include <asio.hpp>
+#include <asio/error_code.hpp>
 
 #include <boost/function.hpp>
 #include <boost/bind.hpp>
@@ -24,6 +35,8 @@ using namespace isc::d2;
 
 namespace {
 
+const size_t MAX_MSG_SIZE = 1024;
+
 /// @brief Test derivation of NameChangeTransaction for exercising state
 /// model mechanics.
 ///
@@ -40,6 +53,8 @@ public:
     // NameChangeStub events
     static const int SEND_UPDATE_EVT = NCT_DERIVED_EVENT_MIN + 2;
 
+    bool use_stub_callback_;
+
     /// @brief Constructor
     ///
     /// Parameters match those needed by NameChangeTransaction.
@@ -48,13 +63,47 @@ public:
                    DdnsDomainPtr forward_domain,
                    DdnsDomainPtr reverse_domain)
         : NameChangeTransaction(io_service, ncr, forward_domain,
-                                reverse_domain) {
+                                reverse_domain),
+                                use_stub_callback_(false) {
     }
 
     /// @brief Destructor
     virtual ~NameChangeStub() {
     }
 
+    /// @brief DNSClient callback
+    /// Overrides the callback in NameChangeTranscation to allow testing
+    /// sendUpdate without incorporating exectution of the state model
+    /// into the test.
+    /// It sets the DNS status update and posts IO_COMPLETED_EVT as does
+    /// the normal callback, but rather than invoking runModel it stops
+    /// the IO service.  This allows tests to be constructed that consisted
+    /// of generating a DNS request and invoking sendUpdate to post it and
+    /// wait for response.
+    virtual void operator()(DNSClient::Status status) {
+        if (use_stub_callback_) {
+            setDnsUpdateStatus(status);
+            postNextEvent(IO_COMPLETED_EVT);
+            getIOService()->stop();
+        } else {
+            // For tests which need to use the real callback.
+            NameChangeTransaction::operator()(status);
+        }
+    }
+
+    /// @brief Some tests require a server to be selected.  This method
+    /// selects the first server in the forward domain without involving
+    /// state model execution to do so.
+    bool selectFwdServer() {
+        if (getForwardDomain()) {
+            initServerSelection(getForwardDomain());
+            selectNextServer();
+            return (getCurrentServer());
+        }
+
+        return (false);
+    }
+
     /// @brief Empty handler used to statisfy map verification.
     void dummyHandler() {
         isc_throw(NameChangeTransactionError,
@@ -182,12 +231,15 @@ public:
         // Invoke the base call implementation first.
         NameChangeTransaction::verifyStates();
 
-        // Define our states.
+        // Check our states.
         getState(DOING_UPDATE_ST);
     }
 
     // Expose the protected methods to be tested.
     using StateModel::runModel;
+    using StateModel::postNextEvent;
+    using StateModel::setState;
+    using StateModel::initDictionaries;
     using NameChangeTransaction::initServerSelection;
     using NameChangeTransaction::selectNextServer;
     using NameChangeTransaction::getCurrentServer;
@@ -203,6 +255,151 @@ public:
     using NameChangeTransaction::getReverseChangeCompleted;
     using NameChangeTransaction::setForwardChangeCompleted;
     using NameChangeTransaction::setReverseChangeCompleted;
+    using NameChangeTransaction::setUpdateAttempts;
+    using NameChangeTransaction::transition;
+    using NameChangeTransaction::retryTransition;
+    using NameChangeTransaction::sendUpdate;
+};
+
+typedef boost::shared_ptr<asio::ip::udp::socket> SocketPtr;
+
+/// @brief This class simulates a DNS server.  It is capable of performing
+/// an asynchronous read, governed by an IOService, and responding to received
+/// requests in a given manner.
+class FauxServer {
+public:
+    enum  ResponseMode {
+        USE_RCODE,   // Generate a response with a given RCODE
+        CORRUPT_RESP  // Generate a corrupt response
+    };
+
+    asiolink::IOService& io_service_;
+    asiolink::IOAddress& address_;
+    size_t port_;
+    SocketPtr server_socket_;
+    asio::ip::udp::endpoint remote_;
+    uint8_t receive_buffer_[MAX_MSG_SIZE];
+
+    /// @brief Constructor
+    ///
+    /// @param io_service IOService to be used for socket IO.
+    /// @param address  IP address at which the server should listen.
+    /// @param port Port number at which the server should listen.
+    FauxServer(asiolink::IOService& io_service, asiolink::IOAddress& address,
+               size_t port)
+        : io_service_(io_service), address_(address), port_(port),
+          server_socket_() {
+        server_socket_.reset(new asio::ip::udp::
+                             socket(io_service_.get_io_service(),
+                                    asio::ip::udp::v4()));
+        server_socket_->set_option(asio::socket_base::reuse_address(true));
+        server_socket_->bind(asio::ip::udp::
+                             endpoint(address_.getAddress(), port_));
+    }
+
+    /// @brief Destructor
+    virtual ~FauxServer() {
+    }
+
+    /// @brief Initiates an asyncrhonrous receive
+    ///
+    /// Starts the server listening for requests.  Upon completion of the
+    /// the listen, the callback method, requestHandler, is invoked.
+    ///
+    /// @param response_mode Selects how the server responds to a request
+    /// @param response_rcode The Rcode value set in the response. Not used
+    /// for all modes.
+    void receive (const ResponseMode& response_mode,
+                  const dns::Rcode& response_rcode=dns::Rcode::NOERROR()) {
+
+        server_socket_->async_receive_from(asio::buffer(receive_buffer_,
+                                                   sizeof(receive_buffer_)),
+                                      remote_,
+                                      boost::bind(&FauxServer::requestHandler,
+                                                  this, _1, _2,
+                                                  response_mode,
+                                                  response_rcode));
+    }
+
+    /// @brief Socket IO Completion callback
+    ///
+    /// This method servers as the Server's UDP socket receive callback handler.
+    /// When the receive completes the handler is invoked with the
+    /// @param error result code of the recieve (determined by asio layer)
+    /// @param bytes_recvd number of bytes received, if any
+    /// @param response_mode type of response the handler should produce
+    /// @param response_rcode value of Rcode in the response constructed by
+    /// handler
+    void requestHandler(const asio::error_code& error,
+                        std::size_t bytes_recvd,
+                        const ResponseMode& response_mode,
+                        const dns::Rcode& response_rcode) {
+
+        // If we encountered an error or received no data then fail.
+        // We expect the client to send good requests.
+        if (error.value() != 0 || bytes_recvd < 1) {
+            ADD_FAILURE() << "FauxServer receive failed" << error.message();
+            return;
+        }
+
+        // We have a successfully received data. We need to turn it into
+        // a request in order to build a proper response.
+        // Note D2UpdateMessage is geared towards making requests and
+        // reading responses.  This is the opposite perspective so we have
+        // to a bit of roll-your-own here.
+        dns::Message request(dns::Message::PARSE);
+        util::InputBuffer request_buf(receive_buffer_, bytes_recvd);
+        try {
+            request.fromWire(request_buf);
+        } catch (const std::exception& ex) {
+            // If the request cannot be parsed, then fail the test.
+            // We expect the client to send good requests.
+            ADD_FAILURE() << "FauxServer request is corrupt:" << ex.what();
+            return;
+        }
+
+        // The request parsed ok, so let's build a response.
+        // We must use the QID we received in the response or IOFetch will
+        // toss the response out, resulting in eventual timeout.
+        // We fill in the zone with data we know is from the "server".
+        dns::Message response(dns::Message::RENDER);
+        response.setQid(request.getQid());
+        dns::Question question(dns::Name("response.example.com"),
+                               dns::RRClass::ANY(), dns::RRType::SOA());
+        response.addQuestion(question);
+        response.setOpcode(dns::Opcode(dns::Opcode::UPDATE_CODE));
+        response.setHeaderFlag(dns::Message::HEADERFLAG_QR, true);
+
+        // Set the response Rcode to value passed in, default is NOERROR.
+        response.setRcode(response_rcode);
+
+        // Render the response to a buffer.
+        dns::MessageRenderer renderer;
+        util::OutputBuffer response_buf(MAX_MSG_SIZE);
+        renderer.setBuffer(&response_buf);
+        response.toWire(renderer);
+
+        // If mode is to ship garbage, then stomp on part of the rendered
+        // message.
+        if (response_mode == CORRUPT_RESP) {
+            response_buf.writeUint16At(0xFFFF, 2);
+        }
+
+        // Ship the reponse via synchronous send.
+        try {
+            int cnt = server_socket_->send_to(asio::
+                                              buffer(response_buf.getData(),
+                                                     response_buf.getLength()),
+                                              remote_);
+            // Make sure we sent what we expect to send.
+            if (cnt != response_buf.getLength()) {
+                ADD_FAILURE() << "FauxServer sent: " << cnt << " expected: "
+                          << response_buf.getLength();
+            }
+        } catch (const std::exception& ex) {
+            ADD_FAILURE() << "FauxServer send failed: " << ex.what();
+        }
+    }
 };
 
 /// @brief Defines a pointer to a NameChangeStubPtr instance.
@@ -217,16 +414,45 @@ public:
     IOServicePtr io_service_;
     DdnsDomainPtr forward_domain_;
     DdnsDomainPtr reverse_domain_;
-    
-    NameChangeTransactionTest() : io_service_(new isc::asiolink::IOService()) {
-    } 
+    asiolink::IntervalTimer timer_;
+    int run_time_;
+
+    NameChangeTransactionTest()
+        : io_service_(new isc::asiolink::IOService()), timer_(*io_service_),
+         run_time_(0) {
+    }
 
     virtual ~NameChangeTransactionTest() {
     }
 
-    /// @brief Instantiates a NameChangeStub built around a canned
-    /// NameChangeRequest.
+    /// @brief Run the IO service for no more than a given amount of time.
+    ///
+    /// Uses an IntervalTimer to interrupt the invocation of IOService run(),
+    /// after the given number of milliseconds elapse.  The timer executes
+    /// the timesUp() method if it expires.
+    ///
+    /// @param run_time amount of time in milliseconds to allow run to execute.
+    void runTimedIO(int run_time) {
+        run_time_ = run_time;
+        timer_.setup(boost::bind(&NameChangeTransactionTest::timesUp, this),
+                     run_time_);
+        io_service_->run();
+    }
+
+    /// @brief IO Timer expiration handler
+    ///
+    /// Stops the IOSerivce and FAILs the current test.
+    void timesUp() {
+        io_service_->stop();
+        FAIL() << "Test Time: " << run_time_ << " expired";
+    }
+
+    /// @brief  Instantiates a NameChangeStub test transaction
+    /// The transaction is constructed around a predefined (i.e "canned")
+    /// NameChangeRequest. The request has both forward and reverse DNS
+    /// changes requested, and both forward and reverse domains are populated.
     NameChangeStubPtr makeCannedTransaction() {
+        // NCR in JSON form.
         const char* msg_str =
             "{"
             " \"change_type\" : 0 , "
@@ -239,28 +465,36 @@ public:
             " \"lease_length\" : 1300 "
             "}";
 
+        // Create the request from JSON.
         dhcp_ddns::NameChangeRequestPtr ncr;
-
         DnsServerInfoStoragePtr servers(new DnsServerInfoStorage());
         DnsServerInfoPtr server;
-
         ncr = dhcp_ddns::NameChangeRequest::fromJSON(msg_str);
 
-        // make forward server list
+        // Make forward DdnsDomain with 2 forward servers.
         server.reset(new DnsServerInfo("forward.example.com",
-                                       isc::asiolink::IOAddress("1.1.1.1")));
+                                       isc::asiolink::IOAddress("127.0.0.1"),
+                                       5301));
+        servers->push_back(server);
+        server.reset(new DnsServerInfo("forward2.example.com",
+                                       isc::asiolink::IOAddress("127.0.0.1"),
+                                       5302));
+
         servers->push_back(server);
-        forward_domain_.reset(new DdnsDomain("*", "", servers));
+        forward_domain_.reset(new DdnsDomain("example.com.", "", servers));
 
-        // make reverse server list
-        servers->clear();
+        // Make reverse DdnsDomain with one reverse server.
+        servers.reset(new DnsServerInfoStorage());
         server.reset(new DnsServerInfo("reverse.example.com",
-                                       isc::asiolink::IOAddress("2.2.2.2")));
+                                       isc::asiolink::IOAddress("127.0.0.1"),
+                                       5301));
         servers->push_back(server);
-        reverse_domain_.reset(new DdnsDomain("*", "", servers));
+        reverse_domain_.reset(new DdnsDomain("2.168.192.in.addr.arpa.",
+                                             "", servers));
+
+        // Instantiate the transaction as would be done by update manager.
         return (NameChangeStubPtr(new NameChangeStub(io_service_, ncr,
                                   forward_domain_, reverse_domain_)));
-
     }
 
 };
@@ -386,21 +620,24 @@ TEST_F(NameChangeTransactionTest, accessors) {
     EXPECT_TRUE(name_change->getReverseChangeCompleted());
 }
 
+/// @brief Tests DNS update request accessor methods.
 TEST_F(NameChangeTransactionTest, dnsUpdateRequestAccessors) {
+    // Create a transction.
     NameChangeStubPtr name_change;
     ASSERT_NO_THROW(name_change = makeCannedTransaction());
-    // Verify that the DNS update request accessors.
-    D2UpdateMessagePtr req;
-    ASSERT_NO_THROW(req.reset(new D2UpdateMessage(D2UpdateMessage::OUTBOUND)));
 
-    // Post construction it is empty.
+    // Post transaction construction, there should not be an update request.
     EXPECT_FALSE(name_change->getDnsUpdateRequest());
 
-    /// @param request is the new request packet to assign.
+    // Create a request.
+    D2UpdateMessagePtr req;
+    ASSERT_NO_THROW(req.reset(new D2UpdateMessage(D2UpdateMessage::OUTBOUND)));
+
+    // Use the setter and then verify we can fetch the request.
     ASSERT_NO_THROW(name_change->setDnsUpdateRequest(req));
 
     // Post set, we should be able to fetch it.
-    EXPECT_TRUE(name_change->getDnsUpdateRequest());
+    ASSERT_TRUE(name_change->getDnsUpdateRequest());
 
     // Should be able to clear it.
     ASSERT_NO_THROW(name_change->clearDnsUpdateRequest());
@@ -409,18 +646,20 @@ TEST_F(NameChangeTransactionTest, dnsUpdateRequestAccessors) {
     EXPECT_FALSE(name_change->getDnsUpdateRequest());
 }
 
+/// @brief Tests DNS update request accessor methods.
 TEST_F(NameChangeTransactionTest, dnsUpdateResponseAccessors) {
+    // Create a transction.
     NameChangeStubPtr name_change;
     ASSERT_NO_THROW(name_change = makeCannedTransaction());
 
-    // Verify that the DNS update response accessors.
+    // Post transaction construction, there should not be an update response.
+    EXPECT_FALSE(name_change->getDnsUpdateResponse());
+
+    // Create a response.
     D2UpdateMessagePtr resp;
     ASSERT_NO_THROW(resp.reset(new D2UpdateMessage(D2UpdateMessage::INBOUND)));
 
-    // Post construction it is empty.
-    EXPECT_FALSE(name_change->getDnsUpdateResponse());
-
-    /// @param request is the new request packet to assign.
+    // Use the setter and then verify we can fetch the response.
     ASSERT_NO_THROW(name_change->setDnsUpdateResponse(resp));
 
     // Post set, we should be able to fetch it.
@@ -434,7 +673,6 @@ TEST_F(NameChangeTransactionTest, dnsUpdateResponseAccessors) {
 }
 
 
-
 /// @brief Tests event and state dictionary construction and verification.
 TEST_F(NameChangeTransactionTest, dictionaryCheck) {
     NameChangeStubPtr name_change;
@@ -656,4 +894,228 @@ TEST_F(NameChangeTransactionTest, failedUpdateTest) {
     EXPECT_FALSE(name_change->getForwardChangeCompleted());
 }
 
+/// @brief Tests update attempt accessors.
+TEST_F(NameChangeTransactionTest, updateAttempts) {
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+
+    // Post transaction construction, update attempts should be 0.
+    EXPECT_EQ(0, name_change->getUpdateAttempts());
+
+    // Set it to a known value.
+    name_change->setUpdateAttempts(5);
+
+    // Verify that the value is as expected.
+    EXPECT_EQ(5, name_change->getUpdateAttempts());
+}
+
+/// @brief Tests retryTransition method
+///
+/// Verifes that while the maximum number of update attempts has not
+/// been exceeded, the method will leave the state unchanged but post a
+/// SERVER_SELECTED_EVT.  Once the maximum is exceeded, the method should
+/// transition to the state given with a next event of SERVER_IO_ERROR_EVT.
+TEST_F(NameChangeTransactionTest, retryTransition) {
+    // Create the transaction.
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+
+    // Define dictionaries.
+    ASSERT_NO_THROW(name_change->initDictionaries());
+
+    // Transition to a known spot.
+    ASSERT_NO_THROW(name_change->transition(
+                    NameChangeStub::DOING_UPDATE_ST,
+                    NameChangeStub::SEND_UPDATE_EVT));
+
+    // Verify we are at the known spot.
+    ASSERT_EQ(NameChangeStub::DOING_UPDATE_ST,
+              name_change->getCurrState());
+    ASSERT_EQ(NameChangeStub::SEND_UPDATE_EVT,
+              name_change->getNextEvent());
+
+    // Verify that we have not exceeded maximum number of attempts.
+    EXPECT_TRUE(name_change->getUpdateAttempts() <
+                NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER);
+
+    // Call retryTransition.
+    ASSERT_NO_THROW(name_change->retryTransition(
+                    NameChangeTransaction::PROCESS_TRANS_FAILED_ST));
+
+    // Since the number of udpate attempts is less than the maximum allowed
+    // we should remain in our current state but with next event of
+    // SERVER_SELECTED_EVT posted.
+    ASSERT_EQ(NameChangeStub::DOING_UPDATE_ST,
+              name_change->getCurrState());
+    ASSERT_EQ(NameChangeTransaction::SERVER_SELECTED_EVT,
+              name_change->getNextEvent());
+
+    // Now set the number of attempts to the maximum.
+    name_change->setUpdateAttempts(NameChangeTransaction::
+                                   MAX_UPDATE_TRIES_PER_SERVER);
+    // Call retryTransition.
+    ASSERT_NO_THROW(name_change->retryTransition(
+                    NameChangeTransaction::PROCESS_TRANS_FAILED_ST));
+
+    // Since we have exceeded maximum attempts, we should tranisition to
+    // PROCESS_UPDATE_FAILD_ST with a next event of SERVER_IO_ERROR_EVT.
+    ASSERT_EQ(NameChangeTransaction::PROCESS_TRANS_FAILED_ST,
+              name_change->getCurrState());
+    ASSERT_EQ(NameChangeTransaction::SERVER_IO_ERROR_EVT,
+              name_change->getNextEvent());
+}
+
+/// @brief Tests sendUpdate method when underlying doUpdate throws.
+///
+/// DNSClient::doUpdate can throw for a variety of reasons. This tests
+/// sendUpdate handling of such a throw by passing doUpdate a request
+/// that will not render.
+TEST_F(NameChangeTransactionTest, sendUpdateDoUpdateFailure) {
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+    ASSERT_NO_THROW(name_change->initDictionaries());
+    ASSERT_TRUE(name_change->selectFwdServer());
+
+    // Set the transaction's request to an empty DNS update.
+    D2UpdateMessagePtr req;
+    ASSERT_NO_THROW(req.reset(new D2UpdateMessage(D2UpdateMessage::OUTBOUND)));
+    ASSERT_NO_THROW(name_change->setDnsUpdateRequest(req));
+
+    // Verify that sendUpdate does not throw, but it should fail because
+    // the requset won't render.
+    ASSERT_NO_THROW(name_change->sendUpdate());
+
+    // Verify that we transition to failed state and event.
+    ASSERT_EQ(NameChangeTransaction::PROCESS_TRANS_FAILED_ST,
+              name_change->getCurrState());
+    ASSERT_EQ(NameChangeTransaction::UPDATE_FAILED_EVT,
+              name_change->getNextEvent());
+}
+
+/// @brief Tests sendUpdate method when underlying doUpdate times out.
+TEST_F(NameChangeTransactionTest, sendUpdateTimeout) {
+    log::initLogger();
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+    ASSERT_NO_THROW(name_change->initDictionaries());
+    ASSERT_TRUE(name_change->selectFwdServer());
+
+    // Create a valid request.
+    D2UpdateMessagePtr req;
+    ASSERT_NO_THROW(req.reset(new D2UpdateMessage(D2UpdateMessage::OUTBOUND)));
+    ASSERT_NO_THROW(name_change->setDnsUpdateRequest(req));
+    req->setZone(dns::Name("request.example.com"), dns::RRClass::ANY());
+    req->setRcode(dns::Rcode(dns::Rcode::NOERROR_CODE));
+
+    // Set the flag to use the NameChangeStub's DNSClient callback.
+    name_change->use_stub_callback_ = true;
+
+    // Invoke sendUdpate.
+    ASSERT_NO_THROW(name_change->sendUpdate());
+
+    // Update attempt count should be 1, next event should be NOP_EVT.
+    EXPECT_EQ(1, name_change->getUpdateAttempts());
+    ASSERT_EQ(NameChangeTransaction::NOP_EVT,
+              name_change->getNextEvent());
+
+    // Run IO a bit longer than maximum allowed to permit timeout logic to
+    // execute.
+    runTimedIO(NameChangeTransaction::DNS_UPDATE_DEFAULT_TIMEOUT + 100);
+
+    // Verify that next event is IO_COMPLETED_EVT and DNS status is TIMEOUT.
+    ASSERT_EQ(NameChangeTransaction::IO_COMPLETED_EVT,
+              name_change->getNextEvent());
+    ASSERT_EQ(DNSClient::TIMEOUT, name_change->getDnsUpdateStatus());
+}
+
+/// @brief Tests sendUpdate method when it receives a corrupt respons from
+/// the server.
+TEST_F(NameChangeTransactionTest, sendUpdateCorruptResponse) {
+    log::initLogger();
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+    ASSERT_NO_THROW(name_change->initDictionaries());
+    ASSERT_TRUE(name_change->selectFwdServer());
+
+    // Create a server and start it listening.
+    asiolink::IOAddress address("127.0.0.1");
+    FauxServer server(*io_service_, address, 5301);
+    server.receive(FauxServer::CORRUPT_RESP);
+
+    // Create a valid request for the transaction.
+    D2UpdateMessagePtr req;
+    ASSERT_NO_THROW(req.reset(new D2UpdateMessage(D2UpdateMessage::OUTBOUND)));
+    ASSERT_NO_THROW(name_change->setDnsUpdateRequest(req));
+    req->setZone(dns::Name("request.example.com"), dns::RRClass::ANY());
+    req->setRcode(dns::Rcode(dns::Rcode::NOERROR_CODE));
+
+    // Set the flag to use the NameChangeStub's DNSClient callback.
+    name_change->use_stub_callback_ = true;
+
+    // Invoke sendUdpate.
+    ASSERT_NO_THROW(name_change->sendUpdate());
+
+    // Update attempt count should be 1, next event should be NOP_EVT.
+    EXPECT_EQ(1, name_change->getUpdateAttempts());
+    ASSERT_EQ(NameChangeTransaction::NOP_EVT,
+              name_change->getNextEvent());
+
+    // Run the IO for 500 ms.  This should be more than enough time.
+    runTimedIO(500);
+
+    // Verify that next event is IO_COMPLETED_EVT and DNS status is INVALID.
+    ASSERT_EQ(NameChangeTransaction::IO_COMPLETED_EVT,
+              name_change->getNextEvent());
+    ASSERT_EQ(DNSClient::INVALID_RESPONSE, name_change->getDnsUpdateStatus());
+}
+
+/// @brief Tests sendUpdate method when the exchange succeeds.
+TEST_F(NameChangeTransactionTest, sendUpdate) {
+    log::initLogger();
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+    ASSERT_NO_THROW(name_change->initDictionaries());
+    ASSERT_TRUE(name_change->selectFwdServer());
+
+    // Create a server and start it listening.
+    asiolink::IOAddress address("127.0.0.1");
+    FauxServer server(*io_service_, address, 5301);
+    server.receive (FauxServer::USE_RCODE, dns::Rcode::NOERROR());
+
+    // Create a valid request for the transaction.
+    D2UpdateMessagePtr req;
+    ASSERT_NO_THROW(req.reset(new D2UpdateMessage(D2UpdateMessage::OUTBOUND)));
+    ASSERT_NO_THROW(name_change->setDnsUpdateRequest(req));
+    req->setZone(dns::Name("request.example.com"), dns::RRClass::ANY());
+    req->setRcode(dns::Rcode(dns::Rcode::NOERROR_CODE));
+
+    // Set the flag to use the NameChangeStub's DNSClient callback.
+    name_change->use_stub_callback_ = true;
+
+    // Invoke sendUdpate.
+    ASSERT_NO_THROW(name_change->sendUpdate());
+
+    // Update attempt count should be 1, next event should be NOP_EVT.
+    EXPECT_EQ(1, name_change->getUpdateAttempts());
+    ASSERT_EQ(NameChangeTransaction::NOP_EVT,
+              name_change->getNextEvent());
+
+    // Run the IO for 500 ms.  This should be more than enough time.
+    runTimedIO(500);
+
+    // Verify that next event is IO_COMPLETED_EVT and DNS status is SUCCESS.
+    ASSERT_EQ(NameChangeTransaction::IO_COMPLETED_EVT,
+              name_change->getNextEvent());
+    ASSERT_EQ(DNSClient::SUCCESS, name_change->getDnsUpdateStatus());
+
+    // Verify that we have a response and it's Rcode is NOERROR,
+    // and the zone is as expected.
+    D2UpdateMessagePtr response = name_change->getDnsUpdateResponse();
+    ASSERT_TRUE(response);
+    ASSERT_EQ(dns::Rcode::NOERROR().getCode(), response->getRcode().getCode());
+    D2ZonePtr zone = response->getZone();
+    EXPECT_TRUE(zone);
+    EXPECT_EQ("response.example.com.", zone->getName().toText());
+}
+
 }