diff --git a/ChangeLog b/ChangeLog index b60a53db98ec94418de9be563b16609fa3c3001e..b0296088c6cca63f09c4354e51927f7a801b383c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +1867. [bug] andrei + MySQL connection unit tests have been modified to work with + Percona cluster. This change doesn't fix all problems, but it + improves the situation sufficiently to be able to run unit tests + with positive results on a Percona cluster. + (Gitlab #1708) + 1866. [func] marcin Added new log messages issued when a dynamic lease allocation fails. The new messages provide comprehensive information about diff --git a/src/lib/mysql/mysql_connection.cc b/src/lib/mysql/mysql_connection.cc index 4d205ce14a84935b89bf01d7209ecf2a85646185..4784aaefd3f9f3797ba68c62b62ce3f382946401 100644 --- a/src/lib/mysql/mysql_connection.cc +++ b/src/lib/mysql/mysql_connection.cc @@ -437,5 +437,5 @@ MySqlConnection::rollback() { } } -} // namespace isc::db +} // namespace db } // namespace isc diff --git a/src/lib/mysql/mysql_connection.h b/src/lib/mysql/mysql_connection.h index c48221f1c5d61d83dd9b0676b98515181e862892..4a5bed9b2062e0dd45a993565bb5380d2667fa88 100644 --- a/src/lib/mysql/mysql_connection.h +++ b/src/lib/mysql/mysql_connection.h @@ -71,7 +71,6 @@ private: /// /// Each statement is associated with an index, which is used to reference the /// associated prepared statement. - struct TaggedStatement { uint32_t index; const char* text; @@ -565,7 +564,6 @@ public: return (static_cast(mysql_stmt_affected_rows(statements_[index]))); } - /// @brief Commit Transactions /// /// Commits all pending database operations. On databases that don't diff --git a/src/lib/mysql/tests/mysql_connection_unittest.cc b/src/lib/mysql/tests/mysql_connection_unittest.cc index 0a12b132a074188e97d9511e6c04180fcea5aa63..7e1627d1ab0e09b6d2d140b38d2a10c443d2d6cf 100644 --- a/src/lib/mysql/tests/mysql_connection_unittest.cc +++ b/src/lib/mysql/tests/mysql_connection_unittest.cc @@ -17,6 +17,19 @@ using namespace isc::db::test; namespace { +/// @brief RAII wrapper over MYSQL_RES obtained from MySQL library functions like +/// mysql_use_result(). +struct MySqlResult { + MySqlResult(MYSQL_RES* result) : result_(result) { + } + + ~MySqlResult() { + mysql_free_result(result_); + } + + MYSQL_RES* const result_; +}; + /// @brief Test fixture class for @c MySqlConnection class. class MySqlConnectionTest : public ::testing::Test { public: @@ -30,11 +43,10 @@ public: }; /// @brief Array of tagged MySQL statements. - typedef std::array - TaggedStatementArray; + typedef std::array TaggedStatementArray; /// @brief Prepared MySQL statements used in the tests. - TaggedStatementArray tagged_statements = { { + TaggedStatementArray tagged_statements = {{ { GET_BY_INT_VALUE, "SELECT tinyint_value, int_value, bigint_value, string_value," " blob_value, timestamp_value" @@ -47,9 +59,7 @@ public: "INSERT INTO mysql_connection_test (tinyint_value, int_value," "bigint_value, string_value, blob_value, timestamp_value)" " VALUES (?, ?, ?, ?, ?, ?)" } - } - }; - + }}; /// @brief Constructor. /// @@ -57,7 +67,7 @@ public: /// prepared statements used in tests. Created schema contains a test /// table @c mysql_connection_test which includes 6 columns of various /// types. - MySqlConnectionTest() + MySqlConnectionTest(bool const primary_key = false) : conn_(DatabaseConnection::parse(validMySQLConnectionString())) { try { @@ -65,12 +75,26 @@ public: conn_.openDatabase(); // Create mysql_connection_test table. - createTestTable(); + createTestTable(primary_key); + + // In Percona XtraDB cluster, you can't do much on tables with + // primary keys. So far the connection and the table creation have + // been tested. Continue only if: + // * we are in primary key mode + // * a MySQL database other than Percona is running + // * Percona's pxc_strict_mode is set to "DISABLED" or "PERMISSIVE" + // The last two checks are done with inverse logic against the two + // modes that restrict this: "ENFORCING" and "MASTER". This check is + // to be paired inside the tests without a primary key to disable + // those tests. + if (!primary_key && + (showPxcStrictMode() == "ENFORCING" || showPxcStrictMode() == "MASTER")) { + return; + } // Created prepared statements for basic queries to test table. conn_.prepareStatements(tagged_statements.begin(), tagged_statements.end()); - } catch (...) { std::cerr << "*** ERROR: unable to open database. The test\n" "*** environment is broken and must be fixed before\n" @@ -93,7 +117,7 @@ public: /// /// The new table contains 6 columns of various data types. All of /// the columns accept null values. - void createTestTable() { + void createTestTable(bool const primary_key = false) { /// @todo TIMESTAMP value lacks sub second precision because /// it is supported since MySQL 5.6.4, which is still not a /// default version on some OSes. When the subsecond precision @@ -101,7 +125,8 @@ public: /// column should be turned to TIMESTAMP(6). Until then, it /// must remain TIMESTAMP. runQuery("CREATE TABLE IF NOT EXISTS mysql_connection_test (" - "tinyint_value TINYINT NULL," + "tinyint_value TINYINT " + + std::string(primary_key ? "PRIMARY KEY NOT NULL," : "NULL,") + "int_value INT NULL," "bigint_value BIGINT NULL," "string_value TEXT NULL," @@ -222,167 +247,323 @@ public: } } + /// @brief Run a raw, unprepared statement and return the result. + /// + /// This is useful when running statements that can't be parametrized with a + /// question mark in place of a bound variable e.g. "SHOW GLOBAL VARIABLES" + /// and thus cannot be prepared beforehand. All the results are string, the + /// output should be the same as that which one would see in a mysql command + /// line client. + /// + /// @param statement the statement in string form + /// @throw DbOperationError if the statement could not be run + /// @return the list of rows, each row consisting of a list of values for + /// each column + std::vector> + rawStatement(std::string const& statement) const { + // Execute a SQL statement. + if (mysql_query(conn_.mysql_, statement.c_str())) { + isc_throw(DbOperationError, + statement << ": " << mysql_error(conn_.mysql_)); + } + + // Get a result set. + MySqlResult result(mysql_use_result(conn_.mysql_)); + + // Fetch a result set. + std::vector> output; + size_t r(0); + MYSQL_ROW row; + size_t const column_count(mysql_num_fields(result.result_)); + while ((row = mysql_fetch_row(result.result_)) != NULL) { + output.push_back(std::vector()); + for (size_t i = 0; i < column_count; ++i) { + output[r].push_back(row[i]); + } + ++r; + } + + return output; + } + + /// @brief Get pxc_strict_mode global variable from the database. + /// For Percona, they can be: DISABLED, PERMISSIVE, ENFORCING, MASTER. + std::string showPxcStrictMode() { + // Store in a static variable so this only runs once per gtest binary + // invocation. + static std::string const pxc_strict_mode([&]() -> std::string { + // Execute select statement. We expect one row to be returned. For + // this returned row the lambda provided as 4th argument should be + // executed. + std::vector> const result( + rawStatement("SHOW GLOBAL VARIABLES LIKE 'pxc_strict_mode'")); + if (result.size() < 1 || result[0].size() < 2) { + // Not Percona + return ""; + } + + return result[0][1]; + }()); + return pxc_strict_mode; + } + + /// *** Test definitions start here. Tests are invoked *** + /// *** multiple times further below in different test fixtures. *** + + /// @brief Test that non-null values of various types can be inserted and + /// retrieved from the dataabse. + void select() { + std::string blob = "myblob"; + MySqlBindingCollection in_bindings = { + MySqlBinding::createInteger(123), + MySqlBinding::createInteger(1024), + MySqlBinding::createInteger(-4096), + MySqlBinding::createString("shellfish"), + MySqlBinding::createBlob(blob.begin(), blob.end()), + /// @todo Change it to microsec_clock once we transition to + /// subsecond precision. + MySqlBinding::createTimestamp( + boost::posix_time::second_clock::local_time()), + }; + + testInsertSelect(in_bindings); + } + + /// @brief Test that null value can be inserted to a column having numeric + /// type and retrieved. + void selectNullInteger() { + std::string blob = "myblob"; + MySqlBindingCollection in_bindings = { + MySqlBinding::createInteger(123), + MySqlBinding::createInteger(1024), + MySqlBinding::createNull(), + MySqlBinding::createString("shellfish"), + MySqlBinding::createBlob(blob.begin(), blob.end()), + /// @todo Change it to microsec_clock once we transition to + /// subsecond precision. + MySqlBinding::createTimestamp( + boost::posix_time::second_clock::local_time()), + }; + + testInsertSelect(in_bindings); + } + + /// @brief Test that null value can be inserted to a column having string + /// type and retrieved. + void selectNullString() { + std::string blob = "myblob"; + + MySqlBindingCollection in_bindings = { + MySqlBinding::createInteger(123), + MySqlBinding::createInteger(1024), + MySqlBinding::createInteger(-4096), + MySqlBinding::createNull(), + MySqlBinding::createBlob(blob.begin(), blob.end()), + /// @todo Change it to microsec_clock once we transition to + /// subsecond precision. + MySqlBinding::createTimestamp( + boost::posix_time::second_clock::local_time()), + }; + + testInsertSelect(in_bindings); + } + + /// @brief Test that null value can be inserted to a column having blob type + /// and retrieved. + void selectNullBlob() { + MySqlBindingCollection in_bindings = { + MySqlBinding::createInteger(123), + MySqlBinding::createInteger(1024), + MySqlBinding::createInteger(-4096), + MySqlBinding::createString("shellfish"), + MySqlBinding::createNull(), + /// @todo Change it to microsec_clock once we transition to + /// subsecond precision. + MySqlBinding::createTimestamp( + boost::posix_time::second_clock::local_time()), + }; + + testInsertSelect(in_bindings); + } + + /// @brief Test that null value can be inserted to a column having timestamp + /// type and retrieved. + void selectNullTimestamp() { + std::string blob = "myblob"; + MySqlBindingCollection in_bindings = { + MySqlBinding::createInteger(123), + MySqlBinding::createInteger(1024), + MySqlBinding::createInteger(-4096), + MySqlBinding::createString("shellfish"), + MySqlBinding::createBlob(blob.begin(), blob.end()), + MySqlBinding::createNull(), + }; + + testInsertSelect(in_bindings); + } + + /// @brief Test that empty string and empty blob can be inserted to a + /// database. + void selectEmptyStringBlob() { + std::string blob = ""; + MySqlBindingCollection in_bindings = { + MySqlBinding::createInteger(123), + MySqlBinding::createInteger(1024), + MySqlBinding::createInteger(-4096), + MySqlBinding::createString(""), + MySqlBinding::createBlob(blob.begin(), blob.end()), + /// @todo Change it to microsec_clock once we transition to + /// subsecond precision. + MySqlBinding::createTimestamp( + boost::posix_time::second_clock::local_time()), + }; + + testInsertSelect(in_bindings); + } + + /// @brief Test that a row can be deleted from the database. + void deleteByValue() { + // Insert a row with numeric values. + MySqlBindingCollection in_bindings = { + MySqlBinding::createInteger(123), + MySqlBinding::createInteger(1024), + MySqlBinding::createInteger(-4096), + MySqlBinding::createNull(), + MySqlBinding::createNull(), + MySqlBinding::createNull(), + }; + + ASSERT_NO_THROW( + conn_.insertQuery(MySqlConnectionTest::INSERT_VALUE, in_bindings)); + + // This variable will be checked to see if the row has been deleted + // from the database. + bool deleted = false; + + // Execute delete query but use int_value of non existing row. + // The row should not be deleted. + in_bindings = {MySqlBinding::createInteger(1)}; + ASSERT_NO_THROW( + deleted = conn_.updateDeleteQuery( + MySqlConnectionTest::DELETE_BY_INT_VALUE, in_bindings)); + ASSERT_FALSE(deleted); + + // This time use the correct value. + in_bindings = {MySqlBinding::createInteger(1024)}; + ASSERT_NO_THROW( + deleted = conn_.updateDeleteQuery( + MySqlConnectionTest::DELETE_BY_INT_VALUE, in_bindings)); + // The row should have been deleted. + ASSERT_TRUE(deleted); + + // Let's confirm that it has been deleted by issuing a select query. + MySqlBindingCollection out_bindings = { + MySqlBinding::createInteger(), + MySqlBinding::createInteger(), + MySqlBinding::createInteger(), + MySqlBinding::createString(512), + MySqlBinding::createBlob(512), + MySqlBinding::createTimestamp(), + }; + + ASSERT_NO_THROW(conn_.selectQuery(MySqlConnectionTest::GET_BY_INT_VALUE, + in_bindings, out_bindings, + [&deleted](MySqlBindingCollection&) { + // This will be executed if the + // row is returned as a result of + // select query. We expect that + // this is not executed. + deleted = false; + })); + // Make sure that select query returned nothing. + EXPECT_TRUE(deleted); + } + /// @brief Test MySQL connection. MySqlConnection conn_; +}; +struct MySqlConnectionWithPrimaryKeyTest : MySqlConnectionTest { + MySqlConnectionWithPrimaryKeyTest() + : MySqlConnectionTest(/* primary_key = */ true) { + } }; -// Test that non-null values of various types can be inserted and retrieved -// from the dataabse. TEST_F(MySqlConnectionTest, select) { - std::string blob = "myblob"; - MySqlBindingCollection in_bindings = { - MySqlBinding::createInteger(123), - MySqlBinding::createInteger(1024), - MySqlBinding::createInteger(-4096), - MySqlBinding::createString("shellfish"), - MySqlBinding::createBlob(blob.begin(), blob.end()), - /// @todo Change it to microsec_clock once we transition to subsecond - /// precision. - MySqlBinding::createTimestamp(boost::posix_time::second_clock::local_time()) - }; - - testInsertSelect(in_bindings); + if (showPxcStrictMode() == "ENFORCING" || showPxcStrictMode() == "MASTER") { + return; + } + select(); } -// Test that null value can be inserted to a column having numeric type and -// retrieved. TEST_F(MySqlConnectionTest, selectNullInteger) { - std::string blob = "myblob"; - MySqlBindingCollection in_bindings = { - MySqlBinding::createNull(), - MySqlBinding::createInteger(1024), - MySqlBinding::createInteger(-4096), - MySqlBinding::createString("shellfish"), - MySqlBinding::createBlob(blob.begin(), blob.end()), - /// @todo Change it to microsec_clock once we transition to subsecond - /// precision. - MySqlBinding::createTimestamp(boost::posix_time::second_clock::local_time()) - }; - - testInsertSelect(in_bindings); + if (showPxcStrictMode() == "ENFORCING" || showPxcStrictMode() == "MASTER") { + return; + } + selectNullInteger(); } -// Test that null value can be inserted to a column having string type and -// retrieved. TEST_F(MySqlConnectionTest, selectNullString) { - std::string blob = "myblob"; - - MySqlBindingCollection in_bindings = { - MySqlBinding::createInteger(123), - MySqlBinding::createInteger(1024), - MySqlBinding::createInteger(-4096), - MySqlBinding::createNull(), - MySqlBinding::createBlob(blob.begin(), blob.end()), - /// @todo Change it to microsec_clock once we transition to subsecond - /// precision. - MySqlBinding::createTimestamp(boost::posix_time::second_clock::local_time()) - }; - - testInsertSelect(in_bindings); + if (showPxcStrictMode() == "ENFORCING" || showPxcStrictMode() == "MASTER") { + return; + } + selectNullString(); } -// Test that null value can be inserted to a column having blob type and -// retrieved. TEST_F(MySqlConnectionTest, selectNullBlob) { - MySqlBindingCollection in_bindings = { - MySqlBinding::createInteger(123), - MySqlBinding::createInteger(1024), - MySqlBinding::createInteger(-4096), - MySqlBinding::createString("shellfish"), - MySqlBinding::createNull(), - /// @todo Change it to microsec_clock once we transition to subsecond - /// precision. - MySqlBinding::createTimestamp(boost::posix_time::second_clock::local_time()) - }; - - testInsertSelect(in_bindings); + if (showPxcStrictMode() == "ENFORCING" || showPxcStrictMode() == "MASTER") { + return; + } + selectNullBlob(); } -// Test that null value can be inserted to a column having timestamp type and -// retrieved. TEST_F(MySqlConnectionTest, selectNullTimestamp) { - std::string blob = "myblob"; - MySqlBindingCollection in_bindings = { - MySqlBinding::createInteger(123), - MySqlBinding::createInteger(1024), - MySqlBinding::createInteger(-4096), - MySqlBinding::createString("shellfish"), - MySqlBinding::createBlob(blob.begin(), blob.end()), - MySqlBinding::createNull() - }; - - testInsertSelect(in_bindings); + if (showPxcStrictMode() == "ENFORCING" || showPxcStrictMode() == "MASTER") { + return; + } + selectNullTimestamp(); } -// Test that empty string and empty blob can be inserted to a database. TEST_F(MySqlConnectionTest, selectEmptyStringBlob) { - std::string blob = ""; - MySqlBindingCollection in_bindings = { - MySqlBinding::createInteger(123), - MySqlBinding::createInteger(1024), - MySqlBinding::createInteger(-4096), - MySqlBinding::createString(""), - MySqlBinding::createBlob(blob.begin(), blob.end()), - /// @todo Change it to microsec_clock once we transition to subsecond - /// precision. - MySqlBinding::createTimestamp(boost::posix_time::second_clock::local_time()) - }; - - testInsertSelect(in_bindings); + if (showPxcStrictMode() == "ENFORCING" || showPxcStrictMode() == "MASTER") { + return; + } + selectEmptyStringBlob(); } -// Test that a row can be deleted from the database. TEST_F(MySqlConnectionTest, deleteByValue) { - // Insert a row with numeric values. - MySqlBindingCollection in_bindings = { - MySqlBinding::createInteger(123), - MySqlBinding::createInteger(1024), - MySqlBinding::createInteger(-4096), - MySqlBinding::createNull(), - MySqlBinding::createNull(), - MySqlBinding::createNull() - }; - ASSERT_NO_THROW(conn_.insertQuery(MySqlConnectionTest::INSERT_VALUE, - in_bindings)); - - // This variable will be checked to see if the row has been deleted - // from the database. - bool deleted = false; - - // Execute delete query but use int_value of non existing row. - // The row should not be deleted. - in_bindings = { MySqlBinding::createInteger(1) }; - ASSERT_NO_THROW(deleted = conn_.updateDeleteQuery(MySqlConnectionTest::DELETE_BY_INT_VALUE, - in_bindings)); - ASSERT_FALSE(deleted); - - // This time use the correct value. - in_bindings = { MySqlBinding::createInteger(1024) }; - ASSERT_NO_THROW(deleted = conn_.updateDeleteQuery(MySqlConnectionTest::DELETE_BY_INT_VALUE, - in_bindings)); - // The row should have been deleted. - ASSERT_TRUE(deleted); - - // Let's confirm that it has been deleted by issuing a select query. - MySqlBindingCollection out_bindings = { - MySqlBinding::createInteger(), - MySqlBinding::createInteger(), - MySqlBinding::createInteger(), - MySqlBinding::createString(512), - MySqlBinding::createBlob(512), - MySqlBinding::createTimestamp() - }; + if (showPxcStrictMode() == "ENFORCING" || showPxcStrictMode() == "MASTER") { + return; + } + deleteByValue(); +} + +TEST_F(MySqlConnectionWithPrimaryKeyTest, select) { + select(); +} + +TEST_F(MySqlConnectionWithPrimaryKeyTest, selectNullInteger) { + selectNullInteger(); +} + +TEST_F(MySqlConnectionWithPrimaryKeyTest, selectNullString) { + selectNullString(); +} + +TEST_F(MySqlConnectionWithPrimaryKeyTest, selectNullBlob) { + selectNullBlob(); +} + +TEST_F(MySqlConnectionWithPrimaryKeyTest, selectNullTimestamp) { + selectNullTimestamp(); +} - ASSERT_NO_THROW(conn_.selectQuery(MySqlConnectionTest::GET_BY_INT_VALUE, - in_bindings, out_bindings, - [&deleted](MySqlBindingCollection&) { - // This will be executed if the row is returned as a result of - // select query. We expect that this is not executed. - deleted = false; - })); - // Make sure that select query returned nothing. - EXPECT_TRUE(deleted); +TEST_F(MySqlConnectionWithPrimaryKeyTest, selectEmptyStringBlob) { + selectEmptyStringBlob(); +} + +TEST_F(MySqlConnectionWithPrimaryKeyTest, deleteByValue) { + deleteByValue(); } /// @brief Test fixture class for @c MySqlConnection class methods. @@ -410,4 +591,4 @@ TEST_F(MySqlSchemaTest, checkVersion) { EXPECT_EQ(MYSQL_SCHEMA_VERSION_MINOR, version.second); } -} +} // namespace