From: Thomas Markwalder Date: Tue, 17 Dec 2013 13:43:10 +0000 (-0500) Subject: [3088] Addressed review changes. X-Git-Tag: bind10-1.2.0beta1-release~173^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=59bd021c72b3e2d9538b534f3ba00ae4b410da62;p=thirdparty%2Fkea.git [3088] Addressed review changes. Changes are largely clean up and commentary. --- diff --git a/src/bin/d2/d2_messages.mes b/src/bin/d2/d2_messages.mes index cd21f11d92..a5bd5ec706 100644 --- a/src/bin/d2/d2_messages.mes +++ b/src/bin/d2/d2_messages.mes @@ -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. diff --git a/src/bin/d2/nc_add.cc b/src/bin/d2/nc_add.cc index 3773d6edbf..da20763ace 100644 --- a/src/bin/d2/nc_add.cc +++ b/src/bin/d2/nc_add.cc @@ -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; diff --git a/src/bin/d2/nc_add.h b/src/bin/d2/nc_add.h index a9e748fd0c..1fe167b99e 100644 --- a/src/bin/d2/nc_add.h +++ b/src/bin/d2/nc_add.h @@ -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 NameAddTransactionPtr; } // namespace isc::d2 diff --git a/src/bin/d2/nc_remove.cc b/src/bin/d2/nc_remove.cc index 401333eb8c..d7135bdd9f 100644 --- a/src/bin/d2/nc_remove.cc +++ b/src/bin/d2/nc_remove.cc @@ -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; diff --git a/src/bin/d2/nc_remove.h b/src/bin/d2/nc_remove.h index 3790a7c897..f7b0dcc081 100644 --- a/src/bin/d2/nc_remove.h +++ b/src/bin/d2/nc_remove.h @@ -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 NameRemoveTransactionPtr; diff --git a/src/bin/d2/nc_trans.cc b/src/bin/d2/nc_trans.cc index 0d9fc77d2a..e264df579a 100644 --- a/src/bin/d2/nc_trans.cc +++ b/src/bin/d2/nc_trans.cc @@ -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& 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: " diff --git a/src/bin/d2/nc_trans.h b/src/bin/d2/nc_trans.h index 7415d88d65..86a89d7198 100644 --- a/src/bin/d2/nc_trans.h +++ b/src/bin/d2/nc_trans.h @@ -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(); diff --git a/src/bin/d2/tests/nc_remove_unittests.cc b/src/bin/d2/tests/nc_remove_unittests.cc index 2ae93263f5..2f12feb8b0 100644 --- a/src/bin/d2/tests/nc_remove_unittests.cc +++ b/src/bin/d2/tests/nc_remove_unittests.cc @@ -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);