Always use savepoints when nesting transactions (#1135)

atk4 · Oct 2, 2023 · 947664c · 947664c
1 parent cb65dbc
commit 947664c
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 5 deletions.
diff --git a/src/Model.php b/src/Model.php
@@ -1640,16 +1640,16 @@ public function insert(array $row)
     {
         $entity = $this->createEntity();
 
-        $hasArrayValue = false;
+        $hasRefs = false;
         foreach ($row as $v) {
             if (is_array($v)) {
-                $hasArrayValue = true;
+                $hasRefs = true;
 
                 break;
             }
         }
 
-        if (!$hasArrayValue) {
+        if (!$hasRefs) {
             $entity->_insert($row);
         } else {
             $this->atomic(static function () use ($entity, $row) {

diff --git a/src/Persistence/Sql/Connection.php b/src/Persistence/Sql/Connection.php
@@ -201,6 +201,8 @@ public static function connect($dsn, $user = null, $password = null, $defaults =
             $dbalConnection = $connectionClass::connectFromDbalDriverConnection($dbalDriverConnection);
         }
 
+        $dbalConnection->setNestTransactionsWithSavepoints(true); // remove once DBAL 3.x support is dropped
+
         $connection = new $connectionClass($defaults);
         $connection->_connection = $dbalConnection;
 

diff --git a/src/Schema/TestSqlPersistence.php b/src/Schema/TestSqlPersistence.php
@@ -36,6 +36,12 @@ public function getConnection(): Persistence\Sql\Connection
                     new class() implements SQLLogger {
                         public function startQuery($sql, array $params = null, array $types = null): void
                         {
+                            // log transaction savepoint operations only once
+                            // https://github.com/doctrine/dbal/blob/3.6.7/src/Connection.php#L1365
+                            if (preg_match('~^(?:SAVEPOINT|RELEASE SAVEPOINT|ROLLBACK TO SAVEPOINT|SAVE TRANSACTION|ROLLBACK TRANSACTION) DOCTRINE2_SAVEPOINT_\d+;?$~', $sql)) {
+                                return;
+                            }
+
                             // fix https://github.com/doctrine/dbal/issues/5525
                             if ($params !== null && $params !== [] && array_is_list($params)) {
                                 $params = array_combine(range(1, count($params)), $params);

diff --git a/tests/Schema/TestCaseTest.php b/tests/Schema/TestCaseTest.php
@@ -56,6 +56,9 @@ public function testLogQuery(): void
                 "START TRANSACTION";
 
 
+                "SAVEPOINT";
+
+
                 insert into `t` (`name`, `int`, `float`, `null`)
                 values
                   (
@@ -86,9 +89,9 @@ public function testLogQuery(): void
                   and `id` = 1
                 EOF
             . $makeLimitSqlFx(2)
+            . ";\n\n"
+            . ($this->getDatabasePlatform()->supportsReleaseSavepoints() ? "\n\"RELEASE SAVEPOINT\";\n\n" : '')
             . <<<'EOF'
-                ;
-
 
                 "COMMIT";
 

diff --git a/tests/TransactionTest.php b/tests/TransactionTest.php
@@ -82,6 +82,47 @@ public function testAtomicInMiddleOfResultIteration(): void
         ], $this->getDb()['item']);
     }
 
+    public function testAtomicWithRollbackToSavepoint(): void
+    {
+        $this->setDb([
+            'item' => [
+                ['name' => 'John'],
+                ['name' => 'Sue'],
+                ['name' => 'Smith'],
+            ],
+        ]);
+
+        $m = new Model($this->db, ['table' => 'item']);
+        $m->addField('name');
+        $m->setOrder('id');
+
+        $this->db->atomic(function () use ($m) {
+            foreach ($m as $entity) {
+                $rollback = $entity->get('name') === 'Sue';
+
+                try {
+                    $this->db->atomic(static function () use ($entity, $rollback) {
+                        $entity->set('name', $entity->get('name') . ' 2');
+                        $entity->save();
+
+                        if ($rollback) {
+                            throw new \Exception('Rollback to savepoint');
+                        }
+                    });
+                } catch (\Exception $e) {
+                    self::assertSame('Rollback to savepoint', $e->getMessage());
+                    self::assertTrue($rollback);
+                }
+            }
+        });
+
+        self::assertSame([
+            1 => ['id' => 1, 'name' => 'John 2'],
+            ['id' => 2, 'name' => 'Sue'],
+            ['id' => 3, 'name' => 'Smith 2'],
+        ], $this->getDb()['item']);
+    }
+
     public function testBeforeSaveHook(): void
     {
         $this->setDb([