From b93bfb2d8981d3255aeb4c1eb8de6efe91826452 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 15 Dec 2023 23:56:19 +0100 Subject: [PATCH 1/3] PHPLIB-1324 Implement rename using stream wrapper Rename all the revisions as defined in the spec https://github.com/mongodb/specifications/blob/0b47194538aa817978fae0f77f684f6d5e62ebab/source/gridfs/gridfs-spec.rst#renaming-stored-files --- ...ucket-registerGlobalStreamWrapperAlias.txt | 3 ++- phpcs.xml.dist | 7 +++++ src/GridFS/Exception/LogicException.php | 10 +++++++ src/GridFS/ReadableStream.php | 23 ++++++++++++++++ src/GridFS/StreamWrapper.php | 27 +++++++++++++++++++ tests/GridFS/StreamWrapperFunctionalTest.php | 24 +++++++++++++++++ 6 files changed, 93 insertions(+), 1 deletion(-) diff --git a/docs/reference/method/MongoDBGridFSBucket-registerGlobalStreamWrapperAlias.txt b/docs/reference/method/MongoDBGridFSBucket-registerGlobalStreamWrapperAlias.txt index 649915eea..4dcc79ce9 100644 --- a/docs/reference/method/MongoDBGridFSBucket-registerGlobalStreamWrapperAlias.txt +++ b/docs/reference/method/MongoDBGridFSBucket-registerGlobalStreamWrapperAlias.txt @@ -47,7 +47,8 @@ Supported stream functions: - :php:`filemtime() ` - :php:`filesize() ` - :php:`file() ` -- :php:`fopen() ` (with "r", "rb", "w", and "wb" modes) +- :php:`fopen() ` with "r", "rb", "w", and "wb" modes +- :php:`rename() ` rename all revisions of a file in the same bucket In read mode, the stream context can contain the option ``gridfs['revision']`` to specify the revision number of the file to read. If omitted, the most recent diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 9cee3b66a..096011786 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -162,4 +162,11 @@ /docs/examples/codecs/ + + + + + + /src/GridFS/StreamWrapper.php + diff --git a/src/GridFS/Exception/LogicException.php b/src/GridFS/Exception/LogicException.php index 907e27f07..a2b6378c7 100644 --- a/src/GridFS/Exception/LogicException.php +++ b/src/GridFS/Exception/LogicException.php @@ -67,4 +67,14 @@ public static function openModeNotSupported(string $mode): self { return new self(sprintf('Mode "%s" is not supported by "gridfs://" files. Use one of "r", "rb", "w", or "wb".', $mode)); } + + /** + * Thrown when the origin and destination paths are not in the same bucket. + * + * @internal + */ + public static function renamePathMismatch(string $from, string $to): self + { + return new self(sprintf('Cannot rename "%s" to "%s" because they are not in the same GridFS bucket.', $from, $to)); + } } diff --git a/src/GridFS/ReadableStream.php b/src/GridFS/ReadableStream.php index a0664e821..6a65de154 100644 --- a/src/GridFS/ReadableStream.php +++ b/src/GridFS/ReadableStream.php @@ -19,6 +19,7 @@ use Iterator; use MongoDB\BSON\Binary; +use MongoDB\BSON\Document; use MongoDB\Driver\CursorInterface; use MongoDB\Exception\InvalidArgumentException; use MongoDB\GridFS\Exception\CorruptFileException; @@ -176,6 +177,28 @@ public function readBytes(int $length): string return $data; } + /** + * Rename all revisions of the file. + * + * @return int The number of revisions renamed + */ + public function rename(string $newFilename): int + { + /** @var Document[] $revisions */ + $revisions = $this->collectionWrapper->findFiles( + ['filename' => $this->file->filename], + ['typeMap' => ['root' => 'bson'], 'projection' => ['_id' => 1]], + ); + $count = 0; + + foreach ($revisions as $revision) { + $this->collectionWrapper->updateFilenameForId($revision->get('_id'), $newFilename); + $count++; + } + + return $count; + } + /** * Seeks the chunk and buffer offsets for the next read operation. * diff --git a/src/GridFS/StreamWrapper.php b/src/GridFS/StreamWrapper.php index b5a4d6d70..6cf175209 100644 --- a/src/GridFS/StreamWrapper.php +++ b/src/GridFS/StreamWrapper.php @@ -22,12 +22,15 @@ use MongoDB\GridFS\Exception\FileNotFoundException; use MongoDB\GridFS\Exception\LogicException; +use function array_slice; use function assert; use function explode; +use function implode; use function in_array; use function is_array; use function is_integer; use function is_resource; +use function str_starts_with; use function stream_context_get_options; use function stream_get_wrappers; use function stream_wrapper_register; @@ -90,6 +93,30 @@ public static function register(string $protocol = 'gridfs'): void stream_wrapper_register($protocol, static::class, STREAM_IS_URL); } + /** + * Rename all revisions of a filename. + * + * @return bool True on success or false on failure. + */ + public function rename(string $path_from, string $path_to): bool + { + $prefix = implode('/', array_slice(explode('/', $path_from, 4), 0, 3)) . '/'; + if (! str_starts_with($path_to, $prefix)) { + throw LogicException::renamePathMismatch($path_from, $path_to); + } + + try { + $this->stream_open($path_from, 'r', 0, $openedPath); + } catch (FileNotFoundException $e) { + return false; + } + + $newName = explode('/', $path_to, 4)[3] ?? ''; + assert($this->stream instanceof ReadableStream); + + return $this->stream->rename($newName) > 0; + } + /** * @see Bucket::resolveStreamContext() * diff --git a/tests/GridFS/StreamWrapperFunctionalTest.php b/tests/GridFS/StreamWrapperFunctionalTest.php index a2f1fe701..d9ab66552 100644 --- a/tests/GridFS/StreamWrapperFunctionalTest.php +++ b/tests/GridFS/StreamWrapperFunctionalTest.php @@ -25,6 +25,7 @@ use function is_dir; use function is_file; use function is_link; +use function rename; use function stream_context_create; use function stream_get_contents; use function time; @@ -363,4 +364,27 @@ public function testCopy(): void $this->assertSame('foobar', file_get_contents($path . '.copy')); $this->assertSame('foobar', file_get_contents($path)); } + + public function testRenameAllRevisions(): void + { + $this->bucket->registerGlobalStreamWrapperAlias('bucket'); + $path = 'gridfs://bucket/filename'; + + $this->assertSame(6, file_put_contents($path, 'foobar')); + $this->assertSame(6, file_put_contents($path, 'foobar')); + $this->assertSame(6, file_put_contents($path, 'foobar')); + + $this->assertTrue(rename($path, $path . '.renamed')); + $this->assertTrue(file_exists($path . '.renamed')); + $this->assertFalse(file_exists($path)); + $this->assertSame('foobar', file_get_contents($path . '.renamed')); + } + + public function testRenamePathMismatch(): void + { + $this->expectException(LogicException::class); + $this->expectExceptionMessage('Cannot rename "gridfs://bucket/filename" to "gridfs://other/newname" because they are not in the same GridFS bucket.'); + + rename('gridfs://bucket/filename', 'gridfs://other/newname'); + } } From 7a799a410fdeba3b5a97964f102cac3e5680c20c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Mon, 18 Dec 2023 17:48:13 +0100 Subject: [PATCH 2/3] Rename `rename` arguments to match CS rules --- phpcs.xml.dist | 7 ------- src/GridFS/StreamWrapper.php | 12 ++++++------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 096011786..9cee3b66a 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -162,11 +162,4 @@ /docs/examples/codecs/ - - - - - - /src/GridFS/StreamWrapper.php - diff --git a/src/GridFS/StreamWrapper.php b/src/GridFS/StreamWrapper.php index 6cf175209..c5602e2e2 100644 --- a/src/GridFS/StreamWrapper.php +++ b/src/GridFS/StreamWrapper.php @@ -98,20 +98,20 @@ public static function register(string $protocol = 'gridfs'): void * * @return bool True on success or false on failure. */ - public function rename(string $path_from, string $path_to): bool + public function rename(string $fromPath, string $toPath): bool { - $prefix = implode('/', array_slice(explode('/', $path_from, 4), 0, 3)) . '/'; - if (! str_starts_with($path_to, $prefix)) { - throw LogicException::renamePathMismatch($path_from, $path_to); + $prefix = implode('/', array_slice(explode('/', $fromPath, 4), 0, 3)) . '/'; + if (! str_starts_with($toPath, $prefix)) { + throw LogicException::renamePathMismatch($fromPath, $toPath); } try { - $this->stream_open($path_from, 'r', 0, $openedPath); + $this->stream_open($fromPath, 'r', 0, $openedPath); } catch (FileNotFoundException $e) { return false; } - $newName = explode('/', $path_to, 4)[3] ?? ''; + $newName = explode('/', $toPath, 4)[3] ?? ''; assert($this->stream instanceof ReadableStream); return $this->stream->rename($newName) > 0; From 58526b775344619b555178ca3f23a2bc837f76cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 19 Dec 2023 11:49:25 +0100 Subject: [PATCH 3/3] Optimize rename using a single updateMany command --- src/GridFS/CollectionWrapper.php | 11 +++++++++++ src/GridFS/ReadableStream.php | 23 ++++++++--------------- src/GridFS/StreamWrapper.php | 2 +- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/GridFS/CollectionWrapper.php b/src/GridFS/CollectionWrapper.php index 16ef8343a..31c5df7bd 100644 --- a/src/GridFS/CollectionWrapper.php +++ b/src/GridFS/CollectionWrapper.php @@ -253,6 +253,17 @@ public function insertFile($file): void $this->filesCollection->insertOne($file); } + /** + * Updates the filename field in the file document for all the files with a given filename. + */ + public function updateFilenameForFilename(string $filename, string $newFilename): UpdateResult + { + return $this->filesCollection->updateMany( + ['filename' => $filename], + ['$set' => ['filename' => $newFilename]], + ); + } + /** * Updates the filename field in the file document for a given ID. * diff --git a/src/GridFS/ReadableStream.php b/src/GridFS/ReadableStream.php index 6a65de154..402f21280 100644 --- a/src/GridFS/ReadableStream.php +++ b/src/GridFS/ReadableStream.php @@ -19,16 +19,17 @@ use Iterator; use MongoDB\BSON\Binary; -use MongoDB\BSON\Document; use MongoDB\Driver\CursorInterface; use MongoDB\Exception\InvalidArgumentException; use MongoDB\GridFS\Exception\CorruptFileException; +use MongoDB\GridFS\Exception\LogicException; use function assert; use function ceil; use function floor; use function is_integer; use function is_object; +use function is_string; use function property_exists; use function sprintf; use function strlen; @@ -179,24 +180,16 @@ public function readBytes(int $length): string /** * Rename all revisions of the file. - * - * @return int The number of revisions renamed */ - public function rename(string $newFilename): int + public function rename(string $newFilename): bool { - /** @var Document[] $revisions */ - $revisions = $this->collectionWrapper->findFiles( - ['filename' => $this->file->filename], - ['typeMap' => ['root' => 'bson'], 'projection' => ['_id' => 1]], - ); - $count = 0; - - foreach ($revisions as $revision) { - $this->collectionWrapper->updateFilenameForId($revision->get('_id'), $newFilename); - $count++; + if (! isset($this->file->filename) || ! is_string($this->file->filename)) { + throw new LogicException('Cannot rename file without a filename'); } - return $count; + $this->collectionWrapper->updateFilenameForFilename($this->file->filename, $newFilename); + + return true; } /** diff --git a/src/GridFS/StreamWrapper.php b/src/GridFS/StreamWrapper.php index c5602e2e2..7a77f11bd 100644 --- a/src/GridFS/StreamWrapper.php +++ b/src/GridFS/StreamWrapper.php @@ -114,7 +114,7 @@ public function rename(string $fromPath, string $toPath): bool $newName = explode('/', $toPath, 4)[3] ?? ''; assert($this->stream instanceof ReadableStream); - return $this->stream->rename($newName) > 0; + return $this->stream->rename($newName); } /**