From: Marcin Siodelski <marcin@isc.org>
Date: Mon, 16 May 2016 14:44:09 +0000 (+0200)
Subject: [4281] Creation of a new host in MySQL is wrapped in transaction.
X-Git-Tag: trac4106_update_base~7^2~3^2~9
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=60192f6630bfdc6db683f6f1b9477f63b614b430;p=thirdparty%2Fkea.git

[4281] Creation of a new host in MySQL is wrapped in transaction.
---

diff --git a/src/lib/dhcpsrv/mysql_connection.cc b/src/lib/dhcpsrv/mysql_connection.cc
index 196ecce850..8096cfd7aa 100755
--- a/src/lib/dhcpsrv/mysql_connection.cc
+++ b/src/lib/dhcpsrv/mysql_connection.cc
@@ -27,6 +27,29 @@ const my_bool MLM_TRUE = 1;
 const int MLM_MYSQL_FETCH_SUCCESS = 0;
 const int MLM_MYSQL_FETCH_FAILURE = 1;
 
+
+MySqlTransaction::MySqlTransaction(MySqlConnection& conn)
+    : conn_(conn), committed_(false) {
+    int status = mysql_query(conn_.mysql_, "START TRANSACTION");
+    if (status != 0) {
+        isc_throw(DbOperationError, "unable to start transaction, "
+                  "reason: " << mysql_error(conn_.mysql_));
+    }
+}
+
+MySqlTransaction::~MySqlTransaction() {
+    if (!committed_) {
+        conn_.rollback();
+    }
+}
+
+void
+MySqlTransaction::commit() {
+    conn_.commit();
+    committed_ = true;
+}
+
+
 // Open the database using the parameters passed to the constructor.
 
 void
diff --git a/src/lib/dhcpsrv/mysql_connection.h b/src/lib/dhcpsrv/mysql_connection.h
index ba0798cba2..866bf6472a 100755
--- a/src/lib/dhcpsrv/mysql_connection.h
+++ b/src/lib/dhcpsrv/mysql_connection.h
@@ -140,6 +140,26 @@ private:
     MYSQL* mysql_;      ///< Initialization context
 };
 
+class MySqlConnection;
+
+class MySqlTransaction : public boost::noncopyable {
+public:
+
+    MySqlTransaction(MySqlConnection& conn);
+
+    ~MySqlTransaction();
+
+    void commit();
+
+private:
+
+    MySqlConnection& conn_;
+
+    bool committed_;
+
+};
+
+
 /// @brief Common MySQL Connector Pool
 ///
 /// This class provides common operations for MySQL database connection
@@ -294,8 +314,6 @@ public:
     MySqlHolder mysql_;
 };
 
-
-
 }; // end of isc::dhcp namespace
 }; // end of isc namespace
 
diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc
index ada7c49647..3bd484dd45 100644
--- a/src/lib/dhcpsrv/mysql_host_data_source.cc
+++ b/src/lib/dhcpsrv/mysql_host_data_source.cc
@@ -1982,6 +1982,8 @@ MySqlHostDataSource::~MySqlHostDataSource() {
 
 void
 MySqlHostDataSource::add(const HostPtr& host) {
+    MySqlTransaction transaction(impl_->conn_);
+
     // Create the MYSQL_BIND array for the host
     std::vector<MYSQL_BIND> bind = impl_->host_exchange_->createBindForSend(host);
 
@@ -2009,6 +2011,8 @@ MySqlHostDataSource::add(const HostPtr& host) {
     if (cfg_option6) {
         impl_->addOptions(INSERT_V6_OPTION, cfg_option6, host_id);
     }
+
+    transaction.commit();
 }
 
 ConstHostCollection
diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc
index 94c3ff8ce7..7c0d8c363e 100644
--- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc
+++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc
@@ -12,6 +12,7 @@
 #include <dhcp/option_string.h>
 #include <dhcp/option_int.h>
 #include <dhcp/option_vendor.h>
+#include <dhcpsrv/mysql_connection.h>
 #include <dhcpsrv/tests/generic_host_data_source_unittest.h>
 #include <dhcpsrv/tests/test_utils.h>
 #include <dhcpsrv/database_connection.h>
@@ -967,6 +968,37 @@ void GenericHostDataSourceTest::testAddDuplicate4() {
     EXPECT_NO_THROW(hdsptr_->add(host));
 }
 
+void GenericHostDataSourceTest::testAddRollback() {
+    // Make sure we have the pointer to the host data source.
+    ASSERT_TRUE(hdsptr_);
+
+    MySqlConnection::ParameterMap params;
+    params["name"] = "keatest";
+    params["user"] = "keatest";
+    params["password"] = "keatest";
+    MySqlConnection conn(params);
+    ASSERT_NO_THROW(conn.openDatabase());
+    int status = mysql_query(conn.mysql_, "DROP TABLE IF EXISTS ipv6_reservations");
+    ASSERT_EQ(0, status) << mysql_error(conn.mysql_);
+
+    // Create a host reservations.
+    HostPtr host = initializeHost6("2001:db8:1::1", Host::IDENT_HWADDR, false);
+
+    host->setIPv4SubnetID(SubnetID(4));
+
+    // Create IPv6 reservation (for an address) and add it to the host
+    IPv6Resrv resv(IPv6Resrv::TYPE_NA, IOAddress("2001:db8::2"), 128);
+    host->addReservation(resv);
+
+    ASSERT_THROW(hdsptr_->add(host), DbOperationError);
+
+    ConstHostPtr from_hds = hdsptr_->get4(host->getIPv4SubnetID(),
+                                          host->getIdentifierType(),
+                                          &host->getIdentifier()[0],
+                                          host->getIdentifier().size());
+    ASSERT_FALSE(from_hds);
+}
+
 void GenericHostDataSourceTest::testAddr6AndPrefix(){
     // Make sure we have the pointer to the host data source.
     ASSERT_TRUE(hdsptr_);
diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h
index def3d9b60f..14a68e6071 100644
--- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h
+++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h
@@ -342,6 +342,8 @@ public:
     /// Uses gtest macros to report failures.
     void testAddDuplicate4();
 
+    void testAddRollback();
+
     /// @brief Test that DHCPv4 options can be inserted and retrieved from
     /// the database.
     ///
diff --git a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc
index 3fae611c3c..64801a6c6d 100644
--- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc
+++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc
@@ -442,4 +442,8 @@ TEST_F(MySqlHostDataSourceTest, formattedOptionsReservations46) {
     testOptionsReservations46(true);
 }
 
+TEST_F(MySqlHostDataSourceTest, testAddRollback) {
+    testAddRollback();
+}
+
 }; // Of anonymous namespace