From: Marcin Siodelski Date: Tue, 9 Jul 2019 09:20:33 +0000 (+0200) Subject: [#716,!412] Addressed easy review comments. X-Git-Tag: Kea-1.6.0-beta2~84 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=cc8b6fc5cee988b6c93f939d990df524de1ea6a1;p=thirdparty%2Fkea.git [#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 6b2f59358f..21320a45f3 100644 --- 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(mysql_insert_id(conn_.mysql_)), - in_bindings[11]); + MySqlBinding::createInteger(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 103f1b2e0c..9dfb2f47e5 100644 --- 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(mysql_insert_id(conn_.mysql_)), - in_bindings[11]); + MySqlBinding::createInteger(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 f56d2dab72..8e1b0eeecb 100644 --- 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 336f7b74ae..020d8f59c9 100644 --- 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 0998b1f1b6..19fbe316b3 100644 --- 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)); {