[master] Merge branch 'trac3059'

Adds initial implementation of D2UpdateMgr class to
src/bin/d2.

src/bin/d2/Makefile.am

@@ -56,6 +56,7 @@ b10_dhcp_ddns_SOURCES += d2_config.cc d2_config.h
 b10_dhcp_ddns_SOURCES += d2_cfg_mgr.cc d2_cfg_mgr.h
 b10_dhcp_ddns_SOURCES += d2_queue_mgr.cc d2_queue_mgr.h
 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
 
@@ -69,7 +70,7 @@ b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/asiodns/libb10-asiodns.la
 b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
 b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/config/libb10-cfgclient.la
 b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/dhcp_ddns/libb10-dhcp_ddns.la
-b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/dhcpsrv/libb10-dhcpsrv.la 
+b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/dhcpsrv/libb10-dhcpsrv.la
 b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/dns/libb10-dns++.la
 b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/util/libb10-util.la
 b10_dhcp_ddns_LDADD += $(top_builddir)/src/lib/hooks/libb10-hooks.la

src/bin/d2/d2_cfg_mgr.cc

@@ -14,6 +14,7 @@
 
 #include <d2/d2_log.h>
 #include <d2/d2_cfg_mgr.h>
+#include <util/encode/hex.h>
 
 #include <boost/foreach.hpp>
 
@@ -39,7 +40,7 @@ D2CfgContext::D2CfgContext(const D2CfgContext& rhs) : DCfgContextBase(rhs) {
         reverse_mgr_->setDomains(rhs.reverse_mgr_->getDomains());
     }
 
-    keys_ = rhs.keys_; 
+    keys_ = rhs.keys_;
 }
 
 D2CfgContext::~D2CfgContext() {
@@ -47,6 +48,10 @@ D2CfgContext::~D2CfgContext() {
 
 // *********************** D2CfgMgr  *************************
 
+const char* D2CfgMgr::IPV4_REV_ZONE_SUFFIX = "in-addr.arpa.";
+
+const char* D2CfgMgr::IPV6_REV_ZONE_SUFFIX = "ip6.arpa.";
+
 D2CfgMgr::D2CfgMgr() : DCfgMgrBase(DCfgContextBasePtr(new D2CfgContext())) {
     // TSIG keys need to parse before the Domains, so we can catch Domains
     // that specify undefined keys. Create the necessary parsing order now.
@@ -76,17 +81,79 @@ D2CfgMgr::matchForward(const std::string& fqdn, DdnsDomainPtr& domain) {
 }
 
 bool
-D2CfgMgr::matchReverse(const std::string& fqdn, DdnsDomainPtr& domain) {
-    if (fqdn.empty()) {
-        // This is a programmatic error and should not happen.
-        isc_throw(D2CfgError, "matchReverse passed a null or empty fqdn");
-    }
+D2CfgMgr::matchReverse(const std::string& ip_address, DdnsDomainPtr& domain) {
+    // Note, reverseIpAddress will throw if the ip_address is invalid.
+    std::string reverse_address = reverseIpAddress(ip_address);
 
     // Fetch the reverse manager from the D2 context.
     DdnsDomainListMgrPtr mgr = getD2CfgContext()->getReverseMgr();
 
-    // Call the manager's match method and return the result.
-    return (mgr->matchDomain(fqdn, domain));
+    return (mgr->matchDomain(reverse_address, domain));
+}
+
+std::string
+D2CfgMgr::reverseIpAddress(const std::string& address) {
+    try {
+        // Convert string address into an IOAddress and invoke the
+        // appropriate reverse method.
+        isc::asiolink::IOAddress ioaddr(address);
+        if (ioaddr.isV4()) {
+            return (reverseV4Address(ioaddr));
+        }
+
+        return (reverseV6Address(ioaddr));
+
+    } catch (const isc::Exception& ex) {
+        isc_throw(D2CfgError, "D2CfgMgr cannot reverse address: "
+                               << address << " : " << ex.what());
+    }
+}
+
+std::string
+D2CfgMgr::reverseV4Address(const isc::asiolink::IOAddress& ioaddr) {
+    if (!ioaddr.isV4()) {
+        isc_throw(D2CfgError, "D2CfgMgr address is not IPv4 address :"
+                              << ioaddr.toText());
+    }
+
+    // Get the address in byte vector form.
+    std::vector<uint8_t> bytes = ioaddr.toBytes();
+
+    // Walk backwards through vector outputting each octet and a dot.
+    std::ostringstream stream;
+    std::vector<uint8_t>::const_reverse_iterator rit;
+
+    for (rit = bytes.rbegin(); rit != bytes.rend(); ++rit) {
+        stream << static_cast<unsigned int>(*rit) << ".";
+    }
+
+    // Tack on the suffix and we're done.
+    stream << IPV4_REV_ZONE_SUFFIX;
+    return(stream.str());
+}
+
+std::string
+D2CfgMgr::reverseV6Address(const isc::asiolink::IOAddress& ioaddr) {
+    if (!ioaddr.isV6()) {
+        isc_throw(D2CfgError, "D2Cfg address is not IPv6 address: "
+                              << ioaddr.toText());
+    }
+
+    // Turn the address into a string of digits.
+    std::vector<uint8_t> bytes = ioaddr.toBytes();
+    std::string digits;
+    digits = isc::util::encode::encodeHex(bytes);
+
+    // Walk backwards through string outputting each digits and a dot.
+    std::ostringstream stream;
+    std::string::const_reverse_iterator rit;
+    for (rit = digits.rbegin(); rit != digits.rend(); ++rit) {
+        stream << static_cast<char>(*rit) << ".";
+    }
+
+    // Tack on the suffix and we're done.
+    stream << IPV6_REV_ZONE_SUFFIX;
+    return(stream.str());
 }
 
 
@@ -99,10 +166,10 @@ D2CfgMgr::createConfigParser(const std::string& config_id) {
     isc::dhcp::DhcpConfigParser* parser = NULL;
     if ((config_id == "interface")  ||
         (config_id == "ip_address")) {
-        parser = new isc::dhcp::StringParser(config_id, 
+        parser = new isc::dhcp::StringParser(config_id,
                                              context->getStringStorage());
     } else if (config_id == "port") {
-        parser = new isc::dhcp::Uint32Parser(config_id, 
+        parser = new isc::dhcp::Uint32Parser(config_id,
                                              context->getUint32Storage());
     } else if (config_id ==  "forward_ddns") {
         parser = new DdnsDomainListMgrParser("forward_mgr",

src/bin/d2/d2_cfg_mgr.h

@@ -105,6 +105,14 @@ typedef boost::shared_ptr<DdnsDomainListMgr> DdnsDomainListMgrPtr;
 /// and retrieving the information on demand.
 class D2CfgMgr : public DCfgMgrBase {
 public:
+    /// @brief Reverse zone suffix added to IPv4 addresses for reverse lookups
+    /// @todo This should be configurable.
+    static const char* IPV4_REV_ZONE_SUFFIX;
+
+    /// @brief Reverse zone suffix added to IPv6 addresses for reverse lookups
+    /// @todo This should be configurable.
+    static const char* IPV6_REV_ZONE_SUFFIX;
+
     /// @brief Constructor
     D2CfgMgr();
 
@@ -119,30 +127,84 @@ public:
     }
 
     /// @brief Matches a given FQDN to a forward domain.
-    /// 
+    ///
     /// This calls the matchDomain method of the forward domain manager to
-    /// match the given FQDN to a forward domain.  
+    /// match the given FQDN to a forward domain.
     ///
     /// @param fqdn is the name for which to look.
     /// @param domain receives the matching domain. Note that it will be reset
     /// upon entry and only set if a match is subsequently found.
     ///
     /// @return returns true if a match is found, false otherwise.
-    /// @throw throws D2CfgError if given an invalid fqdn. 
-    bool matchForward(const std::string& fqdn, DdnsDomainPtr &domain);
+    /// @throw throws D2CfgError if given an invalid fqdn.
+    bool matchForward(const std::string& fqdn, DdnsDomainPtr& domain);
 
-    /// @brief Matches a given FQDN to a reverse domain.
+    /// @brief Matches a given IP address to a reverse domain.
     ///
     /// This calls the matchDomain method of the reverse domain manager to
-    /// match the given FQDN to a forward domain.  
+    /// match the given IPv4 or IPv6 address to a reverse domain.
     ///
-    /// @param fqdn is the name for which to look.
+    /// @param ip_address is the name for which to look.
     /// @param domain receives the matching domain. Note that it will be reset
     /// upon entry and only set if a match is subsequently found.
     ///
     /// @return returns true if a match is found, false otherwise.
-    /// @throw throws D2CfgError if given an invalid fqdn. 
-    bool matchReverse(const std::string& fqdn, DdnsDomainPtr &domain);
+    /// @throw throws D2CfgError if given an invalid fqdn.
+    bool matchReverse(const std::string& ip_address, DdnsDomainPtr& domain);
+
+    /// @brief Generate a reverse order string for the given IP address
+    ///
+    /// This method creates a string containing the given IP address
+    /// contents in reverse order.  This format is used for matching
+    /// against reverse DDNS domains in DHCP_DDNS configuration.
+    /// After reversing the syllables of the address, it appends the
+    /// appropriate suffix.
+    ///
+    /// @param address string containing a valid IPv4 or IPv6 address.
+    ///
+    /// @return a std::string containing the reverse order address.
+    ///
+    /// @throw D2CfgError if given an invalid address.
+    static std::string reverseIpAddress(const std::string& address);
+
+    /// @brief Generate a reverse order string for the given IP address
+    ///
+    /// This method creates a string containing the given IP address
+    /// contents in reverse order.  This format is used for matching
+    /// against reverse DDNS domains in DHCP_DDNS configuration.
+    /// After reversing the syllables of the address, it appends the
+    /// appropriate suffix.
+    ///
+    /// Example:
+    ///   input:  192.168.1.15
+    ///  output:  15.1.168.192.in-addr.arpa.
+    ///
+    /// @param ioaddr is the IPv4 IOaddress to convert
+    ///
+    /// @return a std::string containing the reverse order address.
+    ///
+    /// @throw D2CfgError if not given an IPv4  address.
+    static std::string reverseV4Address(const isc::asiolink::IOAddress& ioaddr);
+
+    /// @brief Generate a reverse order string for the given IP address
+    ///
+    /// This method creates a string containing the given IPv6 address
+    /// contents in reverse order.  This format is used for matching
+    /// against reverse DDNS domains in DHCP_DDNS configuration.
+    /// After reversing the syllables of the address, it appends the
+    /// appropriate suffix.
+    ///
+    /// IPv6 example:
+    /// input:  2001:db8:302:99::
+    /// output:
+    ///0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.9.9.0.0.2.0.3.0.8.B.D.0.1.0.0.2.ip6.arpa.
+    ///
+    /// @param address string containing a valid IPv6 address.
+    ///
+    /// @return a std::string containing the reverse order address.
+    ///
+    /// @throw D2CfgError if not given an IPv6 address.
+    static std::string reverseV6Address(const isc::asiolink::IOAddress& ioaddr);
 
 protected:
     /// @brief Given an element_id returns an instance of the appropriate

src/bin/d2/d2_config.cc

@@ -91,44 +91,67 @@ DdnsDomainListMgr::setDomains(DdnsDomainMapPtr domains) {
 
 bool
 DdnsDomainListMgr::matchDomain(const std::string& fqdn, DdnsDomainPtr& domain) {
-    // Clear the return parameter.
-    domain.reset();
-
     // First check the case of one domain to rule them all.
     if ((size() == 1) && (wildcard_domain_)) {
         domain = wildcard_domain_;
         return (true);
     }
 
-    // Start with the longest version of the fqdn and search the list.
-    // Continue looking for shorter versions of fqdn so long as no match is
-    // found.
-    // @todo This can surely be optimized, time permitting.
-    std::string match_name = fqdn;
-    std::size_t start_pos = 0;
-    while (start_pos != std::string::npos) {
-        match_name = match_name.substr(start_pos, std::string::npos);
-        DdnsDomainMap::iterator gotit = domains_->find(match_name);
-        if (gotit != domains_->end()) {
-            domain = gotit->second;
-            return (true);
+    // Iterate over the domain map looking for the domain which matches
+    // the longest portion of the given fqdn.
+
+    size_t req_len = fqdn.size();
+    size_t match_len = 0;
+    DdnsDomainMapPair map_pair;
+    DdnsDomainPtr best_match;
+    BOOST_FOREACH (map_pair, *domains_) {
+        std::string domain_name = map_pair.first;
+        size_t dom_len = domain_name.size();
+
+        // If the domain name is longer than the fqdn, then it cant be match.
+        if (req_len < dom_len) {
+            continue;
         }
 
-        start_pos = match_name.find_first_of(".");
-        if (start_pos != std::string::npos) {
-            ++start_pos;
+        // If the lengths are identical and the names match we're done.
+        if (req_len == dom_len) {
+            if (fqdn == domain_name) {
+                // exact match, done
+                domain = map_pair.second;
+                return (true);
+            }
+        } else {
+            // The fqdn is longer than the domain name.  Adjust the start
+            // point of comparison by the excess in length.  Only do the
+            // comparison if the adjustment lands on a boundary. This
+            // prevents "onetwo.net" from matching "two.net".
+            size_t offset = req_len - dom_len;
+            if ((fqdn[offset - 1] == '.')  &&
+               (fqdn.compare(offset, std::string::npos, domain_name) == 0)) {
+                // Fqdn contains domain name, keep it if its better than
+                // any we have matched so far.
+                if (dom_len > match_len) {
+                    match_len = dom_len;
+                    best_match = map_pair.second;
+                }
+            }
         }
     }
 
-    // There's no match. If they specified a wild card domain use it
-    // otherwise there's no domain for this entry.
-    if (wildcard_domain_) {
-        domain = wildcard_domain_;
-        return (true);
+    if (!best_match) {
+        // There's no match. If they specified a wild card domain use it
+        // otherwise there's no domain for this entry.
+        if (wildcard_domain_) {
+            domain = wildcard_domain_;
+            return (true);
+        }
+
+        LOG_WARN(dctl_logger, DHCP_DDNS_NO_MATCH).arg(fqdn);
+        return (false);
     }
 
-    LOG_WARN(dctl_logger, DHCP_DDNS_NO_MATCH).arg(fqdn);
-    return (false);
+    domain = best_match;
+    return (true);
 }
 
 // *************************** PARSERS ***********************************

src/bin/d2/d2_config.h

@@ -321,8 +321,8 @@ typedef boost::shared_ptr<DnsServerInfoStorage> DnsServerInfoStoragePtr;
 /// it.  It's primary use is to map a domain to the DNS server(s) responsible
 /// for it.
 /// @todo Currently the name entry for a domain is just an std::string. It
-/// may be worthwhile to change this to a dns::Name for purposes of better 
-/// validation and matching capabilities. 
+/// may be worthwhile to change this to a dns::Name for purposes of better
+/// validation and matching capabilities.
 class DdnsDomain {
 public:
     /// @brief Constructor
@@ -385,7 +385,11 @@ typedef boost::shared_ptr<DdnsDomainMap> DdnsDomainMapPtr;
 /// services.  These services are used to match a FQDN to a domain.  Currently
 /// it supports a single matching service, which will return the matching
 /// domain or a wild card domain if one is specified.  The wild card domain is
-/// specified as a domain whose name is "*".
+/// specified as a domain whose name is "*".  The wild card domain will match
+/// any entry and is provided for flexibility in FQDNs  If for instance, all
+/// forward requests are handled by the same servers, the configuration could
+/// specify the wild card domain as the only forward domain. All forward DNS
+/// updates would be sent to that one list of servers, regardless of the FQDN.
 /// As matching capabilities evolve this class is expected to expand.
 class DdnsDomainListMgr {
 public:
@@ -410,8 +414,8 @@ public:
     /// it will be returned immediately for any FQDN.
     ///
     /// @param fqdn is the name for which to look.
-    /// @param domain receives the matching domain. Note that it will be reset
-    /// upon entry and only set if a match is subsequently found.
+    /// @param domain receives the matching domain. If no match is found its
+    /// contents will be unchanged.
     ///
     /// @return returns true if a match is found, false otherwise.
     /// @todo This is a very basic match method, which expects valid FQDNs

src/bin/d2/d2_messages.mes

@@ -112,6 +112,10 @@ service first starts.
 This is an informational message issued when the controller is exiting
 following a shut down (normal or otherwise) of the service.
 
+% DHCP_DDNS_AT_MAX_TRANSACTIONS application has %1 queued requests but has reached maximum number of %2 concurrent transactions
+This is a debug message that indicates that the application has DHCP_DDNS
+requests in the queue but is working as many concurrent requests as allowed.
+
 % DHCP_DDNS_COMMAND command directive received, command: %1 - args: %2
 This is a debug message issued when the Dhcp-Ddns application command method
 has been invoked.
@@ -129,16 +133,35 @@ This is a debug message issued when the DHCP-DDNS application encountered an
 error while decoding a response to DNS Update message. Typically, this error
 will be encountered when a response message is malformed.
 
-% DHCP_DDNS_NO_MATCH No DNS servers match FQDN: %1
+% DHCP_DDNS_NO_ELIGIBLE_JOBS although there are queued requests, there are pending transactions for each Queue count: %1  Transaction count: %2
+This is a debug messge issued when all of the queued requests represent clients
+for which there is a an update already in progress.  This may occur under
+normal operations but should be temporary situation.
+
+% DHCP_DDNS_NO_FWD_MATCH_ERROR the configured list of forward DDNS domains does not contain a match for FQDN %1  The request has been discarded.
+This is an error message that indicates that DHCP_DDNS received a request to
+update a the forward DNS information for the given FQDN but for which there are
+no configured DDNS domains in the DHCP_DDNS configuration.  Either the DHCP_DDNS
+configuration needs to be updated or the source of the FQDN itself should be
+investigated.
+
+% DHCP_DDNS_NO_MATCH No DNS servers match FQDN %1
 This is warning message issued when there are no domains in the configuration
 which match the cited fully qualified domain name (FQDN).  The DNS Update
 request for the FQDN cannot be processed.
 
+% DHCP_DDNS_NO_REV_MATCH_ERROR the configured list of reverse DDNS domains does not contain a match for FQDN %1  The request has been discarded.
+This is an error message that indicates that DHCP_DDNS received a request to
+update a the reverse DNS information for the given FQDN but for which there are
+no configured DDNS domains in the DHCP_DDNS configuration.  Either the DHCP_DDNS
+configuration needs to be updated or the source of the FQDN itself should be
+investigated.
+
 % DHCP_DDNS_PROCESS_INIT application init invoked
 This is a debug message issued when the Dhcp-Ddns application enters
 its init method.
 
-% DHCP_DDNS_QUEUE_MGR_QUEUE_FULL application request queue has reached maximum number of entries: %1
+% DHCP_DDNS_QUEUE_MGR_QUEUE_FULL application request queue has reached maximum number of entries %1
 This an error message indicating that DHCP-DDNS is receiving DNS update
 requests faster than they can be processed.  This may mean the maximum queue
 needs to be increased, the DHCP-DDNS clients are simply generating too many

src/bin/d2/d2_update_mgr.cc

@@ -0,0 +1,227 @@
+// 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_update_mgr.h>
+
+#include <sstream>
+#include <iostream>
+#include <vector>
+
+namespace isc {
+namespace d2 {
+
+const size_t D2UpdateMgr::MAX_TRANSACTIONS_DEFAULT;
+
+D2UpdateMgr::D2UpdateMgr(D2QueueMgrPtr& queue_mgr, D2CfgMgrPtr& cfg_mgr,
+                         isc::asiolink::IOService& io_service,
+                         const size_t max_transactions)
+    :queue_mgr_(queue_mgr), cfg_mgr_(cfg_mgr), io_service_(io_service) {
+    if (!queue_mgr_) {
+        isc_throw(D2UpdateMgrError, "D2UpdateMgr queue manager cannot be null");
+    }
+
+    if (!cfg_mgr_) {
+        isc_throw(D2UpdateMgrError,
+                  "D2UpdateMgr configuration manager cannot be null");
+    }
+
+    // Use setter to do validation.
+    setMaxTransactions(max_transactions);
+}
+
+D2UpdateMgr::~D2UpdateMgr() {
+    transaction_list_.clear();
+}
+
+void D2UpdateMgr::sweep() {
+    // cleanup finished transactions;
+    checkFinishedTransactions();
+
+    // if the queue isn't empty, find the next suitable job and
+    // start a transaction for it.
+    // @todo - Do we want to queue max transactions? The logic here will only
+    // start one new transaction per invocation.  On the other hand a busy
+    // system will generate many IO events and this method will be called
+    // frequently.  It will likely achieve max transactions quickly on its own.
+    if (getQueueCount() > 0)  {
+        if (getTransactionCount() >= max_transactions_) {
+            LOG_DEBUG(dctl_logger, DBGLVL_TRACE_DETAIL_DATA,
+                      DHCP_DDNS_AT_MAX_TRANSACTIONS).arg(getQueueCount())
+                      .arg(getMaxTransactions());
+
+            return;
+        }
+
+        // We are not at maximum transactions, so pick and start the next job.
+        pickNextJob();
+    }
+}
+
+void
+D2UpdateMgr::checkFinishedTransactions() {
+    // Cycle through transaction list and do whatever needs to be done
+    // for finished transactions.
+    // At the moment all we do is remove them from the list. This is likely
+    // to expand as DHCP_DDNS matures.
+    TransactionList::iterator it = transaction_list_.begin();
+    while (it != transaction_list_.end()) {
+        NameChangeTransactionPtr trans = (*it).second;
+        switch (trans->getNcrStatus())  {
+        case dhcp_ddns::ST_COMPLETED:
+            transaction_list_.erase(it);
+            break;
+        case dhcp_ddns::ST_FAILED:
+            transaction_list_.erase(it);
+            break;
+        default:
+            break;
+        }
+
+        ++it;
+    }
+}
+
+void D2UpdateMgr::pickNextJob() {
+    // Start at the front of the queue, looking for the first entry for
+    // which no transaction is in progress.  If we find an eligible entry
+    // remove it from the queue and  make a transaction for it.
+    // Requests and transactions are associated by DHCID.  If a request has
+    // the same DHCID as a transaction, they are presumed to be for the same
+    // "end user".
+    size_t queue_count = getQueueCount();
+    for (size_t index = 0; index < queue_count; ++index) {
+        dhcp_ddns::NameChangeRequestPtr found_ncr = queue_mgr_->peekAt(index);
+        if (!hasTransaction(found_ncr->getDhcid())) {
+            queue_mgr_->dequeueAt(index);
+            makeTransaction(found_ncr);
+            return;
+        }
+    }
+
+    // There were no eligible jobs. All of the current DHCIDs already have
+    // transactions pending.
+    LOG_DEBUG(dctl_logger, DBGLVL_TRACE_DETAIL_DATA, DHCP_DDNS_NO_ELIGIBLE_JOBS)
+              .arg(getQueueCount()).arg(getTransactionCount());
+}
+
+void
+D2UpdateMgr::makeTransaction(dhcp_ddns::NameChangeRequestPtr& next_ncr) {
+    // First lets ensure there is not a transaction in progress for this
+    // DHCID. (pickNextJob should ensure this, as it is the only real caller
+    // but for safety's sake we'll check).
+    const TransactionKey& key = next_ncr->getDhcid();
+    if (findTransaction(key) != transactionListEnd()) {
+        // This is programmatic error.  Caller(s) should be checking this.
+        isc_throw(D2UpdateMgrError, "Transaction already in progress for: "
+            << key.toStr());
+    }
+
+    // If forward change is enabled, match to forward servers.
+    DdnsDomainPtr forward_domain;
+    if (next_ncr->isForwardChange()) {
+        bool matched = cfg_mgr_->matchForward(next_ncr->getFqdn(),
+                                             forward_domain);
+        // Could not find a match for forward DNS server. Log it and get out.
+        // This has the net affect of dropping the request on the floor.
+        if (!matched) {
+            LOG_ERROR(dctl_logger, DHCP_DDNS_NO_FWD_MATCH_ERROR)
+                      .arg(next_ncr->getFqdn());
+            return;
+        }
+    }
+
+    // If reverse change is enabled, match to reverse servers.
+    DdnsDomainPtr reverse_domain;
+    if (next_ncr->isReverseChange()) {
+        bool matched = cfg_mgr_->matchReverse(next_ncr->getIpAddress(),
+                                              reverse_domain);
+        // Could not find a match for reverse DNS server. Log it and get out.
+        // This has the net affect of dropping the request on the floor.
+        if (!matched) {
+            LOG_ERROR(dctl_logger, DHCP_DDNS_NO_REV_MATCH_ERROR)
+                      .arg(next_ncr->getIpAddress());
+            return;
+        }
+    }
+
+    // We matched to the required servers, so construct the transaction.
+    NameChangeTransactionPtr trans(new NameChangeTransaction(io_service_,
+                                                             next_ncr,
+                                                             forward_domain,
+                                                             reverse_domain));
+    // Add the new transaction to the list.
+    transaction_list_[key] = trans;
+}
+
+TransactionList::iterator
+D2UpdateMgr::findTransaction(const TransactionKey& key) {
+    return (transaction_list_.find(key));
+}
+
+bool
+D2UpdateMgr::hasTransaction(const TransactionKey& key) {
+   return (findTransaction(key) != transactionListEnd());
+}
+
+void
+D2UpdateMgr::removeTransaction(const TransactionKey& key) {
+    TransactionList::iterator pos = findTransaction(key);
+    if (pos != transactionListEnd()) {
+        transaction_list_.erase(pos);
+    }
+}
+
+TransactionList::iterator
+D2UpdateMgr::transactionListEnd() {
+    return (transaction_list_.end());
+}
+
+void
+D2UpdateMgr::clearTransactionList() {
+    // @todo for now this just wipes them out. We might need something
+    // more elegant, that allows a cancel first.
+    transaction_list_.clear();
+}
+
+void
+D2UpdateMgr::setMaxTransactions(const size_t new_trans_max) {
+    // Obviously we need at room for at least one transaction.
+    if (new_trans_max < 1) {
+        isc_throw(D2UpdateMgrError, "D2UpdateMgr"
+                  " maximum transactions limit must be greater than zero");
+    }
+
+    // Do not allow the list maximum to be set to less then current list size.
+    if (new_trans_max < getTransactionCount()) {
+        isc_throw(D2UpdateMgrError, "D2UpdateMgr maximum transaction limit "
+                  "cannot be less than the current transaction count :"
+                  << getTransactionCount());
+    }
+
+    max_transactions_ = new_trans_max;
+}
+
+size_t
+D2UpdateMgr::getQueueCount() const {
+    return (queue_mgr_->getQueueSize());
+}
+
+size_t
+D2UpdateMgr::getTransactionCount() const {
+    return (transaction_list_.size());
+}
+
+
+} // namespace isc::d2
+} // namespace isc

src/bin/d2/d2_update_mgr.h

@@ -0,0 +1,294 @@
+// 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.
+
+#ifndef D2_UPDATE_MGR_H
+#define D2_UPDATE_MGR_H
+
+/// @file d2_update_mgr.h This file defines the class D2UpdateMgr.
+
+#include <asiolink/io_service.h>
+#include <exceptions/exceptions.h>
+#include <d2/d2_log.h>
+#include <d2/d2_queue_mgr.h>
+#include <d2/d2_cfg_mgr.h>
+
+#include <boost/noncopyable.hpp>
+#include <boost/shared_ptr.hpp>
+#include <map>
+
+namespace isc {
+namespace d2 {
+
+/// @brief Thrown if the update manager encounters a general error.
+class D2UpdateMgrError : public isc::Exception {
+public:
+    D2UpdateMgrError(const char* file, size_t line, const char* what) :
+        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
+/// transactions that execute the DNS updates needed to fulfill the requests
+/// (NameChangeRequests) received from DHCP_DDNS clients (e.g. DHCP servers).
+///
+/// D2UpdateMgr uses the services of D2QueueMgr to monitor the queue of
+/// NameChangeRequests and select and dequeue requests for processing.
+/// When request is dequeued for processing it is removed from the queue and
+/// wrapped in NameChangeTransaction and added to the D2UpdateMgr's list of
+/// transactions.
+///
+/// As part of the process of forming transactions, D2UpdateMgr matches each
+/// request with the appropriate list of DNS servers.  This matching is  based
+/// upon request attributes, primarily the FQDN and update direction (forward
+/// or reverse).  D2UpdateMgr uses the services of D2CfgMgr to match requests
+/// to DNS server lists.
+///
+/// Once created, each transaction is responsible for carrying out the steps
+/// required to fulfill its specific request.  These steps typically consist of
+/// one or more DNS packet exchanges with the appropriate DNS server.  As
+/// transactions complete,  D2UpdateMgr removes them from the transaction list,
+/// replacing them with new transactions.
+///
+/// D2UpdateMgr carries out each of the above steps, from with a method called
+/// sweep().  This method is intended to be called as IO events complete.
+/// The upper layer(s) are responsible for calling sweep in a timely and cyclic
+/// manner.
+///
+class D2UpdateMgr : public boost::noncopyable {
+public:
+    /// @brief Maximum number of concurrent transactions
+    /// NOTE that 32 is an arbitrary choice picked for the initial
+    /// implementation.
+    static const size_t MAX_TRANSACTIONS_DEFAULT = 32;
+
+    // @todo This structure is not yet used. It is here in anticipation of
+    // enabled statistics capture.
+    struct Stats {
+        uint64_t start_time_;
+        uint64_t stop_time_;
+        uint64_t update_count_;
+        uint64_t min_update_time_;
+        uint64_t max_update_time_;
+        uint64_t server_rejects_;
+        uint64_t server_timeouts_;
+    };
+
+    /// @brief Constructor
+    ///
+    /// @param queue_mgr reference to the queue manager receiving requests
+    /// @param cfg_mgr reference to the configuration manager
+    /// @param io_service IO service used by the upper layer(s) to manage
+    /// IO events
+    /// @param max_transactions the maximum number of concurrent transactions
+    ///
+    /// @throw D2UpdateMgrError if either the queue manager or configuration
+    /// managers are NULL, or max transactions is less than one.
+    D2UpdateMgr(D2QueueMgrPtr& queue_mgr, D2CfgMgrPtr& cfg_mgr,
+                isc::asiolink::IOService& io_service,
+                const size_t max_transactions = MAX_TRANSACTIONS_DEFAULT);
+
+    /// @brief Destructor
+    virtual ~D2UpdateMgr();
+
+    /// @brief Check current transactions; start transactions for new requests.
+    ///
+    /// This method is the primary public interface used by the upper layer. It
+    /// should be called as IO events complete.  During each invocation it does
+    /// the following:
+    ///
+    /// - Removes all completed transactions from the transaction list.
+    ///
+    /// - If the request queue is not empty and the number of transactions
+    /// in the transaction list has not reached maximum allowed, then select
+    /// a request from the queue.
+    ///
+    /// - If a request was selected, start a new transaction for it and
+    /// add the transaction to the list of transactions.
+    void sweep();
+
+protected:
+    /// @brief Performs post-completion cleanup on completed transactions.
+    ///
+    /// Iterates through the list of transactions and removes any that have
+    /// reached completion.  This method may expand in complexity or even
+    /// disappear altogether as the implementation matures.
+    void checkFinishedTransactions();
+
+    /// @brief Starts a transaction for the next eligible request in the queue.
+    ///
+    /// This method will scan the request queue for the next request to
+    /// dequeue.  The current implementation starts at the front of the queue
+    /// and looks for the first request for whose DHCID there is no current
+    /// transaction in progress.
+    ///
+    /// If a request is selected, it is removed from the queue and transaction
+    /// is constructed for it.
+    ///
+    /// It is possible that no such request exists, though this is likely to be
+    /// rather rare unless a system is frequently seeing requests for the same
+    /// clients in quick succession.
+    void pickNextJob();
+
+    /// @brief Create a new transaction for the given request.
+    ///
+    /// This method will attempt to match the request to a list of configured
+    /// DNS servers.  If a list of servers is found, it will instantiate a
+    /// transaction for it and add the transaction to the transaction list.
+    ///
+    /// If no servers are found that match the request, this constitutes a
+    /// configuration error.  The error will be logged and the request will
+    /// be discarded.
+    ///
+    /// @param ncr the NameChangeRequest for which to create a transaction.
+    ///
+    /// @throw D2UpdateMgrError if a transaction for this DHCID already
+    /// exists. Note this would be programmatic error.
+    void makeTransaction(isc::dhcp_ddns::NameChangeRequestPtr& ncr);
+
+public:
+    /// @brief Returns the maximum number of concurrent transactions.
+    size_t getMaxTransactions() const {
+        return (max_transactions_);
+    }
+
+    /// @brief Sets the maximum number of entries allowed in the queue.
+    ///
+    /// @param max_transactions is the new maximum number of transactions
+    ///
+    /// @throw Throws D2QueueMgrError if the new value is less than one or if
+    /// the new value is less than the number of entries currently in the
+    /// queue.
+    void setMaxTransactions(const size_t max_transactions);
+
+    /// @brief Search the transaction list for the given key.
+    ///
+    /// @param key the transaction key value for which to search.
+    ///
+    /// @return Iterator pointing to the entry found.  If no entry is
+    /// it will point to the list end position.
+    TransactionList::iterator findTransaction(const TransactionKey& key);
+
+    /// @brief Returns the transaction list end position.
+    TransactionList::iterator transactionListEnd();
+
+    /// @brief Convenience method that checks transaction list for the given key
+    ///
+    /// @param key the transaction key value for which to search.
+    ///
+    /// @return Returns true if the key is found within the list, false
+    /// otherwise.
+    bool hasTransaction(const TransactionKey& key);
+
+    /// @brief Removes the entry pointed to by key from the transaction list.
+    ///
+    /// Removes the entry referred to by key if it exists.  It has no effect
+    /// if the entry is not found.
+    ///
+    /// @param key of the transaction to remove
+    void removeTransaction(const TransactionKey& key);
+
+    /// @brief Immediately discards all entries in the transaction list.
+    ///
+    /// @todo For now this just wipes them out. We might need something
+    /// more elegant, that allows a cancel first.
+    void clearTransactionList();
+
+    /// @brief Convenience method that returns the number of requests queued.
+    size_t getQueueCount() const;
+
+    /// @brief Returns the current number of transactions.
+    size_t getTransactionCount() const;
+
+private:
+    /// @brief Pointer to the queue manager.
+    D2QueueMgrPtr queue_mgr_;
+
+    /// @brief Pointer to the configuration manager.
+    D2CfgMgrPtr cfg_mgr_;
+
+    /// @brief Primary IOService instance.
+    /// This is the IOService that the upper layer(s) use for IO events, such
+    /// as shutdown and configuration commands.  It is the IOService that is
+    /// passed into transactions to manager their IO events.
+    /// (For future reference, multi-threaded transactions would each use their
+    /// own IOService instance.)
+    isc::asiolink::IOService& io_service_;
+
+    /// @brief Maximum number of concurrent transactions.
+    size_t max_transactions_;
+
+    /// @brief List of transactions.
+    TransactionList transaction_list_;
+};
+
+/// @brief Defines a pointer to a D2UpdateMgr instance.
+typedef boost::shared_ptr<D2UpdateMgr> D2UpdateMgrPtr;
+
+
+} // namespace isc::d2
+} // namespace isc
+#endif

src/bin/d2/tests/Makefile.am

@@ -61,6 +61,7 @@ d2_unittests_SOURCES += ../d2_config.cc ../d2_config.h
 d2_unittests_SOURCES += ../d2_cfg_mgr.cc ../d2_cfg_mgr.h
 d2_unittests_SOURCES += ../d2_queue_mgr.cc ../d2_queue_mgr.h
 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 += d_test_stubs.cc d_test_stubs.h
@@ -72,6 +73,7 @@ d2_unittests_SOURCES += d_cfg_mgr_unittests.cc
 d2_unittests_SOURCES += d2_cfg_mgr_unittests.cc
 d2_unittests_SOURCES += d2_queue_mgr_unittests.cc
 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
 nodist_d2_unittests_SOURCES = ../d2_messages.h ../d2_messages.cc

src/bin/d2/tests/d2_cfg_mgr_unittests.cc

@@ -53,7 +53,7 @@ public:
 /// @brief Tests that the spec file is valid.
 /// Verifies that the BIND10 DHCP-DDNS configuration specification file
 //  is valid.
-TEST(D2SpecTest, basicSpecTest) {
+TEST(D2SpecTest, basicSpec) {
     ASSERT_NO_THROW(isc::config::
                     moduleSpecFromFile(specfile("dhcp-ddns.spec")));
 }
@@ -252,7 +252,7 @@ public:
 /// 3. Secret cannot be blank.
 /// @TODO TSIG keys are not fully functional. Only basic validation is
 /// currently supported. This test will need to expand as they evolve.
-TEST_F(TSIGKeyInfoTest, invalidEntryTests) {
+TEST_F(TSIGKeyInfoTest, invalidEntry) {
     // Config with a blank name entry.
     std::string config = "{"
                          " \"name\": \"\" , "
@@ -294,7 +294,7 @@ TEST_F(TSIGKeyInfoTest, invalidEntryTests) {
 
 /// @brief Verifies that TSIGKeyInfo parsing creates a proper TSIGKeyInfo
 /// when given a valid combination of entries.
-TEST_F(TSIGKeyInfoTest, validEntryTests) {
+TEST_F(TSIGKeyInfoTest, validEntry) {
     // Valid entries for TSIG key, all items are required.
     std::string config = "{"
                          " \"name\": \"d2_key_one\" , "
@@ -448,7 +448,7 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) {
 /// 1. Specifying both a hostname and an ip address is not allowed.
 /// 2. Specifying both blank a hostname and blank ip address is not allowed.
 /// 3. Specifying a negative port number is not allowed.
-TEST_F(DnsServerInfoTest, invalidEntryTests) {
+TEST_F(DnsServerInfoTest, invalidEntry) {
     // Create a config in which both host and ip address are supplied.
     // Verify that it builds without throwing but commit fails.
     std::string config = "{ \"hostname\": \"pegasus.tmark\", "
@@ -480,7 +480,7 @@ TEST_F(DnsServerInfoTest, invalidEntryTests) {
 /// 1. A DnsServerInfo entry is correctly made, when given only a hostname.
 /// 2. A DnsServerInfo entry is correctly made, when given ip address and port.
 /// 3. A DnsServerInfo entry is correctly made, when given only an ip address.
-TEST_F(DnsServerInfoTest, validEntryTests) {
+TEST_F(DnsServerInfoTest, validEntry) {
     // Valid entries for dynamic host
     std::string config = "{ \"hostname\": \"pegasus.tmark\" }";
     ASSERT_TRUE(fromJSON(config));
@@ -831,7 +831,7 @@ TEST_F(DdnsDomainTest, DdnsDomainListParsing) {
 }
 
 /// @brief Tests that a domain list configuration cannot contain duplicates.
-TEST_F(DdnsDomainTest, duplicateDomainTest) {
+TEST_F(DdnsDomainTest, duplicateDomain) {
     // Create a domain list configuration that contains two domains with
     // the same name.
     std::string config =
@@ -885,7 +885,7 @@ TEST(D2CfgMgr, construction) {
 /// This tests passes the configuration into an instance of D2CfgMgr just
 /// as it would be done by d2_process in response to a configuration update
 /// event.
-TEST_F(D2CfgMgrTest, fullConfigTest) {
+TEST_F(D2CfgMgrTest, fullConfig) {
     // Create a configuration with all of application level parameters, plus
     // both the forward and reverse ddns managers.  Both managers have two
     // domains with three servers per domain.
@@ -1018,7 +1018,7 @@ TEST_F(D2CfgMgrTest, fullConfigTest) {
 /// 2. Given a FQDN for sub-domain in the list, returns the proper match.
 /// 3. Given a FQDN that matches no domain name, returns the wild card domain
 /// as a match.
-TEST_F(D2CfgMgrTest, forwardMatchTest) {
+TEST_F(D2CfgMgrTest, forwardMatch) {
     // Create  configuration with one domain, one sub domain, and the wild
     // card.
     std::string config = "{ "
@@ -1190,14 +1190,22 @@ TEST_F(D2CfgMgrTest, matchReverse) {
                         "\"forward_ddns\" : {}, "
                         "\"reverse_ddns\" : {"
                         "\"ddns_domains\": [ "
-                        "{ \"name\": \"100.168.192.in-addr.arpa\" , "
+                        "{ \"name\": \"5.100.168.192.in-addr.arpa.\" , "
                         "  \"dns_servers\" : [ "
                         "  { \"ip_address\": \"127.0.0.1\" } "
                         "  ] }, "
-                        "{ \"name\": \"168.192.in-addr.arpa\" , "
+                        "{ \"name\": \"100.200.192.in-addr.arpa.\" , "
                         "  \"dns_servers\" : [ "
                         "  { \"ip_address\": \"127.0.0.1\" } "
                         "  ] }, "
+                        "{ \"name\": \"170.192.in-addr.arpa.\" , "
+                        "  \"dns_servers\" : [ "
+                        "  { \"ip_address\": \"127.0.0.1\" } "
+                        "  ] }, "
+                        "{ \"name\": \"2.0.3.0.8.B.D.0.1.0.0.2.ip6.arpa.\" , "
+                        "  \"dns_servers\" : [ "
+                        "  { \"ip_address\": \"127.0.0.1\" } "
+                        "  ] },"
                         "{ \"name\": \"*\" , "
                         "  \"dns_servers\" : [ "
                         "  { \"ip_address\": \"127.0.0.1\" } "
@@ -1215,23 +1223,32 @@ TEST_F(D2CfgMgrTest, matchReverse) {
     ASSERT_NO_THROW(context = cfg_mgr_->getD2CfgContext());
 
     DdnsDomainPtr match;
+
     // Verify an exact match.
-    EXPECT_TRUE(cfg_mgr_->matchReverse("100.168.192.in-addr.arpa", match));
-    EXPECT_EQ("100.168.192.in-addr.arpa", match->getName());
+    EXPECT_TRUE(cfg_mgr_->matchReverse("192.168.100.5", match));
+    EXPECT_EQ("5.100.168.192.in-addr.arpa.", match->getName());
 
     // Verify a sub-domain match.
-    EXPECT_TRUE(cfg_mgr_->matchReverse("27.100.168.192.in-addr.arpa", match));
-    EXPECT_EQ("100.168.192.in-addr.arpa", match->getName());
+    EXPECT_TRUE(cfg_mgr_->matchReverse("192.200.100.27", match));
+    EXPECT_EQ("100.200.192.in-addr.arpa.", match->getName());
 
     // Verify a sub-domain match.
-    EXPECT_TRUE(cfg_mgr_->matchReverse("30.133.168.192.in-addr.arpa", match));
-    EXPECT_EQ("168.192.in-addr.arpa", match->getName());
+    EXPECT_TRUE(cfg_mgr_->matchReverse("192.170.50.30", match));
+    EXPECT_EQ("170.192.in-addr.arpa.", match->getName());
 
     // Verify a wild card match.
-    EXPECT_TRUE(cfg_mgr_->matchReverse("shouldbe.wildcard", match));
+    EXPECT_TRUE(cfg_mgr_->matchReverse("1.1.1.1", match));
     EXPECT_EQ("*", match->getName());
 
-    // Verify that an attempt to match an empty FQDN throws.
+    // Verify a IPv6 match.
+    EXPECT_TRUE(cfg_mgr_->matchReverse("2001:db8:302:99::",match));
+    EXPECT_EQ("2.0.3.0.8.B.D.0.1.0.0.2.ip6.arpa.", match->getName());
+
+    // Verify a IPv6 wild card match.
+    EXPECT_TRUE(cfg_mgr_->matchReverse("2001:db8:99:302::",match));
+    EXPECT_EQ("*", match->getName());
+
+    // Verify that an attempt to match an invalid IP address throws.
     ASSERT_THROW(cfg_mgr_->matchReverse("", match), D2CfgError);
 }
 

src/bin/d2/tests/d2_update_mgr_unittests.cc

@@ -0,0 +1,443 @@
+// 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 <asiolink/interval_timer.h>
+#include <d2/d2_update_mgr.h>
+#include <util/time_utilities.h>
+#include <d_test_stubs.h>
+
+#include <boost/function.hpp>
+#include <boost/bind.hpp>
+#include <gtest/gtest.h>
+#include <gtest/gtest.h>
+#include <algorithm>
+#include <vector>
+
+using namespace std;
+using namespace isc;
+using namespace isc::dhcp_ddns;
+using namespace isc::d2;
+
+namespace {
+
+/// @brief Wrapper class for D2UpdateMgr to provide acces non-public methods.
+///
+/// This class faciliates testing by making non-public methods accessible so
+/// they can be invoked directly in test routines.
+class D2UpdateMgrWrapper : public D2UpdateMgr {
+public:
+    /// @brief Constructor
+    ///
+    /// Parameters match those needed by D2UpdateMgr.
+    D2UpdateMgrWrapper(D2QueueMgrPtr& queue_mgr, D2CfgMgrPtr& cfg_mgr,
+                       isc::asiolink::IOService& io_service,
+                       const size_t max_transactions = MAX_TRANSACTIONS_DEFAULT)
+        : D2UpdateMgr(queue_mgr, cfg_mgr, io_service, max_transactions) {
+    }
+
+    /// @brief Destructor
+    virtual ~D2UpdateMgrWrapper() {
+    }
+
+    // Expose the protected methods to be tested.
+    using D2UpdateMgr::checkFinishedTransactions;
+    using D2UpdateMgr::pickNextJob;
+    using D2UpdateMgr::makeTransaction;
+};
+
+/// @brief Defines a pointer to a D2UpdateMgr instance.
+typedef boost::shared_ptr<D2UpdateMgrWrapper> D2UpdateMgrWrapperPtr;
+
+/// @brief Test fixture for testing D2UpdateMgr.
+///
+/// Note this class uses D2UpdateMgrWrapper class to exercise non-public
+/// aspects of D2UpdateMgr. D2UpdateMgr depends on both D2QueueMgr and
+/// D2CfgMgr.  This fixture provides an instance of each, plus a canned,
+/// valid DHCP_DDNS configuration sufficient to test D2UpdateMgr's basic
+/// functions.
+class D2UpdateMgrTest : public ConfigParseTest {
+public:
+    isc::asiolink::IOService io_service_;
+    D2QueueMgrPtr queue_mgr_;
+    D2CfgMgrPtr cfg_mgr_;
+    //D2UpdateMgrPtr update_mgr_;
+    D2UpdateMgrWrapperPtr update_mgr_;
+    std::vector<NameChangeRequestPtr> canned_ncrs_;
+    size_t canned_count_;
+
+    D2UpdateMgrTest() {
+        queue_mgr_.reset(new D2QueueMgr(io_service_));
+        cfg_mgr_.reset(new D2CfgMgr());
+        update_mgr_.reset(new D2UpdateMgrWrapper(queue_mgr_, cfg_mgr_,
+                                                 io_service_));
+        makeCannedNcrs();
+        makeCannedConfig();
+    }
+
+    ~D2UpdateMgrTest() {
+    }
+
+    /// @brief Creates a list of valid NameChangeRequest.
+    ///
+    /// This method builds a list of NameChangeRequests from a single
+    /// JSON string request. Each request is assigned a unique DHCID.
+    void makeCannedNcrs() {
+        const char* msg_str =
+        "{"
+        " \"change_type\" : 0 , "
+        " \"forward_change\" : true , "
+        " \"reverse_change\" : false , "
+        " \"fqdn\" : \"walah.walah.org.\" , "
+        " \"ip_address\" : \"192.168.2.1\" , "
+        " \"dhcid\" : \"0102030405060708\" , "
+        " \"lease_expires_on\" : \"20130121132405\" , "
+        " \"lease_length\" : 1300 "
+        "}";
+
+        const char* dhcids[] = { "111111", "222222", "333333", "444444"};
+        canned_count_ = 4;
+        for (int i = 0; i < canned_count_; i++) {
+            dhcp_ddns::NameChangeRequestPtr ncr = NameChangeRequest::
+                                                  fromJSON(msg_str);
+            ncr->setDhcid(dhcids[i]);
+            canned_ncrs_.push_back(ncr);
+        }
+    }
+
+    /// @brief Seeds configuration manager with a valid DHCP_DDNS configuration.
+    void makeCannedConfig() {
+        std::string canned_config_ =
+                 "{ "
+                  "\"interface\" : \"eth1\" , "
+                  "\"ip_address\" : \"192.168.1.33\" , "
+                  "\"port\" : 88 , "
+                  "\"tsig_keys\": [] ,"
+                  "\"forward_ddns\" : {"
+                  "\"ddns_domains\": [ "
+                  "{ \"name\": \"two.three.org.\" , "
+                  "  \"dns_servers\" : [ "
+                  "  { \"ip_address\": \"127.0.0.1\" } "
+                  "  ] },"
+                  "{ \"name\": \"org.\" , "
+                  "  \"dns_servers\" : [ "
+                  "  { \"ip_address\": \"127.0.0.1\" } "
+                  "  ] }, "
+                  "] }, "
+                  "\"reverse_ddns\" : { "
+                  "\"ddns_domains\": [ "
+                  "{ \"name\": \"1.168.192.in-addr.arpa.\" , "
+                  "  \"dns_servers\" : [ "
+                  "  { \"ip_address\": \"127.0.0.1\" } "
+                  "  ] }, "
+                  "{ \"name\": \"2.0.3.0.8.B.D.0.1.0.0.2.ip6.arpa.\" , "
+                  "  \"dns_servers\" : [ "
+                  "  { \"ip_address\": \"127.0.0.1\" } "
+                  "  ] } "
+                  "] } }";
+
+        // If this configuration fails to parse most tests will fail.
+        ASSERT_TRUE(fromJSON(canned_config_));
+        answer_ = cfg_mgr_->parseConfig(config_set_);
+        ASSERT_TRUE(checkAnswer(0));
+    }
+
+};
+
+/// @brief Tests the D2UpdateMgr construction.
+/// This test verifies that:
+/// 1. Construction with invalid queue manager is not allowed
+/// 2. Construction with invalid configuration manager is not allowed
+/// 3. Construction with max transactions of zero is not allowed
+/// 4. Default construction works and max transactions is defaulted properly
+/// 5. Construction with custom max transactions works properly
+TEST(D2UpdateMgr, construction) {
+    isc::asiolink::IOService io_service;
+    D2QueueMgrPtr queue_mgr;
+    D2CfgMgrPtr cfg_mgr;
+    D2UpdateMgrPtr update_mgr;
+
+    // Verify that constrctor fails if given an invalid queue manager.
+    ASSERT_NO_THROW(cfg_mgr.reset(new D2CfgMgr()));
+    EXPECT_THROW(D2UpdateMgr(queue_mgr, cfg_mgr, io_service),
+                 D2UpdateMgrError);
+
+    // Verify that constrctor fails if given an invalid config manager.
+    ASSERT_NO_THROW(queue_mgr.reset(new D2QueueMgr(io_service)));
+    ASSERT_NO_THROW(cfg_mgr.reset());
+    EXPECT_THROW(D2UpdateMgr(queue_mgr, cfg_mgr, io_service),
+                 D2UpdateMgrError);
+
+    ASSERT_NO_THROW(cfg_mgr.reset(new D2CfgMgr()));
+
+    // Verify that max transactions cannot be zero.
+    EXPECT_THROW(D2UpdateMgr(queue_mgr, cfg_mgr, io_service, 0),
+                 D2UpdateMgrError);
+
+    // Verify that given valid values, constructor works.
+    ASSERT_NO_THROW(update_mgr.reset(new D2UpdateMgr(queue_mgr, cfg_mgr,
+                                                      io_service)));
+
+    // Verify that max transactions defaults properly.
+    EXPECT_EQ(D2UpdateMgr::MAX_TRANSACTIONS_DEFAULT,
+              update_mgr->getMaxTransactions());
+
+
+    // Verify that constructor permits custom  max transactions.
+    ASSERT_NO_THROW(update_mgr.reset(new D2UpdateMgr(queue_mgr, cfg_mgr,
+                                                     io_service, 100)));
+
+    // Verify that max transactions is correct.
+    EXPECT_EQ(100, update_mgr->getMaxTransactions());
+}
+
+/// @brief Tests the D2UpdateManager's transaction list services
+/// This test verifies that:
+/// 1. A transaction can be added to the list.
+/// 2. Finding a transaction in the list by key works correctly.
+/// 3. Looking for a non-existant transaction works properly.
+/// 4. Attempting to add a transaction for a DHCID already in the list fails.
+/// 5. Removing a transaction by key works properly.
+/// 6. Attempting to remove an non-existant transaction does no harm.
+TEST_F(D2UpdateMgrTest, transactionList) {
+    // Grab a canned request for test purposes.
+    NameChangeRequestPtr& ncr = canned_ncrs_[0];
+    TransactionList::iterator pos;
+
+    // Verify that we can add a transaction.
+    EXPECT_NO_THROW(update_mgr_->makeTransaction(ncr));
+    EXPECT_EQ(1, update_mgr_->getTransactionCount());
+
+    // Verify that we can find a transaction by key.
+    EXPECT_NO_THROW(pos = update_mgr_->findTransaction(ncr->getDhcid()));
+    EXPECT_TRUE(pos != update_mgr_->transactionListEnd());
+
+    // Verify that convenience method has same result.
+    EXPECT_TRUE(update_mgr_->hasTransaction(ncr->getDhcid()));
+
+    // Verify that we will not find a transaction that isn't there.
+    dhcp_ddns::D2Dhcid bogus_id("FFFF");
+    EXPECT_NO_THROW(pos = update_mgr_->findTransaction(bogus_id));
+    EXPECT_TRUE(pos == update_mgr_->transactionListEnd());
+
+    // Verify that convenience method has same result.
+    EXPECT_FALSE(update_mgr_->hasTransaction(bogus_id));
+
+    // Verify that adding a transaction for the same key fails.
+    EXPECT_THROW(update_mgr_->makeTransaction(ncr), D2UpdateMgrError);
+    EXPECT_EQ(1, update_mgr_->getTransactionCount());
+
+    // Verify the we can remove a transaction by key.
+    EXPECT_NO_THROW(update_mgr_->removeTransaction(ncr->getDhcid()));
+    EXPECT_EQ(0, update_mgr_->getTransactionCount());
+
+    // Verify the we can try to remove a non-existant transaction without harm.
+    EXPECT_NO_THROW(update_mgr_->removeTransaction(ncr->getDhcid()));
+}
+
+/// @brief Tests D2UpdateManager's checkFinishedTransactions method.
+/// This test verifies that:
+/// 1. Completed transactions are removed from the transaction list.
+/// 2. Failed transactions are removed from the transaction list.
+/// @todo This test will need to expand if and when checkFinishedTransactions
+/// method expands to do more than remove them from the list.
+TEST_F(D2UpdateMgrTest, checkFinishedTransaction) {
+    // Ensure we have at least 4 canned requests with which to work.
+    ASSERT_TRUE(canned_count_ >= 4);
+
+    // Create a transaction for each canned request.
+    for (int i = 0; i < canned_count_; i++) {
+        EXPECT_NO_THROW(update_mgr_->makeTransaction(canned_ncrs_[i]));
+    }
+    // Verfiy we have that the transaçtion count is correct.
+    EXPECT_EQ(canned_count_, update_mgr_->getTransactionCount());
+
+    // Set two of the transactions to finished states.
+    (canned_ncrs_[1])->setStatus(dhcp_ddns::ST_COMPLETED);
+    (canned_ncrs_[3])->setStatus(dhcp_ddns::ST_FAILED);
+
+    // Verify that invoking checkFinishedTransactions does not throw.
+    EXPECT_NO_THROW(update_mgr_->checkFinishedTransactions());
+
+    // Verify that the list of transactions has decreased by two.
+    EXPECT_EQ(canned_count_ - 2, update_mgr_->getTransactionCount());
+
+    // Vefity that the transaction list is correct.
+    EXPECT_TRUE(update_mgr_->hasTransaction(canned_ncrs_[0]->getDhcid()));
+    EXPECT_FALSE(update_mgr_->hasTransaction(canned_ncrs_[1]->getDhcid()));
+    EXPECT_TRUE(update_mgr_->hasTransaction(canned_ncrs_[2]->getDhcid()));
+    EXPECT_FALSE(update_mgr_->hasTransaction(canned_ncrs_[3]->getDhcid()));
+}
+
+/// @brief Tests D2UpdateManager's pickNextJob method.
+/// This test verifies that:
+/// 1. pickNextJob will select and make transactions from NCR queue.
+/// 2. Requests are removed from the queue once selected
+/// 3. Requests for DHCIDs with transactions already in progress are not
+/// selected.
+/// 4. Requests with no matching servers are removed from the queue and
+/// discarded.
+TEST_F(D2UpdateMgrTest, pickNextJob) {
+    // Ensure we have at least 4 canned requests with which to work.
+    ASSERT_TRUE(canned_count_ >= 4);
+
+    // Put each transaction on the queue.
+    for (int i = 0; i < canned_count_; i++) {
+        ASSERT_NO_THROW(queue_mgr_->enqueue(canned_ncrs_[i]));
+    }
+
+    // Invoke pickNextJob canned_count_ times which should create a
+    // transaction for each canned ncr.
+    for (int i = 0; i < canned_count_; i++) {
+        EXPECT_NO_THROW(update_mgr_->pickNextJob());
+        EXPECT_EQ(i + 1, update_mgr_->getTransactionCount());
+        EXPECT_TRUE(update_mgr_->hasTransaction(canned_ncrs_[i]->getDhcid()));
+    }
+
+    // Verify that the queue has been drained.
+    EXPECT_EQ(0, update_mgr_->getQueueCount());
+
+    // Now verify that a subsequent request for a DCHID  for which a
+    // transaction is in progress, is not dequeued.
+    // First add the "subsequent" request.
+    dhcp_ddns::NameChangeRequestPtr
+        subsequent_ncr(new dhcp_ddns::NameChangeRequest(*(canned_ncrs_[2])));
+    EXPECT_NO_THROW(queue_mgr_->enqueue(subsequent_ncr));
+    EXPECT_EQ(1, update_mgr_->getQueueCount());
+
+    // Verify that invoking pickNextJob:
+    // 1. does not throw
+    // 2. does not make a new transaction
+    // 3. does not dequeu the entry
+    EXPECT_NO_THROW(update_mgr_->pickNextJob());
+    EXPECT_EQ(canned_count_, update_mgr_->getTransactionCount());
+    EXPECT_EQ(1, update_mgr_->getQueueCount());
+
+    // Clear out the queue and transaction list.
+    queue_mgr_->clearQueue();
+    update_mgr_->clearTransactionList();
+
+    // Make a forward change NCR with an FQDN that has no forward match.
+    dhcp_ddns::NameChangeRequestPtr
+        bogus_ncr(new dhcp_ddns::NameChangeRequest(*(canned_ncrs_[0])));
+    bogus_ncr->setForwardChange(true);
+    bogus_ncr->setReverseChange(false);
+    bogus_ncr->setFqdn("bogus.forward.domain.com");
+
+    // Put it on the queue up
+    ASSERT_NO_THROW(queue_mgr_->enqueue(bogus_ncr));
+
+    // Verify that invoking pickNextJob:
+    // 1. does not throw
+    // 2. does not make a new transaction
+    // 3. does dequeue the entry
+    EXPECT_NO_THROW(update_mgr_->pickNextJob());
+    EXPECT_EQ(0, update_mgr_->getTransactionCount());
+    EXPECT_EQ(0, update_mgr_->getQueueCount());
+
+    // Make a reverse change NCR with an FQDN that has no reverse match.
+    bogus_ncr.reset(new dhcp_ddns::NameChangeRequest(*(canned_ncrs_[0])));
+    bogus_ncr->setForwardChange(false);
+    bogus_ncr->setReverseChange(true);
+    bogus_ncr->setIpAddress("77.77.77.77");
+
+    // Verify that invoking pickNextJob:
+    // 1. does not throw
+    // 2. does not make a new transaction
+    // 3. does dequeue the entry
+    EXPECT_NO_THROW(update_mgr_->pickNextJob());
+    EXPECT_EQ(0, update_mgr_->getTransactionCount());
+    EXPECT_EQ(0, update_mgr_->getQueueCount());
+}
+
+/// @brief Tests D2UpdateManager's sweep method.
+/// Since sweep is primarly a wrapper around chechFinishedTransactions and
+/// pickNextJob, along with checks on maximum transaction limits, it mostly
+/// verifies that these three pieces work togther to move process jobs.
+/// Most of what is tested here is tested above.
+TEST_F(D2UpdateMgrTest, sweep) {
+    // Ensure we have at least 4 canned requests with which to work.
+    ASSERT_TRUE(canned_count_ >= 4);
+
+    // Set max transactions to same as current transaction count.
+    EXPECT_NO_THROW(update_mgr_->setMaxTransactions(canned_count_));
+    EXPECT_EQ(canned_count_, update_mgr_->getMaxTransactions());
+
+    // Put each transaction on the queue.
+    for (int i = 0; i < canned_count_; i++) {
+        EXPECT_NO_THROW(queue_mgr_->enqueue(canned_ncrs_[i]));
+    }
+
+    // Invoke sweep canned_count_ times which should create a
+    // transaction for each canned ncr.
+    for (int i = 0; i < canned_count_; i++) {
+        EXPECT_NO_THROW(update_mgr_->sweep());
+        EXPECT_EQ(i + 1, update_mgr_->getTransactionCount());
+        EXPECT_TRUE(update_mgr_->hasTransaction(canned_ncrs_[i]->getDhcid()));
+    }
+
+    // Verify that the queue has been drained.
+    EXPECT_EQ(0, update_mgr_->getQueueCount());
+
+    // Verify max transactions can't be less than current transaction count.
+    EXPECT_THROW(update_mgr_->setMaxTransactions(1), D2UpdateMgrError);
+
+    // Queue up a request for a DCHID which has a transaction in progress.
+    dhcp_ddns::NameChangeRequestPtr
+        subsequent_ncr(new dhcp_ddns::NameChangeRequest(*(canned_ncrs_[2])));
+    EXPECT_NO_THROW(queue_mgr_->enqueue(subsequent_ncr));
+    EXPECT_EQ(1, update_mgr_->getQueueCount());
+
+    // Verify that invoking sweep, does not dequeue the job nor make a
+    // transaction for it.
+    EXPECT_NO_THROW(update_mgr_->sweep());
+    EXPECT_EQ(canned_count_, update_mgr_->getTransactionCount());
+    EXPECT_EQ(1, update_mgr_->getQueueCount());
+
+    // Mark the transaction complete.
+    (canned_ncrs_[2])->setStatus(dhcp_ddns::ST_COMPLETED);
+
+    // Verify that invoking sweep, cleans up the completed transaction,
+    // dequeues the queued job and adds its transaction to the list.
+    EXPECT_NO_THROW(update_mgr_->sweep());
+    EXPECT_EQ(canned_count_, update_mgr_->getTransactionCount());
+    EXPECT_EQ(0, update_mgr_->getQueueCount());
+
+    // Queue up a request from a new DHCID.
+    dhcp_ddns::NameChangeRequestPtr
+        another_ncr(new dhcp_ddns::NameChangeRequest(*(canned_ncrs_[0])));
+    another_ncr->setDhcid("AABBCCDDEEFF");
+    EXPECT_NO_THROW(queue_mgr_->enqueue(another_ncr));
+    EXPECT_EQ(1, update_mgr_->getQueueCount());
+
+    // Verify that sweep does not dequeue the new request as we are at
+    // transaction count.
+    EXPECT_NO_THROW(update_mgr_->sweep());
+    EXPECT_EQ(canned_count_, update_mgr_->getTransactionCount());
+    EXPECT_EQ(1, update_mgr_->getQueueCount());
+
+    // Set max transactions to same as current transaction count.
+    EXPECT_NO_THROW(update_mgr_->setMaxTransactions(canned_count_ + 1));
+
+    // Verify that invoking sweep, dequeues the request and creates
+    // a transaction for it.
+    EXPECT_NO_THROW(update_mgr_->sweep());
+    EXPECT_EQ(canned_count_ + 1, update_mgr_->getTransactionCount());
+    EXPECT_EQ(0, update_mgr_->getQueueCount());
+
+    // Verify that clearing transaction list works.
+    EXPECT_NO_THROW(update_mgr_->clearTransactionList());
+    EXPECT_EQ(0, update_mgr_->getTransactionCount());
+}
+
+}

src/lib/dhcp_ddns/ncr_msg.h

@@ -107,7 +107,12 @@ public:
     /// @brief Compares two D2Dhcids for inequality
     bool operator!=(const D2Dhcid& other) const {
         return (this->bytes_ != other.bytes_);
-}
+    }
+
+    /// @brief Compares two D2Dhcids lexcially
+    bool operator<(const D2Dhcid& other) const {
+        return (this->bytes_ < other.bytes_);
+    }
 
 private:
     /// @brief Storage for the DHCID value in unsigned bytes.

src/lib/dhcp_ddns/tests/ncr_unittests.cc

@@ -240,6 +240,7 @@ TEST(NameChangeRequestTest, constructionTests) {
 /// 2. DHCID input strings must contain only hexadecimal character digits
 /// 3. A valid DHCID string converts correctly.
 /// 4. Converting a D2Dhcid to a string works correctly.
+/// 5. Equality, inequality, and less-than-equal operators work.
 TEST(NameChangeRequestTest, dhcidTest) {
     D2Dhcid dhcid;
 
@@ -271,6 +272,20 @@ TEST(NameChangeRequestTest, dhcidTest) {
     // DHCID input string.
     std::string next_str = dhcid.toStr();
     EXPECT_EQ(test_str, next_str);
+
+    // Test equality, inequality, and less-than-equal operators
+    test_str="AABBCCDD";
+    EXPECT_NO_THROW(dhcid.fromStr(test_str));
+
+    D2Dhcid other_dhcid;
+    EXPECT_NO_THROW(other_dhcid.fromStr(test_str));
+
+    EXPECT_TRUE(dhcid == other_dhcid);
+    EXPECT_FALSE(dhcid != other_dhcid);
+
+    EXPECT_NO_THROW(other_dhcid.fromStr("BBCCDDEE"));
+    EXPECT_TRUE(dhcid < other_dhcid);
+
 }
 
 /// @brief Verifies the fundamentals of converting from and to JSON.