Skip to content

Commit e3305f6

Browse files
abnegateabnegate
authored
Merge pull request #817 from utopia-php/fix-transaction-state-reset
Fix transaction state management across adapters
2 parents 5e49f32 + 143e3bb commit e3305f6

File tree

6 files changed

+222
-4
lines changed

6 files changed

+222
-4
lines changed

‎src/Database/Adapter.php‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,6 @@ public function withTransaction(callable $callback): mixed
430430
$action instanceof ConflictException ||
431431
$action instanceof LimitException
432432
) {
433-
$this->inTransaction = 0;
434433
throw $action;
435434
}
436435

@@ -439,7 +438,6 @@ public function withTransaction(callable $callback): mixed
439438
continue;
440439
}
441440

442-
$this->inTransaction = 0;
443441
throw $action;
444442
}
445443
}
@@ -1531,4 +1529,18 @@ public function getSupportForTTLIndexes(): bool
15311529
{
15321530
return false;
15331531
}
1532+
1533+
/**
1534+
* Does the adapter support transaction retries?
1535+
*
1536+
* @return bool
1537+
*/
1538+
abstract public function getSupportForTransactionRetries(): bool;
1539+
1540+
/**
1541+
* Does the adapter support nested transactions?
1542+
*
1543+
* @return bool
1544+
*/
1545+
abstract public function getSupportForNestedTransactions(): bool;
15341546
}

‎src/Database/Adapter/Mongo.php‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,13 @@ public function withTransaction(callable $callback): mixed
103103
{
104104
// If the database is not a replica set, we can't use transactions
105105
if (!$this->client->isReplicaSet()) {
106-
$result = $callback();
107-
return $result;
106+
return $callback();
107+
}
108+
109+
// MongoDB doesn't support nested transactions/savepoints.
110+
// If already in a transaction, just run the callback directly.
111+
if ($this->inTransaction > 0) {
112+
return $callback();
108113
}
109114

110115
try {
@@ -3687,6 +3692,16 @@ public function getSupportForTTLIndexes(): bool
36873692
return true;
36883693
}
36893694

3695+
public function getSupportForTransactionRetries(): bool
3696+
{
3697+
return false;
3698+
}
3699+
3700+
public function getSupportForNestedTransactions(): bool
3701+
{
3702+
return false;
3703+
}
3704+
36903705
protected function isExtendedISODatetime(string $val): bool
36913706
{
36923707
/**

‎src/Database/Adapter/Pool.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -703,4 +703,14 @@ public function getSupportForTTLIndexes(): bool
703703
{
704704
return $this->delegate(__FUNCTION__, \func_get_args());
705705
}
706+
707+
public function getSupportForTransactionRetries(): bool
708+
{
709+
return $this->delegate(__FUNCTION__, \func_get_args());
710+
}
711+
712+
public function getSupportForNestedTransactions(): bool
713+
{
714+
return $this->delegate(__FUNCTION__, \func_get_args());
715+
}
706716
}

‎src/Database/Adapter/Postgres.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ public function startTransaction(): bool
4747

4848
$result = $this->getPDO()->beginTransaction();
4949
} else {
50+
$this->getPDO()->exec('SAVEPOINT transaction' . $this->inTransaction);
5051
$result = true;
5152
}
5253
} catch (PDOException $e) {
@@ -72,9 +73,16 @@ public function rollbackTransaction(): bool
7273
}
7374

7475
try {
76+
if ($this->inTransaction > 1) {
77+
$this->getPDO()->exec('ROLLBACK TO transaction' . ($this->inTransaction - 1));
78+
$this->inTransaction--;
79+
return true;
80+
}
81+
7582
$result = $this->getPDO()->rollBack();
7683
$this->inTransaction = 0;
7784
} catch (PDOException $e) {
85+
$this->inTransaction = 0;
7886
throw new DatabaseException('Failed to rollback transaction: ' . $e->getMessage(), $e->getCode(), $e);
7987
}
8088

‎src/Database/Adapter/SQL.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3577,4 +3577,14 @@ public function getLockType(): string
35773577

35783578
return '';
35793579
}
3580+
3581+
public function getSupportForTransactionRetries(): bool
3582+
{
3583+
return true;
3584+
}
3585+
3586+
public function getSupportForNestedTransactions(): bool
3587+
{
3588+
return true;
3589+
}
35803590
}

‎tests/e2e/Adapter/Scopes/GeneralTests.php‎

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,169 @@ public function testTransactionAtomicity(): void
845845
$database->deleteCollection('transactionAtomicity');
846846
}
847847

848+
/**
849+
* Test that withTransaction correctly resets inTransaction state
850+
* when a known exception (DuplicateException) is thrown after successful rollback.
851+
*/
852+
public function testTransactionStateAfterKnownException(): void
853+
{
854+
/** @var Database $database */
855+
$database = $this->getDatabase();
856+
857+
$database->createCollection('txKnownException');
858+
$database->createAttribute('txKnownException', 'title', Database::VAR_STRING, 128, true);
859+
860+
$database->createDocument('txKnownException', new Document([
861+
'$id' => 'existing_doc',
862+
'$permissions' => [
863+
Permission::read(Role::any()),
864+
],
865+
'title' => 'Original',
866+
]));
867+
868+
// Trigger a DuplicateException inside withTransaction by inserting a duplicate ID
869+
try {
870+
$database->withTransaction(function () use ($database) {
871+
$database->createDocument('txKnownException', new Document([
872+
'$id' => 'existing_doc',
873+
'$permissions' => [
874+
Permission::read(Role::any()),
875+
],
876+
'title' => 'Duplicate',
877+
]));
878+
});
879+
$this->fail('Expected DuplicateException was not thrown');
880+
} catch (DuplicateException $e) {
881+
// Expected
882+
}
883+
884+
// inTransaction must be false after the exception
885+
$this->assertFalse(
886+
$database->getAdapter()->inTransaction(),
887+
'Adapter should not be in transaction after DuplicateException'
888+
);
889+
890+
// Database should still be functional
891+
$doc = $database->getDocument('txKnownException', 'existing_doc');
892+
$this->assertEquals('Original', $doc->getAttribute('title'));
893+
894+
$database->deleteCollection('txKnownException');
895+
}
896+
897+
/**
898+
* Test that withTransaction correctly resets inTransaction state
899+
* when retries are exhausted for a generic exception.
900+
*
901+
* MongoDB's withTransaction has no retry logic, so this test
902+
* only applies to SQL-based adapters.
903+
*/
904+
public function testTransactionStateAfterRetriesExhausted(): void
905+
{
906+
/** @var Database $database */
907+
$database = $this->getDatabase();
908+
909+
if (!$database->getAdapter()->getSupportForTransactionRetries()) {
910+
$this->expectNotToPerformAssertions();
911+
return;
912+
}
913+
914+
$attempts = 0;
915+
916+
try {
917+
$database->withTransaction(function () use (&$attempts) {
918+
$attempts++;
919+
throw new \RuntimeException('Persistent failure');
920+
});
921+
} catch (\RuntimeException $e) {
922+
$this->assertEquals('Persistent failure', $e->getMessage());
923+
}
924+
925+
// Should have attempted 3 times (initial + 2 retries)
926+
$this->assertEquals(3, $attempts, 'Should have exhausted all retry attempts');
927+
928+
// inTransaction must be false after retries exhausted
929+
$this->assertFalse(
930+
$database->getAdapter()->inTransaction(),
931+
'Adapter should not be in transaction after retries exhausted'
932+
);
933+
}
934+
935+
/**
936+
* Test that nested withTransaction calls maintain correct inTransaction state
937+
* when the inner transaction throws a known exception.
938+
*
939+
* MongoDB does not support nested transactions or savepoints, so a duplicate
940+
* key error inside an inner transaction aborts the entire transaction.
941+
*/
942+
public function testNestedTransactionState(): void
943+
{
944+
/** @var Database $database */
945+
$database = $this->getDatabase();
946+
947+
if (!$database->getAdapter()->getSupportForNestedTransactions()) {
948+
$this->expectNotToPerformAssertions();
949+
return;
950+
}
951+
952+
$database->createCollection('txNested');
953+
$database->createAttribute('txNested', 'title', Database::VAR_STRING, 128, true);
954+
955+
$database->createDocument('txNested', new Document([
956+
'$id' => 'nested_existing',
957+
'$permissions' => [
958+
Permission::read(Role::any()),
959+
],
960+
'title' => 'Original',
961+
]));
962+
963+
// Outer transaction should succeed even if inner transaction throws
964+
$result = $database->withTransaction(function () use ($database) {
965+
$database->createDocument('txNested', new Document([
966+
'$id' => 'outer_doc',
967+
'$permissions' => [
968+
Permission::read(Role::any()),
969+
],
970+
'title' => 'Outer',
971+
]));
972+
973+
// Inner transaction throws a DuplicateException
974+
try {
975+
$database->withTransaction(function () use ($database) {
976+
$database->createDocument('txNested', new Document([
977+
'$id' => 'nested_existing',
978+
'$permissions' => [
979+
Permission::read(Role::any()),
980+
],
981+
'title' => 'Duplicate',
982+
]));
983+
});
984+
} catch (DuplicateException $e) {
985+
// Caught and handled — outer transaction should continue
986+
}
987+
988+
return true;
989+
});
990+
991+
$this->assertTrue($result);
992+
993+
// inTransaction must be false after everything completes
994+
$this->assertFalse(
995+
$database->getAdapter()->inTransaction(),
996+
'Adapter should not be in transaction after nested transactions complete'
997+
);
998+
999+
// Outer document should have been committed
1000+
$outerDoc = $database->getDocument('txNested', 'outer_doc');
1001+
$this->assertFalse($outerDoc->isEmpty(), 'Outer transaction document should exist');
1002+
$this->assertEquals('Outer', $outerDoc->getAttribute('title'));
1003+
1004+
// Original document should be unchanged
1005+
$existingDoc = $database->getDocument('txNested', 'nested_existing');
1006+
$this->assertEquals('Original', $existingDoc->getAttribute('title'));
1007+
1008+
$database->deleteCollection('txNested');
1009+
}
1010+
8481011
/**
8491012
* Wait for Redis to be ready with a readiness probe
8501013
*/

0 commit comments

Comments
 (0)