[3086] Initial implementation of NameChangeTransaction class

Interim commit for 3086 which includes the preliminary implementation
of the base class, NameChangeTransaction.  b10-dhcp-ddns module will use
derivations of this class to carry out NameChangeRequests.

src/bin/d2/Makefile.am

@@ -59,6 +59,7 @@ b10_dhcp_ddns_SOURCES += d2_update_message.cc d2_update_message.h
 b10_dhcp_ddns_SOURCES += d2_update_mgr.cc d2_update_mgr.h
 b10_dhcp_ddns_SOURCES += d2_zone.cc d2_zone.h
 b10_dhcp_ddns_SOURCES += dns_client.cc dns_client.h
+b10_dhcp_ddns_SOURCES += nc_trans.cc nc_trans.h
 
 nodist_b10_dhcp_ddns_SOURCES = d2_messages.h d2_messages.cc
 EXTRA_DIST += d2_messages.mes

src/bin/d2/d2_messages.mes

@@ -252,3 +252,9 @@ in event loop.
 % DHCP_DDNS_SHUTDOWN application received shutdown command with args: %1
 This is informational message issued when the application has been instructed
 to shut down by the controller.
+
+% DHCP_DDNS_TRANS_PROCESS_EROR application encountered an unexpected error while carrying out a NameChangeRequest: %1
+This is error message issued when the application fails to process a 
+NameChangeRequest correctly. Some or all of the DNS updates requested as part
+of this update did not succeed. This is a programmatic error and should be
+reported. 

src/bin/d2/d2_update_mgr.cc

@@ -108,6 +108,12 @@ 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;
         }
     }

src/bin/d2/d2_update_mgr.h

@@ -22,6 +22,7 @@
 #include <d2/d2_log.h>
 #include <d2/d2_queue_mgr.h>
 #include <d2/d2_cfg_mgr.h>
+#include <d2/nc_trans.h>
 
 #include <boost/noncopyable.hpp>
 #include <boost/shared_ptr.hpp>
@@ -37,58 +38,9 @@ public:
         isc::Exception(file, line, what) { };
 };
 
-//@{
-/// @todo  This is a stub implementation of NameChangeTransaction that is here
-/// strictly to facilitate development of D2UpdateMgr. It will move to its own
-/// source file(s) once NameChangeTransaction class development begins.
-
-/// @brief Defines the key for transactions.
-typedef isc::dhcp_ddns::D2Dhcid TransactionKey;
-
-class NameChangeTransaction {
-public:
-    NameChangeTransaction(isc::asiolink::IOService& io_service,
-                          dhcp_ddns::NameChangeRequestPtr& ncr,
-                          DdnsDomainPtr forward_domain,
-                          DdnsDomainPtr reverse_domain)
-    : io_service_(io_service), ncr_(ncr), forward_domain_(forward_domain),
-      reverse_domain_(reverse_domain) {
-    }
-
-    ~NameChangeTransaction(){
-    }
-
-    const dhcp_ddns::NameChangeRequestPtr& getNcr() const {
-        return (ncr_);
-    }
-
-    const TransactionKey& getTransactionKey() const {
-        return (ncr_->getDhcid());
-    }
-
-    dhcp_ddns::NameChangeStatus getNcrStatus() const {
-        return (ncr_->getStatus());
-    }
-
-private:
-    isc::asiolink::IOService& io_service_;
-
-    dhcp_ddns::NameChangeRequestPtr ncr_;
-
-    DdnsDomainPtr forward_domain_;
-
-    DdnsDomainPtr reverse_domain_;
-};
-
-/// @brief Defines a pointer to a NameChangeTransaction.
-typedef boost::shared_ptr<NameChangeTransaction> NameChangeTransactionPtr;
-
-//@}
-
 /// @brief Defines a list of transactions.
 typedef std::map<TransactionKey, NameChangeTransactionPtr> TransactionList;
 
-
 /// @brief D2UpdateMgr creates and manages update transactions.
 ///
 /// D2UpdateMgr is the DHCP_DDNS task master, instantiating and then supervising

src/bin/d2/nc_trans.cc

+// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <d2/d2_log.h>
+#include <d2/nc_trans.h>
+
+namespace isc {
+namespace d2 {
+
+// Common transaction states
+const int NameChangeTransaction::NEW_ST;
+const int NameChangeTransaction::READY_ST;
+const int NameChangeTransaction::SELECTING_FWD_SERVER_ST;
+const int NameChangeTransaction::SELECTING_REV_SERVER_ST;
+const int NameChangeTransaction::DONE_ST;
+
+const int NameChangeTransaction::DERIVED_STATES;
+
+// Common transaction events
+const int NameChangeTransaction::NOP_EVT;
+const int NameChangeTransaction::START_TRANSACTION_EVT;
+const int NameChangeTransaction::SELECT_SERVER_EVT;
+const int NameChangeTransaction::SERVER_SELECTED_EVT;
+const int NameChangeTransaction::SERVER_IO_ERROR_EVT;
+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;
+
+NameChangeTransaction::
+NameChangeTransaction(isc::asiolink::IOService& io_service,
+                      dhcp_ddns::NameChangeRequestPtr& ncr,
+                      DdnsDomainPtr forward_domain,
+                      DdnsDomainPtr reverse_domain)
+    : state_handlers_(), io_service_(io_service), ncr_(ncr),
+     forward_domain_(forward_domain), reverse_domain_(reverse_domain),
+     dns_client_(), state_(NEW_ST), next_event_(NOP_EVT),
+     dns_update_status_(DNSClient::OTHER), dns_update_response_(),
+     forward_change_completed_(false), reverse_change_completed_(false) {
+    if (!ncr_) {
+        isc_throw(NameChangeTransactionError, "NameChangeRequest cannot null");
+    }
+
+    if (ncr_->isForwardChange() && !(forward_domain_)) {
+        isc_throw(NameChangeTransactionError,
+                 "Forward change must have a forward domain");
+    }
+
+    if (ncr_->isReverseChange() && !(reverse_domain_)) {
+        isc_throw(NameChangeTransactionError,
+                 "Reverse change must have a reverse domain");
+    }
+    
+    // Use setters here so we get proper values for previous state, last event.
+    setState(state_);
+    setNextEvent(NOP_EVT);
+}
+
+NameChangeTransaction::~NameChangeTransaction(){
+}
+
+void
+NameChangeTransaction::startTransaction() {
+    // Initialize the state handler map first.
+    initStateHandlerMap();
+
+    // 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.
+    // runStateModel is exception safe so we are good to call it here.  
+    // It won't exit until we hit the next IO wait or the state model ends.
+    setDnsUpdateStatus(status);
+    runStateModel(IO_COMPLETED_EVT);
+}
+
+void
+NameChangeTransaction::runStateModel(unsigned int run_event) {
+    try {
+        // Seed the loop with the given event as the next to process.
+        setNextEvent(run_event);
+        do {
+            // Invoke the current state's handler.  It should consume the 
+            // next event, then determine what happens next by setting
+            // current state and/or the next event.  
+            (getStateHandler(state_))();
+
+            // Keep going until a handler sets next event to a NOP_EVT. 
+        } while (getNextEvent() != NOP_EVT);
+    }
+    catch (const std::exception& ex) {
+        // Transaction has suffered an unexpected exception.  This indicates
+        // a programmatic shortcoming.  Log it and set status to ST_FAILED.
+        // In theory, the model should account for all error scenarios and
+        // deal with them accordingly.
+        LOG_ERROR(dctl_logger, DHCP_DDNS_TRANS_PROCESS_EROR).arg(ex.what());
+        setNcrStatus(dhcp_ddns::ST_FAILED);
+    }
+}
+
+
+StateHandler
+NameChangeTransaction::getStateHandler(unsigned int state) {
+    StateHandlerMap::iterator it = state_handlers_.find(state);
+    if (it == state_handlers_.end()) {
+        isc_throw(NameChangeTransactionError, "Invalid state: " << state);
+    }
+
+    return ((*it).second);
+}
+
+void
+NameChangeTransaction::addToMap(unsigned int state, StateHandler handler) {
+    StateHandlerMap::iterator it = state_handlers_.find(state);
+    if (it != state_handlers_.end()) {
+        isc_throw(NameChangeTransactionError,
+                  "Attempted duplicate entry in state handler mape, state: "
+                   << state);
+    }
+
+    state_handlers_[state] = handler;
+}
+
+void
+NameChangeTransaction::setState(unsigned int state) {
+    prev_state_ = state_;
+    state_ = state;
+}
+
+void
+NameChangeTransaction::setNextEvent(unsigned int event) {
+    last_event_ = next_event_;
+    next_event_ = event;
+}
+
+void
+NameChangeTransaction::setDnsUpdateStatus(const DNSClient::Status& status) {
+    dns_update_status_ = status;
+}
+
+void
+NameChangeTransaction::setDnsUpdateResponse(D2UpdateMessagePtr& response) {
+    dns_update_response_ = response;
+}
+
+void
+NameChangeTransaction::setForwardChangeCompleted(const bool value) {
+    forward_change_completed_ = value;
+}
+
+void
+NameChangeTransaction::setReverseChangeCompleted(const bool value) {
+    reverse_change_completed_ = value;
+}
+
+const dhcp_ddns::NameChangeRequestPtr& 
+NameChangeTransaction::getNcr() const {
+    return (ncr_);
+}
+
+const TransactionKey& 
+NameChangeTransaction::getTransactionKey() const {
+    return (ncr_->getDhcid());
+}
+
+dhcp_ddns::NameChangeStatus
+NameChangeTransaction::getNcrStatus() const {
+    return (ncr_->getStatus());
+}
+
+void
+NameChangeTransaction::setNcrStatus(const dhcp_ddns::NameChangeStatus& status) {
+    return (ncr_->setStatus(status));
+}
+
+unsigned int
+NameChangeTransaction::getState() const {
+    return (state_);
+}
+
+unsigned int
+NameChangeTransaction::getPrevState() const {
+    return (prev_state_);
+}
+
+unsigned int
+NameChangeTransaction::getLastEvent() const {
+    return (last_event_);
+}
+
+unsigned int
+NameChangeTransaction::getNextEvent() const {
+    return (next_event_);
+}
+
+DNSClient::Status
+NameChangeTransaction::getDnsUpdateStatus() const {
+    return (dns_update_status_);
+}
+
+const D2UpdateMessagePtr&
+NameChangeTransaction::getDnsUpdateResponse() const {
+    return (dns_update_response_);
+}
+
+bool
+NameChangeTransaction::getForwardChangeCompleted() const {
+    return (forward_change_completed_);
+}
+
+bool
+NameChangeTransaction::getReverseChangeCompleted() const {
+    return (reverse_change_completed_);
+}
+
+
+} // namespace isc::d2
+} // namespace isc

src/bin/d2/tests/Makefile.am

@@ -64,6 +64,7 @@ d2_unittests_SOURCES += ../d2_update_message.cc ../d2_update_message.h
 d2_unittests_SOURCES += ../d2_update_mgr.cc ../d2_update_mgr.h
 d2_unittests_SOURCES += ../d2_zone.cc ../d2_zone.h
 d2_unittests_SOURCES += ../dns_client.cc ../dns_client.h
+d2_unittests_SOURCES += ../nc_trans.cc ../nc_trans.h
 d2_unittests_SOURCES += d_test_stubs.cc d_test_stubs.h
 d2_unittests_SOURCES += d2_unittests.cc
 d2_unittests_SOURCES += d2_process_unittests.cc
@@ -76,6 +77,7 @@ d2_unittests_SOURCES += d2_update_message_unittests.cc
 d2_unittests_SOURCES += d2_update_mgr_unittests.cc
 d2_unittests_SOURCES += d2_zone_unittests.cc
 d2_unittests_SOURCES += dns_client_unittests.cc
+d2_unittests_SOURCES += nc_trans_unittests.cc
 nodist_d2_unittests_SOURCES = ../d2_messages.h ../d2_messages.cc
 
 d2_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)

src/bin/d2/tests/nc_trans_unittests.cc

+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <d2/nc_trans.h>
+
+#include <boost/function.hpp>
+#include <boost/bind.hpp>
+#include <gtest/gtest.h>
+
+using namespace std;
+using namespace isc;
+using namespace isc::d2;
+
+namespace {
+
+/// @brief Test derivation of NameChangeTransaction for exercising state
+/// model mechanics.
+///
+/// This class faciliates 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.
+class NameChangeStub : public NameChangeTransaction {
+public:
+
+    // NameChangeStub states
+    static const int DO_WORK_ST = DERIVED_STATES + 1;
+
+    // NameChangeStub events
+    static const int START_WORK_EVT = DERIVED_EVENTS + 1;
+
+    /// @brief Constructor
+    ///
+    /// Parameters match those needed by NameChangeTransaction.
+    NameChangeStub(isc::asiolink::IOService& io_service,
+                   dhcp_ddns::NameChangeRequestPtr& ncr,
+                   DdnsDomainPtr forward_domain,
+                   DdnsDomainPtr reverse_domain)
+        : NameChangeTransaction(io_service, ncr, forward_domain,
+                                reverse_domain) {
+    }
+
+    /// @brief Destructor
+    virtual ~NameChangeStub() {
+    }
+
+    /// @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
+    /// sets the next event to START_WORK_EVT.
+    void readyHandler() {
+        switch(getNextEvent()) {
+        case START_TRANSACTION_EVT:
+            setState(DO_WORK_ST);
+            setNextEvent(START_WORK_EVT);
+            break;
+        default:
+            // its bogus
+            isc_throw(NameChangeTransactionError, "invalid event: "
+                      << getNextEvent() << " for state: " << getState());
+        }
+    }
+
+    /// @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
+    /// and signals the state model must "wait" for an event by setting
+    /// next event to NOP_EVT. 
+    ///
+    /// When next event is IO_COMPLETED_EVT, it transitions to the state,
+    /// DONE_ST, and sets the next event to ALL_DONE_EVT.
+    void doWorkHandler() {
+        switch(getNextEvent()) {
+        case START_WORK_EVT:
+            setNcrStatus(dhcp_ddns::ST_PENDING);
+            setNextEvent(NOP_EVT);
+            break;
+        //case WORK_DONE_EVT:
+        case IO_COMPLETED_EVT:
+            setState(DONE_ST);
+            setNextEvent(ALL_DONE_EVT);
+            break;
+        default:
+            // its bogus
+            isc_throw(NameChangeTransactionError, "invalid event: "
+                      << getNextEvent() << " for state: " << getState());
+        }
+    }
+
+    /// @brief State handler for the DONE_ST.
+    ///
+    /// This is the last state in the model.  Note that it sets the
+    /// status to completed and next event to NOP_EVT. 
+    void doneHandler() {
+        switch(getNextEvent()) {
+        case ALL_DONE_EVT:
+            setNcrStatus(dhcp_ddns::ST_COMPLETED);
+            setNextEvent(NOP_EVT);
+            break;
+        default:
+            // its bogus
+            isc_throw(NameChangeTransactionError, "invalid event: "
+                      << getNextEvent() << " for state: " << getState());
+        }
+    }
+
+    /// @brief Initializes the state handler map.
+    void initStateHandlerMap() {
+        addToMap(READY_ST,
+            boost::bind(&NameChangeStub::readyHandler, this));
+
+         addToMap(DO_WORK_ST,
+            boost::bind(&NameChangeStub::doWorkHandler, this));
+
+         addToMap(DONE_ST,
+            boost::bind(&NameChangeStub::doneHandler, this));
+    }
+
+    // Expose the protected methods to be tested.
+    using NameChangeTransaction::addToMap;
+    using NameChangeTransaction::getStateHandler;
+    using NameChangeTransaction::initStateHandlerMap;
+    using NameChangeTransaction::runStateModel;
+    using NameChangeTransaction::setState;
+    using NameChangeTransaction::setNextEvent;
+};
+
+const int NameChangeStub::DO_WORK_ST;
+const int NameChangeStub::START_WORK_EVT;
+
+/// @brief Defines a pointer to a D2UpdateMgr instance.
+typedef boost::shared_ptr<NameChangeStub> NameChangeStubPtr;
+
+/// @brief Test fixture for testing NameChangeTransaction
+///
+/// Note this class uses NameChangeStub class to exercise non-public
+/// aspects of NameChangeTransaction.
+class NameChangeTransactionTest : public ::testing::Test {
+public:
+    isc::asiolink::IOService io_service_;
+
+    virtual ~NameChangeTransactionTest() {
+    }
+
+    /// @brief Instantiates a NameChangeStub built around a canned 
+    /// NameChangeRequest.
+    NameChangeStubPtr makeCannedTransaction() {
+        const char* msg_str =
+            "{"
+            " \"change_type\" : 0 , "
+            " \"forward_change\" : true , "
+            " \"reverse_change\" : true , "
+            " \"fqdn\" : \"walah.walah.org.\" , "
+            " \"ip_address\" : \"192.168.2.1\" , "
+            " \"dhcid\" : \"0102030405060708\" , "
+            " \"lease_expires_on\" : \"20130121132405\" , "
+            " \"lease_length\" : 1300 "
+            "}";
+
+        dhcp_ddns::NameChangeRequestPtr ncr;
+        DnsServerInfoStoragePtr servers;
+        DdnsDomainPtr forward_domain;
+        DdnsDomainPtr reverse_domain;
+
+        ncr = dhcp_ddns::NameChangeRequest::fromJSON(msg_str);
+        forward_domain.reset(new DdnsDomain("*", "", servers));
+        reverse_domain.reset(new DdnsDomain("*", "", servers));
+        return (NameChangeStubPtr(new NameChangeStub(io_service_, ncr,
+                                  forward_domain, reverse_domain)));
+
+    }
+
+};
+
+/// @brief Tests NameChangeTransaction construction.
+/// This test verifies that:
+/// 1. Construction with null NameChangeRequest
+/// 2. Construction with null forward domain is not allowed when the request
+/// requires forward change.
+/// 3. Construction with null reverse domain is not allowed when the request
+/// requires reverse change.
+/// 4. Valid construction functions properly
+TEST(NameChangeTransaction, construction) {
+    isc::asiolink::IOService io_service;
+
+    const char* msg_str =
+        "{"
+        " \"change_type\" : 0 , "
+        " \"forward_change\" : true , "
+        " \"reverse_change\" : true , "
+        " \"fqdn\" : \"walah.walah.org.\" , "
+        " \"ip_address\" : \"192.168.2.1\" , "
+        " \"dhcid\" : \"0102030405060708\" , "
+        " \"lease_expires_on\" : \"20130121132405\" , "
+        " \"lease_length\" : 1300 "
+        "}";
+
+    dhcp_ddns::NameChangeRequestPtr ncr;
+
+    dhcp_ddns::NameChangeRequestPtr empty_ncr;
+    DnsServerInfoStoragePtr servers;
+    DdnsDomainPtr forward_domain;
+    DdnsDomainPtr reverse_domain;
+    DdnsDomainPtr empty_domain;
+
+    ASSERT_NO_THROW(ncr = dhcp_ddns::NameChangeRequest::fromJSON(msg_str));
+    ASSERT_NO_THROW(forward_domain.reset(new DdnsDomain("*", "", servers)));
+    ASSERT_NO_THROW(reverse_domain.reset(new DdnsDomain("*", "", servers)));
+
+    // Verify that construction with an empty NameChangeRequest throws. 
+    EXPECT_THROW(NameChangeTransaction(io_service, empty_ncr,
+                                       forward_domain, reverse_domain),
+                                        NameChangeTransactionError);
+
+    // Verify that construction with an empty forward domain when the
+    // NameChangeRequest calls for a forward change throws. 
+    EXPECT_THROW(NameChangeTransaction(io_service, ncr,
+                                       empty_domain, reverse_domain),
+                                       NameChangeTransactionError);
+
+    // Verify that construction with an empty reverse domain when the
+    // NameChangeRequest calls for a reverse change throws. 
+    EXPECT_THROW(NameChangeTransaction(io_service, ncr,
+                                       forward_domain, empty_domain),
+                                       NameChangeTransactionError);
+
+    // Verify that a valid construction attempt works.
+    EXPECT_NO_THROW(NameChangeTransaction(io_service, ncr,
+                                          forward_domain, reverse_domain));
+
+    // Verify that an empty forward domain is allowed when the requests does
+    // include a forward change. 
+    ncr->setForwardChange(false);
+    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. 
+    ncr->setReverseChange(false);
+    EXPECT_NO_THROW(NameChangeTransaction(io_service, ncr,
+                                          empty_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.