From b2fb5b2deb111461b8645a9899b09f7ed8d5a2fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 29 Oct 2024 16:34:48 +0100 Subject: [PATCH 1/5] PHPLIB-1568 Add GridFS\Bucket::deleteByName(filename) --- src/GridFS/Bucket.php | 13 ++++++++++ tests/GridFS/BucketFunctionalTest.php | 35 +++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/GridFS/Bucket.php b/src/GridFS/Bucket.php index 56a4e26e2..91b54dff9 100644 --- a/src/GridFS/Bucket.php +++ b/src/GridFS/Bucket.php @@ -242,6 +242,19 @@ public function delete(mixed $id) } } + /** + * Delete all the revisions of a file name from the GridFS bucket. + * + * @param string $filename Filename + * + * @throws FileNotFoundException if no file could be selected + * @throws DriverRuntimeException for other driver errors (e.g. connection errors) + */ + public function deleteByName(string $filename): void + { + $this->collectionWrapper->deleteFileAndChunksByFilename($filename); + } + /** * Writes the contents of a GridFS file to a writable stream. * diff --git a/tests/GridFS/BucketFunctionalTest.php b/tests/GridFS/BucketFunctionalTest.php index 0b85b8d9a..3b78d6c8a 100644 --- a/tests/GridFS/BucketFunctionalTest.php +++ b/tests/GridFS/BucketFunctionalTest.php @@ -160,6 +160,41 @@ public function testDeleteStillRemovesChunksIfFileDoesNotExist($input, $expected $this->assertCollectionCount($this->chunksCollection, 0); } + public function testDeleteByName(): void + { + $this->bucket->uploadFromStream('filename', self::createStream('foobar1')); + $this->bucket->uploadFromStream('filename', self::createStream('foobar2')); + $this->bucket->uploadFromStream('filename', self::createStream('foobar3')); + + $this->bucket->uploadFromStream('other', self::createStream('foobar')); + + $this->assertCollectionCount($this->filesCollection, 4); + $this->assertCollectionCount($this->chunksCollection, 4); + + $this->bucket->deleteByName('filename'); + + $this->assertCollectionCount($this->filesCollection, 1); + $this->assertCollectionCount($this->chunksCollection, 1); + + $this->bucket->deleteByName('other'); + + $this->assertCollectionCount($this->filesCollection, 0); + $this->assertCollectionCount($this->chunksCollection, 0); + } + + public function testDeleteByNameShouldIgnoreNonexistentFiles(): void + { + $this->bucket->uploadFromStream('filename', self::createStream('foobar')); + + $this->assertCollectionCount($this->filesCollection, 1); + $this->assertCollectionCount($this->chunksCollection, 1); + + $this->bucket->deleteByName('nonexistent-filename'); + + $this->assertCollectionCount($this->filesCollection, 1); + $this->assertCollectionCount($this->chunksCollection, 1); + } + public function testDownloadingFileWithMissingChunk(): void { $id = $this->bucket->uploadFromStream('filename', self::createStream('foobar')); From f1926a15dd7049ca93c0023bada5ffef0cd6cfca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 29 Oct 2024 17:22:14 +0100 Subject: [PATCH 2/5] Add GridFS\Bucket::renameByName() --- src/GridFS/Bucket.php | 18 ++++++++++++++++++ tests/GridFS/BucketFunctionalTest.php | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/GridFS/Bucket.php b/src/GridFS/Bucket.php index 91b54dff9..8a116bbfd 100644 --- a/src/GridFS/Bucket.php +++ b/src/GridFS/Bucket.php @@ -661,6 +661,24 @@ public function rename(mixed $id, string $newFilename) } } + /** + * Renames all the revisions of a file name in the GridFS bucket. + * + * @param string $filename Filename + * @param string $newFilename New filename + * + * @throws FileNotFoundException if no file could be selected + * @throws DriverRuntimeException for other driver errors (e.g. connection errors) + */ + public function renameByName(string $filename, string $newFilename): void + { + $matchedCount = $this->collectionWrapper->updateFilenameForFilename($filename, $newFilename); + + if (! $matchedCount) { + throw FileNotFoundException::byFilename($filename); + } + } + /** * Writes the contents of a readable stream to a GridFS file. * diff --git a/tests/GridFS/BucketFunctionalTest.php b/tests/GridFS/BucketFunctionalTest.php index 3b78d6c8a..22da8a346 100644 --- a/tests/GridFS/BucketFunctionalTest.php +++ b/tests/GridFS/BucketFunctionalTest.php @@ -758,6 +758,24 @@ public function testRenameShouldRequireFileToExist(): void $this->bucket->rename('nonexistent-id', 'b'); } + public function testRenameByName(): void + { + $this->bucket->uploadFromStream('filename', self::createStream('foo')); + $this->bucket->uploadFromStream('filename', self::createStream('foo')); + $this->bucket->uploadFromStream('filename', self::createStream('foo')); + + $this->bucket->renameByName('filename', 'newname'); + + $this->assertNull($this->bucket->findOne(['filename' => 'filename']), 'No file has the old name'); + $this->assertStreamContents('foo', $this->bucket->openDownloadStreamByName('newname')); + } + + public function testRenameByNameShouldRequireFileToExist(): void + { + $this->expectException(FileNotFoundException::class); + $this->bucket->renameByName('nonexistent-name', 'b'); + } + public function testUploadFromStream(): void { $options = [ From cc47ff7d72a428b141eb2504f127f358075f5263 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 29 Oct 2024 17:31:50 +0100 Subject: [PATCH 3/5] Improves static analysis --- psalm-baseline.xml | 9 +++++++++ src/GridFS/CollectionWrapper.php | 5 +---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 039b43007..253df75fb 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -284,9 +284,18 @@ + + + + + filesCollection->updateMany( + ['filename' => $filename], + ['$set' => ['filename' => $newFilename]], + )->getMatchedCount()]]> + diff --git a/src/GridFS/CollectionWrapper.php b/src/GridFS/CollectionWrapper.php index 0c6b9b96c..736e47cfb 100644 --- a/src/GridFS/CollectionWrapper.php +++ b/src/GridFS/CollectionWrapper.php @@ -150,9 +150,6 @@ public function findChunksByFileId(mixed $id, int $fromChunk = 0) */ public function findFileByFilenameAndRevision(string $filename, int $revision): ?object { - $filename = $filename; - $revision = $revision; - if ($revision < 0) { $skip = abs($revision) - 1; $sortOrder = -1; @@ -266,7 +263,7 @@ public function insertFile(array|object $file): void /** * Updates the filename field in the file document for all the files with a given filename. */ - public function updateFilenameForFilename(string $filename, string $newFilename): ?int + public function updateFilenameForFilename(string $filename, string $newFilename): int { return $this->filesCollection->updateMany( ['filename' => $filename], From a5ee44c56ee9a12789ec84954ed53f94f189ec74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 30 Oct 2024 12:57:25 +0100 Subject: [PATCH 4/5] Add spec tests --- psalm-baseline.xml | 2 ++ src/GridFS/Bucket.php | 10 +++++++--- src/GridFS/CollectionWrapper.php | 2 +- tests/GridFS/BucketFunctionalTest.php | 13 +++---------- tests/UnifiedSpecTests/Operation.php | 14 ++++++++++++++ tests/UnifiedSpecTests/Util.php | 2 ++ tests/specifications | 2 +- 7 files changed, 30 insertions(+), 15 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 253df75fb..ba4f3cb4f 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -286,11 +286,13 @@ + + filesCollection->updateMany( ['filename' => $filename], ['$set' => ['filename' => $newFilename]], diff --git a/src/GridFS/Bucket.php b/src/GridFS/Bucket.php index 8a116bbfd..85046906c 100644 --- a/src/GridFS/Bucket.php +++ b/src/GridFS/Bucket.php @@ -252,7 +252,11 @@ public function delete(mixed $id) */ public function deleteByName(string $filename): void { - $this->collectionWrapper->deleteFileAndChunksByFilename($filename); + $count = $this->collectionWrapper->deleteFileAndChunksByFilename($filename); + + if ($count === 0) { + throw FileNotFoundException::byFilename($filename); + } } /** @@ -672,9 +676,9 @@ public function rename(mixed $id, string $newFilename) */ public function renameByName(string $filename, string $newFilename): void { - $matchedCount = $this->collectionWrapper->updateFilenameForFilename($filename, $newFilename); + $count = $this->collectionWrapper->updateFilenameForFilename($filename, $newFilename); - if (! $matchedCount) { + if ($count === 0) { throw FileNotFoundException::byFilename($filename); } } diff --git a/src/GridFS/CollectionWrapper.php b/src/GridFS/CollectionWrapper.php index 736e47cfb..09b06ba23 100644 --- a/src/GridFS/CollectionWrapper.php +++ b/src/GridFS/CollectionWrapper.php @@ -74,7 +74,7 @@ public function deleteChunksByFilesId(mixed $id): void /** * Delete all GridFS files and chunks for a given filename. */ - public function deleteFileAndChunksByFilename(string $filename): ?int + public function deleteFileAndChunksByFilename(string $filename): int { /** @var iterable $files */ $files = $this->findFiles(['filename' => $filename], [ diff --git a/tests/GridFS/BucketFunctionalTest.php b/tests/GridFS/BucketFunctionalTest.php index 22da8a346..8faef8e38 100644 --- a/tests/GridFS/BucketFunctionalTest.php +++ b/tests/GridFS/BucketFunctionalTest.php @@ -182,17 +182,10 @@ public function testDeleteByName(): void $this->assertCollectionCount($this->chunksCollection, 0); } - public function testDeleteByNameShouldIgnoreNonexistentFiles(): void + public function testDeleteByNameShouldRequireFileToExist(): void { - $this->bucket->uploadFromStream('filename', self::createStream('foobar')); - - $this->assertCollectionCount($this->filesCollection, 1); - $this->assertCollectionCount($this->chunksCollection, 1); - - $this->bucket->deleteByName('nonexistent-filename'); - - $this->assertCollectionCount($this->filesCollection, 1); - $this->assertCollectionCount($this->chunksCollection, 1); + $this->expectException(FileNotFoundException::class); + $this->bucket->deleteByName('nonexistent-name'); } public function testDownloadingFileWithMissingChunk(): void diff --git a/tests/UnifiedSpecTests/Operation.php b/tests/UnifiedSpecTests/Operation.php index 390e278be..d2550f498 100644 --- a/tests/UnifiedSpecTests/Operation.php +++ b/tests/UnifiedSpecTests/Operation.php @@ -789,6 +789,12 @@ private function executeForBucket(Bucket $bucket) return $bucket->delete($args['id']); + case 'deleteByName': + assertArrayHasKey('filename', $args); + assertIsString($args['filename']); + + return $bucket->deleteByName($args['filename']); + case 'downloadByName': assertArrayHasKey('filename', $args); assertIsString($args['filename']); @@ -812,6 +818,14 @@ private function executeForBucket(Bucket $bucket) return null; + case 'renameByName': + assertArrayHasKey('filename', $args); + assertArrayHasKey('newFilename', $args); + assertIsString($args['filename']); + assertIsString($args['newFilename']); + + return $bucket->renameByName($args['filename'], $args['newFilename']); + case 'uploadWithId': assertArrayHasKey('id', $args); $args['_id'] = $args['id']; diff --git a/tests/UnifiedSpecTests/Util.php b/tests/UnifiedSpecTests/Util.php index e09e3f942..b0d575d39 100644 --- a/tests/UnifiedSpecTests/Util.php +++ b/tests/UnifiedSpecTests/Util.php @@ -132,9 +132,11 @@ final class Util ], Bucket::class => [ 'delete' => ['id'], + 'deleteByName' => ['filename'], 'downloadByName' => ['filename', 'revision'], 'download' => ['id'], 'rename' => ['id', 'newFilename'], + 'renameByName' => ['filename', 'newFilename'], 'uploadWithId' => ['id', 'filename', 'source', 'chunkSizeBytes', 'disableMD5', 'contentType', 'metadata'], 'upload' => ['filename', 'source', 'chunkSizeBytes', 'disableMD5', 'contentType', 'metadata'], ], diff --git a/tests/specifications b/tests/specifications index 34f9a579c..841a72983 160000 --- a/tests/specifications +++ b/tests/specifications @@ -1 +1 @@ -Subproject commit 34f9a579cb7b74023a2bf59c61c39145cf9aec45 +Subproject commit 841a72983e94d001bff9d1c8fc1c381df0af6bba From d4cadcadce1ac198bde94ff160926f62268d06b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Thu, 31 Oct 2024 18:42:47 +0100 Subject: [PATCH 5/5] Revert submodule update --- tests/specifications | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/specifications b/tests/specifications index 841a72983..34f9a579c 160000 --- a/tests/specifications +++ b/tests/specifications @@ -1 +1 @@ -Subproject commit 841a72983e94d001bff9d1c8fc1c381df0af6bba +Subproject commit 34f9a579cb7b74023a2bf59c61c39145cf9aec45