[#95] Addressed minor review comments

src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc
    Explicitly handle ddns-replace-cient-name-mode binding

src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc
    GenericConfigBackendDHCPv4Test::testNewAuditEntry(0
    - Fail on empty tag list.

src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.h
    Add doxygen

src/share/database/scripts/pgsql/dhcpdb_create.pgsql
src/share/database/scripts/pgsql/upgrade_008_to_009.sh.in
    Fix function discrepancies on argument types

diff --git a/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc b/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc
index 48fa140afd5357a5e7f5904aadac8f2a09f8bdb6..124f6a2dac1657eaac6e651053f2cda2c55fc43c 100644 (file)
--- a/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc
+++ b/src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc
@@ -824,9 +824,7 @@ public:
                 in_bindings.addInet4(pool_end_address);
 
                 getPools(GET_POOL4_RANGE, in_bindings, pools, pool_ids);
-                if (!pools.empty()) {
-                    break;
-                }
+                // Break if something is found?
             }
         }
 
@@ -932,7 +930,14 @@ public:
         in_bindings.addOptional(subnet->getDdnsSendUpdates(Network::Inheritance::NONE));
         in_bindings.addOptional(subnet->getDdnsOverrideNoUpdate(Network::Inheritance::NONE));
         in_bindings.addOptional(subnet->getDdnsOverrideClientUpdate(Network::Inheritance::NONE));
-        in_bindings.addOptional(subnet->getDdnsReplaceClientNameMode(Network::Inheritance::NONE));
+
+        auto ddns_rcn_mode = subnet->getDdnsReplaceClientNameMode(Network::Inheritance::NONE);
+        if (!ddns_rcn_mode.unspecified()) {
+            in_bindings.add(static_cast<uint8_t>(ddns_rcn_mode.get()));
+        } else {
+            in_bindings.addNull();
+        }
+
         in_bindings.addOptional(subnet->getDdnsGeneratedPrefix(Network::Inheritance::NONE));
         in_bindings.addOptional(subnet->getDdnsQualifyingSuffix(Network::Inheritance::NONE));
         in_bindings.addOptional(subnet->getReservationsInSubnet(Network::Inheritance::NONE));
@@ -1494,7 +1499,14 @@ public:
         in_bindings.addOptional(shared_network->getDdnsSendUpdates(Network::Inheritance::NONE));
         in_bindings.addOptional(shared_network->getDdnsOverrideNoUpdate(Network::Inheritance::NONE));
         in_bindings.addOptional(shared_network->getDdnsOverrideClientUpdate(Network::Inheritance::NONE));
-        in_bindings.addOptional(shared_network->getDdnsReplaceClientNameMode(Network::Inheritance::NONE));
+
+        auto ddns_rcn_mode = shared_network->getDdnsReplaceClientNameMode(Network::Inheritance::NONE);
+        if (!ddns_rcn_mode.unspecified()) {
+            in_bindings.add(static_cast<uint8_t>(ddns_rcn_mode.get()));
+        } else {
+            in_bindings.addNull();
+        }
+
         in_bindings.addOptional(shared_network->getDdnsGeneratedPrefix(Network::Inheritance::NONE));
         in_bindings.addOptional(shared_network->getDdnsQualifyingSuffix(Network::Inheritance::NONE));
         in_bindings.addOptional(shared_network->getReservationsInSubnet(Network::Inheritance::NONE));

diff --git a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc
index 31b3396248cab4fc1944d3a858cd989d106dbb80..d5778b84a5673614975663ba9e01f030e0fe1cec 100644 (file)
--- a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc
+++ b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc
@@ -517,12 +517,13 @@ GenericConfigBackendDHCPv4Test::testNewAuditEntry(const std::vector<ExpAuditEntr
     } else {
         const auto& tags = server_selector.getTags();
         // This test is not meant to handle multiple server tags all at once.
-        if (tags.size() > 1) {
-            ADD_FAILURE() << "Test error: do not use multiple server tags";
-        } else if (tags.size() == 1) {
-            // Get the server tag for which we run the current test.
-            tag = tags.begin()->get();
-        }
+        if (tags.size() != 1) {
+            ADD_FAILURE() << "Test error: tags.size(): " << tags.size() 
+                          << ", you must specify one and only one server tag";
+        } 
+
+        // Get the server tag for which we run the current test.
+        tag = tags.begin()->get();
     }
 
     size_t new_entries_num = exp_entries.size();

diff --git a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.h b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.h
index d15a5291ccdca71d1b5f8275bd63f1d06dfd530a..040158c96c9962ad94dd3ea1f2b29a9b4dba4542 100644 (file)
--- a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.h
+++ b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.h
@@ -15,9 +15,13 @@ namespace isc {
 namespace dhcp {
 namespace test {
 
+/// @brief Describes an expected audit table entry.
 struct ExpAuditEntry {
+    /// @brief Type of object changed.
     std::string object_type;
+    /// @brief Timestamp the change occurred.
     db::AuditEntry::ModificationType modification_type;
+    /// @brief Log message describing the change.
     std::string log_message;
 };
 

diff --git a/src/share/database/scripts/pgsql/dhcpdb_create.pgsql b/src/share/database/scripts/pgsql/dhcpdb_create.pgsql
index 7558b620ebcdc69c84c1c70c023ea59071ee79d6..c8d18f680cdccda244ff6049b86dcd4783ebce55 100644 (file)
--- a/src/share/database/scripts/pgsql/dhcpdb_create.pgsql
+++ b/src/share/database/scripts/pgsql/dhcpdb_create.pgsql
@@ -4338,7 +4338,7 @@ DECLARE
     -- we will select.
     snid BIGINT;
     sid BIGINT;
-    cascade_transaction BOOLEAN := true;
+    cascade_transaction BOOLEAN;
 BEGIN
     -- Cascade transaction flag is set to true to prevent creation of
     -- the audit entries for the options when the options are
@@ -4419,12 +4419,12 @@ END;$$;
 --   allow table aliases to be used with column names in update
 --   set expressions.
 -- -----------------------------------------------------
-CREATE OR REPLACE FUNCTION createOptionAuditDHCP6(modification_type VARCHAR(32),
+CREATE OR REPLACE FUNCTION createOptionAuditDHCP6(modification_type VARCHAR,
                                                   scope_id SMALLINT,
                                                   option_id INT,
                                                   p_subnet_id BIGINT,
                                                   host_id INT,
-                                                  network_name VARCHAR(128),
+                                                  network_name VARCHAR,
                                                   pool_id BIGINT,
                                                   pd_pool_id BIGINT,
                                                   p_modification_ts TIMESTAMP WITH TIME ZONE)
@@ -4436,7 +4436,7 @@ DECLARE
     -- we will select.
     snid BIGINT;
     sid BIGINT;
-    cascade_transaction BOOLEAN := false;
+    cascade_transaction BOOLEAN;
 BEGIN
     -- Cascade transaction flag is set to true to prevent creation of
     -- the audit entries for the options when the options are

diff --git a/src/share/database/scripts/pgsql/upgrade_008_to_009.sh.in b/src/share/database/scripts/pgsql/upgrade_008_to_009.sh.in
index 87315bb8d05342e19282c4649fd56790859b2c1b..04cc8d44141c20665410c36e5ae886229adee6ac 100644 (file)
--- a/src/share/database/scripts/pgsql/upgrade_008_to_009.sh.in
+++ b/src/share/database/scripts/pgsql/upgrade_008_to_009.sh.in
@@ -110,7 +110,7 @@ DECLARE
     -- we will select.
     snid BIGINT;
     sid BIGINT;
-    cascade_transaction BOOLEAN := true;
+    cascade_transaction BOOLEAN;
 BEGIN
     -- Cascade transaction flag is set to true to prevent creation of
     -- the audit entries for the options when the options are
@@ -191,12 +191,12 @@ END;\$\$;
 --   allow table aliases to be used with column names in update
 --   set expressions.
 -- -----------------------------------------------------
-CREATE OR REPLACE FUNCTION createOptionAuditDHCP6(modification_type VARCHAR(32),
+CREATE OR REPLACE FUNCTION createOptionAuditDHCP6(modification_type VARCHAR,
                                                   scope_id SMALLINT,
                                                   option_id INT,
                                                   p_subnet_id BIGINT,
                                                   host_id INT,
-                                                  network_name VARCHAR(128),
+                                                  network_name VARCHAR,
                                                   pool_id BIGINT,
                                                   pd_pool_id BIGINT,
                                                   p_modification_ts TIMESTAMP WITH TIME ZONE)
@@ -208,7 +208,7 @@ DECLARE
     -- we will select.
     snid BIGINT;
     sid BIGINT;
-    cascade_transaction BOOLEAN := false;
+    cascade_transaction BOOLEAN;
 BEGIN
     -- Cascade transaction flag is set to true to prevent creation of
     -- the audit entries for the options when the options are