[#1386] Addressed review comments

src/bin/d2/d2_update_mgr.h
src/bin/d2/simple_add.cc
    Fixed mispellings

src/bin/d2/tests/d2_update_mgr_unittests.cc
    Fixed compiler warning/errors for unused variables
    Fixed mispellings

     // and sent the DNS request.
src/bin/d2/tests/nc_remove_unittests.cc
    fixed copy-paste typo

src/bin/d2/tests/nc_test_utils.cc
    Added commentary, removed superfluous whitespace

src/bin/d2/tests/nc_test_utils.h
    Fixed mispellings

src/lib/dhcp_ddns/ncr_msg.*
    Fixed mispellings

diff --git a/src/bin/d2/d2_update_mgr.h b/src/bin/d2/d2_update_mgr.h
index 1717cdd31f63d4cdb137ce911db6f8ef961b37b2..8ef128c66b592deec391f3202ce2a794dd5a3abc 100644 (file)
--- a/src/bin/d2/d2_update_mgr.h
+++ b/src/bin/d2/d2_update_mgr.h
@@ -156,8 +156,8 @@ protected:
     /// discarded.
     ///
     /// Finally, If conflict resolution is enabled, it will instantiate either
-    /// a NameAddTranscation or a NameRemoveTransaction. If disabled it will
-    /// instantiate either a SimpleAddTransaction or a SimpleRemoveTranscation.
+    /// a NameAddTransaction or a NameRemoveTransaction. If disabled it will
+    /// instantiate either a SimpleAddTransaction or a SimpleRemoveTransaction.
     ///
     /// @param ncr the NameChangeRequest for which to create a transaction.
     ///

diff --git a/src/bin/d2/simple_add.cc b/src/bin/d2/simple_add.cc
index 652966f37436d6af243d4feef545771e84258614..6ca7a3cfe2ddde0e93918a661f939da8ae6b6b60 100644 (file)
--- a/src/bin/d2/simple_add.cc
+++ b/src/bin/d2/simple_add.cc
@@ -489,7 +489,7 @@ SimpleAddTransaction::buildReplaceFwdAddressRequest() {
                                 dns::RRType::DHCID(), lease_ttl));
 
     // We add the DHCID for auditing purposes and in the event
-    // conflict resoloution is later enabled.
+    // conflict resolution is later enabled.
     addDhcidRdata(update);
     request->addRRset(D2UpdateMessage::SECTION_UPDATE, update);
 

diff --git a/src/bin/d2/tests/d2_update_mgr_unittests.cc b/src/bin/d2/tests/d2_update_mgr_unittests.cc
index 9ddee7534e8b7b6561e5b653f673494ebc8e4ddb..8ab2b246e9dc6270a0bab082ea3bf3c89b58f7c5 100644 (file)
--- a/src/bin/d2/tests/d2_update_mgr_unittests.cc
+++ b/src/bin/d2/tests/d2_update_mgr_unittests.cc
@@ -681,8 +681,9 @@ TEST_F(D2UpdateMgrTest, addTransaction) {
     NameChangeTransactionPtr trans = (*pos).second;
     ASSERT_TRUE(trans);
    
-    // Verify the correct type of transcation was created. 
-    ASSERT_NO_THROW(dynamic_cast<NameAddTransaction&>(*trans)); 
+    // Verify the correct type of transaction was created. 
+    NameAddTransaction* t = dynamic_cast<NameAddTransaction*>(trans.get());
+    ASSERT_TRUE(t);
 
     // At this point the transaction should have constructed
     // and sent the DNS request.
@@ -737,8 +738,9 @@ TEST_F(D2UpdateMgrTest, removeTransaction) {
     NameChangeTransactionPtr trans = (*pos).second;
     ASSERT_TRUE(trans);
 
-    // Verify the correct type of transcation was created. 
-    ASSERT_NO_THROW(dynamic_cast<NameRemoveTransaction&>(*trans)); 
+    // Verify the correct type of transaction was created. 
+    NameRemoveTransaction* t = dynamic_cast<NameRemoveTransaction*>(trans.get());
+    ASSERT_TRUE(t);
 
     // At this point the transaction should have constructed
     // and sent the DNS request.
@@ -895,8 +897,9 @@ TEST_F(D2UpdateMgrTest, simpleAddTransaction) {
     NameChangeTransactionPtr trans = (*pos).second;
     ASSERT_TRUE(trans);
    
-    // Verify the correct type of transcation was created. 
-    ASSERT_NO_THROW(dynamic_cast<SimpleAddTransaction&>(*trans)); 
+    // Verify the correct type of transaction was created. 
+    SimpleAddTransaction* t = dynamic_cast<SimpleAddTransaction*>(trans.get());
+    ASSERT_TRUE(t);
 
     // At this point the transaction should have constructed
     // and sent the DNS request.
@@ -952,8 +955,9 @@ TEST_F(D2UpdateMgrTest, simpleRemoveTransaction) {
     NameChangeTransactionPtr trans = (*pos).second;
     ASSERT_TRUE(trans);
 
-    // Verify the correct type of transcation was created. 
-    ASSERT_NO_THROW(dynamic_cast<SimpleRemoveTransaction&>(*trans)); 
+    // Verify the correct type of transaction was created. 
+    SimpleRemoveTransaction* t = dynamic_cast<SimpleRemoveTransaction*>(trans.get());
+    ASSERT_TRUE(t);
 
     // At this point the transaction should have constructed
     // and sent the DNS request.

diff --git a/src/bin/d2/tests/nc_remove_unittests.cc b/src/bin/d2/tests/nc_remove_unittests.cc
index f5ced0a61b280f29887e7b09f3504d1d081c018f..e19000460aa2aa12314c64f3493a4773962103e6 100644 (file)
--- a/src/bin/d2/tests/nc_remove_unittests.cc
+++ b/src/bin/d2/tests/nc_remove_unittests.cc
@@ -583,7 +583,7 @@ TEST_F(NameRemoveTransactionTest, removingFwdAddrsHandler_FwdOnlyOK) {
               name_remove->getNextEvent());
 }
 
-// Tests addingFwdAddrsHandler with the following scenario:
+// Tests removingFwdAddrsHandler with the following scenario:
 //
 //  The request includes only a forward change.
 //  Initial posted event is SERVER_SELECTED_EVT.

diff --git a/src/bin/d2/tests/nc_test_utils.cc b/src/bin/d2/tests/nc_test_utils.cc
index 79b3dc54b43a1d65b7e74d0649d99477c9241c08..f6392e07a22ddc99137ade653d42cefe36f8ddc0 100644 (file)
--- a/src/bin/d2/tests/nc_test_utils.cc
+++ b/src/bin/d2/tests/nc_test_utils.cc
@@ -834,16 +834,16 @@ void checkSimpleReplaceFwdAddressRequest(NameChangeTransaction& tran) {
     checkZone(request, exp_zone_name);
 
     // Verify the PREREQUISITE SECTION
-    // There should be no prerequisites. 
+    // There should be no prerequisites.
     dns::RRsetPtr rrset;
     checkRRCount(request, D2UpdateMessage::SECTION_PREREQUISITE, 0);
 
     // Verify the UPDATE SECTION
     // Should be 4
     // 1. delete of the FQDN/IP RR
-    // 2. delete of the DHCID RR 
+    // 2. delete of the DHCID RR
     // 3. add of the FQDN/IP RR
-    // 4. add of the DHCID RR 
+    // 4. add of the DHCID RR
     checkRRCount(request, D2UpdateMessage::SECTION_UPDATE, 4);
 
     // Fetch ttl.
@@ -917,6 +917,9 @@ void checkSimpleRemoveFwdRRsRequest(NameChangeTransaction& tran) {
     ASSERT_NO_THROW(request->toWire(renderer));
 }
 
+// Verifies that the contents of the given transaction's  DNS update request
+// is correct for removing a reverse DNS entry when not using conflict
+// resolution.
 void checkSimpleRemoveRevPtrsRequest(NameChangeTransaction& tran) {
     const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest();
     ASSERT_TRUE(request);
@@ -927,7 +930,6 @@ void checkSimpleRemoveRevPtrsRequest(NameChangeTransaction& tran) {
 
     std::string exp_zone_name = tran.getReverseDomain()->getName();
     std::string exp_rev_addr = D2CfgMgr::reverseIpAddress(ncr->getIpAddress());
-    const dns::RRType& exp_ip_rr_type = tran.getAddressRRType();
 
     // Verify the zone section.
     checkZone(request, exp_zone_name);
@@ -954,6 +956,7 @@ void checkSimpleRemoveRevPtrsRequest(NameChangeTransaction& tran) {
     ASSERT_NO_THROW(request->toWire(renderer));
 }
 
+// Verifies the current state and next event in a transaction
 void checkContext(NameChangeTransactionPtr trans, const int exp_state,
                   const int exp_evt, const std::string& file, int line) {
     ASSERT_TRUE(trans);
@@ -964,5 +967,5 @@ void checkContext(NameChangeTransactionPtr trans, const int exp_state,
             << " at " << file << ":" << line;
 }
 
-}; // namespace isc::d2
-}; // namespace isc
+} // namespace isc::d2
+} // namespace isc

diff --git a/src/bin/d2/tests/nc_test_utils.h b/src/bin/d2/tests/nc_test_utils.h
index bfb9240938c04598263a8f7ee666d456898ba317..26eacd82d05b15e745bf58c3cdac06347ff23921 100644 (file)
--- a/src/bin/d2/tests/nc_test_utils.h
+++ b/src/bin/d2/tests/nc_test_utils.h
@@ -472,7 +472,7 @@ extern std::string toHexText(const uint8_t* data, size_t len);
 /// @param exp_state expected current state of the transaction
 /// @param exp_event  expected next event of the transaction
 /// @param file source file name
-/// @param line souce line number
+/// @param line source line number
 extern void checkContext(NameChangeTransactionPtr trans, const int exp_state,
                          const int exp_evt, const std::string& file, int line);
 
@@ -480,7 +480,7 @@ extern void checkContext(NameChangeTransactionPtr trans, const int exp_state,
 #define CHECK_CONTEXT(a,b,c) checkContext(a,b,c,__FILE__,__LINE__)
 
 
-}; // namespace isc::d2
-}; // namespace isc
+} // namespace isc::d2
+} // namespace isc
 
 #endif

diff --git a/src/lib/dhcp_ddns/ncr_msg.cc b/src/lib/dhcp_ddns/ncr_msg.cc
index f2b2718a259d669d1b1c39bf4ea174a4a168bdd9..545a85f32f3af9ebd49071a2ac8a2d3d004b5b1a 100644 (file)
--- a/src/lib/dhcp_ddns/ncr_msg.cc
+++ b/src/lib/dhcp_ddns/ncr_msg.cc
@@ -344,7 +344,7 @@ NameChangeRequest::fromJSON(const std::string& json) {
     element = ncr->getElement("lease-length", element_map);
     ncr->setLeaseLength(element);
 
-    // For backward compatiblity  use-conflict-resolution is optional
+    // For backward compatibility  use-conflict-resolution is optional
     // and defaults to true.
     auto found = element_map.find("use-conflict-resolution"); 
     if (found != element_map.end()) {

diff --git a/src/lib/dhcp_ddns/ncr_msg.h b/src/lib/dhcp_ddns/ncr_msg.h
index 1a718a1f65d5a219b82117e3c75d8894c74e564a..0acab2bb48accb2533d667885d2d4c95f4391f0d 100644 (file)
--- a/src/lib/dhcp_ddns/ncr_msg.h
+++ b/src/lib/dhcp_ddns/ncr_msg.h
@@ -747,7 +747,7 @@ private:
     /// @brief The amount of time in seconds for which the lease is valid (TTL).
     uint32_t lease_length_;
 
-    /// @brief Indicates if conflict resoltuion is enabled.
+    /// @brief Indicates if conflict resolution is enabled.
     bool conflict_resolution_;
 
     /// @brief The processing status of the request.  Used internally.