From: Francis Dupont <fdupont@isc.org>
Date: Tue, 12 Mar 2019 14:04:29 +0000 (+0100)
Subject: [481-remote-subnet4-set-inconsistent-work-when-id-subnet-is-duplicated] Addressed... 
X-Git-Tag: Kea-1.6.0-beta~387
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=5d9614502516a2edf31df301b807ba5c0cfa3965;p=thirdparty%2Fkea.git

[481-remote-subnet4-set-inconsistent-work-when-id-subnet-is-duplicated] Addressed comments
---

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 eeea6e9aef..435de2bba3 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
@@ -622,7 +622,7 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getSubnet4) {
     EXPECT_EQ(subnet2->toElement()->str(), returned_subnet->toElement()->str());
 
     // Fetching the subnet for an explicitly specified server tag should
-    // succeeed too.
+    // succeed too.
     returned_subnet = cbptr_->getSubnet4(ServerSelector::ONE("server1"),
                                          SubnetID(1024));
     EXPECT_EQ(subnet2->toElement()->str(), returned_subnet->toElement()->str());
@@ -639,19 +639,26 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getSubnet4) {
 
     // Fetch this subnet by prefix and verify it matches.
     returned_subnet = cbptr_->getSubnet4(ServerSelector::ALL(),
-					 test_subnets_[2]->toText());
+                                         test_subnets_[2]->toText());
     ASSERT_TRUE(returned_subnet);
     EXPECT_EQ(test_subnets_[2]->toElement()->str(), returned_subnet->toElement()->str());
 
-    // Update the the subnet in the database (both use the same prefix).
+    // Update the subnet in the database (both use the same prefix).
     subnet2.reset(new Subnet4(IOAddress("192.0.3.0"), 24, 30, 40, 60, 8192));
     cbptr_->createUpdateSubnet4(ServerSelector::ALL(),  subnet2);
 
     // Fetch again and verify.
     returned_subnet = cbptr_->getSubnet4(ServerSelector::ALL(),
-					 test_subnets_[2]->toText());
+                                         test_subnets_[2]->toText());
     ASSERT_TRUE(returned_subnet);
     EXPECT_EQ(subnet2->toElement()->str(), returned_subnet->toElement()->str());
+
+    // Update the subnet when it conflicts same id and same prefix both
+    // with different subnets. This should throw.
+    // Subnets are 10.0.0.0/8 id 1024 and 192.0.3.0/24 id 8192
+    subnet2.reset(new Subnet4(IOAddress("10.0.0.0"), 8, 30, 40, 60, 8192));
+    EXPECT_THROW(cbptr_->createUpdateSubnet4(ServerSelector::ALL(),  subnet2),
+                 DbOperationError);
 }
 
 // Test that the information about unspecified optional parameters gets
diff --git a/src/lib/mysql/mysql_binding.h b/src/lib/mysql/mysql_binding.h
index f46f45f623..7511bfd669 100644
--- a/src/lib/mysql/mysql_binding.h
+++ b/src/lib/mysql/mysql_binding.h
@@ -569,7 +569,7 @@ private:
     /// created using static factory functions.
     ///
     /// @param buffer_type MySQL buffer type as defined in MySQL C API.
-    /// @param length Buffer length. 
+    /// @param length Buffer length.
     MySqlBinding(enum_field_types buffer_type, const size_t length);
 
     /// @brief Assigns new value to a buffer.
diff --git a/src/lib/mysql/mysql_connection.h b/src/lib/mysql/mysql_connection.h
index 183934ca59..2b59d8dbbb 100644
--- a/src/lib/mysql/mysql_connection.h
+++ b/src/lib/mysql/mysql_connection.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2018 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2019 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -442,7 +442,7 @@ public:
             // Failure: check for the special case of duplicate entry.
             if (mysql_errno(mysql_) == ER_DUP_ENTRY) {
                 isc_throw(DuplicateEntry, "Database duplicate entry error");
-        }
+            }
             checkError(status, index, "unable to execute");
         }
     }
@@ -477,6 +477,10 @@ public:
         status = mysql_stmt_execute(statements_[index]);
 
         if (status != 0) {
+            // Failure: check for the special case of duplicate entry.
+            if (mysql_errno(mysql_) == ER_DUP_ENTRY) {
+                isc_throw(DuplicateEntry, "Database duplicate entry error");
+            }
             checkError(status, index, "unable to execute");
         }
 
diff --git a/src/share/database/scripts/mysql/dhcpdb_create.mysql b/src/share/database/scripts/mysql/dhcpdb_create.mysql
index 6ca8a3eead..d4bfa892ce 100644
--- a/src/share/database/scripts/mysql/dhcpdb_create.mysql
+++ b/src/share/database/scripts/mysql/dhcpdb_create.mysql
@@ -1909,7 +1909,7 @@ ALTER TABLE dhcp6_shared_network_server
     ON DELETE CASCADE ON UPDATE NO ACTION;
 
 -- Update dhcp4_subnet_server and dhcp6_subnet_server to allow update
--- on the prefix too.
+-- on the prefix too by setting the CASCADE action.
 
 ALTER TABLE dhcp4_subnet_server
     DROP FOREIGN KEY fk_dhcp4_subnet_server_subnet_id;
diff --git a/src/share/database/scripts/mysql/upgrade_7.0_to_8.0.sh.in b/src/share/database/scripts/mysql/upgrade_7.0_to_8.0.sh.in
index 959f4b823a..70ec6a6717 100644
--- a/src/share/database/scripts/mysql/upgrade_7.0_to_8.0.sh.in
+++ b/src/share/database/scripts/mysql/upgrade_7.0_to_8.0.sh.in
@@ -591,7 +591,7 @@ ALTER TABLE dhcp6_shared_network_server
     ON DELETE CASCADE ON UPDATE NO ACTION;
 
 -- Update dhcp4_subnet_server and dhcp6_subnet_server to allow update
--- on the prefix too.
+-- on the prefix too by setting the CASCADE action.
 
 ALTER TABLE dhcp4_subnet_server
     DROP FOREIGN KEY fk_dhcp4_subnet_server_subnet_id;