diff --git a/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php index 38d57e317f..8847857fb5 100644 --- a/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php +++ b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php @@ -510,6 +510,7 @@ protected function installParameters() { unset($connection_info['default']['autoload']); unset($connection_info['default']['pdo']); unset($connection_info['default']['init_commands']); + unset($connection_info['default']['isolation_level']); // Remove database connection info that is not used by SQLite. if ($driver === 'sqlite') { unset($connection_info['default']['username']); diff --git a/core/modules/migrate_drupal_ui/tests/src/Functional/MigrateUpgradeTestBase.php b/core/modules/migrate_drupal_ui/tests/src/Functional/MigrateUpgradeTestBase.php index df5f4230ed..241fb99b41 100644 --- a/core/modules/migrate_drupal_ui/tests/src/Functional/MigrateUpgradeTestBase.php +++ b/core/modules/migrate_drupal_ui/tests/src/Functional/MigrateUpgradeTestBase.php @@ -301,6 +301,8 @@ protected function getCredentials() { $drivers = drupal_get_database_types(); $form = $drivers[$driver]->getFormOptions($connection_options); $connection_options = array_intersect_key($connection_options, $form + $form['advanced_options']); + // Remove isolation_level since that options is not configurable in the UI. + unset($connection_options['isolation_level']); $edit = [ $driver => $connection_options, 'source_private_file_path' => $this->getSourceBasePath(), diff --git a/core/modules/migrate_drupal_ui/tests/src/Functional/d7/FilePathTest.php b/core/modules/migrate_drupal_ui/tests/src/Functional/d7/FilePathTest.php index 6ccb1f97ff..59487a7488 100644 --- a/core/modules/migrate_drupal_ui/tests/src/Functional/d7/FilePathTest.php +++ b/core/modules/migrate_drupal_ui/tests/src/Functional/d7/FilePathTest.php @@ -127,6 +127,8 @@ public function testFilePath(string $file_private_path, string $file_public_path $drivers = drupal_get_database_types(); $form = $drivers[$driver]->getFormOptions($connection_options); $connection_options = array_intersect_key($connection_options, $form + $form['advanced_options']); + // Remove isolation_level since that options is not configurable in the UI. + unset($connection_options['isolation_level']); $edit = [ $driver => $connection_options, 'version' => '7', diff --git a/core/modules/mysql/mysql.install b/core/modules/mysql/mysql.install index 42ed7344af..bdebc7b547 100644 --- a/core/modules/mysql/mysql.install +++ b/core/modules/mysql/mysql.install @@ -41,6 +41,31 @@ function mysql_requirements($phase) { ':performance_doc' => 'https://www.drupal.org/docs/system-requirements/setting-the-mysql-transaction-isolation-level', ]), ]; + + $tables_missing_keys = []; + $missing_primary_key = FALSE; + $tables = $connection->schema()->findTables('%'); + $description = 'To insure correct operation with READ-COMMITTED, ensure all tables have a primary key.'; + + foreach ($tables as $table) { + $primary_key_column = Database::getConnection()->query("SHOW KEYS FROM {" . $table . "} WHERE Key_name = 'PRIMARY'")->fetchAllAssoc('Column_name'); + if (empty($primary_key_column)) { + $missing_primary_key = TRUE; + $tables_missing_keys[] = $table; + } + } + + if ($missing_primary_key) { + $tables_missing_key_text = implode(', ', $tables_missing_keys); + $description = "To insure correct operation with READ-COMMITTED, ensure all tables have a primary key. The following table(s) do not contain a primary key: $tables_missing_key_text"; + } + + $requirements['mysql_primary_key_existence'] = [ + 'title' => t('MySQL primary keys'), + 'severity' => (!$missing_primary_key) ? REQUIREMENT_OK : REQUIREMENT_WARNING, + 'value' => (!$missing_primary_key) ? t('Primary keys exist') : t('Primary keys missing'), + 'description' => t($description), + ]; } } diff --git a/core/modules/mysql/src/Driver/Database/mysql/Connection.php b/core/modules/mysql/src/Driver/Database/mysql/Connection.php index 5062d5ebab..6a9e8e5332 100644 --- a/core/modules/mysql/src/Driver/Database/mysql/Connection.php +++ b/core/modules/mysql/src/Driver/Database/mysql/Connection.php @@ -204,6 +204,14 @@ public static function open(array &$connection_options = []) { 'sql_mode' => "SET sql_mode = 'ANSI,TRADITIONAL'", ]; + // Only set the transaction isolation level when the connection option has + // been set to 'READ COMMITTED' or 'REPEATABLE READ'. + if (!empty($connection_options['isolation_level']) && in_array($connection_options['isolation_level'], ['READ COMMITTED', 'REPEATABLE READ'], TRUE)) { + $connection_options['init_commands'] += [ + 'isolation_level' => 'SET SESSION TRANSACTION ISOLATION LEVEL ' . $connection_options['isolation_level'], + ]; + } + // Execute initial commands. foreach ($connection_options['init_commands'] as $sql) { $pdo->exec($sql); diff --git a/core/modules/mysql/src/Driver/Database/mysql/Install/Tasks.php b/core/modules/mysql/src/Driver/Database/mysql/Install/Tasks.php index a6ad642425..17b6779548 100644 --- a/core/modules/mysql/src/Driver/Database/mysql/Install/Tasks.php +++ b/core/modules/mysql/src/Driver/Database/mysql/Install/Tasks.php @@ -175,6 +175,18 @@ public function getFormOptions(array $database) { $form['advanced_options']['port']['#default_value'] = '3306'; } + $form['advanced_options']['isolation_level'] = [ + '#type' => 'select', + '#title' => $this->t('Transaction isolation level'), + '#options' => [ + 'READ COMMITTED' => $this->t('Read Committed'), + 'REPEATABLE READ' => $this->t('Repeatable Read'), + 'DO NOT SET BY DRUPAL' => $this->t('Do not set by Drupal'), + ], + '#default_value' => $database['isolation_level'] ?? 'READ COMMITTED', + '#description' => $this->t('To ensure database consistency, we recommend setting the transaction isolation level to "READ COMMITTED". For existing sites, Drupal does not set this option. Instead, leave it to the database servers configuration. Sites without this option will use the database default transaction isolation level, which is typically "REPEATABLE READ".'), + ]; + return $form; } diff --git a/core/modules/mysql/tests/src/Functional/InstallerIsolationLevelExistingSettingsTest.php b/core/modules/mysql/tests/src/Functional/InstallerIsolationLevelExistingSettingsTest.php new file mode 100644 index 0000000000..ace161dc5b --- /dev/null +++ b/core/modules/mysql/tests/src/Functional/InstallerIsolationLevelExistingSettingsTest.php @@ -0,0 +1,70 @@ +markTestSkipped("This test does not support the {$connection_info['default']['driver']} database driver."); + } + } + + /** + * Verifies that isolation_level is not set in the database settings. + */ + public function testInstaller() { + $contents = file_get_contents($this->container->getParameter('app.root') . '/' . $this->siteDirectory . '/settings.php'); + + // Test that isolation_level was not set. + $this->assertStringNotContainsString("'isolation_level' => 'READ COMMITTED'", $contents); + $this->assertStringNotContainsString("'isolation_level' => 'REPEATABLE READ'", $contents); + + // Change the default database connection to use the isolation level from + // the test. + $connection_info = Database::getConnectionInfo(); + $driver_test_connection = $connection_info['default']; + // We have asserted that the isolation level was not set. + unset($driver_test_connection['isolation_level']); + unset($driver_test_connection['init_commands']); + + Database::renameConnection('default', 'original_database_connection'); + Database::addConnectionInfo('default', 'default', $driver_test_connection); + // Close and reopen the database connection, so the database init commands + // get executed. + Database::closeConnection('default', 'default'); + $connection = Database::getConnection('default', 'default'); + + $query = 'SELECT @@SESSION.tx_isolation'; + // The database variable "tx_isolation" has been removed in MySQL v8.0 and + // has been replaced by "transaction_isolation". + // @see https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_tx_isolation + if (!$connection->isMariaDb() && version_compare($connection->version(), '8.0.0-AnyName', '>')) { + $query = 'SELECT @@SESSION.transaction_isolation'; + } + + // Test that transaction level is REPEATABLE READ. + $this->assertEquals('REPEATABLE-READ', $connection->query($query)->fetchField()); + + // Restore the old database connection. + Database::addConnectionInfo('default', 'default', $connection_info['default']); + Database::closeConnection('default', 'default'); + Database::getConnection('default', 'default'); + } + +} diff --git a/core/modules/mysql/tests/src/Functional/InstallerIsolationLevelNoDatabaseSettingsTest.php b/core/modules/mysql/tests/src/Functional/InstallerIsolationLevelNoDatabaseSettingsTest.php new file mode 100644 index 0000000000..f54dca0595 --- /dev/null +++ b/core/modules/mysql/tests/src/Functional/InstallerIsolationLevelNoDatabaseSettingsTest.php @@ -0,0 +1,74 @@ +markTestSkipped("This test does not support the {$connection_info['default']['driver']} database driver."); + } + } + + /** + * Verifies that the isolation_level was added to the database settings. + */ + public function testInstaller() { + $contents = file_get_contents($this->container->getParameter('app.root') . '/' . $this->siteDirectory . '/settings.php'); + + // Test that isolation_level was set to "READ COMMITTED". + $this->assertStringContainsString("'isolation_level' => 'READ COMMITTED',", $contents); + + // Change the default database connection to use the isolation level from + // the test. + $connection_info = Database::getConnectionInfo(); + $driver_test_connection = $connection_info['default']; + // We have asserted that the isolation level was set to 'READ COMMITTED'. + $driver_test_connection['isolation_level'] = 'READ COMMITTED'; + unset($driver_test_connection['init_commands']); + + Database::renameConnection('default', 'original_database_connection'); + Database::addConnectionInfo('default', 'default', $driver_test_connection); + // Close and reopen the database connection, so the database init commands + // get executed. + Database::closeConnection('default', 'default'); + $connection = Database::getConnection('default', 'default'); + + $query = 'SELECT @@SESSION.tx_isolation'; + // The database variable "tx_isolation" has been removed in MySQL v8.0 and + // has been replaced by "transaction_isolation". + // @see https://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_tx_isolation + if (!$connection->isMariaDb() && version_compare($connection->version(), '8.0.0-AnyName', '>')) { + $query = 'SELECT @@SESSION.transaction_isolation'; + } + + // Test that transaction level is READ-COMMITTED. + $this->assertEquals('READ-COMMITTED', $connection->query($query)->fetchField()); + + // Restore the old database connection. + Database::addConnectionInfo('default', 'default', $connection_info['default']); + Database::closeConnection('default', 'default'); + Database::getConnection('default', 'default'); + } + +} diff --git a/core/modules/mysql/tests/src/Functional/RequirementsTest.php b/core/modules/mysql/tests/src/Functional/RequirementsTest.php index d7ba5f18dc..d3039d79f3 100644 --- a/core/modules/mysql/tests/src/Functional/RequirementsTest.php +++ b/core/modules/mysql/tests/src/Functional/RequirementsTest.php @@ -81,7 +81,7 @@ public function testIsolationLevelWarningNotDisplaying() { private function writeIsolationLevelSettings(string $isolation_level) { $settings['databases']['default']['default']['init_commands'] = (object) [ 'value' => [ - 'isolation' => "SET SESSION TRANSACTION ISOLATION LEVEL {$isolation_level}", + 'isolation_level' => "SET SESSION TRANSACTION ISOLATION LEVEL {$isolation_level}", ], 'required' => TRUE, ]; diff --git a/core/modules/mysql/tests/src/Functional/mysql/PrimaryKeyExistsTest.php b/core/modules/mysql/tests/src/Functional/mysql/PrimaryKeyExistsTest.php new file mode 100644 index 0000000000..bc02cb80d8 --- /dev/null +++ b/core/modules/mysql/tests/src/Functional/mysql/PrimaryKeyExistsTest.php @@ -0,0 +1,68 @@ +drupalCreateUser([ + 'administer site configuration', + 'access site reports', + ]); + $this->drupalLogin($admin_user); + + // Check the message is not a warning. + $this->drupalGet('admin/reports/status'); + $elements = $this->xpath('//details[@class="system-status-report__entry"]//div[contains(text(), :text)]', [ + ':text' => 'To insure correct operation with READ-COMMITTED, ensure all tables have a primary key.', + ]); + $this->assertCount(1, $elements); + $this->assertStringNotContainsString('warning', $elements[0]->getParent()->getParent()->find('css', 'summary')->getAttribute('class')); + + $connection->schema()->createTable('test_primary_key', [ + 'fields' => [ + 'id' => [ + 'type' => 'int', + 'unsigned' => TRUE, + 'not null' => TRUE, + ], + ], + ]); + + $this->assertTrue($connection->schema()->tableExists('test_primary_key')); + + // Check the message is not a warning. + $this->drupalGet('admin/reports/status'); + $elements = $this->xpath('//details[@class="system-status-report__entry"]//div[contains(text(), :text)]', [ + ':text' => 'To insure correct operation with READ-COMMITTED, ensure all tables have a primary key. The following table(s) do not contain a primary key: test_primary_key', + ]); + $this->assertCount(1, $elements); + // Ensure it is a warning. + $this->assertStringContainsString('warning', $elements[0]->getParent()->getParent()->find('css', 'summary')->getAttribute('class')); + + } + +} diff --git a/core/phpstan-baseline.neon b/core/phpstan-baseline.neon index 955f2987dd..60168a877a 100644 --- a/core/phpstan-baseline.neon +++ b/core/phpstan-baseline.neon @@ -870,6 +870,11 @@ parameters: count: 1 path: modules/block_content/tests/src/Functional/BlockContentTypeTest.php + - + message: "#^Variable \\$goto in isset\\(\\) always exists and is not nullable\\.$#" + count: 1 + path: modules/block_content/tests/src/Functional/PageEditTest.php + - message: "#^Call to deprecated constant REQUEST_TIME\\: Deprecated in drupal\\:8\\.3\\.0 and is removed from drupal\\:11\\.0\\.0\\. Use \\\\Drupal\\:\\:time\\(\\)\\-\\>getRequestTime\\(\\); $#" count: 2 @@ -2175,11 +2180,6 @@ parameters: count: 2 path: modules/system/tests/modules/entity_test/entity_test.install - - - message: "#^Variable \\$goto in isset\\(\\) always exists and is not nullable\\.$#" - count: 1 - path: modules/system/tests/src/Functional/Menu/AssertBreadcrumbTrait.php - - message: "#^Call to deprecated constant REQUEST_TIME\\: Deprecated in drupal\\:8\\.3\\.0 and is removed from drupal\\:11\\.0\\.0\\. Use \\\\Drupal\\:\\:time\\(\\)\\-\\>getRequestTime\\(\\); $#" count: 1 @@ -3005,11 +3005,6 @@ parameters: count: 12 path: tests/Drupal/KernelTests/Core/Cache/GenericCacheBackendUnitTestBase.php - - - message: "#^Variable \\$expected_driver might not be defined\\.$#" - count: 2 - path: tests/Drupal/KernelTests/Core/Database/DriverSpecificKernelTestBase.php - - message: "#^Relying on entity queries to check access by default is deprecated in drupal\\:9\\.2\\.0 and an error will be thrown from drupal\\:10\\.0\\.0\\. Call \\\\Drupal\\\\Core\\\\Entity\\\\Query\\\\QueryInterface\\:\\:accessCheck\\(\\) with TRUE or FALSE to specify whether access should be checked\\.$#" count: 2 diff --git a/core/tests/Drupal/FunctionalTests/Core/Database/DriverSpecificBrowserTestBase.php b/core/tests/Drupal/FunctionalTests/Core/Database/DriverSpecificBrowserTestBase.php new file mode 100644 index 0000000000..aed0c4589c --- /dev/null +++ b/core/tests/Drupal/FunctionalTests/Core/Database/DriverSpecificBrowserTestBase.php @@ -0,0 +1,21 @@ +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++) { - if ($test_class_parts[$i] === 'Kernel') { - $expected_driver = $test_class_parts[$i + 1] ?? ''; - 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'."); - } - } + use DriverSpecificTrait; } diff --git a/core/tests/Drupal/Tests/DriverSpecificTrait.php b/core/tests/Drupal/Tests/DriverSpecificTrait.php new file mode 100644 index 0000000000..81f3063653 --- /dev/null +++ b/core/tests/Drupal/Tests/DriverSpecificTrait.php @@ -0,0 +1,56 @@ +root = static::getDrupalRoot(); + $expected_driver = ''; + + if (method_exists($this, 'getDatabaseConnectionInfo')) { + $connectionInfo = $this->getDatabaseConnectionInfo(); + } + else { + $connectionInfo = Database::getConnectionInfo(); + } + $test_class_parts = explode('\\', get_class($this)); + $expected_provider = $test_class_parts[2] ?? ''; + for ($i = 3; $i < count($test_class_parts); $i++) { + if (in_array($test_class_parts[$i], ['Kernel', 'Functional'], TRUE)) { + $expected_driver = $test_class_parts[$i + 1] ?? ''; + 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'."); + } + + } + +}