Anticipate Transaction Manager implementation

.github/workflows/pr.yml

@@ -104,7 +104,7 @@ jobs:
         continue-on-error: true
         run: vendor/bin/phpunit -c core --color=always ${{ matrix.test-args }}
 
-      - uses: actions/upload-artifact@v3
-        with:
-          name: test-results
-          path: sites/simpletest/browser_output
+#      - uses: actions/upload-artifact@v3
+#        with:
+#          name: test-results
+#          path: sites/simpletest/browser_output

.github/workflows/test.yml

@@ -24,7 +24,7 @@ jobs:
     runs-on: ubuntu-20.04
 
     strategy:
-      fail-fast: true
+      fail-fast: false
       matrix:
         php-version:
           - "8.1"

README.md

@@ -11,6 +11,7 @@ Requires patches for the following issues to be applied:
 Issue              | Description
 -------------------|----------------------------------------------------------------------------------------------|
 #3110546           | Allow contributed modules (mostly database drivers) to override tests in core |
+#3364706           | Refactor transactions |
 
 
 Known issues

src/Driver/Database/mysqli/Connection.php

@@ -4,12 +4,14 @@
 
 use Drupal\Core\Database\Connection as BaseConnection;
 use Drupal\Core\Database\Database;
+use Drupal\Core\Database\Transaction\TransactionManagerInterface;
 use Drupal\Core\Database\TransactionNameNonUniqueException;
 use Drupal\Core\Database\TransactionNoActiveException;
 use Drupal\Core\Database\TransactionOutOfOrderException;
 use Drupal\mysql\Driver\Database\mysql\Connection as BaseMySqlConnection;
 use Drupal\mysqli\Driver\Database\mysqli\Parser\Parser;
 use Drupal\mysqli\Driver\Database\mysqli\Parser\Visitor;
+
 /**
  * MySQLi implementation of \Drupal\Core\Database\Connection.
  */
@@ -181,151 +183,6 @@ public function lastInsertId(?string $name = NULL): string {
     return $this->connection->insert_id;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function pushTransaction($name) {
-// global $xxx; if ($xxx) dump(['pushTransaction in', $name]);
-    if (isset($this->transactionLayers[$name])) {
-      throw new TransactionNameNonUniqueException($name . " is already in use.");
-    }
-    // If we're already in a transaction then we want to create a savepoint
-    // rather than try to create another transaction.
-    if ($this->inTransaction()) {
-// if ($xxx) dump(['pushTransaction savepoint', $name]);
-      $this->connection->savepoint($name);
-    }
-    else {
-// if ($xxx) dump(['pushTransaction begin_transaction', $name]);
-      $this->connection->begin_transaction(0, $name);
-    }
-    $this->transactionLayers[$name] = $name;
-// if ($xxx) dump(['pushTransaction out', $this->transactionLayers]);
-  }
-
-  /**
-   * {@inheritdoc}
-   *
-   * mysqli does not support query('RELEASE SAVEPOINT ' . $name), we
-   * need to use direct rollback on the connection.
-   */
-  protected function popCommittableTransactions() {
-// global $xxx; if ($xxx) dump(['popCommittableTransactions in', $this->transactionLayers]);
-    // Commit all the committable layers.
-    foreach (array_reverse($this->transactionLayers) as $name => $active) {
-      // Stop once we found an active transaction.
-      if ($active) {
-        break;
-      }
-
-      // If there are no more layers left then we should commit.
-      unset($this->transactionLayers[$name]);
-      if (empty($this->transactionLayers)) {
-//dump(['popCommittableTransactions 1', $name]);
-        $this->doCommit();
-      }
-      else {
-//dump(['popCommittableTransactions 2', $name]);
-        if (!$this->connection->release_savepoint($name)) {
-//dump(['popCommittableTransactions 3', $name]);
-          $this->transactionLayers = [];
-          $this->doCommit();
-        }
-      }
-    }
-// if ($xxx) dump(['popCommittableTransactions out', $this->transactionLayers]);
-  }
-
-  /**
-   * {@inheritdoc}
-   *
-   * mysqli does not support query('ROLLBACK TO SAVEPOINT ' . $savepoint), we
-   * need to use direct rollback on the connection.
-   */
-  public function rollBack($savepoint_name = 'drupal_transaction') {
-// global $xxx; if ($xxx) dump(['rollBack in', $savepoint_name, $this->transactionLayers]);
-    if (!$this->inTransaction()) {
-      throw new TransactionNoActiveException();
-    }
-    // A previous rollback to an earlier savepoint may mean that the savepoint
-    // in question has already been accidentally committed.
-    if (!isset($this->transactionLayers[$savepoint_name])) {
-      throw new TransactionNoActiveException();
-    }
-
-    // We need to find the point we're rolling back to, all other savepoints
-    // before are no longer needed. If we rolled back other active savepoints,
-    // we need to throw an exception.
-    $rolled_back_other_active_savepoints = FALSE;
-    while ($savepoint = array_pop($this->transactionLayers)) {
-      if ($savepoint == $savepoint_name) {
-        // If it is the last the transaction in the stack, then it is not a
-        // savepoint, it is the transaction itself so we will need to roll back
-        // the transaction rather than a savepoint.
-        if (empty($this->transactionLayers)) {
-//dump(['rollBack 2', $savepoint_name, $this->transactionLayers]);
-          break;
-        }
-//dump($this->query('SELECT * FROM {test}')->fetchAll());
-//        $success = $this->connection->rollback(0, $savepoint);
-        $success = $this->connection->query('ROLLBACK TO SAVEPOINT ' . $savepoint_name);
-//dump(['rollBack 3', $savepoint_name, $this->transactionLayers, $success]);
-//dump($this->query('SELECT * FROM {test}')->fetchAll());
-        $this->popCommittableTransactions();
-        if ($rolled_back_other_active_savepoints) {
-          throw new TransactionOutOfOrderException();
-        }
-        return;
-      }
-      else {
-//dump(['rollBack 4', $savepoint, $savepoint_name, $this->transactionLayers]);
-        $rolled_back_other_active_savepoints = TRUE;
-      }
-// if ($xxx) dump(['rollBack out', $savepoint_name, $this->transactionLayers]);
-    }
-
-    // Notify the callbacks about the rollback.
-    $callbacks = $this->rootTransactionEndCallbacks;
-    $this->rootTransactionEndCallbacks = [];
-    foreach ($callbacks as $callback) {
-      call_user_func($callback, FALSE);
-    }
-
-//dump(['in rollback 1']);
-    if (!$this->connection->rollBack()) {
-//dump(['in rollback 2']);
-      trigger_error('Invalid rollback', E_USER_WARNING);
-    }
-    if ($rolled_back_other_active_savepoints) {
-      throw new TransactionOutOfOrderException();
-    }
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  protected function doCommit() {
-    try {
-      $this->connection->commit();
-      $success = TRUE;
-    }
-    catch (\mysqli_sql_exception $e) {
-      $success = FALSE;
-    }
-
-    if (!empty($this->rootTransactionEndCallbacks)) {
-      $callbacks = $this->rootTransactionEndCallbacks;
-      $this->rootTransactionEndCallbacks = [];
-      foreach ($callbacks as $callback) {
-        call_user_func($callback, $success);
-      }
-    }
-
-    if (!$success) {
-      throw new TransactionCommitFailedException();
-    }
-  }
-
   /**
    * @todo
    */
@@ -356,4 +213,18 @@ public function exceptionHandler() {
     return new ExceptionHandler();
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function driverTransactionManager(): TransactionManagerInterface {
+    return new TransactionManager($this);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function startTransaction($name = '') {
+    return $this->transactionManager()->push($name);
+  }
+
 }

src/Driver/Database/mysqli/TransactionManager.php

@@ -0,0 +1,60 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\mysqli\Driver\Database\mysqli;
+
+use Drupal\Core\Database\Transaction\ClientConnectionTransactionState;
+use Drupal\Core\Database\Transaction\TransactionManagerBase;
+
+/**
+ * MySqli implementation of TransactionManagerInterface.
+ */
+class TransactionManager extends TransactionManagerBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function beginClientTransaction(): bool {
+    return $this->connection->getClientConnection()->begin_transaction();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function addClientSavepoint(string $name): bool {
+    return $this->connection->getClientConnection()->savepoint($name);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function rollbackClientSavepoint(string $name): bool {
+    // Mysqli does not have a rollback_to_savepoint method, and it does not
+    // allow a prepared statement for 'ROLLBACK TO SAVEPOINT', so we need to
+    // fallback to query on the client connection directly.
+    return (bool) $this->connection->getClientConnection()->query('ROLLBACK TO SAVEPOINT ' . $name);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function releaseClientSavepoint(string $name): bool {
+    return $this->connection->getClientConnection()->release_savepoint($name);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function rollbackClientTransaction(): bool {
+    return $this->connection->getClientConnection()->rollback();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function commitClientTransaction(): bool {
+    return $this->connection->getClientConnection()->commit();
+  }
+
+}

tests/github/drupal_patch.sh

@@ -3,5 +3,8 @@
 #3110546 Allow contributed modules (mostly database drivers) to override tests in core
 curl https://git.drupalcode.org/project/drupal/-/merge_requests/291.diff | git apply -v
 
+#3364706 Refactor transactions
+curl https://git.drupalcode.org/project/drupal/-/merge_requests/4101.diff | git apply -v
+
 # Extra patch
 # git apply -v ./mysqli_staging/tests/github/extra_patch.patch

tests/src/Kernel/mysqli/TransactionTest.php

@@ -104,4 +104,13 @@ public function testTransactionWithDdlStatement() {
     }
   }
 
+  /**
+   * Tests deprecation of Connection methods.
+   *
+   * @group legacy
+   */
+  public function testConnectionDeprecations(): void {
+    $this->markTestSkipped('Skipping this for mysqli');
+  }
+
 }