[#716,!412] Addressed easy review comments.

- Use variable to hold mysql_insert_id in insertOption4/6 function.
- Use variable for timestamp binding in insertOption4/6 function
- Removed unused MYSQL_DELETE_OPTION_SERVER query
- Removed LAST_INSERT_ID from a query
- Unit test tries to insert shared network for non-existing server.

diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc
index 6b2f59358f9650663693173260e345b8c788413a..21320a45f30c66dd13dd5201a24444dcd7048a78 100644 (file)
--- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc
+++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc
@@ -1406,11 +1406,18 @@ public:
         conn_.insertQuery(MySqlConfigBackendDHCPv4Impl::INSERT_OPTION4,
                           in_bindings);
 
+        // Fetch primary key value of the inserted option. We will use it in the
+        // next INSERT statement to associate this option with the server.
+        auto option_id = mysql_insert_id(conn_.mysql_);
+
+        // Timestamp is expected to be in this input binding.
+        auto timestamp_binding = in_bindings[11];
+
         // Associate the option with the servers.
         attachElementToServers(MySqlConfigBackendDHCPv4Impl::INSERT_OPTION4_SERVER,
                                server_selector,
-                               MySqlBinding::createInteger<uint64_t>(mysql_insert_id(conn_.mysql_)),
-                               in_bindings[11]);
+                               MySqlBinding::createInteger<uint64_t>(option_id),
+                               timestamp_binding);
     }
 
     /// @brief Sends query to insert or update global DHCP option.

diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc
index 103f1b2e0c618301181a1c84a4c1e9a359795074..9dfb2f47e518ba1aaf5f30c17ab1ec81914f2048 100644 (file)
--- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc
+++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc
@@ -1621,11 +1621,18 @@ public:
         conn_.insertQuery(MySqlConfigBackendDHCPv6Impl::INSERT_OPTION6,
                           in_bindings);
 
+        // Fetch primary key value of the inserted option. We will use it in the
+        // next INSERT statement to associate this option with the server.
+        auto option_id = mysql_insert_id(conn_.mysql_);
+
+        // Timestamp is expected to be in this input binding.
+        auto timestamp_binding = in_bindings[11];
+
         // Associate the option with the servers.
         attachElementToServers(MySqlConfigBackendDHCPv6Impl::INSERT_OPTION6_SERVER,
                                server_selector,
-                               MySqlBinding::createInteger<uint64_t>(mysql_insert_id(conn_.mysql_)),
-                               in_bindings[11]);
+                               MySqlBinding::createInteger<uint64_t>(option_id),
+                               timestamp_binding);
     }
 
     /// @brief Sends query to insert or update global DHCP option.

diff --git a/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h b/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h
index f56d2dab72f9b43adccf262adfc5bd84f7aeee83..8e1b0eeecb260af0c0717e9cd35ac10d8d411188 100644 (file)
--- a/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h
+++ b/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h
@@ -610,7 +610,6 @@ namespace {
     "INNER JOIN " #table_prefix "_server AS s" \
     "  ON a.server_id = s.id " \
     "SET" \
-    "  o.option_id = LAST_INSERT_ID(o.option_id)," \
     "  o.code = ?," \
     "  o.value = ?," \
     "  o.formatted_value = ?," \
@@ -782,18 +781,6 @@ namespace {
     "   WHERE prefix = ? AND prefix_length = ?)"
 #endif
 
-#ifndef MYSQL_DELETE_OPTION_SERVER
-#define MYSQL_DELETE_OPTION_SERVER(table_prefix) \
-    "DELETE os FROM " #table_prefix "_options_server AS os " \
-    "WHERE os.option_id = " \
-    "  (SELECT o.option_id FROM " #table_prefix "_options AS o" \
-    "   INNER JOIN " #table_prefix "_options_server AS a" \
-    "      ON o.option_id = a.option_id " \
-    "   INNER JOIN " #table_prefix "_server AS s" \
-    "      ON a.server_id = s.id " \
-    "   WHERE s.tag = ? AND o.scope_id = ? AND o.code = ? AND o.space = ?)"
-#endif
-
 #ifndef MYSQL_DELETE_SERVER
 #define MYSQL_DELETE_SERVER(table_prefix) \
     "DELETE FROM " #table_prefix "_server " \

diff --git a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc
index 336f7b74aee2439e3cb9dfe6201616347902a27b..020d8f59c9244a92dc2dc717e007edc37106d1d5 100644 (file)
--- a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc
+++ b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc
@@ -1531,6 +1531,13 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getSharedNetwork4) {
 // Test that shared network may be created and updated and the server tags
 // are properly assigned to it.
 TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateSharedNetwork4) {
+    auto shared_network = test_networks_[0];
+
+    // An attempto insert the shared network for non-existing server should fail.
+    EXPECT_THROW(cbptr_->createUpdateSharedNetwork4(ServerSelector::ONE("server1"),
+                                                    shared_network),
+                 DbOperationError);
+
     // Insert the server1 into the database.
     EXPECT_NO_THROW(cbptr_->createUpdateServer4(test_servers_[0]));
     {
@@ -1549,8 +1556,6 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateSharedNetwork4) {
                           "server set");
     }
 
-    auto shared_network = test_networks_[0];
-
     EXPECT_NO_THROW(cbptr_->createUpdateSharedNetwork4(ServerSelector::ALL(),
                                                        shared_network));
     {

diff --git a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc
index 0998b1f1b6b422f53b917cc829b8e505bf7af1ae..19fbe316b30495246a107d48879ef39ed7fc71ab 100644 (file)
--- a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc
+++ b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc
@@ -1546,6 +1546,13 @@ TEST_F(MySqlConfigBackendDHCPv6Test, getSharedNetwork6) {
 // Test that shared network may be created and updated and the server tags
 // are properly assigned to it.
 TEST_F(MySqlConfigBackendDHCPv6Test, createUpdateSharedNetwork6) {
+    auto shared_network = test_networks_[0];
+
+    // An attempto insert the shared network for non-existing server should fail.
+    EXPECT_THROW(cbptr_->createUpdateSharedNetwork6(ServerSelector::ONE("server1"),
+                                                    shared_network),
+                 DbOperationError);
+
     // Insert the server1 into the database.
     EXPECT_NO_THROW(cbptr_->createUpdateServer6(test_servers_[0]));
     {
@@ -1564,8 +1571,6 @@ TEST_F(MySqlConfigBackendDHCPv6Test, createUpdateSharedNetwork6) {
                           "server set");
     }
 
-    auto shared_network = test_networks_[0];
-
     EXPECT_NO_THROW(cbptr_->createUpdateSharedNetwork6(ServerSelector::ALL(),
                                                        shared_network));
     {