]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[3088] Addressed review changes.
authorThomas Markwalder <tmark@isc.org>
Tue, 17 Dec 2013 13:43:10 +0000 (08:43 -0500)
committerThomas Markwalder <tmark@isc.org>
Tue, 17 Dec 2013 13:43:10 +0000 (08:43 -0500)
Changes are largely clean up and commentary.

src/bin/d2/d2_messages.mes
src/bin/d2/nc_add.cc
src/bin/d2/nc_add.h
src/bin/d2/nc_remove.cc
src/bin/d2/nc_remove.h
src/bin/d2/nc_trans.cc
src/bin/d2/nc_trans.h
src/bin/d2/tests/nc_remove_unittests.cc

index cd21f11d92297b1088ebdc9e1b3d856c2e78a2d4..a5bd5ec706e321f5563713b734802371d561913f 100644 (file)
@@ -347,12 +347,12 @@ aborted.  This is most likely a configuration issue.
 This is a debug message issued after DHCP_DDNS has submitted DNS mapping
 additions which were received and accepted by an appropriate DNS server.
 
-% DHCP_DDNS_ADD_FAILED DHCP_DDNS failed attempting to make DNS mapping additions for this request: %1
+% DHCP_DDNS_ADD_FAILED DHCP_DDNS failed attempting to make DNS mapping additions for this request: %1, event: %2
 This is an error message issued after DHCP_DDNS attempts to submit DNS mapping
 entry additions have failed.  The precise reason for the failure should be
 documented in preceding log entries.
 
-% DHCP_DDNS_FORWARD_REMOVE_ADDRS_BUILD_FAILURE DNS udpate message to remove a forward DNS Address entry could not be constructed for this request: %1  reason: %2
+% DHCP_DDNS_FORWARD_REMOVE_ADDRS_BUILD_FAILURE DNS udpate message to remove a forward DNS Address entry could not be constructed for this request: %1,  reason: %2
 This is an error message issued when an error occurs attempting to construct
 the server bound packet requesting a forward address (A or AAAA) removal.  This
 is due to invalid data contained in the NameChangeRequest. The request will be
@@ -378,7 +378,7 @@ This is an error message issued when DNSClient returns an unrecognized status
 while DHCP_DDNS was removing a forward address mapping.  The request will be
 aborted.  This is most likely a programmatic issue and should be reported.
 
-% DHCP_DDNS_FORWARD_REMOVE_RRS_BUILD_FAILURE DNS udpate message to remove forward DNS RR entries could not be constructed for this request: %1  reason: %2
+% DHCP_DDNS_FORWARD_REMOVE_RRS_BUILD_FAILURE DNS udpate message to remove forward DNS RR entries could not be constructed for this request: %1,  reason: %2
 This is an error message issued when an error occurs attempting to construct
 the server bound packet requesting forward RR (DHCID RR) removal.  This is due
 to invalid data contained in the NameChangeRequest. The request will be aborted.This is most likely a configuration issue.
@@ -403,7 +403,7 @@ This is an error message issued when DNSClient returns an unrecognized status
 while DHCP_DDNS was removing forward RRs.  The request will be aborted. This is
 most likely a programmatic issue and should be reported.
 
-% DHCP_DDNS_REVERSE_REMOVE_BUILD_FAILURE DNS update message to remove a reverse DNS entry could not be constructed from this request: %1  reason: %2
+% DHCP_DDNS_REVERSE_REMOVE_BUILD_FAILURE DNS update message to remove a reverse DNS entry could not be constructed from this request: %1,  reason: %2
 This is an error message issued when an error occurs attempting to construct
 the server bound packet requesting a reverse PTR removal.  This is
 due to invalid data contained in the NameChangeRequest. The request will be
@@ -433,8 +433,8 @@ aborted.  This is most likely a programmatic issue and should be reported.
 This is a debug message issued after DHCP_DDNS has submitted DNS mapping
 removals which were received and accepted by an appropriate DNS server.
 
-% DHCP_DDNS_REMOVE_FAILED DHCP_DDNS failed attempting to make DNS mapping removals for this request: %1
+% DHCP_DDNS_REMOVE_FAILED DHCP_DDNS failed attempting to make DNS mapping removals for this request: %1, event: %2
 This is an error message issued after DHCP_DDNS attempts to submit DNS mapping
-entry removals have failed.  There precise reason for the failure should be
+entry removals have failed.  The precise reason for the failure should be
 documented in preceding log entries.
 
index 3773d6edbf46a66b13a5f7862d771fb1af64edfb..da20763ace7424106a348ef34e08d8de510da59f 100644 (file)
@@ -61,10 +61,18 @@ NameAddTransaction::defineEvents() {
 
 void
 NameAddTransaction::verifyEvents() {
-    // Call superclass impl first.
+    // Call superclass implementation first to verify its events. These are
+    // events common to all transactions, and they must be defined.
+    // SELECT_SERVER_EVT
+    // SERVER_SELECTED_EVT
+    // SERVER_IO_ERROR_EVT
+    // NO_MORE_SERVERS_EVT
+    // IO_COMPLETED_EVT
+    // UPDATE_OK_EVT
+    // UPDATE_FAILED_EVT
     NameChangeTransaction::verifyEvents();
 
-    // Verify NameAddTransaction events.
+    // Verify NameAddTransaction events by attempting to fetch them.
     getEvent(FQDN_IN_USE_EVT);
     getEvent(FQDN_NOT_IN_USE_EVT);
 }
@@ -102,10 +110,16 @@ NameAddTransaction::defineStates() {
 }
 void
 NameAddTransaction::verifyStates() {
-    // Call superclass impl first.
+    // Call superclass implementation first to verify its states. These are
+    // states common to all transactions, and they must be defined.
+    // READY_ST
+    // SELECTING_FWD_SERVER_ST
+    // SELECTING_REV_SERVER_ST
+    // PROCESS_TRANS_OK_ST
+    // PROCESS_TRANS_FAILED_ST
     NameChangeTransaction::verifyStates();
 
-    // Verify NameAddTransaction states.
+    // Verify NameAddTransaction states by attempting to fetch them.
     getState(ADDING_FWD_ADDRS_ST);
     getState(REPLACING_FWD_ADDRS_ST);
     getState(REPLACING_REV_PTRS_ST);
@@ -542,7 +556,8 @@ NameAddTransaction::processAddFailedHandler() {
     switch(getNextEvent()) {
     case UPDATE_FAILED_EVT:
     case NO_MORE_SERVERS_EVT:
-        LOG_ERROR(dctl_logger, DHCP_DDNS_ADD_FAILED).arg(getNcr()->toText());
+        LOG_ERROR(dctl_logger, DHCP_DDNS_ADD_FAILED).arg(getNcr()->toText())
+        .arg(getContextStr());
         setNcrStatus(dhcp_ddns::ST_FAILED);
         endModel();
         break;
index a9e748fd0c715ae1c1045628974882922f38ab27..1fe167b99e0aa7cf3c846cb32147d92583d07708 100644 (file)
@@ -110,7 +110,8 @@ protected:
     /// @brief Validates the contents of the set of events.
     ///
     /// Invokes NameChangeTransaction's implementation and then verifies the
-    /// Add transaction's events.
+    /// Add transaction's.  This tests that the needed events are in the event
+    /// dictionary.
     ///
     /// @throw StateModelError if an event value is undefined.
     virtual void verifyEvents();
@@ -126,7 +127,8 @@ protected:
     /// @brief Validates the contents of the set of states.
     ///
     /// Invokes NameChangeTransaction's implementation and then verifies the
-    /// Add transaction's states.
+    /// Add transaction's states. This tests that the needed states are in the
+    /// state dictionary.
     ///
     /// @throw StateModelError if an event value is undefined.
     virtual void verifyStates();
@@ -441,7 +443,7 @@ protected:
     void buildReplaceRevPtrsRequest();
 };
 
-/// @brief Defines a pointer to a NameChangeTransaction.
+/// @brief Defines a pointer to a NameAddTransaction.
 typedef boost::shared_ptr<NameAddTransaction> NameAddTransactionPtr;
 
 } // namespace isc::d2
index 401333eb8ca3d726a43652dfd4e4a11038aa0b71..d7135bdd9ff5b44b3646de7912632746d3bd699e 100644 (file)
@@ -58,10 +58,18 @@ NameRemoveTransaction::defineEvents() {
 
 void
 NameRemoveTransaction::verifyEvents() {
-    // Call superclass impl first.
+    // Call superclass implementation first to verify its events. These are
+    // events common to all transactions, and they must be defined.
+    // SELECT_SERVER_EVT
+    // SERVER_SELECTED_EVT
+    // SERVER_IO_ERROR_EVT
+    // NO_MORE_SERVERS_EVT
+    // IO_COMPLETED_EVT
+    // UPDATE_OK_EVT
+    // UPDATE_FAILED_EVT
     NameChangeTransaction::verifyEvents();
 
-    // Verify NameRemoveTransaction events.
+    // Verify NameRemoveTransaction events by attempting to fetch them.
     // Currently NameRemoveTransaction does not define any events.
     // getEvent(TBD_EVENT);
 }
@@ -106,10 +114,16 @@ NameRemoveTransaction::defineStates() {
 
 void
 NameRemoveTransaction::verifyStates() {
-    // Call superclass impl first.
+    // Call superclass implementation first to verify its states. These are
+    // states common to all transactions, and they must be defined.
+    // READY_ST
+    // SELECTING_FWD_SERVER_ST
+    // SELECTING_REV_SERVER_ST
+    // PROCESS_TRANS_OK_ST
+    // PROCESS_TRANS_FAILED_ST
     NameChangeTransaction::verifyStates();
 
-    // Verify NameRemoveTransaction states.
+    // Verify NameRemoveTransaction states by attempting to fetch them.
     getState(REMOVING_FWD_ADDRS_ST);
     getState(REMOVING_FWD_RRS_ST);
     getState(REMOVING_REV_PTRS_ST);
@@ -551,7 +565,8 @@ NameRemoveTransaction::processRemoveFailedHandler() {
     case UPDATE_FAILED_EVT:
     case NO_MORE_SERVERS_EVT:
     case SERVER_IO_ERROR_EVT:
-        LOG_ERROR(dctl_logger, DHCP_DDNS_REMOVE_FAILED).arg(getNcr()->toText());
+        LOG_ERROR(dctl_logger, DHCP_DDNS_REMOVE_FAILED).arg(getNcr()->toText())
+        .arg(getEventLabel(getNextEvent()));
         setNcrStatus(dhcp_ddns::ST_FAILED);
         endModel();
         break;
index 3790a7c8972e02e32b61036a8e4ac4cf3e878382..f7b0dcc081495b375838cbf229c65c3792341bc9 100644 (file)
@@ -95,7 +95,7 @@ public:
     virtual ~NameRemoveTransaction();
 
 protected:
-    /// @brief Removes events defined by NameRemoveTransaction to the event set.
+    /// @brief Adds events defined by NameRemoveTransaction to the event set.
     ///
     /// Invokes NameChangeTransaction's implementation and then defines the
     /// events unique to NCR Remove transaction processing.
@@ -106,12 +106,13 @@ protected:
     /// @brief Validates the contents of the set of events.
     ///
     /// Invokes NameChangeTransaction's implementation and then verifies the
-    /// Remove transaction's events.
+    /// Remove transaction's events. This tests that the needed events are in
+    /// the event dictionary.
     ///
     /// @throw StateModelError if an event value is undefined.
     virtual void verifyEvents();
 
-    /// @brief Removes states defined by NameRemoveTransaction to the state set.
+    /// @brief Adds states defined by NameRemoveTransaction to the state set.
     ///
     /// Invokes NameChangeTransaction's implementation and then defines the
     /// states unique to NCR Remove transaction processing.
@@ -122,7 +123,8 @@ protected:
     /// @brief Validates the contents of the set of states.
     ///
     /// Invokes NameChangeTransaction's implementation and then verifies the
-    /// Remove transaction's states.
+    /// Remove transaction's states.  This tests that the needed states are in
+    /// the state dictionary.
     ///
     /// @throw StateModelError if an event value is undefined.
     virtual void verifyStates();
@@ -134,7 +136,7 @@ protected:
     ///
     /// The READY_ST is the state the model transitions into when the inherited
     /// method, startTransaction() is invoked.  This handler, therefore, is the
-    /// entry point into the state model execution.h  Its primary task is to
+    /// entry point into the state model execution.  Its primary task is to
     /// determine whether to start with a forward DNS change or a reverse DNS
     /// change.
     ///
@@ -424,7 +426,7 @@ protected:
     void buildRemoveRevPtrsRequest();
 };
 
-/// @brief Defines a pointer to a NameChangeTransaction.
+/// @brief Defines a pointer to a NameRemoveTransaction.
 typedef boost::shared_ptr<NameRemoveTransaction> NameRemoveTransactionPtr;
 
 
index 0d9fc77d2ac3ad2625fc23f6343af1a0ba0a97b5..e264df579a674ffbe5e18cc804e74ecad574dff6 100644 (file)
@@ -263,12 +263,13 @@ NameChangeTransaction::addLeaseAddressRdata(dns::RRsetPtr& rrset) {
 
     try {
         // Manufacture an RData from the lease address then add it to the RR.
-        dns::rdata::ConstRdataPtr rdata;
         if (ncr_->isV4()) {
-            rdata.reset(new dns::rdata::in::A(ncr_->getIpAddress()));
+            dns::rdata::ConstRdataPtr rdata(new dns::rdata::in::
+                                            A(ncr_->getIpAddress()));
             rrset->addRdata(rdata);
         } else {
-            rdata.reset(new dns::rdata::in::AAAA(ncr_->getIpAddress()));
+            dns::rdata::ConstRdataPtr rdata(new dns::rdata::in::
+                                            AAAA(ncr_->getIpAddress()));
             rrset->addRdata(rdata);
         }
     } catch (const std::exception& ex) {
@@ -286,10 +287,10 @@ NameChangeTransaction::addDhcidRdata(dns::RRsetPtr& rrset) {
     }
 
     try {
-        dns::rdata::ConstRdataPtr rdata;
         const std::vector<uint8_t>& ncr_dhcid = ncr_->getDhcid().getBytes();
         util::InputBuffer buffer(ncr_dhcid.data(), ncr_dhcid.size());
-        rdata.reset(new dns::rdata::in::DHCID(buffer, ncr_dhcid.size()));
+        dns::rdata::ConstRdataPtr rdata (new dns::rdata::in::
+                                         DHCID(buffer, ncr_dhcid.size()));
         rrset->addRdata(rdata);
     } catch (const std::exception& ex) {
         isc_throw(NameChangeTransactionError, "Cannot add DCHID rdata: "
@@ -306,8 +307,8 @@ NameChangeTransaction::addPtrRdata(dns::RRsetPtr& rrset) {
     }
 
     try {
-        dns::rdata::ConstRdataPtr rdata;
-        rdata.reset(new dns::rdata::generic::PTR(getNcr()->getFqdn()));
+        dns::rdata::ConstRdataPtr rdata(new dns::rdata::generic::
+                                        PTR(getNcr()->getFqdn()));
         rrset->addRdata(rdata);
     } catch (const std::exception& ex) {
         isc_throw(NameChangeTransactionError, "Cannot add PTR rdata: "
index 7415d88d655dd4764d7796e8ce891133f17b7b94..86a89d71987a262750d714d83f52bd3ecf971e7c 100644 (file)
@@ -277,7 +277,7 @@ protected:
     /// If the maximum number of attempts has been reached, it will transition
     /// to the given state with a next event of SERVER_IO_ERROR_EVT.
     ///
-    /// @param server_sel_state  State to transition to if maximum attempts
+    /// @param fail_to_state  State to transition to if maximum attempts
     /// have been tried.
     ///
     void retryTransition(const int fail_to_state);
@@ -435,13 +435,13 @@ public:
 
     /// @brief Fetches the forward DdnsDomain.
     ///
-    /// @return A pointer reference to the forward DdnsDomain.  If 
+    /// @return A pointer reference to the forward DdnsDomain.  If
     /// the request does not include a forward change, the pointer will empty.
     DdnsDomainPtr& getForwardDomain();
 
     /// @brief Fetches the reverse DdnsDomain.
     ///
-    /// @return A pointer reference to the reverse DdnsDomain.  If 
+    /// @return A pointer reference to the reverse DdnsDomain.  If
     /// the request does not include a reverse change, the pointer will empty.
     DdnsDomainPtr& getReverseDomain();
 
index 2ae93263f58cf79af93194630ca49681b704611b..2f12feb8b0618c3065ae266891ab8d7c4e8b3cde 100644 (file)
@@ -33,9 +33,9 @@ namespace {
 class NameRemoveStub : public NameRemoveTransaction {
 public:
     NameRemoveStub(IOServicePtr& io_service,
-                dhcp_ddns::NameChangeRequestPtr& ncr,
-                DdnsDomainPtr& forward_domain,
-                DdnsDomainPtr& reverse_domain)
+                   dhcp_ddns::NameChangeRequestPtr& ncr,
+                   DdnsDomainPtr& forward_domain,
+                   DdnsDomainPtr& reverse_domain)
         : NameRemoveTransaction(io_service, ncr, forward_domain,
                                 reverse_domain),
           simulate_send_exception_(false),
@@ -328,7 +328,6 @@ TEST_F(NameRemoveTransactionTest, buildRemoveFwdAddressRequest) {
     // and then verify the request contents.
     NameRemoveStubPtr name_remove;
     ASSERT_NO_THROW(name_remove = makeTransaction4(FORWARD_CHG));
-    (name_remove->buildRemoveFwdAddressRequest());
     ASSERT_NO_THROW(name_remove->buildRemoveFwdAddressRequest());
     checkRemoveFwdAddressRequest(*name_remove);