refactor: keep errno 1213 state on the MySQLi connection

The flags are MySQLi-only, so BaseConnection stays unchanged.
_transBegin() and _transRollback() clear them after success.
When InnoDB has already rolled back, _transCommit() calls
transRollback() and returns false.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: KevinBlakeLopez

system/Database/BaseConnection.php

@@ -337,20 +337,6 @@ abstract class BaseConnection implements ConnectionInterface
      */
     protected $transFailure = false;
 
-    /**
-     * The server has already rolled this transaction back (MySQLi errno 1213).
-     * mysqli::commit() still returns true, so transCommit() must not report success.
-     */
-    protected bool $transServerRolledBack = false;
-
-    /**
-     * The next statement must open a server transaction before it runs.
-     * InnoDB ended the previous one and the connection is back in autocommit.
-     * Errno 1205 does not set this: innodb_rollback_on_timeout decides whether
-     * that error rolled the transaction back.
-     */
-    protected bool $transNeedsServerRestart = false;
-
     /**
      * Whether to throw exceptions during transaction
      */
@@ -1060,7 +1046,6 @@ public function transBegin(bool $testMode = false): bool
 
         if ($this->_transBegin()) {
             $this->transDepth++;
-            $this->clearServerRolledBackTransaction();
 
             return true;
         }
@@ -1078,24 +1063,7 @@ public function transCommit(): bool
         }
 
         // When transactions are nested we only begin/commit/rollback the outermost ones
-        if ($this->transDepth > 1) {
-            $this->transDepth--;
-
-            return true;
-        }
-
-        // InnoDB already removed the victim transaction. mysqli::commit()
-        // returns true anyway, which would publish later writes and report success.
-        // Roll back the restarted server transaction so a false return ends the
-        // transaction, as transComplete() does. Otherwise a caller that retries
-        // without transRollback() nests inside it and nothing commits.
-        if ($this->transServerRolledBack) {
-            $this->transRollback();
-
-            return false;
-        }
-
-        if ($this->_transCommit()) {
+        if ($this->transDepth > 1 || $this->_transCommit()) {
             $this->transDepth--;
 
             return true;
@@ -1117,10 +1085,6 @@ public function transRollback(): bool
         if ($this->transDepth > 1 || $this->_transRollback()) {
             $this->transDepth--;
 
-            if ($this->transDepth === 0) {
-                $this->clearServerRolledBackTransaction();
-            }
-
             return true;
         }
 
@@ -1149,37 +1113,6 @@ public function handleTransStatus(): void
         }
     }
 
-    /**
-     * Remember that the server has rolled the open transaction back.
-     */
-    protected function markServerRolledBackTransaction(): void
-    {
-        $this->transServerRolledBack   = true;
-        $this->transNeedsServerRestart = true;
-    }
-
-    /**
-     * True while the next statement must BEGIN before it runs.
-     */
-    protected function serverTransactionRestartPending(): bool
-    {
-        return $this->transNeedsServerRestart && $this->transDepth > 0;
-    }
-
-    /**
-     * BEGIN succeeded. Clear the restart only now, so a failed BEGIN keeps it pending.
-     */
-    protected function serverTransactionRestarted(): void
-    {
-        $this->transNeedsServerRestart = false;
-    }
-
-    private function clearServerRolledBackTransaction(): void
-    {
-        $this->transServerRolledBack   = false;
-        $this->transNeedsServerRestart = false;
-    }
-
     /**
      * Begin Transaction
      */

system/Database/MySQLi/Connection.php

@@ -91,6 +91,16 @@ class Connection extends BaseConnection
      */
     public $foundRows = false;
 
+    /**
+     * InnoDB errno 1213 ended this transaction.
+     */
+    private bool $transServerRolledBack = false;
+
+    /**
+     * The next statement must BEGIN before it runs.
+     */
+    private bool $transNeedsServerRestart = false;
+
     /**
      * Connect to the database.
      *
@@ -291,20 +301,16 @@ protected function execute(string $sql)
         }
 
         try {
-            // Errno 1213 ends the server transaction. Begin again before this
-            // statement so it does not autocommit. A failed BEGIN goes through
-            // the normal error path below, and the restart stays pending.
-            // Errno 1205 is not handled: innodb_rollback_on_timeout decides
-            // whether InnoDB rolled that transaction back.
+            // 1213 ended the server transaction. BEGIN again so this statement
+            // does not autocommit. A failed BEGIN stays pending. 1205 is unchanged:
+            // innodb_rollback_on_timeout decides that case.
             if ($this->connID instanceof mysqli && $this->serverTransactionRestartPending()) {
-                // With mysqli exception reporting off, a failed BEGIN returns
-                // false instead of throwing. Do not run the statement without
-                // the transaction.
+                // Exception reporting off: a failed BEGIN returns false.
                 if (! $this->connID->begin_transaction()) {
                     return false;
                 }
 
-                $this->serverTransactionRestarted();
+                $this->transNeedsServerRestart = false;
             }
 
             return $this->connID->query($this->prepQuery($sql), $this->resultMode);
@@ -594,16 +600,53 @@ public function insertID(): int
 
     protected function _transBegin(): bool
     {
-        return $this->connID->begin_transaction();
+        if (! $this->connID->begin_transaction()) {
+            return false;
+        }
+
+        $this->clearDeadlockTransactionState();
+
+        return true;
     }
 
     protected function _transCommit(): bool
     {
+        // mysqli::commit() returns true after 1213. End the transaction
+        // and report failure instead.
+        if ($this->transServerRolledBack) {
+            $this->transRollback();
+
+            return false;
+        }
+
         return $this->connID->commit();
     }
 
     protected function _transRollback(): bool
     {
-        return $this->connID->rollback();
+        if (! $this->connID->rollback()) {
+            return false;
+        }
+
+        $this->clearDeadlockTransactionState();
+
+        return true;
+    }
+
+    private function markServerRolledBackTransaction(): void
+    {
+        $this->transServerRolledBack   = true;
+        $this->transNeedsServerRestart = true;
+    }
+
+    private function serverTransactionRestartPending(): bool
+    {
+        return $this->transNeedsServerRestart && $this->transDepth > 0;
+    }
+
+    private function clearDeadlockTransactionState(): void
+    {
+        $this->transServerRolledBack   = false;
+        $this->transNeedsServerRestart = false;
     }
 }