Merge pull request #622 from utopia-php/dat-569

abnegate · web-flow · commit f7a26a2189a8 · 2025-07-24T01:25:32.000+12:00
Add error handling callbacks for batch document operations
diff --git a/src/Database/Database.php b/src/Database/Database.php
@@ -5734,6 +5734,7 @@ private function deleteCascade(Document $collection, Document $relatedCollection
      * @param array<Query> $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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/tests/e2e/Adapter/Scopes/Relationships/OneToOneTests.php b/tests/e2e/Adapter/Scopes/Relationships/OneToOneTests.php
