From de160624b25fa906fd760df0b824d36f51363ece Mon Sep 17 00:00:00 2001 From: Claudear <262350598+claudear@users.noreply.github.com> Date: Wed, 11 Mar 2026 09:40:58 +0000 Subject: [PATCH] Fix: Handle duplicate relationship attribute gracefully in Databases worker When a queue message is retried and the relationship already exists in the database, the worker now catches the Duplicate exception and marks the attribute as 'available' instead of 'failed'. This prevents spurious Sentry errors (CLOUD-3JA4) from queue retries. Co-Authored-By: Claude Opus 4.6 --- .../Modules/Databases/Workers/Databases.php | 13 + .../Databases/Workers/DatabasesTest.php | 272 ++++++++++++++++++ 2 files changed, 285 insertions(+) create mode 100644 tests/unit/Platform/Modules/Databases/Workers/DatabasesTest.php diff --git a/src/Appwrite/Platform/Modules/Databases/Workers/Databases.php b/src/Appwrite/Platform/Modules/Databases/Workers/Databases.php index 9a98d77d2d..c4ea153fd1 100644 --- a/src/Appwrite/Platform/Modules/Databases/Workers/Databases.php +++ b/src/Appwrite/Platform/Modules/Databases/Workers/Databases.php @@ -10,6 +10,7 @@ use Utopia\Database\Document; use Utopia\Database\Exception as DatabaseException; use Utopia\Database\Exception\Authorization; use Utopia\Database\Exception\Conflict; +use Utopia\Database\Exception\Duplicate as DuplicateException; use Utopia\Database\Exception\NotFound; use Utopia\Database\Exception\Restricted; use Utopia\Database\Exception\Structure; @@ -187,6 +188,18 @@ class Databases extends Action } $dbForProject->updateDocument('attributes', $attribute->getId(), $attribute->setAttribute('status', 'available')); + } catch (DuplicateException) { + // Attribute/relationship already exists (e.g. queue retry), treat as success + Console::warning('Attribute already exists, marking as available'); + + $dbForProject->updateDocument('attributes', $attribute->getId(), $attribute->setAttribute('status', 'available')); + + if ($type === Database::VAR_RELATIONSHIP && $options['twoWay'] && !$relatedCollection->isEmpty()) { + $relatedAttribute = $dbForProject->getDocument('attributes', $database->getSequence() . '_' . $relatedCollection->getSequence() . '_' . $options['twoWayKey']); + if (!$relatedAttribute->isEmpty()) { + $dbForProject->updateDocument('attributes', $relatedAttribute->getId(), $relatedAttribute->setAttribute('status', 'available')); + } + } } catch (\Throwable $e) { Console::error($e->getMessage()); diff --git a/tests/unit/Platform/Modules/Databases/Workers/DatabasesTest.php b/tests/unit/Platform/Modules/Databases/Workers/DatabasesTest.php new file mode 100644 index 0000000000..bf8fe77273 --- /dev/null +++ b/tests/unit/Platform/Modules/Databases/Workers/DatabasesTest.php @@ -0,0 +1,272 @@ + $attributeId, + '$sequence' => $attributeId, + 'key' => 'testRelation', + 'type' => Database::VAR_RELATIONSHIP, + 'status' => 'processing', + 'size' => 0, + 'required' => false, + 'default' => null, + 'signed' => true, + 'array' => false, + 'format' => '', + 'formatOptions' => [], + 'filters' => [], + 'options' => [ + 'relatedCollection' => 'relatedCol', + 'relationType' => Database::RELATION_ONE_TO_ONE, + 'twoWay' => false, + 'twoWayKey' => 'reverse', + 'onDelete' => Database::RELATION_MUTATE_CASCADE, + ], + ]); + + $collection = new Document([ + '$id' => 'testCol', + '$sequence' => $collectionSequence, + ]); + + $relatedCollection = new Document([ + '$id' => 'relatedCol', + '$sequence' => $relatedCollectionSequence, + ]); + + $database = new Document([ + '$id' => 'testDb', + '$sequence' => $databaseSequence, + ]); + + $project = new Document([ + '$id' => 'testProject', + ]); + + // Mock dbForProject + $dbForProject = $this->createMock(Database::class); + + // getDocument calls + $dbForProject->method('getDocument') + ->willReturnCallback(function (string $collection, string $id) use ($attribute, $relatedCollection, $databaseSequence) { + if ($collection === 'attributes' && $id === $attribute->getId()) { + return $attribute; + } + if ($collection === 'database_' . $databaseSequence) { + return $relatedCollection; + } + return new Document(); + }); + + // createRelationship should throw DuplicateException (simulating a queue retry) + $dbForProject->method('createRelationship') + ->willThrowException(new DuplicateException('Related attribute already exists')); + + // Expect updateDocument to be called with status 'available' (NOT 'failed') + $dbForProject->expects($this->once()) + ->method('updateDocument') + ->with( + 'attributes', + $attribute->getId(), + $this->callback(function (Document $doc) { + return $doc->getAttribute('status') === 'available'; + }) + ) + ->willReturnArgument(2); + + $dbForProject->method('purgeCachedDocument')->willReturn(true); + $dbForProject->method('purgeCachedCollection')->willReturn(true); + + // Mock dbForPlatform + $dbForPlatform = $this->createMock(Database::class); + $dbForPlatform->method('getDocument') + ->willReturn($project); + + // Mock Realtime + $queueForRealtime = $this->createMock(Realtime::class); + $queueForRealtime->method('setProject')->willReturnSelf(); + $queueForRealtime->method('setSubscribers')->willReturnSelf(); + $queueForRealtime->method('setEvent')->willReturnSelf(); + $queueForRealtime->method('setParam')->willReturnSelf(); + $queueForRealtime->method('setPayload')->willReturnSelf(); + $queueForRealtime->method('trigger')->willReturn(true); + + // Mock Log + $log = $this->createMock(Log::class); + + // Mock Message + $message = $this->createMock(Message::class); + $message->method('getPayload')->willReturn([ + 'type' => DATABASE_TYPE_CREATE_ATTRIBUTE, + 'document' => $attribute->getArrayCopy(), + 'collection' => $collection->getArrayCopy(), + 'database' => $database->getArrayCopy(), + ]); + + // Execute the worker action - should NOT throw + $worker = new Databases(); + $worker->action($message, $project, $dbForPlatform, $dbForProject, $queueForRealtime, $log); + } + + /** + * Test that a Duplicate exception during two-way createRelationship + * sets both the primary and related attribute status to 'available'. + */ + public function testCreateTwoWayRelationshipDuplicateIsHandledGracefully(): void + { + $databaseSequence = '1'; + $collectionSequence = '2'; + $relatedCollectionSequence = '3'; + $attributeId = $databaseSequence . '_' . $collectionSequence . '_testRelation'; + $relatedAttributeId = $databaseSequence . '_' . $relatedCollectionSequence . '_reverse'; + + $attribute = new Document([ + '$id' => $attributeId, + '$sequence' => $attributeId, + 'key' => 'testRelation', + 'type' => Database::VAR_RELATIONSHIP, + 'status' => 'processing', + 'size' => 0, + 'required' => false, + 'default' => null, + 'signed' => true, + 'array' => false, + 'format' => '', + 'formatOptions' => [], + 'filters' => [], + 'options' => [ + 'relatedCollection' => 'relatedCol', + 'relationType' => Database::RELATION_ONE_TO_ONE, + 'twoWay' => true, + 'twoWayKey' => 'reverse', + 'onDelete' => Database::RELATION_MUTATE_CASCADE, + ], + ]); + + $relatedAttribute = new Document([ + '$id' => $relatedAttributeId, + '$sequence' => $relatedAttributeId, + 'key' => 'reverse', + 'type' => Database::VAR_RELATIONSHIP, + 'status' => 'processing', + ]); + + $collection = new Document([ + '$id' => 'testCol', + '$sequence' => $collectionSequence, + ]); + + $relatedCollection = new Document([ + '$id' => 'relatedCol', + '$sequence' => $relatedCollectionSequence, + ]); + + $database = new Document([ + '$id' => 'testDb', + '$sequence' => $databaseSequence, + ]); + + $project = new Document([ + '$id' => 'testProject', + ]); + + // Mock dbForProject + $dbForProject = $this->createMock(Database::class); + + $dbForProject->method('getDocument') + ->willReturnCallback(function (string $collection, string $id) use ($attribute, $relatedAttribute, $relatedCollection, $databaseSequence) { + if ($collection === 'attributes' && $id === $attribute->getId()) { + return $attribute; + } + if ($collection === 'attributes' && $id === $relatedAttribute->getId()) { + return $relatedAttribute; + } + if ($collection === 'database_' . $databaseSequence) { + return $relatedCollection; + } + return new Document(); + }); + + // createRelationship throws DuplicateException + $dbForProject->method('createRelationship') + ->willThrowException(new DuplicateException('Related attribute already exists')); + + // Expect updateDocument to be called twice - once for each attribute, both with 'available' status + $updateCalls = []; + $dbForProject->expects($this->exactly(2)) + ->method('updateDocument') + ->willReturnCallback(function (string $collection, string $id, Document $doc) use (&$updateCalls) { + $updateCalls[] = [ + 'collection' => $collection, + 'id' => $id, + 'status' => $doc->getAttribute('status'), + ]; + return $doc; + }); + + $dbForProject->method('purgeCachedDocument')->willReturn(true); + $dbForProject->method('purgeCachedCollection')->willReturn(true); + + // Mock dbForPlatform + $dbForPlatform = $this->createMock(Database::class); + $dbForPlatform->method('getDocument')->willReturn($project); + + // Mock Realtime + $queueForRealtime = $this->createMock(Realtime::class); + $queueForRealtime->method('setProject')->willReturnSelf(); + $queueForRealtime->method('setSubscribers')->willReturnSelf(); + $queueForRealtime->method('setEvent')->willReturnSelf(); + $queueForRealtime->method('setParam')->willReturnSelf(); + $queueForRealtime->method('setPayload')->willReturnSelf(); + $queueForRealtime->method('trigger')->willReturn(true); + + // Mock Log + $log = $this->createMock(Log::class); + + // Mock Message + $message = $this->createMock(Message::class); + $message->method('getPayload')->willReturn([ + 'type' => DATABASE_TYPE_CREATE_ATTRIBUTE, + 'document' => $attribute->getArrayCopy(), + 'collection' => $collection->getArrayCopy(), + 'database' => $database->getArrayCopy(), + ]); + + // Execute - should NOT throw + $worker = new Databases(); + $worker->action($message, $project, $dbForPlatform, $dbForProject, $queueForRealtime, $log); + + // Verify both attributes were marked as 'available' + $this->assertCount(2, $updateCalls); + foreach ($updateCalls as $call) { + $this->assertEquals('attributes', $call['collection']); + $this->assertEquals('available', $call['status']); + } + } +}