[D7] Add ability to register rootEndTransaction callbacks during transactions

+++ b/includes/database/mysql/database.inc
@@ -180,9 +180,7 @@ class DatabaseConnection_mysql extends DatabaseConnection {
@@ -203,7 +201,7 @@ class DatabaseConnection_mysql extends DatabaseConnection {

@@ -203,7 +201,7 @@ class DatabaseConnection_mysql extends DatabaseConnection {
             $this->transactionLayers = array();
             // We also have to explain to PDO that the transaction stack has
             // been cleaned-up.
-            PDO::commit();
+            $this->doCommit();
           }
           else {
             throw $e;

Is it a problem that now we can throw an Exception here -- while before this would not throw an Exception if the commit failed?

RTBC, but this needs to wait for the D8 issue to be in first.

D8 patch landed.

After recent changes by #3185918: [PP-1] [PHP 8] Fix DatabaseConnection::query signature mismatch with PDO::query this patch no longer applies. It would need to be rerolled and checked.

Added reroll of patch #7 on Drupal 7.x.

Thanks for the reroll @ravi.shankar, but is seems like the test for PostgreSQL and SQLite are failing (problems accessing the parent::). It can be caused by the fact, that the DatabaseConnection class no longer extends PDO class after: #3185918: [PP-1] [PHP 8] Fix DatabaseConnection::query signature mismatch with PDO::query .

I have updated the patch with some changes (see interdiff) and compared it to the current D9 code (some minor changes).

1 - Removed the Callable type-hinting, because it has been introduced only in PHP 5.4 (and D7 should support also PHP 5.3), so I think we can live without that.

2 - Changed the order of $this->connection->rollBack(); and $this->executeRootTransactionEndCallbacks(FALSE); functions in the SQLite driver (on multiple places) - this should be the correct order (the same is also in the D9 code).

3 - Fixed the doCommit() function (problem with ::parent, see the previous patch tests).

Let's check the testbot now.

There may be an issue in the patched code from this most recent patch, includes/database/mysql/database.inc in this block:

protected function doCommit() {
    if ($this->connection->inTransaction()) {
      $success = parent::doCommit();
    }
    else {
      // In PHP 8.0 a PDOException is thrown when a commit is attempted with no
      // transaction active. In previous PHP versions this failed silently.
      $success = TRUE;
      // Allow callbacks to perform their own cleanup.
      $this->executeRootTransactionEndCallbacks($success);
    }
    return $success;

Should DatabaseConnection::doCommit() return $success at the end? Without it, the child classes are testing for $success = parent::doCommit()) if in the middle of a transaction, but that parent function currently uses a void return value (due to there being no return statement).

+  /**
+   * Do the actual commit, invoke pre-commit callbacks.
+   *
+   * @internal
+   */
+  protected function doCommit() {
+    $success = $this->connection->commit();
+    $this->executeRootTransactionEndCallbacks($success);
+
+    if (!$success) {
+      throw new DatabaseTransactionCommitFailedException();
+    }
+  }
+

In DatabaseConnection, the patch adds this code.

+  /**
+   * Do the actual commit, invoke pre-commit callbacks.
+   *
+   * @internal
+   */
+  protected function doCommit() {
+    $success = $this->connection->commit();
+    $this->executeRootTransactionEndCallbacks($success);
+
+    if (!$success) {
+      throw new DatabaseTransactionCommitFailedException();
+    }
+  }

In DatabaseConnection_mysql, the code is changed as follows.

   protected function doCommit() {
     if ($this->connection->inTransaction()) {
-      return $this->connection->commit();
+      $success = parent::doCommit();
     }
     else {
       // In PHP 8.0 a PDOException is thrown when a commit is attempted with no
       // transaction active. In previous PHP versions this failed silently.
-      return TRUE;
+      $success = TRUE;
+      // Allow callbacks to perform their own cleanup.
+      $this->executeRootTransactionEndCallbacks($success);
     }
+    return $success;
   }

$success = parent::doCommit(); would just set $success to NULL, since DatabaseConnection::doCommit() does not return any value.