diff --git a/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php index 38d57e317f13ec530686c1cdfd37f6eaa7ccf68d..8847857fb504650f7365beadec4d759d1f499773 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 df5f4230edcd839aebd238bfdf55baa29ca3abbc..241fb99b41800fa66143f54b9364a17b568f6d2c 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 6ccb1f97ff0cebc558d29b0b35a8a9374ff4b835..59487a7488e5797d4afa26013a810285a0bf8e8f 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 42ed7344afb326691cf5c8f10ae8d5e92e8c0e1e..e4ddf8d10b6891708901f5d16533dd3bddec0e0c 100644 --- a/core/modules/mysql/mysql.install +++ b/core/modules/mysql/mysql.install @@ -6,6 +6,7 @@ */ use Drupal\Core\Database\Database; +use Drupal\Core\Render\Markup; /** * Implements hook_requirements(). @@ -33,13 +34,48 @@ function mysql_requirements($phase) { $isolation_level = $connection->query($query)->fetchField(); + $tables_missing_primary_key = []; + $tables = $connection->schema()->findTables('%'); + 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; + } + } + + $description = []; + if ($isolation_level == 'READ-COMMITTED') { + if (empty($tables_missing_primary_key)) { + $severity_level = REQUIREMENT_OK; + } + else { + $severity_level = REQUIREMENT_ERROR; + } + } + else { + if ($isolation_level == 'REPEATABLE-READ') { + $severity_level = REQUIREMENT_WARNING; + } + else { + $severity_level = REQUIREMENT_ERROR; + $description[] = t('This is not supported by Drupal.'); + } + $description[] = t('The recommended level for Drupal is "READ COMMITTED".'); + } + + if (!empty($tables_missing_primary_key)) { + $description[] = t('For this to work correctly, all tables must have a primary key. The following table(s) do not have a primary key: @tables.', ['@tables' => implode(', ', $tables_missing_primary_key)]); + } + + $description[] = t('See the <a href=":performance_doc">setting MySQL transaction isolation level</a> page for more information.', [ + ':performance_doc' => 'https://www.drupal.org/docs/system-requirements/setting-the-mysql-transaction-isolation-level', + ]); + $requirements['mysql_transaction_level'] = [ - 'title' => t('Database Isolation Level'), - 'severity' => $isolation_level === 'READ-COMMITTED' ? REQUIREMENT_OK : REQUIREMENT_WARNING, + 'title' => t('Transaction isolation level'), + '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 <a href=":performance_doc">setting MySQL transaction isolation level</a> page for more information.', [ - ':performance_doc' => 'https://www.drupal.org/docs/system-requirements/setting-the-mysql-transaction-isolation-level', - ]), + 'description' => Markup::create(implode(' ', $description)), ]; } } diff --git a/core/modules/mysql/src/Driver/Database/mysql/Connection.php b/core/modules/mysql/src/Driver/Database/mysql/Connection.php index a04138540596c9add15da8be3225a9a780bd629a..73239a453612e0ea7c49951492cab9a6d3f06726 100644 --- a/core/modules/mysql/src/Driver/Database/mysql/Connection.php +++ b/core/modules/mysql/src/Driver/Database/mysql/Connection.php @@ -232,6 +232,11 @@ public static function open(array &$connection_options = []) { $connection_options['init_commands'] += [ 'sql_mode' => "SET sql_mode = 'ANSI,TRADITIONAL'", ]; + if (!empty($connection_options['isolation_level'])) { + $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 3a41083d68474f0822ff33f91b50320054e26178..e27ef369fa244c012627d90dffe94b926b8a83a5 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'), + '' => $this->t('Use database default'), + ], + '#default_value' => $database['isolation_level'] ?? 'READ COMMITTED', + '#description' => $this->t('The recommended database transaction level for Drupal is "READ COMMITTED". For more information, see the <a href=":performance_doc">setting MySQL transaction isolation level</a> 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 0000000000000000000000000000000000000000..ace161dc5b1a9d57ebe7b980a5db7ebfb5722bf8 --- /dev/null +++ b/core/modules/mysql/tests/src/Functional/InstallerIsolationLevelExistingSettingsTest.php @@ -0,0 +1,70 @@ +<?php + +namespace Drupal\Tests\mysql\Functional; + +use Drupal\Core\Database\Database; +use Drupal\FunctionalTests\Installer\InstallerExistingSettingsTest; + +/** + * Tests the isolation_level setting with existing database settings. + * + * @group Installer + */ +class InstallerIsolationLevelExistingSettingsTest extends InstallerExistingSettingsTest { + + /** + * {@inheritdoc} + */ + protected function prepareEnvironment() { + parent::prepareEnvironment(); + + $connection_info = Database::getConnectionInfo(); + // The isolation_level option is only available for MySQL. + if ($connection_info['default']['driver'] !== 'mysql') { + $this->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 0000000000000000000000000000000000000000..f54dca05954c2cf36c03843272510dbaf4c67da5 --- /dev/null +++ b/core/modules/mysql/tests/src/Functional/InstallerIsolationLevelNoDatabaseSettingsTest.php @@ -0,0 +1,74 @@ +<?php + +namespace Drupal\Tests\mysql\Functional; + +use Drupal\Core\Database\Database; +use Drupal\FunctionalTests\Installer\InstallerTestBase; + +/** + * Tests the isolation_level setting with no database settings. + * + * @group Installer + */ +class InstallerIsolationLevelNoDatabaseSettingsTest extends InstallerTestBase { + + /** + * {@inheritdoc} + */ + protected $defaultTheme = 'stark'; + + /** + * {@inheritdoc} + */ + protected function prepareEnvironment() { + parent::prepareEnvironment(); + + // The isolation_level option is only available for MySQL. + $connection_info = Database::getConnectionInfo(); + if ($connection_info['default']['driver'] !== 'mysql') { + $this->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 d7ba5f18dced022f1bf3fad2305dfc4332b49e38..b9edb5319c1f587acb262ca066541d9c1cc5e10a 100644 --- a/core/modules/mysql/tests/src/Functional/RequirementsTest.php +++ b/core/modules/mysql/tests/src/Functional/RequirementsTest.php @@ -44,6 +44,38 @@ 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'); + + // Check the message is not a warning. + $this->drupalGet('admin/reports/status'); + $elements = $this->xpath('//details[@class="system-status-report__entry"]//div[contains(text(), "REPEATABLE-READ")]'); + $this->assertCount(1, $elements); + // 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(), "READ-COMMITTED")]'); + $this->assertCount(1, $elements); + // 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'); @@ -51,7 +83,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' => 'The recommended level for Drupal is "READ COMMITTED". For this to work correctly, all tables must have a primary key. The following table(s) do not have a primary key: test_table_without_primary_key.', ]); $this->assertCount(1, $elements); $this->assertStringStartsWith('REPEATABLE-READ', $elements[0]->getParent()->getText()); @@ -64,12 +96,12 @@ 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' => 'For this to work correctly, all tables must have a primary key. The following table(s) do not have a primary key: test_table_without_primary_key.', ]); $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')); + // Ensure it is an error. + $this->assertStringContainsString('error', $elements[0]->getParent()->getParent()->find('css', 'summary')->getAttribute('class')); } /** @@ -80,8 +112,8 @@ 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}", + 'value' => [ + 'isolation_level' => "SET SESSION TRANSACTION ISOLATION LEVEL {$isolation_level}", ], 'required' => TRUE, ];