Merge pull request #539 from utopia-php/deleteDocumentSelect

Accumulate delete Documents outside transaction

Author: abnegate

src/Database/Database.php

@@ -5418,71 +5418,71 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba
             throw new DatabaseException("Cursor document must be from the same Collection.");
         }
 
-        $documents = $this->withTransaction(function () use ($collection, $queries, $batchSize, $limit, $cursor, $skipAuth, $authorization) {
-            $documents = [];
-            $originalLimit = $limit;
-            $lastDocument = $cursor;
-
-            while (true) {
-                if ($limit && $limit < $batchSize && $limit > 0) {
-                    $batchSize = $limit;
-                } elseif (!empty($limit)) {
-                    $limit -= $batchSize;
-                }
-
-                $new = [
-                    Query::limit($batchSize)
-                ];
-
-                if (! empty($lastDocument)) {
-                    $new[] = Query::cursorAfter($lastDocument);
-                }
+        $documents = [];
+        $originalLimit = $limit;
+        $lastDocument = $cursor;
+
+        while (true) {
+            if ($limit && $limit < $batchSize && $limit > 0) {
+                $batchSize = $limit;
+            } elseif (!empty($limit)) {
+                $limit -= $batchSize;
+            }
 
-                $affectedDocuments = $this->silent(fn () => $this->find(
-                    $collection->getId(),
-                    array_merge($new, $queries),
-                    forPermission: Database::PERMISSION_DELETE
-                ));
+            $new = [
+                Query::limit($batchSize)
+            ];
 
-                if (empty($affectedDocuments)) {
-                    break;
-                }
+            if (! empty($lastDocument)) {
+                $new[] = Query::cursorAfter($lastDocument);
+            }
 
-                $documents = \array_merge($affectedDocuments, $documents);
+            $affectedDocuments = $this->silent(fn () => $this->find(
+                $collection->getId(),
+                array_merge($new, $queries),
+                forPermission: Database::PERMISSION_DELETE
+            ));
 
-                foreach ($affectedDocuments as $document) {
-                    if ($this->resolveRelationships) {
-                        $document = $this->silent(fn () => $this->deleteDocumentRelationships(
-                            $collection,
-                            $document
-                        ));
-                    }
+            if (empty($affectedDocuments)) {
+                break;
+            }
 
-                    // 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);
-                    }
+            $documents = \array_merge($affectedDocuments, $documents);
 
-                    if (!\is_null($this->timestamp) && $oldUpdatedAt > $this->timestamp) {
-                        throw new ConflictException('Document was updated after the request timestamp');
-                    }
+            foreach ($affectedDocuments as $document) {
+                if ($this->resolveRelationships) {
+                    $document = $this->silent(fn () => $this->deleteDocumentRelationships(
+                        $collection,
+                        $document
+                    ));
                 }
 
-                if (count($affectedDocuments) < $batchSize) {
-                    break;
-                } elseif ($originalLimit && count($documents) == $originalLimit) {
-                    break;
+                // 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);
                 }
 
-                $lastDocument = end($affectedDocuments);
+                if (!\is_null($this->timestamp) && $oldUpdatedAt > $this->timestamp) {
+                    throw new ConflictException('Document was updated after the request timestamp');
+                }
             }
 
-            if (empty($documents)) {
-                return [];
+            if (count($affectedDocuments) < $batchSize) {
+                break;
+            } elseif ($originalLimit && count($documents) == $originalLimit) {
+                break;
             }
 
+            $lastDocument = end($affectedDocuments);
+        }
+
+        if (empty($documents)) {
+            return [];
+        }
+
+        $this->withTransaction(function () use ($documents, $collection, $batchSize, $skipAuth, $authorization) {
             foreach (\array_chunk($documents, $batchSize) as $chunk) {
                 $getResults = fn () => $this->adapter->deleteDocuments(
                     $collection->getId(),
@@ -5491,8 +5491,6 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba
 
                 $skipAuth ? $authorization->skip($getResults) : $getResults();
             }
-
-            return $documents;
         });
 
         foreach ($documents as $document) {

tests/e2e/Adapter/Base.php

@@ -16517,8 +16517,19 @@ public function testDeleteBulkDocuments(): void
         $docs = static::getDatabase()->find('bulk_delete');
         $this->assertCount(10, $docs);
 
+        /**
+         * Test Short select query
+         */
+        $this->assertCount(2, static::getDatabase()->deleteDocuments(
+            'bulk_delete',
+            [
+                Query::select(['$internalId', '$id', '$permissions', '$updatedAt']),
+                Query::limit(2)
+            ]
+        ));
+
         // TEST: Bulk Delete All Documents
-        $this->assertCount(10, static::getDatabase()->deleteDocuments('bulk_delete'));
+        $this->assertCount(8, static::getDatabase()->deleteDocuments('bulk_delete'));
 
         $docs = static::getDatabase()->find('bulk_delete');
         $this->assertCount(0, $docs);