Skip to content

Commit 6aac2eb

Browse files
committed
Remove redundant inTransaction reset after successful rollback
1 parent 5e49f32 commit 6aac2eb

File tree

2 files changed

+148
-2
lines changed

2 files changed

+148
-2
lines changed

src/Database/Adapter.php

Lines changed: 0 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
}

tests/e2e/Adapter/Scopes/GeneralTests.php

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,154 @@ 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+
public function testTransactionStateAfterRetriesExhausted(): void
902+
{
903+
/** @var Database $database */
904+
$database = $this->getDatabase();
905+
906+
$attempts = 0;
907+
908+
try {
909+
$database->withTransaction(function () use (&$attempts) {
910+
$attempts++;
911+
throw new \RuntimeException('Persistent failure');
912+
});
913+
$this->fail('Expected RuntimeException was not thrown');
914+
} catch (\RuntimeException $e) {
915+
$this->assertEquals('Persistent failure', $e->getMessage());
916+
}
917+
918+
// Should have attempted 3 times (initial + 2 retries)
919+
$this->assertEquals(3, $attempts, 'Should have exhausted all retry attempts');
920+
921+
// inTransaction must be false after retries exhausted
922+
$this->assertFalse(
923+
$database->getAdapter()->inTransaction(),
924+
'Adapter should not be in transaction after retries exhausted'
925+
);
926+
}
927+
928+
/**
929+
* Test that nested withTransaction calls maintain correct inTransaction state
930+
* when the inner transaction throws a known exception.
931+
*/
932+
public function testNestedTransactionState(): void
933+
{
934+
/** @var Database $database */
935+
$database = $this->getDatabase();
936+
937+
$database->createCollection('txNested');
938+
$database->createAttribute('txNested', 'title', Database::VAR_STRING, 128, true);
939+
940+
$database->createDocument('txNested', new Document([
941+
'$id' => 'nested_existing',
942+
'$permissions' => [
943+
Permission::read(Role::any()),
944+
],
945+
'title' => 'Original',
946+
]));
947+
948+
// Outer transaction should succeed even if inner transaction throws
949+
$result = $database->withTransaction(function () use ($database) {
950+
$database->createDocument('txNested', new Document([
951+
'$id' => 'outer_doc',
952+
'$permissions' => [
953+
Permission::read(Role::any()),
954+
],
955+
'title' => 'Outer',
956+
]));
957+
958+
// Inner transaction throws a DuplicateException
959+
try {
960+
$database->withTransaction(function () use ($database) {
961+
$database->createDocument('txNested', new Document([
962+
'$id' => 'nested_existing',
963+
'$permissions' => [
964+
Permission::read(Role::any()),
965+
],
966+
'title' => 'Duplicate',
967+
]));
968+
});
969+
} catch (DuplicateException $e) {
970+
// Caught and handled — outer transaction should continue
971+
}
972+
973+
return true;
974+
});
975+
976+
$this->assertTrue($result);
977+
978+
// inTransaction must be false after everything completes
979+
$this->assertFalse(
980+
$database->getAdapter()->inTransaction(),
981+
'Adapter should not be in transaction after nested transactions complete'
982+
);
983+
984+
// Outer document should have been committed
985+
$outerDoc = $database->getDocument('txNested', 'outer_doc');
986+
$this->assertFalse($outerDoc->isEmpty(), 'Outer transaction document should exist');
987+
$this->assertEquals('Outer', $outerDoc->getAttribute('title'));
988+
989+
// Original document should be unchanged
990+
$existingDoc = $database->getDocument('txNested', 'nested_existing');
991+
$this->assertEquals('Original', $existingDoc->getAttribute('title'));
992+
993+
$database->deleteCollection('txNested');
994+
}
995+
848996
/**
849997
* Wait for Redis to be ready with a readiness probe
850998
*/

0 commit comments

Comments
 (0)