diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index b2d115b52..055d25cf3 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -927,7 +927,29 @@ public function createDocument(Document $collection, Document $document): Docume } if (isset($stmtPermissions)) { - $stmtPermissions->execute(); + try { + $stmtPermissions->execute(); + } catch (PDOException $e) { + $isOrphanedPermission = $e->getCode() === '23000' + && isset($e->errorInfo[1]) + && $e->errorInfo[1] === 1062 + && \str_contains($e->getMessage(), '_index1'); + + if (!$isOrphanedPermission) { + throw $e; + } + + // Clean up orphaned permissions from a previous failed delete, then retry + $sql = "DELETE FROM {$this->getSQLTable($name . '_perms')} WHERE _document = :_uid {$this->getTenantQuery($collection)}"; + $cleanup = $this->getPDO()->prepare($sql); + $cleanup->bindValue(':_uid', $document->getId()); + if ($this->sharedTables) { + $cleanup->bindValue(':_tenant', $document->getTenant()); + } + $cleanup->execute(); + + $stmtPermissions->execute(); + } } } catch (PDOException $e) { throw $this->processException($e); @@ -1883,6 +1905,13 @@ protected function processException(PDOException $e): \Exception // Duplicate row if ($e->getCode() === '23000' && isset($e->errorInfo[1]) && $e->errorInfo[1] === 1062) { + $message = $e->getMessage(); + if (\str_contains($message, '_index1')) { + return new DuplicateException('Duplicate permissions for document', $e->getCode(), $e); + } + if (!\str_contains($message, '_uid')) { + return new DuplicateException('Document with the requested unique attributes already exists', $e->getCode(), $e); + } return new DuplicateException('Document already exists', $e->getCode(), $e); } diff --git a/src/Database/Adapter/Mongo.php b/src/Database/Adapter/Mongo.php index fbff1d3c4..679bf559d 100644 --- a/src/Database/Adapter/Mongo.php +++ b/src/Database/Adapter/Mongo.php @@ -3483,12 +3483,11 @@ protected function processException(\Throwable $e): \Throwable } // Duplicate key error - if ($e->getCode() === 11000) { - return new DuplicateException('Document already exists', $e->getCode(), $e); - } - - // Duplicate key error for unique index - if ($e->getCode() === 11001) { + if ($e->getCode() === 11000 || $e->getCode() === 11001) { + $message = $e->getMessage(); + if (!\str_contains($message, '_uid')) { + return new DuplicateException('Document with the requested unique attributes already exists', $e->getCode(), $e); + } return new DuplicateException('Document already exists', $e->getCode(), $e); } diff --git a/src/Database/Adapter/Pool.php b/src/Database/Adapter/Pool.php index ddf1db037..63d3d9a0b 100644 --- a/src/Database/Adapter/Pool.php +++ b/src/Database/Adapter/Pool.php @@ -16,6 +16,12 @@ class Pool extends Adapter */ protected UtopiaPool $pool; + /** + * When a transaction is active, all delegate calls are routed through + * this pinned adapter to ensure they run on the same connection. + */ + protected ?Adapter $pinnedAdapter = null; + /** * @param UtopiaPool $pool The pool to use for connections. Must contain instances of Adapter. */ @@ -36,6 +42,10 @@ public function __construct(UtopiaPool $pool) */ public function delegate(string $method, array $args): mixed { + if ($this->pinnedAdapter !== null) { + return $this->pinnedAdapter->{$method}(...$args); + } + return $this->pool->use(function (Adapter $adapter) use ($method, $args) { // Run setters in case config changed since this connection was last used $adapter->setDatabase($this->getDatabase()); @@ -92,6 +102,52 @@ public function rollbackTransaction(): bool return $this->delegate(__FUNCTION__, \func_get_args()); } + /** + * Pin a single connection from the pool for the entire transaction lifecycle. + * This prevents startTransaction(), the callback, and commitTransaction() + * from running on different connections. + * + * @template T + * @param callable(): T $callback + * @return T + * @throws \Throwable + */ + public function withTransaction(callable $callback): mixed + { + // If already inside a transaction, reuse the pinned adapter + // so nested withTransaction calls use the same connection + if ($this->pinnedAdapter !== null) { + return $this->pinnedAdapter->withTransaction($callback); + } + + return $this->pool->use(function (Adapter $adapter) use ($callback) { + $adapter->setDatabase($this->getDatabase()); + $adapter->setNamespace($this->getNamespace()); + $adapter->setSharedTables($this->getSharedTables()); + $adapter->setTenant($this->getTenant()); + $adapter->setAuthorization($this->authorization); + + if ($this->getTimeout() > 0) { + $adapter->setTimeout($this->getTimeout()); + } + $adapter->resetDebug(); + foreach ($this->getDebug() as $key => $value) { + $adapter->setDebug($key, $value); + } + $adapter->resetMetadata(); + foreach ($this->getMetadata() as $key => $value) { + $adapter->setMetadata($key, $value); + } + + $this->pinnedAdapter = $adapter; + try { + return $adapter->withTransaction($callback); + } finally { + $this->pinnedAdapter = null; + } + }); + } + protected function quote(string $string): string { return $this->delegate(__FUNCTION__, \func_get_args()); diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index a026084c5..a4d304a59 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -2189,6 +2189,10 @@ protected function processException(PDOException $e): \Exception // Duplicate row if ($e->getCode() === '23505' && isset($e->errorInfo[1]) && $e->errorInfo[1] === 7) { + $message = $e->getMessage(); + if (!\str_contains($message, '_uid')) { + return new DuplicateException('Document with the requested unique attributes already exists', $e->getCode(), $e); + } return new DuplicateException('Document already exists', $e->getCode(), $e); } diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index 0c0e88975..12f2406f4 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -1328,6 +1328,9 @@ protected function processException(PDOException $e): \Exception stripos($message, 'unique') !== false || stripos($message, 'duplicate') !== false ) { + if (!\str_contains($message, '_uid')) { + return new DuplicateException('Document with the requested unique attributes already exists', $e->getCode(), $e); + } return new DuplicateException('Document already exists', $e->getCode(), $e); } } diff --git a/tests/e2e/Adapter/PoolTest.php b/tests/e2e/Adapter/PoolTest.php index 8f5f2bd3a..94c2d4147 100644 --- a/tests/e2e/Adapter/PoolTest.php +++ b/tests/e2e/Adapter/PoolTest.php @@ -10,9 +10,12 @@ use Utopia\Database\Adapter\MySQL; use Utopia\Database\Adapter\Pool; use Utopia\Database\Database; +use Utopia\Database\Document; use Utopia\Database\Exception; use Utopia\Database\Exception\Duplicate; use Utopia\Database\Exception\Limit; +use Utopia\Database\Helpers\Permission; +use Utopia\Database\Helpers\Role; use Utopia\Database\PDO; use Utopia\Pools\Adapter\Stack; use Utopia\Pools\Pool as UtopiaPool; @@ -109,4 +112,89 @@ protected function deleteIndex(string $collection, string $index): bool return true; } + + /** + * Execute raw SQL via the pool using reflection to access the adapter's PDO. + * + * @param string $sql + * @param array $binds + */ + private function execRawSQL(string $sql, array $binds = []): void + { + self::$pool->use(function (Adapter $adapter) use ($sql, $binds) { + $class = new ReflectionClass($adapter); + $property = $class->getProperty('pdo'); + $property->setAccessible(true); + $pdo = $property->getValue($adapter); + $stmt = $pdo->prepare($sql); + foreach ($binds as $key => $value) { + $stmt->bindValue($key, $value); + } + $stmt->execute(); + }); + } + + /** + * Test that orphaned permission records from a previous failed delete + * don't block document recreation. The createDocument method should + * clean up orphaned perms and retry. + */ + public function testOrphanedPermissionsRecovery(): void + { + $database = $this->getDatabase(); + $collection = 'orphanedPermsRecovery'; + + $database->createCollection($collection); + $database->createAttribute($collection, 'title', Database::VAR_STRING, 128, true); + + // Step 1: Create a document with permissions + $doc = $database->createDocument($collection, new Document([ + '$id' => 'orphan_test', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'title' => 'Original', + ])); + $this->assertEquals('orphan_test', $doc->getId()); + + // Step 2: Delete the document normally (cleans up both doc and perms) + $database->deleteDocument($collection, 'orphan_test'); + $deleted = $database->getDocument($collection, 'orphan_test'); + $this->assertTrue($deleted->isEmpty()); + + // Step 3: Manually re-insert orphaned permission rows (simulating a partial delete failure) + $namespace = $this->getDatabase()->getNamespace(); + $dbName = $this->getDatabase()->getDatabase(); + $permsTable = "`{$dbName}`.`{$namespace}_{$collection}_perms`"; + + $this->execRawSQL( + "INSERT INTO {$permsTable} (_type, _permission, _document) VALUES (:type, :perm, :doc)", + [':type' => 'read', ':perm' => 'any', ':doc' => 'orphan_test'] + ); + $this->execRawSQL( + "INSERT INTO {$permsTable} (_type, _permission, _document) VALUES (:type, :perm, :doc)", + [':type' => 'update', ':perm' => 'any', ':doc' => 'orphan_test'] + ); + + // Step 4: Recreate a document with the same ID - should succeed by cleaning up orphaned perms + $newDoc = $database->createDocument($collection, new Document([ + '$id' => 'orphan_test', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + ], + 'title' => 'Recreated', + ])); + $this->assertEquals('orphan_test', $newDoc->getId()); + $this->assertEquals('Recreated', $newDoc->getAttribute('title')); + + // Verify the document can be fetched + $found = $database->getDocument($collection, 'orphan_test'); + $this->assertFalse($found->isEmpty()); + $this->assertEquals('Recreated', $found->getAttribute('title')); + + $database->deleteCollection($collection); + } } diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index f3d9d937e..d1241ad26 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -5007,6 +5007,64 @@ public function testUniqueIndexDuplicate(): void $this->assertInstanceOf(DuplicateException::class, $e); } } + + /** + * Test that DuplicateException messages differentiate between + * document ID duplicates and unique index violations. + */ + public function testDuplicateExceptionMessages(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + if (!$database->getAdapter()->getSupportForUniqueIndex()) { + $this->expectNotToPerformAssertions(); + return; + } + + $database->createCollection('duplicateMessages'); + $database->createAttribute('duplicateMessages', 'email', Database::VAR_STRING, 128, true); + $database->createIndex('duplicateMessages', 'emailUnique', Database::INDEX_UNIQUE, ['email'], [128]); + + // Create first document + $database->createDocument('duplicateMessages', new Document([ + '$id' => 'dup_msg_1', + '$permissions' => [ + Permission::read(Role::any()), + ], + 'email' => 'test@example.com', + ])); + + // Test 1: Duplicate document ID should say "Document already exists" + try { + $database->createDocument('duplicateMessages', new Document([ + '$id' => 'dup_msg_1', + '$permissions' => [ + Permission::read(Role::any()), + ], + 'email' => 'different@example.com', + ])); + $this->fail('Expected DuplicateException for duplicate document ID'); + } catch (DuplicateException $e) { + $this->assertStringContainsString('Document already exists', $e->getMessage()); + } + + // Test 2: Unique index violation should mention "unique attributes" + try { + $database->createDocument('duplicateMessages', new Document([ + '$id' => 'dup_msg_2', + '$permissions' => [ + Permission::read(Role::any()), + ], + 'email' => 'test@example.com', + ])); + $this->fail('Expected DuplicateException for unique index violation'); + } catch (DuplicateException $e) { + $this->assertStringContainsString('unique attributes', $e->getMessage()); + } + + $database->deleteCollection('duplicateMessages'); + } /** * @depends testUniqueIndexDuplicate */ diff --git a/tests/e2e/Adapter/Scopes/GeneralTests.php b/tests/e2e/Adapter/Scopes/GeneralTests.php index 9ac5ccd35..d2f18772c 100644 --- a/tests/e2e/Adapter/Scopes/GeneralTests.php +++ b/tests/e2e/Adapter/Scopes/GeneralTests.php @@ -793,6 +793,58 @@ public function testCacheReconnect(): void } } + /** + * Test that withTransaction properly rolls back on failure. + * With the Pool adapter, this verifies that the entire transaction + * (start, callback, commit/rollback) runs on a single pinned connection. + */ + public function testTransactionAtomicity(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + $database->createCollection('transactionAtomicity'); + $database->createAttribute('transactionAtomicity', 'title', Database::VAR_STRING, 128, true); + + // Verify a successful transaction commits + $doc = $database->withTransaction(function () use ($database) { + return $database->createDocument('transactionAtomicity', new Document([ + '$id' => 'tx_success', + '$permissions' => [ + Permission::read(Role::any()), + ], + 'title' => 'Committed', + ])); + }); + $this->assertEquals('tx_success', $doc->getId()); + $found = $database->getDocument('transactionAtomicity', 'tx_success'); + $this->assertFalse($found->isEmpty()); + $this->assertEquals('Committed', $found->getAttribute('title')); + + // Verify a failed transaction rolls back completely + try { + $database->withTransaction(function () use ($database) { + $database->createDocument('transactionAtomicity', new Document([ + '$id' => 'tx_fail', + '$permissions' => [ + Permission::read(Role::any()), + ], + 'title' => 'Should be rolled back', + ])); + + throw new \Exception('Intentional failure to trigger rollback'); + }); + } catch (\Exception $e) { + $this->assertEquals('Intentional failure to trigger rollback', $e->getMessage()); + } + + // Document should NOT exist since the transaction was rolled back + $notFound = $database->getDocument('transactionAtomicity', 'tx_fail'); + $this->assertTrue($notFound->isEmpty(), 'Document should not exist after transaction rollback'); + + $database->deleteCollection('transactionAtomicity'); + } + /** * Wait for Redis to be ready with a readiness probe */