Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions src/Database/Adapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,6 @@ public function withTransaction(callable $callback): mixed
$action instanceof ConflictException ||
$action instanceof LimitException
) {
$this->inTransaction = 0;
throw $action;
}

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

$this->inTransaction = 0;
throw $action;
}
}
Expand Down Expand Up @@ -1531,4 +1529,18 @@ public function getSupportForTTLIndexes(): bool
{
return false;
}

/**
* Does the adapter support transaction retries?
*
* @return bool
*/
abstract public function getSupportForTransactionRetries(): bool;

/**
* Does the adapter support nested transactions?
*
* @return bool
*/
abstract public function getSupportForNestedTransactions(): bool;
}
19 changes: 17 additions & 2 deletions src/Database/Adapter/Mongo.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,13 @@ public function withTransaction(callable $callback): mixed
{
// If the database is not a replica set, we can't use transactions
if (!$this->client->isReplicaSet()) {
$result = $callback();
return $result;
return $callback();
}

// MongoDB doesn't support nested transactions/savepoints.
// If already in a transaction, just run the callback directly.
if ($this->inTransaction > 0) {
return $callback();
}

try {
Expand Down Expand Up @@ -3687,6 +3692,16 @@ public function getSupportForTTLIndexes(): bool
return true;
}

public function getSupportForTransactionRetries(): bool
{
return false;
}

public function getSupportForNestedTransactions(): bool
{
return false;
}

protected function isExtendedISODatetime(string $val): bool
{
/**
Expand Down
10 changes: 10 additions & 0 deletions src/Database/Adapter/Pool.php
Original file line number Diff line number Diff line change
Expand Up @@ -703,4 +703,14 @@ public function getSupportForTTLIndexes(): bool
{
return $this->delegate(__FUNCTION__, \func_get_args());
}

public function getSupportForTransactionRetries(): bool
{
return $this->delegate(__FUNCTION__, \func_get_args());
}

public function getSupportForNestedTransactions(): bool
{
return $this->delegate(__FUNCTION__, \func_get_args());
}
}
8 changes: 8 additions & 0 deletions src/Database/Adapter/Postgres.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public function startTransaction(): bool

$result = $this->getPDO()->beginTransaction();
} else {
$this->getPDO()->exec('SAVEPOINT transaction' . $this->inTransaction);
$result = true;
}
} catch (PDOException $e) {
Expand All @@ -72,9 +73,16 @@ public function rollbackTransaction(): bool
}

try {
if ($this->inTransaction > 1) {
$this->getPDO()->exec('ROLLBACK TO transaction' . ($this->inTransaction - 1));
$this->inTransaction--;
return true;
}

$result = $this->getPDO()->rollBack();
$this->inTransaction = 0;
} catch (PDOException $e) {
$this->inTransaction = 0;
throw new DatabaseException('Failed to rollback transaction: ' . $e->getMessage(), $e->getCode(), $e);
}

Expand Down
10 changes: 10 additions & 0 deletions src/Database/Adapter/SQL.php
Original file line number Diff line number Diff line change
Expand Up @@ -3577,4 +3577,14 @@ public function getLockType(): string

return '';
}

public function getSupportForTransactionRetries(): bool
{
return true;
}

public function getSupportForNestedTransactions(): bool
{
return true;
}
Comment on lines 3580 to 3589
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix trailing whitespace on line 3585 to resolve the Pint lint failure.

The CI pipeline reports a no_whitespace_in_blank_line PSR-12 violation. Line 3585 has trailing whitespace on the blank line between the two methods.

🧹 Fix trailing whitespace
     public function getSupportForTransactionRetries(): bool
     {
         return true;
     }
-    
+
     public function getSupportForNestedTransactions(): bool
     {
         return true;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function getSupportForTransactionRetries(): bool
{
return true;
}
public function getSupportForNestedTransactions(): bool
{
return true;
}
public function getSupportForTransactionRetries(): bool
{
return true;
}
public function getSupportForNestedTransactions(): bool
{
return true;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Database/Adapter/SQL.php` around lines 3580 - 3589, Remove the trailing
whitespace on the blank line between the methods
getSupportForTransactionRetries() and getSupportForNestedTransactions() in class
SQL (the blank line after getSupportForTransactionRetries returns) so the file
no longer triggers the PSR-12 `no_whitespace_in_blank_line` Pint lint error;
after removing the trailing spaces, save and re-run the linter to confirm the
violation is resolved.

}
163 changes: 163 additions & 0 deletions tests/e2e/Adapter/Scopes/GeneralTests.php
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,169 @@ public function testTransactionAtomicity(): void
$database->deleteCollection('transactionAtomicity');
}

/**
* Test that withTransaction correctly resets inTransaction state
* when a known exception (DuplicateException) is thrown after successful rollback.
*/
public function testTransactionStateAfterKnownException(): void
{
/** @var Database $database */
$database = $this->getDatabase();

$database->createCollection('txKnownException');
$database->createAttribute('txKnownException', 'title', Database::VAR_STRING, 128, true);

$database->createDocument('txKnownException', new Document([
'$id' => 'existing_doc',
'$permissions' => [
Permission::read(Role::any()),
],
'title' => 'Original',
]));

// Trigger a DuplicateException inside withTransaction by inserting a duplicate ID
try {
$database->withTransaction(function () use ($database) {
$database->createDocument('txKnownException', new Document([
'$id' => 'existing_doc',
'$permissions' => [
Permission::read(Role::any()),
],
'title' => 'Duplicate',
]));
});
$this->fail('Expected DuplicateException was not thrown');
} catch (DuplicateException $e) {
// Expected
}

// inTransaction must be false after the exception
$this->assertFalse(
$database->getAdapter()->inTransaction(),
'Adapter should not be in transaction after DuplicateException'
);

// Database should still be functional
$doc = $database->getDocument('txKnownException', 'existing_doc');
$this->assertEquals('Original', $doc->getAttribute('title'));

$database->deleteCollection('txKnownException');
}

/**
* Test that withTransaction correctly resets inTransaction state
* when retries are exhausted for a generic exception.
*
* MongoDB's withTransaction has no retry logic, so this test
* only applies to SQL-based adapters.
*/
public function testTransactionStateAfterRetriesExhausted(): void
{
/** @var Database $database */
$database = $this->getDatabase();

if (!$database->getAdapter()->getSupportForTransactionRetries()) {
$this->expectNotToPerformAssertions();
return;
}

$attempts = 0;

try {
$database->withTransaction(function () use (&$attempts) {
$attempts++;
throw new \RuntimeException('Persistent failure');
});
} catch (\RuntimeException $e) {
$this->assertEquals('Persistent failure', $e->getMessage());
}

// Should have attempted 3 times (initial + 2 retries)
$this->assertEquals(3, $attempts, 'Should have exhausted all retry attempts');

// inTransaction must be false after retries exhausted
$this->assertFalse(
$database->getAdapter()->inTransaction(),
'Adapter should not be in transaction after retries exhausted'
);
}

/**
* Test that nested withTransaction calls maintain correct inTransaction state
* when the inner transaction throws a known exception.
*
* MongoDB does not support nested transactions or savepoints, so a duplicate
* key error inside an inner transaction aborts the entire transaction.
*/
public function testNestedTransactionState(): void
{
/** @var Database $database */
$database = $this->getDatabase();

if (!$database->getAdapter()->getSupportForNestedTransactions()) {
$this->expectNotToPerformAssertions();
return;
}

$database->createCollection('txNested');
$database->createAttribute('txNested', 'title', Database::VAR_STRING, 128, true);

$database->createDocument('txNested', new Document([
'$id' => 'nested_existing',
'$permissions' => [
Permission::read(Role::any()),
],
'title' => 'Original',
]));

// Outer transaction should succeed even if inner transaction throws
$result = $database->withTransaction(function () use ($database) {
$database->createDocument('txNested', new Document([
'$id' => 'outer_doc',
'$permissions' => [
Permission::read(Role::any()),
],
'title' => 'Outer',
]));

// Inner transaction throws a DuplicateException
try {
$database->withTransaction(function () use ($database) {
$database->createDocument('txNested', new Document([
'$id' => 'nested_existing',
'$permissions' => [
Permission::read(Role::any()),
],
'title' => 'Duplicate',
]));
});
} catch (DuplicateException $e) {
// Caught and handled — outer transaction should continue
}

return true;
});

$this->assertTrue($result);

// inTransaction must be false after everything completes
$this->assertFalse(
$database->getAdapter()->inTransaction(),
'Adapter should not be in transaction after nested transactions complete'
);

// Outer document should have been committed
$outerDoc = $database->getDocument('txNested', 'outer_doc');
$this->assertFalse($outerDoc->isEmpty(), 'Outer transaction document should exist');
$this->assertEquals('Outer', $outerDoc->getAttribute('title'));

// Original document should be unchanged
$existingDoc = $database->getDocument('txNested', 'nested_existing');
$this->assertEquals('Original', $existingDoc->getAttribute('title'));

$database->deleteCollection('txNested');
}

/**
* Wait for Redis to be ready with a readiness probe
*/
Expand Down