refactor(transaction): inherit postgres startTransaction

Author: abnegate

src/Database/Adapter/Postgres.php

@@ -31,44 +31,6 @@
 class Postgres extends SQL
 {
     public const MAX_IDENTIFIER_NAME = 63;
-    /**
-     * @inheritDoc
-     */
-    public function startTransaction(): bool
-    {
-        try {
-            if ($this->inTransaction === 0) {
-                try {
-                    if ($this->getPDO()->inTransaction()) {
-                        $this->getPDO()->rollBack();
-                    } else {
-                        // If no active transaction, this has no effect.
-                        $this->getPDO()->prepare('ROLLBACK')->execute();
-                    }
-                } catch (PDOException) {
-                    // A pooled connection can report a transaction it no longer
-                    // holds after a reconnect (e.g. Swoole PDOProxy keeps its own
-                    // counter), making this cleanup rollback throw. It is best
-                    // effort; swallow it and begin a fresh transaction below.
-                }
-
-                $result = $this->getPDO()->beginTransaction();
-            } else {
-                $this->getPDO()->exec('SAVEPOINT transaction' . $this->inTransaction);
-                $result = true;
-            }
-        } catch (PDOException $e) {
-            throw new TransactionException('Failed to start transaction: ' . $e->getMessage(), $e->getCode(), $e);
-        }
-
-        if (!$result) {
-            throw new TransactionException('Failed to start transaction');
-        }
-
-        $this->inTransaction++;
-
-        return $result;
-    }
 
     /**
      * @inheritDoc

src/Database/Adapter/SQL.php

@@ -94,18 +94,23 @@ public function startTransaction(): bool
                     // effort; swallow it and begin a fresh transaction below.
                 }
 
-                $this->getPDO()->beginTransaction();
+                $result = $this->getPDO()->beginTransaction();
 
             } else {
                 $this->getPDO()->exec('SAVEPOINT transaction' . $this->inTransaction);
+                $result = true;
             }
         } catch (PDOException $e) {
             throw new TransactionException('Failed to start transaction: ' . $e->getMessage(), $e->getCode(), $e);
         }
 
+        if (!$result) {
+            throw new TransactionException('Failed to start transaction');
+        }
+
         $this->inTransaction++;
 
-        return true;
+        return $result;
     }
 
     /**

tests/unit/SQLTransactionTest.php

@@ -4,11 +4,13 @@
 
 use PDOException;
 use PHPUnit\Framework\TestCase;
+use ReflectionMethod;
 use Utopia\Database\Adapter\MySQL;
 use Utopia\Database\Adapter\Postgres;
+use Utopia\Database\Adapter\SQL;
 use Utopia\Database\Exception\Transaction as TransactionException;
 
-class SQLTransactionTest extends TestCase
+final class SQLTransactionTest extends TestCase
 {
     /**
      * A pooled connection (e.g. Swoole PDOProxy) can keep its own transaction
@@ -59,8 +61,36 @@ public function testStartTransactionDoesNotMaskBeginFailureAfterDesyncedRollback
         $adapter->startTransaction();
     }
 
+    public function testStartTransactionDoesNotMaskFalseBeginResult(): void
+    {
+        $pdo = $this->getMockBuilder(\PDO::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $pdo->method('inTransaction')->willReturn(false);
+        $pdo->expects($this->once())
+            ->method('beginTransaction')
+            ->willReturn(false);
+
+        $cleanup = $this->getMockBuilder(\PDOStatement::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $cleanup->method('execute')->willReturn(true);
+        $pdo->method('prepare')->with('ROLLBACK')->willReturn($cleanup);
+
+        $adapter = new MySQL($pdo);
+
+        $this->expectException(TransactionException::class);
+        $this->expectExceptionMessage('Failed to start transaction');
+
+        $adapter->startTransaction();
+    }
+
     public function testPostgresStartTransactionRecoversFromDesyncedRollback(): void
     {
+        $method = new ReflectionMethod(Postgres::class, 'startTransaction');
+        $this->assertSame(SQL::class, $method->getDeclaringClass()->getName());
+
         $pdo = $this->getMockBuilder(\PDO::class)
             ->disableOriginalConstructor()
             ->getMock();