Issue #2278971 by mondrake, daffie, xjm, longwave: Deprecate Connection::supportsTransactions()

core/assets/scaffold/files/default.settings.php

@@ -105,14 +105,6 @@
  * webserver.  For most other drivers, you must specify a
  * username, password, host, and database name.
  *
- * Transaction support is enabled by default for all drivers that support it,
- * including MySQL. To explicitly disable it, set the 'transactions' key to
- * FALSE.
- * Note that some configurations of MySQL, such as the MyISAM engine, don't
- * support it and will proceed silently even if enabled. If you experience
- * transaction related crashes with such configuration, set the 'transactions'
- * key to FALSE.
- *
  * For each database, you may optionally specify multiple "target" databases.
  * A target database allows Drupal to try to send certain queries to a
  * different database if it can but fall back to the default connection if not.

core/lib/Drupal/Core/Database/Connection.php

@@ -68,13 +68,6 @@ abstract class Connection {
    */
   protected $statementClass = 'Drupal\Core\Database\Statement';
 
-  /**
-   * Whether this database connection supports transactions.
-   *
-   * @var bool
-   */
-  protected $transactionSupport = TRUE;
-
   /**
    * Whether this database connection supports transactional DDL.
    *
@@ -198,6 +191,12 @@ abstract class Connection {
    *   - Other driver-specific options.
    */
   public function __construct(\PDO $connection, array $connection_options) {
+    // The 'transactions' option is deprecated.
+    if (isset($connection_options['transactions'])) {
+      @trigger_error('Passing a \'transactions\' connection option to ' . __METHOD__ . ' is deprecated in drupal:9.1.0 and is removed in drupal:10.0.0. All database drivers must support transactions. See https://www.drupal.org/node/2278745', E_USER_DEPRECATED);
+      unset($connection_options['transactions']);
+    }
+
     // Initialize and prepare the connection prefix.
     $this->setPrefix(isset($connection_options['prefix']) ? $connection_options['prefix'] : '');
 
@@ -1216,9 +1215,6 @@ public function startTransaction($name = '') {
    * @see \Drupal\Core\Database\Transaction::rollBack()
    */
   public function rollBack($savepoint_name = 'drupal_transaction') {
-    if (!$this->supportsTransactions()) {
-      return;
-    }
     if (!$this->inTransaction()) {
       throw new TransactionNoActiveException();
     }
@@ -1278,9 +1274,6 @@ public function rollBack($savepoint_name = 'drupal_transaction') {
    * @see \Drupal\Core\Database\Transaction
    */
   public function pushTransaction($name) {
-    if (!$this->supportsTransactions()) {
-      return;
-    }
     if (isset($this->transactionLayers[$name])) {
       throw new TransactionNameNonUniqueException($name . " is already in use.");
     }
@@ -1311,9 +1304,6 @@ public function pushTransaction($name) {
    * @see \Drupal\Core\Database\Transaction
    */
   public function popTransaction($name) {
-    if (!$this->supportsTransactions()) {
-      return;
-    }
     // The transaction has already been committed earlier. There is nothing we
     // need to do. If this transaction was part of an earlier out-of-order
     // rollback, an exception would already have been thrown by
@@ -1494,9 +1484,15 @@ public function clientVersion() {
    *
    * @return bool
    *   TRUE if this connection supports transactions, FALSE otherwise.
+   *
+   * @deprecated in drupal:9.1.0 and is removed in drupal:10.0.0. All database
+   * drivers must support transactions.
+   *
+   * @see https://www.drupal.org/node/2278745
    */
   public function supportsTransactions() {
-    return $this->transactionSupport;
+    @trigger_error(__METHOD__ . ' is deprecated in drupal:9.1.0 and is removed in drupal:10.0.0. All database drivers must support transactions. See https://www.drupal.org/node/2278745', E_USER_DEPRECATED);
+    return TRUE;
   }
 
   /**

core/lib/Drupal/Core/Database/Driver/mysql/Connection.php

@@ -63,21 +63,6 @@ class Connection extends DatabaseConnection {
    */
   const MIN_MAX_ALLOWED_PACKET = 1024;
 
-  /**
-   * Constructs a Connection object.
-   */
-  public function __construct(\PDO $connection, array $connection_options = []) {
-    parent::__construct($connection, $connection_options);
-
-    // This driver defaults to transaction support, except if explicitly passed FALSE.
-    $this->transactionSupport = !isset($connection_options['transactions']) || ($connection_options['transactions'] !== FALSE);
-
-    // MySQL never supports transactional DDL.
-    $this->transactionalDDLSupport = FALSE;
-
-    $this->connectionOptions = $connection_options;
-  }
-
   /**
    * {@inheritdoc}
    */

core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php

@@ -49,21 +49,17 @@ class Connection extends DatabaseConnection {
     'NOT REGEXP' => ['operator' => '!~*'],
   ];
 
+  /**
+   * {@inheritdoc}
+   */
+  protected $transactionalDDLSupport = TRUE;
+
   /**
    * Constructs a connection object.
    */
   public function __construct(\PDO $connection, array $connection_options) {
     parent::__construct($connection, $connection_options);
 
-    // This driver defaults to transaction support, except if explicitly passed FALSE.
-    $this->transactionSupport = !isset($connection_options['transactions']) || ($connection_options['transactions'] !== FALSE);
-
-    // Transactional DDL is always available in PostgreSQL,
-    // but we'll only enable it if standard transactions are.
-    $this->transactionalDDLSupport = $this->transactionSupport;
-
-    $this->connectionOptions = $connection_options;
-
     // Force PostgreSQL to use the UTF-8 character set by default.
     $this->connection->exec("SET NAMES 'UTF8'");
 

core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php

@@ -55,6 +55,11 @@ class Connection extends DatabaseConnection {
    */
   public $tableDropped = FALSE;
 
+  /**
+   * {@inheritdoc}
+   */
+  protected $transactionalDDLSupport = TRUE;
+
   /**
    * Constructs a \Drupal\Core\Database\Driver\sqlite\Connection object.
    */
@@ -65,11 +70,6 @@ public function __construct(\PDO $connection, array $connection_options) {
 
     parent::__construct($connection, $connection_options);
 
-    // This driver defaults to transaction support, except if explicitly passed FALSE.
-    $this->transactionSupport = $this->transactionalDDLSupport = !isset($connection_options['transactions']) || $connection_options['transactions'] !== FALSE;
-
-    $this->connectionOptions = $connection_options;
-
     // Attach one database for each registered prefix.
     $prefixes = $this->prefixes;
     foreach ($prefixes as &$prefix) {

core/modules/block_content/tests/src/Functional/BlockContentCreationTest.php

@@ -208,28 +208,14 @@ public function testFailedBlockCreation() {
     }
 
     $connection = Database::getConnection();
-    if ($connection->supportsTransactions()) {
-      // Check that the block does not exist in the database.
-      $id = $connection->select('block_content_field_data', 'b')
-        ->fields('b', ['id'])
-        ->condition('info', 'fail_creation')
-        ->execute()
-        ->fetchField();
-      $this->assertFalse($id, 'Transactions supported, and block not found in database.');
-    }
-    else {
-      // Check that the block exists in the database.
-      $id = $connection->select('block_content_field_data', 'b')
-        ->fields('b', ['id'])
-        ->condition('info', 'fail_creation')
-        ->execute()
-        ->fetchField();
-      $this->assertTrue($id, 'Transactions not supported, and block found in database.');
-
-      // Check that the failed rollback was logged.
-      $records = $connection->query("SELECT wid FROM {watchdog} WHERE message LIKE 'Explicit rollback failed%'")->fetchAll();
-      $this->assertTrue(count($records) > 0, 'Transactions not supported, and rollback error logged to watchdog.');
-    }
+
+    // Check that the block does not exist in the database.
+    $id = $connection->select('block_content_field_data', 'b')
+      ->fields('b', ['id'])
+      ->condition('info', 'fail_creation')
+      ->execute()
+      ->fetchField();
+    $this->assertFalse($id);
   }
 
   /**

core/modules/node/tests/src/Functional/NodeCreationTest.php

@@ -108,20 +108,9 @@ public function testFailedPageCreation() {
       $this->pass('Expected exception has been thrown.');
     }
 
-    if (Database::getConnection()->supportsTransactions()) {
-      // Check that the node does not exist in the database.
-      $node = $this->drupalGetNodeByTitle($edit['title']);
-      $this->assertFalse($node, 'Transactions supported, and node not found in database.');
-    }
-    else {
-      // Check that the node exists in the database.
-      $node = $this->drupalGetNodeByTitle($edit['title']);
-      $this->assertTrue($node, 'Transactions not supported, and node found in database.');
-
-      // Check that the failed rollback was logged.
-      $records = static::getWatchdogIdsForFailedExplicitRollback();
-      $this->assertTrue(count($records) > 0, 'Transactions not supported, and rollback error logged to watchdog.');
-    }
+    // Check that the node does not exist in the database.
+    $node = $this->drupalGetNodeByTitle($edit['title']);
+    $this->assertFalse($node);
 
     // Check that the rollback error was logged.
     $records = static::getWatchdogIdsForTestExceptionRollback();
@@ -289,15 +278,4 @@ protected static function getWatchdogIdsForTestExceptionRollback() {
     return $matches;
   }
 
-  /**
-   * Gets the log records with the explicit rollback failed exception message.
-   *
-   * @return \Drupal\Core\Database\StatementInterface
-   *   A prepared statement object (already executed), which contains the log
-   *   records with the explicit rollback failed exception message.
-   */
-  protected static function getWatchdogIdsForFailedExplicitRollback() {
-    return Database::getConnection()->query("SELECT wid FROM {watchdog} WHERE message LIKE 'Explicit rollback failed%'")->fetchAll();
-  }
-
 }

core/tests/Drupal/KernelTests/Core/Database/ConnectionTest.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\KernelTests\Core\Database;
 
+use Drupal\Core\Database\Connection;
 use Drupal\Core\Database\Database;
 use Drupal\Core\Database\DatabaseExceptionWrapper;
 use Drupal\Core\Database\Query\Condition;
@@ -117,6 +118,22 @@ public function testConnectionOptions() {
     $this->assertNotEqual($connection_info['default']['database'], $connectionOptions['database'], 'The test connection info database does not match the current connection options database.');
   }
 
+  /**
+   * Tests the deprecation of the 'transactions' connection option.
+   *
+   * @group legacy
+   * @expectedDeprecation Passing a 'transactions' connection option to Drupal\Core\Database\Connection::__construct is deprecated in drupal:9.1.0 and is removed in drupal:10.0.0. All database drivers must support transactions. See https://www.drupal.org/node/2278745
+   * @expectedDeprecation Drupal\Core\Database\Connection::supportsTransactions is deprecated in drupal:9.1.0 and is removed in drupal:10.0.0. All database drivers must support transactions. See https://www.drupal.org/node/2278745
+   */
+  public function testTransactionsOptionDeprecation() {
+    $connection_info = Database::getConnectionInfo('default');
+    $connection_info['default']['transactions'] = FALSE;
+    Database::addConnectionInfo('default', 'foo', $connection_info['default']);
+    $foo_connection = Database::getConnection('foo', 'default');
+    $this->assertInstanceOf(Connection::class, $foo_connection);
+    $this->assertTrue($foo_connection->supportsTransactions());
+  }
+
   /**
    * Ensure that you cannot execute multiple statements on MySQL.
    */

core/tests/Drupal/KernelTests/Core/Database/DeleteTruncateTest.php

@@ -70,11 +70,6 @@ public function testTruncate() {
    * Confirms that we can truncate a whole table while in transaction.
    */
   public function testTruncateInTransaction() {
-    // This test won't work right if transactions are not supported.
-    if (!$this->connection->supportsTransactions()) {
-      $this->markTestSkipped('The database driver does not support transactions.');
-    }
-
     $num_records_before = $this->connection->select('test')->countQuery()->execute()->fetchField();
     $this->assertGreaterThan(0, $num_records_before, 'The table is not empty.');
 
@@ -109,11 +104,6 @@ public function testTruncateInTransaction() {
    * Confirms that transaction rollback voids a truncate operation.
    */
   public function testTruncateTransactionRollback() {
-    // This test won't work right if transactions are not supported.
-    if (!$this->connection->supportsTransactions()) {
-      $this->markTestSkipped('The database driver does not support transactions.');
-    }
-
     $num_records_before = $this->connection->select('test')->countQuery()->execute()->fetchField();
     $this->assertGreaterThan(0, $num_records_before, 'The table is not empty.');
 

core/tests/Drupal/KernelTests/Core/Database/InvalidDataTest.php

@@ -2,7 +2,6 @@
 
 namespace Drupal\KernelTests\Core\Database;
 
-use Drupal\Core\Database\Database;
 use Drupal\Core\Database\IntegrityConstraintViolationException;
 
 /**
@@ -17,6 +16,7 @@ class InvalidDataTest extends DatabaseTestBase {
    */
   public function testInsertDuplicateData() {
     // Try to insert multiple records where at least one has bad data.
+    $this->expectException(IntegrityConstraintViolationException::class);
     try {
       $this->connection->insert('test')
         ->fields(['name', 'age', 'job'])
@@ -24,8 +24,9 @@ public function testInsertDuplicateData() {
           'name' => 'Elvis',
           'age' => 63,
           'job' => 'Singer',
-        ])->values([
-          // Duplicate value on unique field.
+        ])
+        ->values([
+          // Duplicate value 'John' on unique field 'name'.
           'name' => 'John',
           'age' => 17,
           'job' => 'Consultant',
@@ -39,32 +40,13 @@ public function testInsertDuplicateData() {
       $this->fail('Insert succeeded when it should not have.');
     }
     catch (IntegrityConstraintViolationException $e) {
-      // Check if the first record was inserted.
-      $name = $this->connection->query('SELECT name FROM {test} WHERE age = :age', [':age' => 63])->fetchField();
-
-      if ($name == 'Elvis') {
-        if (!Database::getConnection()->supportsTransactions()) {
-          // This is an expected fail.
-          // Database engines that don't support transactions can leave partial
-          // inserts in place when an error occurs. This is the case for MySQL
-          // when running on a MyISAM table.
-          $this->pass("The whole transaction has not been rolled-back when a duplicate key insert occurs, this is expected because the database doesn't support transactions");
-        }
-        else {
-          $this->fail('The whole transaction is rolled back when a duplicate key insert occurs.');
-        }
-      }
-      else {
-        $this->pass('The whole transaction is rolled back when a duplicate key insert occurs.');
-      }
-
-      // Ensure the other values were not inserted.
-      $record = $this->connection->select('test')
+      // Ensure the whole transaction is rolled back when a duplicate key
+      // insert occurs.
+      $this->assertFalse($this->connection->select('test')
         ->fields('test', ['name', 'age'])
-        ->condition('age', [17, 75], 'IN')
-        ->execute()->fetchObject();
-
-      $this->assertFalse($record, 'The rest of the insert aborted as expected.');
+        ->condition('age', [63, 17, 75], 'IN')
+        ->execute()->fetchObject());
+      throw $e;
     }
   }
 
@@ -81,9 +63,10 @@ public function testInsertDuplicateDataFromSelect() {
         'name' => 'Elvis',
         'age' => 63,
         'job' => 'Singer',
-      ])->values([
-        // Duplicate value on unique field 'name' for later INSERT in 'test'
-        // table.
+      ])
+      ->values([
+        // Duplicate value 'John' on unique field 'name' for later INSERT in
+        // 'test' table.
         'name' => 'John',
         'age' => 17,
         'job' => 'Consultant',
@@ -95,53 +78,34 @@ public function testInsertDuplicateDataFromSelect() {
       ])
       ->execute();
 
-    try {
-      // Define the subselect query. Add ORDER BY to ensure we have consistent
-      // order in results. Will return:
-      // 0 => [name] => Elvis, [age] => 63, [job] => Singer
-      // 1 => [name] => Frank, [age] => 75, [job] => Bass
-      // 2 => [name] => John, [age] => 17, [job] => Consultant
-      // 3 => [name] => Meredith, [age] => 30, [job] => Speaker
-      // Records 0 and 1 should pass, record 2 should lead to integrity
-      // constraint violation.
-      $query = $this->connection->select('test_people', 'tp')
-        ->fields('tp', ['name', 'age', 'job'])
-        ->orderBy('name');
+    // Define the subselect query. Add ORDER BY to ensure we have consistent
+    // order in results. Will return:
+    // 0 => [name] => Elvis, [age] => 63, [job] => Singer
+    // 1 => [name] => Frank, [age] => 75, [job] => Bass
+    // 2 => [name] => John, [age] => 17, [job] => Consultant
+    // 3 => [name] => Meredith, [age] => 30, [job] => Speaker
+    // Records 0 and 1 should pass, record 2 should lead to integrity
+    // constraint violation.
+    $query = $this->connection->select('test_people', 'tp')
+      ->fields('tp', ['name', 'age', 'job'])
+      ->orderBy('name');
 
-      // Try inserting from the subselect.
+    // Try inserting from the subselect.
+    $this->expectException(IntegrityConstraintViolationException::class);
+    try {
       $this->connection->insert('test')
         ->from($query)
         ->execute();
-
       $this->fail('Insert succeeded when it should not have.');
     }
     catch (IntegrityConstraintViolationException $e) {
-      // Check if the second record was inserted.
-      $name = $this->connection->query('SELECT name FROM {test} WHERE age = :age', [':age' => 75])->fetchField();
-
-      if ($name == 'Frank') {
-        if (!Database::getConnection()->supportsTransactions()) {
-          // This is an expected fail.
-          // Database engines that don't support transactions can leave partial
-          // inserts in place when an error occurs. This is the case for MySQL
-          // when running on a MyISAM table.
-          $this->pass("The whole transaction has not been rolled-back when a duplicate key insert occurs, this is expected because the database doesn't support transactions");
-        }
-        else {
-          $this->fail('The whole transaction is rolled back when a duplicate key insert occurs.');
-        }
-      }
-      else {
-        $this->pass('The whole transaction is rolled back when a duplicate key insert occurs.');
-      }
-
-      // Ensure the values for records 2 and 3 were not inserted.
-      $record = $this->connection->select('test')
+      // Ensure the whole transaction is rolled back when a duplicate key
+      // insert occurs.
+      $this->assertFalse($this->connection->select('test')
         ->fields('test', ['name', 'age'])
-        ->condition('age', [17, 30], 'IN')
-        ->execute()->fetchObject();
-
-      $this->assertFalse($record, 'The rest of the insert aborted as expected.');
+        ->condition('age', [63, 75, 17, 30], 'IN')
+        ->execute()->fetchObject());
+      throw $e;
     }
   }
 

core/tests/Drupal/KernelTests/Core/Database/TransactionTest.php

@@ -144,11 +144,6 @@ protected function transactionInnerLayer($suffix, $rollback = FALSE, $ddl_statem
    * nothing.
    */
   public function testTransactionRollBackSupported() {
-    // This test won't work right if transactions are not supported.
-    if (!$this->connection->supportsTransactions()) {
-      $this->markTestSkipped("The '{$this->connection->driver()}' database driver does not support transactions.");
-    }
-
     try {
       // Create two nested transactions. Roll back from the inner one.
       $this->transactionOuterLayer('B', TRUE);
@@ -165,33 +160,6 @@ public function testTransactionRollBackSupported() {
     }
   }
 
-  /**
-   * Tests transaction rollback on a database that doesn't support transactions.
-   *
-   * If the active driver supports transactions, this test does nothing.
-   */
-  public function testTransactionRollBackNotSupported() {
-    // This test won't work right if transactions are supported.
-    if ($this->connection->supportsTransactions()) {
-      $this->markTestSkipped("The '{$this->connection->driver()}' database driver supports transactions.");
-    }
-
-    try {
-      // Create two nested transactions. Attempt to roll back from the inner one.
-      $this->transactionOuterLayer('B', TRUE);
-
-      // Because our current database claims to not support transactions,
-      // the inserted rows should be present despite the attempt to roll back.
-      $saved_age = $this->connection->query('SELECT age FROM {test} WHERE name = :name', [':name' => 'DavidB'])->fetchField();
-      $this->assertIdentical($saved_age, '24', 'DavidB not rolled back, since transactions are not supported.');
-      $saved_age = $this->connection->query('SELECT age FROM {test} WHERE name = :name', [':name' => 'DanielB'])->fetchField();
-      $this->assertIdentical($saved_age, '19', 'DanielB not rolled back, since transactions are not supported.');
-    }
-    catch (\Exception $e) {
-      $this->fail($e->getMessage());
-    }
-  }
-
   /**
    * Tests a committed transaction.
    *
@@ -379,11 +347,6 @@ public function assertRowAbsent($name, $message = NULL) {
    * Tests transaction stacking, commit, and rollback.
    */
   public function testTransactionStacking() {
-    // This test won't work right if transactions are not supported.
-    if (!$this->connection->supportsTransactions()) {
-      $this->markTestSkipped("The '{$this->connection->driver()}' database driver does not support transactions.");
-    }
-
     // Standard case: pop the inner transaction before the outer transaction.
     $transaction = $this->connection->startTransaction();
     $this->insertRow('outer');

core/tests/Drupal/KernelTests/Core/Entity/EntityCrudHookTest.php

@@ -5,7 +5,6 @@
 use Drupal\comment\Entity\Comment;
 use Drupal\comment\Plugin\Field\FieldType\CommentItemInterface;
 use Drupal\comment\Tests\CommentTestTrait;
-use Drupal\Core\Database\Database;
 use Drupal\Core\Language\LanguageInterface;
 use Drupal\block\Entity\Block;
 use Drupal\entity_test\Entity\EntityTest;
@@ -547,16 +546,9 @@ public function testEntityRollback() {
       $this->pass('Expected exception has been thrown.');
     }
 
-    if (Database::getConnection()->supportsTransactions()) {
-      // Check that the block does not exist in the database.
-      $ids = \Drupal::entityQuery('entity_test')->condition('name', 'fail_insert')->execute();
-      $this->assertTrue(empty($ids), 'Transactions supported, and entity not found in database.');
-    }
-    else {
-      // Check that the block exists in the database.
-      $ids = \Drupal::entityQuery('entity_test')->condition('name', 'fail_insert')->execute();
-      $this->assertFalse(empty($ids), 'Transactions not supported, and entity found in database.');
-    }
+    // Check that the block does not exist in the database.
+    $ids = \Drupal::entityQuery('entity_test')->condition('name', 'fail_insert')->execute();
+    $this->assertEmpty($ids);
   }
 
 }

sites/default/default.settings.php

@@ -105,14 +105,6 @@
  * webserver.  For most other drivers, you must specify a
  * username, password, host, and database name.
  *
- * Transaction support is enabled by default for all drivers that support it,
- * including MySQL. To explicitly disable it, set the 'transactions' key to
- * FALSE.
- * Note that some configurations of MySQL, such as the MyISAM engine, don't
- * support it and will proceed silently even if enabled. If you experience
- * transaction related crashes with such configuration, set the 'transactions'
- * key to FALSE.
- *
  * For each database, you may optionally specify multiple "target" databases.
  * A target database allows Drupal to try to send certain queries to a
  * different database if it can but fall back to the default connection if not.