diff --git a/src/Database/Database.php b/src/Database/Database.php index e65b2c013..2f058640d 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5734,6 +5734,7 @@ private function deleteCascade(Document $collection, Document $relatedCollection * @param array $queries * @param int $batchSize * @param callable|null $onNext + * @param callable|null $onError * @return int * @throws AuthorizationException * @throws DatabaseException @@ -5745,6 +5746,7 @@ public function deleteDocuments( array $queries = [], int $batchSize = self::DELETE_BATCH_SIZE, ?callable $onNext = null, + ?callable $onError = null, ): int { if ($this->adapter->getSharedTables() && empty($this->adapter->getTenant())) { throw new DatabaseException('Missing tenant. Tenant must be set when table sharing is enabled.'); @@ -5825,32 +5827,33 @@ public function deleteDocuments( $sequences = []; $permissionIds = []; - foreach ($batch as $document) { - $sequences[] = $document->getSequence(); - if (!empty($document->getPermissions())) { - $permissionIds[] = $document->getId(); - } - if ($this->resolveRelationships) { - $document = $this->silent(fn () => $this->deleteDocumentRelationships( - $collection, - $document - )); - } + $this->withTransaction(function () use ($collection, $sequences, $permissionIds, $batch) { + foreach ($batch as $document) { + $sequences[] = $document->getSequence(); + if (!empty($document->getPermissions())) { + $permissionIds[] = $document->getId(); + } - // Check if document was updated after the request timestamp - try { - $oldUpdatedAt = new \DateTime($document->getUpdatedAt()); - } catch (Exception $e) { - throw new DatabaseException($e->getMessage(), $e->getCode(), $e); - } + if ($this->resolveRelationships) { + $document = $this->silent(fn () => $this->deleteDocumentRelationships( + $collection, + $document + )); + } - if (!\is_null($this->timestamp) && $oldUpdatedAt > $this->timestamp) { - throw new ConflictException('Document was updated after the request timestamp'); + // Check if document was updated after the request timestamp + try { + $oldUpdatedAt = new \DateTime($document->getUpdatedAt()); + } catch (Exception $e) { + throw new DatabaseException($e->getMessage(), $e->getCode(), $e); + } + + if (!\is_null($this->timestamp) && $oldUpdatedAt > $this->timestamp) { + throw new ConflictException('Document was updated after the request timestamp'); + } } - } - $this->withTransaction(function () use ($collection, $sequences, $permissionIds) { $this->adapter->deleteDocuments( $collection->getId(), $sequences, @@ -5866,8 +5869,11 @@ public function deleteDocuments( } else { $this->purgeCachedDocument($collection->getId(), $document->getId()); } - - $onNext && $onNext($document); + try { + $onNext && $onNext($document); + } catch (Throwable $th) { + $onError ? $onError($th) : throw $th; + } $modified++; } diff --git a/src/Database/Mirror.php b/src/Database/Mirror.php index 7671f26cc..5934cba03 100644 --- a/src/Database/Mirror.php +++ b/src/Database/Mirror.php @@ -829,6 +829,7 @@ public function deleteDocuments( array $queries = [], int $batchSize = self::DELETE_BATCH_SIZE, ?callable $onNext = null, + ?callable $onError = null, ): int { $modified = 0; @@ -840,6 +841,7 @@ function ($doc) use (&$modified, $onNext) { $onNext && $onNext($doc); $modified++; }, + $onError ); if ( diff --git a/tests/e2e/Adapter/Scopes/DocumentTests.php b/tests/e2e/Adapter/Scopes/DocumentTests.php index 5a5016701..658ae0569 100644 --- a/tests/e2e/Adapter/Scopes/DocumentTests.php +++ b/tests/e2e/Adapter/Scopes/DocumentTests.php @@ -4214,6 +4214,130 @@ public function testDeleteBulkDocumentsQueries(): void $database->deleteCollection('bulk_delete_queries'); } + public function testDeleteBulkDocumentsWithCallbackSupport(): void + { + /** @var Database $database */ + $database = static::getDatabase(); + + if (!$database->getAdapter()->getSupportForBatchOperations()) { + $this->expectNotToPerformAssertions(); + return; + } + + $database->createCollection( + 'bulk_delete_with_callback', + attributes: [ + new Document([ + '$id' => 'text', + 'type' => Database::VAR_STRING, + 'size' => 100, + 'required' => true, + ]), + new Document([ + '$id' => 'integer', + 'type' => Database::VAR_INTEGER, + 'size' => 10, + 'required' => true, + ]) + ], + permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::delete(Role::any()) + ], + documentSecurity: false + ); + + $this->propagateBulkDocuments('bulk_delete_with_callback'); + + $docs = $database->find('bulk_delete_with_callback'); + $this->assertCount(10, $docs); + + /** + * Test Short select query, test pagination as well, Add order to select + */ + $selects = ['$sequence', '$id', '$collection', '$permissions', '$updatedAt']; + + try { + // a non existent document to test the error thrown + $database->deleteDocuments( + collection: 'bulk_delete_with_callback', + queries: [ + Query::select([...$selects, '$createdAt']), + Query::lessThan('$createdAt', '1800-01-01'), + Query::orderAsc('$createdAt'), + Query::orderAsc(), + Query::limit(1), + ], + batchSize: 1, + onNext: function () { + throw new Exception("Error thrown to test that deletion doesn't stop and error is caught"); + } + ); + } catch (Exception $e) { + $this->assertInstanceOf(Exception::class, $e); + $this->assertEquals("Error thrown to test that deletion doesn't stop and error is caught", $e->getMessage()); + } + + $docs = $database->find('bulk_delete_with_callback'); + $this->assertCount(10, $docs); + + $count = $database->deleteDocuments( + collection: 'bulk_delete_with_callback', + queries: [ + Query::select([...$selects, '$createdAt']), + Query::cursorAfter($docs[6]), + Query::greaterThan('$createdAt', '2000-01-01'), + Query::orderAsc('$createdAt'), + Query::orderAsc(), + Query::limit(2), + ], + batchSize: 1, + onNext: function () { + // simulating error throwing but should not stop deletion + throw new Exception("Error thrown to test that deletion doesn't stop and error is caught"); + }, + onError:function ($e) { + $this->assertInstanceOf(Exception::class, $e); + $this->assertEquals("Error thrown to test that deletion doesn't stop and error is caught", $e->getMessage()); + } + ); + + $this->assertEquals(2, $count); + + // TEST: Bulk Delete All Documents without passing callbacks + $this->assertEquals(8, $database->deleteDocuments('bulk_delete_with_callback')); + + $docs = $database->find('bulk_delete_with_callback'); + $this->assertCount(0, $docs); + + // TEST: Bulk delete documents with queries with callbacks + $this->propagateBulkDocuments('bulk_delete_with_callback'); + + $results = []; + $count = $database->deleteDocuments('bulk_delete_with_callback', [ + Query::greaterThanEqual('integer', 5) + ], onNext: function ($doc) use (&$results) { + $results[] = $doc; + throw new Exception("Error thrown to test that deletion doesn't stop and error is caught"); + }, onError:function ($e) { + $this->assertInstanceOf(Exception::class, $e); + $this->assertEquals("Error thrown to test that deletion doesn't stop and error is caught", $e->getMessage()); + }); + + $this->assertEquals(5, $count); + + foreach ($results as $document) { + $this->assertGreaterThanOrEqual(5, $document->getAttribute('integer')); + } + + $docs = $database->find('bulk_delete_with_callback'); + $this->assertEquals(5, \count($docs)); + + // Teardown + $database->deleteCollection('bulk_delete_with_callback'); + } + public function testUpdateDocumentsQueries(): void { /** @var Database $database */ diff --git a/tests/e2e/Adapter/Scopes/Relationships/ManyToManyTests.php b/tests/e2e/Adapter/Scopes/Relationships/ManyToManyTests.php index ebc43610f..faa025303 100644 --- a/tests/e2e/Adapter/Scopes/Relationships/ManyToManyTests.php +++ b/tests/e2e/Adapter/Scopes/Relationships/ManyToManyTests.php @@ -1681,4 +1681,63 @@ public function testUpdateParentAndChild_ManyToMany(): void } + public function testDeleteDocumentsRelationshipErrorDoesNotDeleteParent_ManyToMany(): void + { + /** @var Database $database */ + $database = static::getDatabase(); + + if (!$database->getAdapter()->getSupportForRelationships() || !$database->getAdapter()->getSupportForBatchOperations()) { + $this->expectNotToPerformAssertions(); + return; + } + + $parentCollection = 'parent_relationship_many_to_many'; + $childCollection = 'child_relationship_many_to_many'; + + $database->createCollection($parentCollection); + $database->createCollection($childCollection); + $database->createAttribute($parentCollection, 'name', Database::VAR_STRING, 255, true); + $database->createAttribute($childCollection, 'name', Database::VAR_STRING, 255, true); + + $database->createRelationship( + collection: $parentCollection, + relatedCollection: $childCollection, + type: Database::RELATION_MANY_TO_MANY, + onDelete: Database::RELATION_MUTATE_RESTRICT + ); + + $parent = $database->createDocument($parentCollection, new Document([ + '$id' => 'parent1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Parent 1', + $childCollection => [ + [ + '$id' => 'child1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Child 1', + ] + ] + ])); + + try { + $database->deleteDocuments($parentCollection, [Query::equal('$id', ['parent1'])]); + $this->fail('Expected exception was not thrown'); + } catch (RestrictedException $e) { + $this->assertEquals('Cannot delete document because it has at least one related document.', $e->getMessage()); + } + $parentDoc = $database->getDocument($parentCollection, 'parent1'); + $childDoc = $database->getDocument($childCollection, 'child1'); + $this->assertFalse($parentDoc->isEmpty(), 'Parent should not be deleted'); + $this->assertFalse($childDoc->isEmpty(), 'Child should not be deleted'); + $database->deleteCollection($parentCollection); + $database->deleteCollection($childCollection); + } } diff --git a/tests/e2e/Adapter/Scopes/Relationships/ManyToOneTests.php b/tests/e2e/Adapter/Scopes/Relationships/ManyToOneTests.php index 460c58fc1..73a1c7a6f 100644 --- a/tests/e2e/Adapter/Scopes/Relationships/ManyToOneTests.php +++ b/tests/e2e/Adapter/Scopes/Relationships/ManyToOneTests.php @@ -1759,4 +1759,64 @@ public function testUpdateParentAndChild_ManyToOne(): void $database->deleteCollection($parentCollection); $database->deleteCollection($childCollection); } + + public function testDeleteDocumentsRelationshipErrorDoesNotDeleteParent_ManyToOne(): void + { + /** @var Database $database */ + $database = static::getDatabase(); + + if (!$database->getAdapter()->getSupportForRelationships() || !$database->getAdapter()->getSupportForBatchOperations()) { + $this->expectNotToPerformAssertions(); + return; + } + + $parentCollection = 'parent_relationship_error_many_to_one'; + $childCollection = 'child_relationship_error_many_to_one'; + + $database->createCollection($parentCollection); + $database->createCollection($childCollection); + $database->createAttribute($parentCollection, 'name', Database::VAR_STRING, 255, true); + $database->createAttribute($childCollection, 'name', Database::VAR_STRING, 255, true); + + $database->createRelationship( + collection: $childCollection, + relatedCollection: $parentCollection, + type: Database::RELATION_MANY_TO_ONE, + onDelete: Database::RELATION_MUTATE_RESTRICT + ); + + $parent = $database->createDocument($parentCollection, new Document([ + '$id' => 'parent1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Parent 1', + ])); + + $child = $database->createDocument($childCollection, new Document([ + '$id' => 'child1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Child 1', + $parentCollection => 'parent1' + ])); + + try { + $database->deleteDocuments($parentCollection, [Query::equal('$id', ['parent1'])]); + $this->fail('Expected exception was not thrown'); + } catch (RestrictedException $e) { + $this->assertEquals('Cannot delete document because it has at least one related document.', $e->getMessage()); + } + $parentDoc = $database->getDocument($parentCollection, 'parent1'); + $childDoc = $database->getDocument($childCollection, 'child1'); + $this->assertFalse($parentDoc->isEmpty(), 'Parent should not be deleted'); + $this->assertFalse($childDoc->isEmpty(), 'Child should not be deleted'); + $database->deleteCollection($parentCollection); + $database->deleteCollection($childCollection); + } } diff --git a/tests/e2e/Adapter/Scopes/Relationships/OneToManyTests.php b/tests/e2e/Adapter/Scopes/Relationships/OneToManyTests.php index 1a3eb4440..b29bf2087 100644 --- a/tests/e2e/Adapter/Scopes/Relationships/OneToManyTests.php +++ b/tests/e2e/Adapter/Scopes/Relationships/OneToManyTests.php @@ -2155,4 +2155,63 @@ public function testUpdateParentAndChild_OneToMany(): void $database->deleteCollection($parentCollection); $database->deleteCollection($childCollection); } + public function testDeleteDocumentsRelationshipErrorDoesNotDeleteParent_OneToMany(): void + { + /** @var Database $database */ + $database = static::getDatabase(); + + if (!$database->getAdapter()->getSupportForRelationships() || !$database->getAdapter()->getSupportForBatchOperations()) { + $this->expectNotToPerformAssertions(); + return; + } + + $parentCollection = 'parent_relationship_error_one_to_many'; + $childCollection = 'child_relationship_error_one_to_many'; + + $database->createCollection($parentCollection); + $database->createCollection($childCollection); + $database->createAttribute($parentCollection, 'name', Database::VAR_STRING, 255, true); + $database->createAttribute($childCollection, 'name', Database::VAR_STRING, 255, true); + + $database->createRelationship( + collection: $parentCollection, + relatedCollection: $childCollection, + type: Database::RELATION_ONE_TO_MANY, + onDelete: Database::RELATION_MUTATE_RESTRICT + ); + + $parent = $database->createDocument($parentCollection, new Document([ + '$id' => 'parent1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Parent 1', + $childCollection => [ + [ + '$id' => 'child1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Child 1', + ] + ] + ])); + + try { + $database->deleteDocuments($parentCollection, [Query::equal('$id', ['parent1'])]); + $this->fail('Expected exception was not thrown'); + } catch (RestrictedException $e) { + $this->assertEquals('Cannot delete document because it has at least one related document.', $e->getMessage()); + } + $parentDoc = $database->getDocument($parentCollection, 'parent1'); + $childDoc = $database->getDocument($childCollection, 'child1'); + $this->assertFalse($parentDoc->isEmpty(), 'Parent should not be deleted'); + $this->assertFalse($childDoc->isEmpty(), 'Child should not be deleted'); + $database->deleteCollection($parentCollection); + $database->deleteCollection($childCollection); + } } diff --git a/tests/e2e/Adapter/Scopes/Relationships/OneToOneTests.php b/tests/e2e/Adapter/Scopes/Relationships/OneToOneTests.php index 7589e7a7e..52881707b 100644 --- a/tests/e2e/Adapter/Scopes/Relationships/OneToOneTests.php +++ b/tests/e2e/Adapter/Scopes/Relationships/OneToOneTests.php @@ -2371,4 +2371,62 @@ public function testUpdateParentAndChild_OneToOne(): void $database->deleteCollection($parentCollection); $database->deleteCollection($childCollection); } + + public function testDeleteDocumentsRelationshipErrorDoesNotDeleteParent_OneToOne(): void + { + /** @var Database $database */ + $database = static::getDatabase(); + + if (!$database->getAdapter()->getSupportForRelationships() || !$database->getAdapter()->getSupportForBatchOperations()) { + $this->expectNotToPerformAssertions(); + return; + } + + $parentCollection = 'parent_relationship_error_one_to_one'; + $childCollection = 'child_relationship_error_one_to_one'; + + $database->createCollection($parentCollection); + $database->createCollection($childCollection); + $database->createAttribute($parentCollection, 'name', Database::VAR_STRING, 255, true); + $database->createAttribute($childCollection, 'name', Database::VAR_STRING, 255, true); + + $database->createRelationship( + collection: $parentCollection, + relatedCollection: $childCollection, + type: Database::RELATION_ONE_TO_ONE, + onDelete: Database::RELATION_MUTATE_RESTRICT + ); + + $parent = $database->createDocument($parentCollection, new Document([ + '$id' => 'parent1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Parent 1', + $childCollection => [ + '$id' => 'child1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Child 1', + ] + ])); + + try { + $database->deleteDocuments($parentCollection, [Query::equal('$id', ['parent1'])]); + $this->fail('Expected exception was not thrown'); + } catch (RestrictedException $e) { + $this->assertEquals('Cannot delete document because it has at least one related document.', $e->getMessage()); + } + $parentDoc = $database->getDocument($parentCollection, 'parent1'); + $childDoc = $database->getDocument($childCollection, 'child1'); + $this->assertFalse($parentDoc->isEmpty(), 'Parent should not be deleted'); + $this->assertFalse($childDoc->isEmpty(), 'Child should not be deleted'); + $database->deleteCollection($parentCollection); + $database->deleteCollection($childCollection); + } }