Skip to content

Commit

Permalink
Ensure nested transactions are not wrongly rolled back
Browse files Browse the repository at this point in the history
  • Loading branch information
simPod committed Oct 13, 2024
1 parent 086213b commit 9879edf
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 9 deletions.
20 changes: 19 additions & 1 deletion src/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Doctrine\DBAL\Cache\QueryCacheProfile;
use Doctrine\DBAL\Driver\API\ExceptionConverter;
use Doctrine\DBAL\Driver\Connection as DriverConnection;
use Doctrine\DBAL\Driver\Exception as TheDriverException;
use Doctrine\DBAL\Driver\ServerInfoAwareConnection;
use Doctrine\DBAL\Driver\Statement as DriverStatement;
use Doctrine\DBAL\Event\TransactionBeginEventArgs;
Expand Down Expand Up @@ -1279,12 +1280,29 @@ public function transactional(Closure $func)
{
$this->beginTransaction();

$successful = false;

try {
$res = $func($this);

$successful = true;
} finally {
if (! $successful) {
$this->rollBack();
}
}

$shouldRollback = true;
try {
$this->commit();

$shouldRollback = false;
} catch (TheDriverException $t) {
$shouldRollback = false;

Check warning on line 1301 in src/Connection.php

View check run for this annotation

Codecov / codecov/patch

src/Connection.php#L1300-L1301

Added lines #L1300 - L1301 were not covered by tests

throw $t;

Check warning on line 1303 in src/Connection.php

View check run for this annotation

Codecov / codecov/patch

src/Connection.php#L1303

Added line #L1303 was not covered by tests
} finally {
if ($this->isTransactionActive()) {
if ($shouldRollback) {
$this->rollBack();

Check warning on line 1306 in src/Connection.php

View check run for this annotation

Codecov / codecov/patch

src/Connection.php#L1306

Added line #L1306 was not covered by tests
}
}
Expand Down
28 changes: 20 additions & 8 deletions tests/Functional/TransactionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,23 @@

namespace Doctrine\DBAL\Tests\Functional;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Driver\Exception as DriverException;
use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
use Doctrine\DBAL\Platforms\OraclePlatform;
use Doctrine\DBAL\Tests\FunctionalTestCase;
use PDOException;

use function sleep;

class TransactionTest extends FunctionalTestCase
{
protected function setUp(): void
public function testCommitFalse(): void
{
if ($this->connection->getDatabasePlatform() instanceof AbstractMySQLPlatform) {
return;
if (! $this->connection->getDatabasePlatform() instanceof AbstractMySQLPlatform) {
$this->markTestSkipped('Restricted to MySQL.');
}

$this->markTestSkipped('Restricted to MySQL.');
}

public function testCommitFalse(): void
{
$this->connection->executeStatement('SET SESSION wait_timeout=1');

self::assertTrue($this->connection->beginTransaction());
Expand All @@ -40,4 +37,19 @@ public function testCommitFalse(): void
$this->connection->close();
}
}

public function testNestedTransactionWalkthrough(): void
{
$query = $this->connection->getDatabasePlatform() instanceof OraclePlatform
? 'SELECT 1 FROM DUAL'
: 'SELECT 1';

$result = $this->connection->transactional(
static fn (Connection $connection) => $connection->transactional(
static fn (Connection $connection) => $connection->fetchOne($query),
),
);

self::assertSame('1', (string) $result);
}
}

0 comments on commit 9879edf

Please sign in to comment.