[3086] Added server selection support to NameChangeTransaction

Added methods to base class for supporting server selection from a list
of DNS servers.

src/bin/d2/nc_trans.cc

@@ -45,13 +45,14 @@ const int NameChangeTransaction::DERIVED_EVENTS;
 NameChangeTransaction::
 NameChangeTransaction(isc::asiolink::IOService& io_service,
                       dhcp_ddns::NameChangeRequestPtr& ncr,
-                      DdnsDomainPtr forward_domain,
-                      DdnsDomainPtr reverse_domain)
+                      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) {
+     forward_change_completed_(false), reverse_change_completed_(false),
+     current_server_list_(), current_server_(), next_server_pos_(0) {
     if (!ncr_) {
         isc_throw(NameChangeTransactionError, "NameChangeRequest cannot null");
     }
@@ -65,7 +66,7 @@ NameChangeTransaction(isc::asiolink::IOService& io_service,
         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);
@@ -94,7 +95,7 @@ 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.  
+    // 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);
@@ -106,12 +107,12 @@ NameChangeTransaction::runStateModel(unsigned int run_event) {
         // 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 
+            // 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.  
+            // current state and/or the next event.
             (getStateHandler(state_))();
 
-            // Keep going until a handler sets next event to a NOP_EVT. 
+            // Keep going until a handler sets next event to a NOP_EVT.
         } while (getNextEvent() != NOP_EVT);
     }
     catch (const std::exception& ex) {
@@ -179,12 +180,12 @@ NameChangeTransaction::setReverseChangeCompleted(const bool value) {
     reverse_change_completed_ = value;
 }
 
-const dhcp_ddns::NameChangeRequestPtr& 
+const dhcp_ddns::NameChangeRequestPtr&
 NameChangeTransaction::getNcr() const {
     return (ncr_);
 }
 
-const TransactionKey& 
+const TransactionKey&
 NameChangeTransaction::getTransactionKey() const {
     return (ncr_->getDhcid());
 }
@@ -194,6 +195,54 @@ NameChangeTransaction::getNcrStatus() const {
     return (ncr_->getStatus());
 }
 
+DdnsDomainPtr&
+NameChangeTransaction::getForwardDomain() {
+    return (forward_domain_);
+}
+
+DdnsDomainPtr&
+NameChangeTransaction::getReverseDomain() {
+    return (reverse_domain_);
+}
+
+void
+NameChangeTransaction::initServerSelection(DdnsDomainPtr& domain) {
+    current_server_list_ = domain->getServers();
+    next_server_pos_ = 0;
+    current_server_.reset();
+}
+
+bool
+NameChangeTransaction::selectNextServer() {
+    if ((current_server_list_) &&
+        (next_server_pos_ < current_server_list_->size())) {
+        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
+        // 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,
+                                        DNSClient::UDP));
+        ++next_server_pos_;
+        return (true);
+    }
+
+    return (false);
+}
+
+const DNSClientPtr&
+NameChangeTransaction::getDNSClient() const {
+    return (dns_client_);
+}
+
+const DnsServerInfoPtr&
+NameChangeTransaction::getCurrentServer() const {
+    return (current_server_);
+}
+
+
 void
 NameChangeTransaction::setNcrStatus(const dhcp_ddns::NameChangeStatus& status) {
     return (ncr_->setStatus(status));

src/bin/d2/nc_trans.h

@@ -187,8 +187,8 @@ public:
     /// reverse change is enabled but reverse domain is null.
     NameChangeTransaction(isc::asiolink::IOService& io_service,
                           dhcp_ddns::NameChangeRequestPtr& ncr,
-                          DdnsDomainPtr forward_domain,
-                          DdnsDomainPtr reverse_domain);
+                          DdnsDomainPtr& forward_domain,
+                          DdnsDomainPtr& reverse_domain);
 
     virtual ~NameChangeTransaction();
 
@@ -330,6 +330,45 @@ protected:
     /// @param value is the new value to assign to the flag.
     void setReverseChangeCompleted(const bool value);
 
+    /// @brief Sets the status of the transaction's NameChangeRequest
+    ///
+    /// @param status is the new value to assign to the NCR status.
+    void setNcrStatus(const dhcp_ddns::NameChangeStatus& status);
+
+    /// @brief Initializes server selection from the given DDNS domain.
+    ///
+    /// Method prepares internal data to conduct server selection from the
+    /// list of servers supplied by the given domain.  This method should be
+    /// called when a transaction is ready to begin selecting servers from
+    /// a new list.  Typically this will be prior to starting the updates for
+    /// a given DNS direction.
+    ///
+    /// @param domain is the domain from which server selection is to be
+    /// conducted.
+    void initServerSelection(DdnsDomainPtr& domain);
+
+    /// @brief Selects the next server in the current server list.
+    ///
+    /// This method is used to iterate over the list of servers.  If there are
+    /// no more servers in the list, it returns false.  Otherwise it sets the
+    /// the current server to the next server and creates a new DNSClient
+    /// instance.
+    ///
+    /// @return True if a server has been selected, false if there are no more
+    /// servers from which to select.
+    bool selectNextServer();
+
+    /// @brief Fetches the currently selected server.
+    ///
+    /// @return A const pointer reference to the DnsServerInfo of the current
+    /// server.
+    const DnsServerInfoPtr& getCurrentServer() const;
+
+    /// @brief Fetches the DNSClient instance
+    ///
+    /// @return A const pointer reference to the DNSClient
+    const DNSClientPtr& getDNSClient() const;
+
 public:
     /// @brief Fetches the NameChangeRequest for this transaction.
     ///
@@ -355,10 +394,19 @@ public:
     /// status of the transaction.
     dhcp_ddns::NameChangeStatus getNcrStatus() const;
 
-    /// @brief Sets the status of the transaction's NameChangeRequest
+    /// @brief Fetches the forward DdnsDomain.
     ///
-    /// @param status is the new value to assign to the NCR status.
-    void setNcrStatus(const dhcp_ddns::NameChangeStatus& status);
+    /// This value is only meaningful if the request calls for a forward change.
+    ///
+    /// @return A pointer reference to the forward DdnsDomain
+    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
+    DdnsDomainPtr& getReverseDomain();
 
     /// @brief Fetches the transaction's current state.
     ///
@@ -468,6 +516,18 @@ private:
 
     /// @brief Indicator for whether or not the reverse change completed ok.
     bool reverse_change_completed_;
+
+    /// @brief Pointer to the current server selection list.
+    DnsServerInfoStoragePtr current_server_list_;
+
+    /// @brief Pointer to the currently selected server.
+    DnsServerInfoPtr current_server_;
+
+    /// @brief Next server position in the list.
+    ///
+    /// This value is always the position of the next selection in the server
+    /// list, which may be beyond the end of the list.
+    size_t next_server_pos_;
 };
 
 /// @brief Defines a pointer to a NameChangeTransaction.

src/bin/d2/tests/nc_trans_unittests.cc

@@ -56,7 +56,7 @@ public:
     }
 
     /// @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.
@@ -74,11 +74,11 @@ 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
     /// and signals the state model must "wait" for an event by setting
-    /// next event to NOP_EVT. 
+    /// 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.
@@ -103,7 +103,7 @@ public:
     /// @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. 
+    /// status to completed and next event to NOP_EVT.
     void doneHandler() {
         switch(getNextEvent()) {
         case ALL_DONE_EVT:
@@ -136,6 +136,10 @@ public:
     using NameChangeTransaction::runStateModel;
     using NameChangeTransaction::setState;
     using NameChangeTransaction::setNextEvent;
+    using NameChangeTransaction::initServerSelection;
+    using NameChangeTransaction::selectNextServer;
+    using NameChangeTransaction::getCurrentServer;
+    using NameChangeTransaction::getDNSClient;
 };
 
 const int NameChangeStub::DO_WORK_ST;
@@ -155,7 +159,7 @@ public:
     virtual ~NameChangeTransactionTest() {
     }
 
-    /// @brief Instantiates a NameChangeStub built around a canned 
+    /// @brief Instantiates a NameChangeStub built around a canned
     /// NameChangeRequest.
     NameChangeStubPtr makeCannedTransaction() {
         const char* msg_str =
@@ -171,12 +175,26 @@ public:
             "}";
 
         dhcp_ddns::NameChangeRequestPtr ncr;
-        DnsServerInfoStoragePtr servers;
+
+        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",
+                                       isc::asiolink::IOAddress("1.1.1.1")));
+        servers->push_back(server);
         forward_domain.reset(new DdnsDomain("*", "", servers));
+
+        // make reverse server list
+        servers->clear();
+        server.reset(new DnsServerInfo("reverse.server.org",
+                                       isc::asiolink::IOAddress("2.2.2.2")));
+        servers->push_back(server);
         reverse_domain.reset(new DdnsDomain("*", "", servers));
         return (NameChangeStubPtr(new NameChangeStub(io_service_, ncr,
                                   forward_domain, reverse_domain)));
@@ -220,19 +238,19 @@ TEST(NameChangeTransaction, construction) {
     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. 
+    // 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. 
+    // 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. 
+    // NameChangeRequest calls for a reverse change throws.
     EXPECT_THROW(NameChangeTransaction(io_service, ncr,
                                        forward_domain, empty_domain),
                                        NameChangeTransactionError);
@@ -242,20 +260,20 @@ TEST(NameChangeTransaction, construction) {
                                           forward_domain, reverse_domain));
 
     // Verify that an empty forward domain is allowed when the requests does
-    // include a forward change. 
+    // 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. 
+    // 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 
+/// It first verifies basic state handle map fucntionality, and then
 /// runs the NameChangeStub state model through from start to finish.
 TEST_F(NameChangeTransactionTest, stateModelTest) {
     NameChangeStubPtr name_change;
@@ -333,4 +351,88 @@ TEST_F(NameChangeTransactionTest, stateModelTest) {
     EXPECT_EQ(NameChangeTransaction::NOP_EVT, name_change->getNextEvent());
 }
 
+/// @brief Tests server selection methods
+TEST_F(NameChangeTransactionTest, serverSelectionTest) {
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+
+    // Verify that the forward domain and its servers can be retrieved.
+    DdnsDomainPtr& domain = name_change->getForwardDomain();
+    ASSERT_TRUE(domain);
+    DnsServerInfoStoragePtr servers = domain->getServers();
+    ASSERT_TRUE(servers);
+
+    // Get the number of entries in the server list.
+    int num_servers = servers->size();
+    ASSERT_TRUE(num_servers > 0);
+
+    ASSERT_NO_THROW(name_change->initServerSelection(domain));
+
+    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()) {
+        DnsServerInfoPtr server = name_change->getCurrentServer();
+        DNSClientPtr client = name_change->getDNSClient();
+        D2UpdateMessagePtr response = name_change->getDnsUpdateResponse();
+        EXPECT_TRUE(server);
+        EXPECT_TRUE(client);
+        EXPECT_TRUE(response);
+
+        EXPECT_NE(server, prev_server);
+        EXPECT_NE(client, prev_client);
+        EXPECT_NE(response, prev_response);
+
+        prev_server = server;
+        prev_client = client;
+        prev_response = response;
+
+        passes++;
+    }
+
+    // Verify that the numer of passes made equal the number of servers.
+    EXPECT_EQ (passes, num_servers);
+
+    // Repeat the same test using the reverse domain.
+    domain = name_change->getReverseDomain();
+    ASSERT_TRUE(domain);
+
+    servers = domain->getServers();
+    ASSERT_TRUE(servers);
+
+    num_servers = servers->size();
+    ASSERT_TRUE(num_servers > 0);
+
+    ASSERT_NO_THROW(name_change->initServerSelection(domain));
+
+    prev_client = name_change->getDNSClient();
+    prev_response = name_change->getDnsUpdateResponse();
+    prev_server = name_change->getCurrentServer();
+
+    passes = 0;
+    while (name_change->selectNextServer()) {
+        DnsServerInfoPtr server = name_change->getCurrentServer();
+        DNSClientPtr client = name_change->getDNSClient();
+        D2UpdateMessagePtr response = name_change->getDnsUpdateResponse();
+        EXPECT_TRUE(server);
+        EXPECT_TRUE(client);
+        EXPECT_TRUE(response);
+
+        EXPECT_NE(server, prev_server);
+        EXPECT_NE(client, prev_client);
+        EXPECT_NE(response, prev_response);
+
+        prev_server = server;
+        prev_client = client;
+        prev_response = response;
+
+        passes++;
+    }
+
+    EXPECT_EQ (passes, num_servers);
+}
+
 }