From 5d90f8958abe00812ef74942e84af42949f8d8dc Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 18 Mar 2025 16:51:09 +0200 Subject: [PATCH 1/4] move while loop --- src/Database/Database.php | 102 ++++++++++++++++++------------------- tests/e2e/Adapter/Base.php | 7 ++- 2 files changed, 57 insertions(+), 52 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 71d381933..fee08cdf6 100644 --- a/src/Database/Database.php +++ b/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 []; + } + + $documents = $this->withTransaction(function () use ($documents, $collection, $batchSize, $skipAuth, $authorization) { foreach (\array_chunk($documents, $batchSize) as $chunk) { $getResults = fn () => $this->adapter->deleteDocuments( $collection->getId(), diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 1a2d686ef..298f427fa 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -16518,7 +16518,12 @@ public function testDeleteBulkDocuments(): void $this->assertCount(10, $docs); // TEST: Bulk Delete All Documents - $this->assertCount(10, static::getDatabase()->deleteDocuments('bulk_delete')); + $this->assertCount(10, static::getDatabase()->deleteDocuments( + 'bulk_delete', + [ + Query::select(['$internalId', '$uid', '$permissions', '$updatedAt']) + ] + )); $docs = static::getDatabase()->find('bulk_delete'); $this->assertCount(0, $docs); From 14b2eae8135757067b50d98377ec450c7987f9e0 Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 18 Mar 2025 16:55:47 +0200 Subject: [PATCH 2/4] Remove return --- src/Database/Database.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index fee08cdf6..068a68b44 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -5482,7 +5482,7 @@ public function deleteDocuments(string $collection, array $queries = [], int $ba return []; } - $documents = $this->withTransaction(function () use ($documents, $collection, $batchSize, $skipAuth, $authorization) { + $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) { From 20af61899843857101ecb8f7a077a67902f0ed15 Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 18 Mar 2025 17:08:55 +0200 Subject: [PATCH 3/4] $id typo --- tests/e2e/Adapter/Base.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 298f427fa..3553c94f4 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -16521,7 +16521,7 @@ public function testDeleteBulkDocuments(): void $this->assertCount(10, static::getDatabase()->deleteDocuments( 'bulk_delete', [ - Query::select(['$internalId', '$uid', '$permissions', '$updatedAt']) + Query::select(['$internalId', '$id', '$permissions', '$updatedAt']) ] )); From 7f6486dec0ff87da713d35d10336c6901679c319 Mon Sep 17 00:00:00 2001 From: fogelito Date: Tue, 18 Mar 2025 17:24:07 +0200 Subject: [PATCH 4/4] short select --- tests/e2e/Adapter/Base.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 3553c94f4..54b38f7c0 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -16517,14 +16517,20 @@ public function testDeleteBulkDocuments(): void $docs = static::getDatabase()->find('bulk_delete'); $this->assertCount(10, $docs); - // TEST: Bulk Delete All Documents - $this->assertCount(10, static::getDatabase()->deleteDocuments( + /** + * Test Short select query + */ + $this->assertCount(2, static::getDatabase()->deleteDocuments( 'bulk_delete', [ - Query::select(['$internalId', '$id', '$permissions', '$updatedAt']) + Query::select(['$internalId', '$id', '$permissions', '$updatedAt']), + Query::limit(2) ] )); + // TEST: Bulk Delete All Documents + $this->assertCount(8, static::getDatabase()->deleteDocuments('bulk_delete')); + $docs = static::getDatabase()->find('bulk_delete'); $this->assertCount(0, $docs);