From 92c09c8f57d5a4546924e0fb0acfbd7d47e5fc6c Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Thu, 3 Nov 2022 09:43:51 +0000 Subject: [PATCH] Issue #3316923 by mondrake, daffie, catch: Sort out more driver specific database kernel tests --- .../tests/src/Kernel/mysql/SchemaTest.php | 18 +++++ .../tests/src/Kernel/mysql/SyntaxTest.php | 13 ++++ .../src/Kernel/mysql/TransactionTest.php | 13 ++++ .../tests/src/Kernel/pgsql/SchemaTest.php | 18 +++++ .../tests/src/Kernel/pgsql/SyntaxTest.php | 13 ++++ .../src/Kernel/pgsql/TransactionTest.php | 13 ++++ .../tests/src/Kernel/sqlite/SchemaTest.php | 20 +++++- .../tests/src/Kernel/sqlite/SyntaxTest.php | 13 ++++ .../src/Kernel/sqlite/TransactionTest.php | 13 ++++ .../Cache/EndOfTransactionQueriesTest.php | 70 +++++++------------ .../Core/Database/BasicSyntaxTest.php | 29 -------- .../Database/DriverSpecificKernelTestBase.php | 21 ++++-- .../Database/DriverSpecificSchemaTestBase.php | 25 ++----- .../Database/DriverSpecificSyntaxTestBase.php | 44 ++++++++++++ ... => DriverSpecificTransactionTestBase.php} | 8 ++- 15 files changed, 230 insertions(+), 101 deletions(-) create mode 100644 core/modules/mysql/tests/src/Kernel/mysql/SyntaxTest.php create mode 100644 core/modules/mysql/tests/src/Kernel/mysql/TransactionTest.php create mode 100644 core/modules/pgsql/tests/src/Kernel/pgsql/SyntaxTest.php create mode 100644 core/modules/pgsql/tests/src/Kernel/pgsql/TransactionTest.php create mode 100644 core/modules/sqlite/tests/src/Kernel/sqlite/SyntaxTest.php create mode 100644 core/modules/sqlite/tests/src/Kernel/sqlite/TransactionTest.php create mode 100644 core/tests/Drupal/KernelTests/Core/Database/DriverSpecificSyntaxTestBase.php rename core/tests/Drupal/KernelTests/Core/Database/{TransactionTest.php => DriverSpecificTransactionTestBase.php} (98%) diff --git a/core/modules/mysql/tests/src/Kernel/mysql/SchemaTest.php b/core/modules/mysql/tests/src/Kernel/mysql/SchemaTest.php index f08a04df893..f219b373280 100644 --- a/core/modules/mysql/tests/src/Kernel/mysql/SchemaTest.php +++ b/core/modules/mysql/tests/src/Kernel/mysql/SchemaTest.php @@ -43,6 +43,24 @@ class SchemaTest extends DriverSpecificSchemaTestBase { $this->assertSame('ascii_general_ci', $string_ascii_check, 'test_field_string_ascii should have a ascii_general_ci collation, but it has not.'); } + /** + * {@inheritdoc} + */ + public function testTableWithSpecificDataType(): void { + $table_specification = [ + 'description' => 'Schema table description.', + 'fields' => [ + 'timestamp' => [ + 'mysql_type' => 'timestamp', + 'not null' => FALSE, + 'default' => NULL, + ], + ], + ]; + $this->schema->createTable('test_timestamp', $table_specification); + $this->assertTrue($this->schema->tableExists('test_timestamp')); + } + /** * Tests that indexes on string fields are limited to 191 characters on MySQL. * diff --git a/core/modules/mysql/tests/src/Kernel/mysql/SyntaxTest.php b/core/modules/mysql/tests/src/Kernel/mysql/SyntaxTest.php new file mode 100644 index 00000000000..b5ad81267ab --- /dev/null +++ b/core/modules/mysql/tests/src/Kernel/mysql/SyntaxTest.php @@ -0,0 +1,13 @@ +assertTrue($sequenceExists, 'Sequence was renamed.'); } + /** + * {@inheritdoc} + */ + public function testTableWithSpecificDataType(): void { + $table_specification = [ + 'description' => 'Schema table description.', + 'fields' => [ + 'timestamp' => [ + 'pgsql_type' => 'timestamp', + 'not null' => FALSE, + 'default' => NULL, + ], + ], + ]; + $this->schema->createTable('test_timestamp', $table_specification); + $this->assertTrue($this->schema->tableExists('test_timestamp')); + } + /** * @covers \Drupal\pgsql\Driver\Database\pgsql\Schema::introspectIndexSchema */ diff --git a/core/modules/pgsql/tests/src/Kernel/pgsql/SyntaxTest.php b/core/modules/pgsql/tests/src/Kernel/pgsql/SyntaxTest.php new file mode 100644 index 00000000000..2da93a315c4 --- /dev/null +++ b/core/modules/pgsql/tests/src/Kernel/pgsql/SyntaxTest.php @@ -0,0 +1,13 @@ + 'Schema table description.', + 'fields' => [ + 'timestamp' => [ + 'sqlite_type' => 'datetime', + 'not null' => FALSE, + 'default' => NULL, + ], + ], + ]; + $this->schema->createTable('test_timestamp', $table_specification); + $this->assertTrue($this->schema->tableExists('test_timestamp')); + } + /** * @covers \Drupal\sqlite\Driver\Database\sqlite\Schema::introspectIndexSchema */ diff --git a/core/modules/sqlite/tests/src/Kernel/sqlite/SyntaxTest.php b/core/modules/sqlite/tests/src/Kernel/sqlite/SyntaxTest.php new file mode 100644 index 00000000000..fd495ffefec --- /dev/null +++ b/core/modules/sqlite/tests/src/Kernel/sqlite/SyntaxTest.php @@ -0,0 +1,13 @@ +getDatabaseConnectionInfo()['default']['namespace'] . '\Connection')) { - $this->markTestSkipped(sprintf('No logging override exists for the %s database driver. Create it, subclass this test class and override ::getDatabaseConnectionInfo().', $this->getDatabaseConnectionInfo()['default']['driver'])); - } - $this->installSchema('system', 'sequences'); $this->installEntitySchema('entity_test'); $this->installEntitySchema('user'); @@ -61,21 +56,23 @@ class EndOfTransactionQueriesTest extends KernelTestBase { } /** - * {@inheritdoc} + * Tests an entity save. */ - public function testEntitySave() { + public function testEntitySave(): void { \Drupal::cache()->set('test_cache_pretransaction_foobar', 'something', Cache::PERMANENT, ['foobar']); \Drupal::cache()->set('test_cache_pretransaction_entity_test_list', 'something', Cache::PERMANENT, ['entity_test_list']); $entity = EntityTest::create(['name' => $this->randomString()]); - \Drupal::database()->resetLoggedStatements(); + Database::startLog('testEntitySave'); $entity->save(); - $executed_statements = \Drupal::database()->getLoggedStatements(); + $executed_statements = []; + foreach (Database::getLog('testEntitySave') as $log) { + $executed_statements[] = $log['query']; + } $last_statement_index = max(array_keys($executed_statements)); - - $cachetag_statements = array_keys($this->getStatementsForTable(\Drupal::database()->getLoggedStatements(), 'cachetags')); + $cachetag_statements = array_keys($this->getStatementsForTable($executed_statements, 'cachetags')); $this->assertSame($last_statement_index - count($cachetag_statements) + 1, min($cachetag_statements), 'All of the last queries in the transaction are for the "cachetags" table.'); // Verify that a nested entity save occurred. @@ -103,9 +100,9 @@ class EndOfTransactionQueriesTest extends KernelTestBase { } /** - * {@inheritdoc} + * Tests an entity save rollback. */ - public function testEntitySaveRollback() { + public function testEntitySaveRollback(): void { \Drupal::cache() ->set('test_cache_pretransaction_entity_test_list', 'something', Cache::PERMANENT, ['entity_test_list']); \Drupal::cache() @@ -148,44 +145,29 @@ class EndOfTransactionQueriesTest extends KernelTestBase { * Filtered statement list. */ protected function getStatementsForTable(array $statements, $table_name) { - $tables = array_filter(array_map([$this, 'statementToTableName'], $statements)); - return array_filter($tables, function ($table_for_statement) use ($table_name) { - return $table_for_statement === $table_name; + return array_filter($statements, function ($statement) use ($table_name) { + return $this->isStatementRelatedToTable($statement, $table_name); }); } /** - * Returns the table name for a statement. + * Determines if a statement is relative to a specified table. + * + * Non-core database drivers can override this method if they have different + * patterns to identify table related statements. * * @param string $statement * The query statement. + * @param string $tableName + * The table name, Drupal style, without curly brackets or prefix. * - * @return string|null - * The name of the table or NULL if none was found. + * @return bool + * TRUE if the statement is relative to the table, FALSE otherwise. */ - protected static function statementToTableName($statement) { - if (preg_match('/.*\{([^\}]+)\}.*/', $statement, $matches)) { - return $matches[1]; - } - else { - return NULL; - } - } - - /** - * {@inheritdoc} - */ - protected function getDatabaseConnectionInfo() { - $info = parent::getDatabaseConnectionInfo(); - // Override default database driver to one that does logging. Third-party - // (non-core) database drivers can achieve the same test coverage by - // subclassing this test class and overriding only this method. - // @see \Drupal\database_statement_monitoring_test\LoggedStatementsTrait - // @see \Drupal\database_statement_monitoring_test\mysql\Connection - // @see \Drupal\database_statement_monitoring_test\pgsql\Connection - // @see \Drupal\database_statement_monitoring_test\sqlite\Connection - $info['default']['namespace'] = '\Drupal\database_statement_monitoring_test\\' . $info['default']['driver']; - return $info; + protected static function isStatementRelatedToTable(string $statement, string $tableName): bool { + $realTableIdentifier = Database::getConnection()->prefixTables('{' . $tableName . '}'); + $pattern = '/.*(INTO|FROM|UPDATE)( |\n)' . preg_quote($realTableIdentifier, '/') . '/'; + return preg_match($pattern, $statement) === 1 ? TRUE : FALSE; } } diff --git a/core/tests/Drupal/KernelTests/Core/Database/BasicSyntaxTest.php b/core/tests/Drupal/KernelTests/Core/Database/BasicSyntaxTest.php index 773ec6a99b7..b1b3bb7a459 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/BasicSyntaxTest.php +++ b/core/tests/Drupal/KernelTests/Core/Database/BasicSyntaxTest.php @@ -141,33 +141,4 @@ class BasicSyntaxTest extends DatabaseTestBase { $this->assertSame('4', $num_matches, 'Found 4 records.'); } - /** - * Tests allowing square brackets in queries. - * - * @see \Drupal\Core\Database\Connection::prepareQuery() - */ - public function testAllowSquareBrackets() { - $this->connection->insert('test') - ->fields(['name']) - ->values([ - 'name' => '[square]', - ]) - ->execute(); - - // Note that this is a very bad example query because arguments should be - // passed in via the $args parameter. - $result = $this->connection->query("select name from {test} where name = '[square]'", [], ['allow_square_brackets' => TRUE]); - $this->assertSame('[square]', $result->fetchField()); - - // Test that allow_square_brackets has no effect on arguments. - $result = $this->connection->query("select [name] from {test} where [name] = :value", [':value' => '[square]']); - $this->assertSame('[square]', $result->fetchField()); - $result = $this->connection->query("select name from {test} where name = :value", [':value' => '[square]'], ['allow_square_brackets' => TRUE]); - $this->assertSame('[square]', $result->fetchField()); - - // Test square brackets using the query builder. - $result = $this->connection->select('test')->fields('test', ['name'])->condition('name', '[square]')->execute(); - $this->assertSame('[square]', $result->fetchField()); - } - } diff --git a/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificKernelTestBase.php b/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificKernelTestBase.php index a84046e7d97..eb459eb3a15 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificKernelTestBase.php +++ b/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificKernelTestBase.php @@ -27,11 +27,11 @@ abstract class DriverSpecificKernelTestBase extends KernelTestBase { * @inheritdoc */ protected function setUp(): void { - parent::setUp(); - $this->connection = Database::getConnection(); - - $running_provider = $this->connection->getProvider(); - $running_driver = $this->connection->driver(); + // Find the current SUT database driver from the connection info. If that + // is not the one the test requires, skip before test database + // initialization so to save cycles. + $this->root = static::getDrupalRoot(); + $connectionInfo = $this->getDatabaseConnectionInfo(); $test_class_parts = explode('\\', get_class($this)); $expected_provider = $test_class_parts[2] ?? ''; for ($i = 3; $i < count($test_class_parts); $i++) { @@ -40,6 +40,17 @@ abstract class DriverSpecificKernelTestBase extends KernelTestBase { break; } } + if ($connectionInfo['default']['driver'] !== $expected_driver) { + $this->markTestSkipped("This test only runs for the database driver '$expected_driver'. Current database driver is '{$connectionInfo['default']['driver']}'."); + } + + parent::setUp(); + $this->connection = Database::getConnection(); + + // After database initialization, the database driver may be not provided + // by the expected module; skip test in that case. + $running_provider = $this->connection->getProvider(); + $running_driver = $this->connection->driver(); if ($running_provider !== $expected_provider || $running_driver !== $expected_driver) { $this->markTestSkipped("This test only runs for the database driver '$expected_driver' provided by the '$expected_provider' module. Connected database driver is '$running_driver' provided by '$running_provider'."); } diff --git a/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificSchemaTestBase.php b/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificSchemaTestBase.php index bbf5de225c6..de74ae5437f 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificSchemaTestBase.php +++ b/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificSchemaTestBase.php @@ -308,28 +308,13 @@ abstract class DriverSpecificSchemaTestBase extends DriverSpecificKernelTestBase // Check that the ID sequence gets renamed when the table is renamed. $this->checkSequenceRenaming($new_table_name); - - // Use database specific data type and ensure that table is created. - $table_specification = [ - 'description' => 'Schema table description.', - 'fields' => [ - 'timestamp' => [ - 'mysql_type' => 'timestamp', - 'pgsql_type' => 'timestamp', - 'sqlite_type' => 'datetime', - 'not null' => FALSE, - 'default' => NULL, - ], - ], - ]; - try { - $this->schema->createTable('test_timestamp', $table_specification); - } - catch (\Exception $e) { - } - $this->assertTrue($this->schema->tableExists('test_timestamp'), 'Table with database specific datatype was created.'); } + /** + * Tests creating a table with database specific data type. + */ + abstract public function testTableWithSpecificDataType(): void; + /** * Tests creating unsigned columns and data integrity thereof. */ diff --git a/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificSyntaxTestBase.php b/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificSyntaxTestBase.php new file mode 100644 index 00000000000..b40d8786e35 --- /dev/null +++ b/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificSyntaxTestBase.php @@ -0,0 +1,44 @@ +connection->insert('test') + ->fields(['name']) + ->values([ + 'name' => '[square]', + ]) + ->execute(); + + // Note that this is a very bad example query because arguments should be + // passed in via the $args parameter. + $result = $this->connection->query("select name from {test} where name = '[square]'", [], ['allow_square_brackets' => TRUE]); + $this->assertSame('[square]', $result->fetchField()); + + // Test that allow_square_brackets has no effect on arguments. + $result = $this->connection->query("select [name] from {test} where [name] = :value", [':value' => '[square]']); + $this->assertSame('[square]', $result->fetchField()); + $result = $this->connection->query("select name from {test} where name = :value", [':value' => '[square]'], ['allow_square_brackets' => TRUE]); + $this->assertSame('[square]', $result->fetchField()); + + // Test square brackets using the query builder. + $result = $this->connection->select('test')->fields('test', ['name'])->condition('name', '[square]')->execute(); + $this->assertSame('[square]', $result->fetchField()); + } + +} diff --git a/core/tests/Drupal/KernelTests/Core/Database/TransactionTest.php b/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificTransactionTestBase.php similarity index 98% rename from core/tests/Drupal/KernelTests/Core/Database/TransactionTest.php rename to core/tests/Drupal/KernelTests/Core/Database/DriverSpecificTransactionTestBase.php index 8adc5840631..ebbf60826ad 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/TransactionTest.php +++ b/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificTransactionTestBase.php @@ -26,9 +26,13 @@ use PHPUnit\Framework\Error\Warning; * Do more stuff * Should still be in transaction A * - * @group Database + * These method can be overridden by non-core database driver if their + * transaction behavior is different from core. For example, both oci8 (Oracle) + * and mysqli (MySql) clients do not have a solution to check if a transaction + * is active, and mysqli does not fail when rolling back and no transaction + * active. */ -class TransactionTest extends DatabaseTestBase { +class DriverSpecificTransactionTestBase extends DriverSpecificDatabaseTestBase { /** * Encapsulates a transaction's "inner layer" with an "outer layer".