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..6b37454412 100644 --- a/core/modules/mysql/mysql.install +++ b/core/modules/mysql/mysql.install @@ -31,13 +31,42 @@ function mysql_requirements($phase) { $query = 'SELECT @@SESSION.transaction_isolation'; } + $tables_missing_primary_key = []; + $tables = $connection->schema()->findTables('%'); $isolation_level = $connection->query($query)->fetchField(); + $severity_level = REQUIREMENT_INFO; + $description = 'This site is using the database transaction level "READ COMMITTED".'; + + 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)) { + $tables_missing_primary_key[] = $table; + } + } + + $tables_missing_primary_key = 'For this to work properly all tables should have a primary key. The following table(s) do no have a primary key: ' . implode(', ', $tables_missing_primary_key); + + if ($isolation_level == 'READ-COMMITTED' && !empty($tables_missing_primary_key)) { + $severity_level = REQUIREMENT_ERROR; + $description = 'This site is using the database transaction level "READ COMMITTED". ' . !empty($tables_missing_primary_key); + } + elseif ($isolation_level == 'REPEATABLE-READ') { + $severity_level = REQUIREMENT_WARNING; + $description = 'This site is using the database transaction level "REPEATABLE READ". The recommended database transaction level for Drupal is "READ COMMITTED".'; + if ($tables_missing_primary_key) { + $description .= ' ' . $tables_missing_primary_key; + } + } + elseif ($isolation_level == 'READ-UNCOMMITTED' || $isolation_level == 'SERIALIZED') { + $severity_level = REQUIREMENT_ERROR; + $description = 'This site is using the database transaction level "' . $isolation_level . '". This database transaction level is not supported by Drupal. The recommended database transaction level for Drupal is "READ COMMITTED".'; + } $requirements['mysql_transaction_level'] = [ 'title' => t('Database Isolation Level'), - 'severity' => $isolation_level === 'READ-COMMITTED' ? REQUIREMENT_OK : REQUIREMENT_WARNING, + 'severity' => $severity_level, 'value' => $isolation_level, - 'description' => t('For the best performance and to minimize locking issues, the READ-COMMITTED transaction isolation level is recommended. See the setting MySQL transaction isolation level page for more information.', [ + 'description' => t($description . ' See the setting MySQL transaction isolation level page for more information.', [ ':performance_doc' => 'https://www.drupal.org/docs/system-requirements/setting-the-mysql-transaction-isolation-level', ]), ]; diff --git a/core/modules/mysql/src/Driver/Database/mysql/Connection.php b/core/modules/mysql/src/Driver/Database/mysql/Connection.php index 7c9fab2836..61f4e94059 100644 --- a/core/modules/mysql/src/Driver/Database/mysql/Connection.php +++ b/core/modules/mysql/src/Driver/Database/mysql/Connection.php @@ -203,6 +203,13 @@ public static function open(array &$connection_options = []) { $connection_options['init_commands'] += [ '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(strtoupper($connection_options['isolation_level']), ['READ COMMITTED', 'REPEATABLE READ'], TRUE)) { + $connection_options['init_commands'] += [ + 'isolation_level' => 'SET SESSION TRANSACTION ISOLATION LEVEL ' . strtoupper($connection_options['isolation_level']), + ]; + } // Execute initial commands. foreach ($connection_options['init_commands'] as $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 3a41083d68..3fbd49c7b7 100644 --- a/core/modules/mysql/src/Driver/Database/mysql/Install/Tasks.php +++ b/core/modules/mysql/src/Driver/Database/mysql/Install/Tasks.php @@ -174,6 +174,19 @@ public function getFormOptions(array $database) { if (empty($form['advanced_options']['port']['#default_value'])) { $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('The recommended database transaction level for Drupal is "READ COMMITTED". By selecting the option "READ COMMITTED" Drupal will set the transaction level on every page request. If possible, we recommend to set database transaction level globally. In that case select the option "DO NOT SET BY DRUPAL". For more information, see the setting MySQL transaction isolation level page.', [ + ':performance_doc' => 'https://www.drupal.org/docs/system-requirements/setting-the-mysql-transaction-isolation-level', + ]), + ]; 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..627c33ab66 100644 --- a/core/modules/mysql/tests/src/Functional/RequirementsTest.php +++ b/core/modules/mysql/tests/src/Functional/RequirementsTest.php @@ -44,6 +44,7 @@ public function testIsolationLevelWarningNotDisplaying() { 'access site reports', ]); $this->drupalLogin($admin_user); + $connection = Database::getConnection(); // Set the isolation level to a level that produces a warning. $this->writeIsolationLevelSettings('REPEATABLE READ'); @@ -51,7 +52,7 @@ public function testIsolationLevelWarningNotDisplaying() { // 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' => 'For the best performance and to minimize locking issues, the READ-COMMITTED', + ':text' => 'This site is using the database transaction level "REPEATABLE READ". The recommended database', ]); $this->assertCount(1, $elements); $this->assertStringStartsWith('REPEATABLE-READ', $elements[0]->getParent()->getText()); @@ -64,12 +65,49 @@ public function testIsolationLevelWarningNotDisplaying() { // 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' => 'For the best performance and to minimize locking issues, the READ-COMMITTED', + ':text' => 'This site is using the database transaction level "READ COMMITTED"', ]); $this->assertCount(1, $elements); $this->assertStringStartsWith('READ-COMMITTED', $elements[0]->getParent()->getText()); // Ensure it is a not a warning. $this->assertStringNotContainsString('warning', $elements[0]->getParent()->getParent()->find('css', 'summary')->getAttribute('class')); + + $specification = [ + 'fields' => [ + 'text' => [ + 'type' => 'text', + 'description' => 'A text field', + ], + ], + ]; + + $connection->schema()->createTable('test_table_without_primary_key', $specification); + + // Set the isolation level to a level that produces a warning. + $this->writeIsolationLevelSettings('REPEATABLE READ'); + + // 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' => 'This site is using the database transaction level "REPEATABLE READ". The recommended database transaction level for Drupal is "READ COMMITTED". For this to work properly all tables should have a primary key. The following table(s) do no have a primary key:', + ]); + $this->assertCount(1, $elements); + $this->assertStringStartsWith('REPEATABLE-READ', $elements[0]->getParent()->getText()); + // Ensure it is a warning. + $this->assertStringContainsString('warning', $elements[0]->getParent()->getParent()->find('css', 'summary')->getAttribute('class')); + + // Rollback the isolation level to read committed. + $this->writeIsolationLevelSettings('READ COMMITTED'); + + // 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' => 'This site is using the database transaction level "READ COMMITTED". For this to work properly all tables should have a primary key. The following table(s) do no have a primary key:', + ]); + $this->assertCount(1, $elements); + $this->assertStringStartsWith('READ-COMMITTED', $elements[0]->getParent()->getText()); + // Ensure it is an error. + $this->assertStringContainsString('error', $elements[0]->getParent()->getParent()->find('css', 'summary')->getAttribute('class')); } /** @@ -81,7 +119,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, ];