PHPLIB-1323 Implement unlink for GridFS stream wrapper (#1206)

GromNaN · web-flow · commit b700b1040f9b · 2024-01-12T13:37:37.000+01:00
* Delete all file revisions by bulk
* Thrown a FileNotFoundException in rename and unlink
* Fix rename with same file name
diff --git a/docs/reference/method/MongoDBGridFSBucket-registerGlobalStreamWrapperAlias.txt b/docs/reference/method/MongoDBGridFSBucket-registerGlobalStreamWrapperAlias.txt
@@ -48,7 +48,8 @@ Supported stream functions:
 - :php:`filesize() <filesize>`
 - :php:`file() <file>`
 - :php:`fopen() <fopen>` with "r", "rb", "w", and "wb" modes
-- :php:`rename() <rename>` rename all revisions of a file in the same bucket
+- :php:`rename() <rename>`
+- :php:`unlink() <unlink>`
 
 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
@@ -57,6 +58,9 @@ revision is read (revision ``-1``).
 In write mode, the stream context can contain the option ``gridfs['chunkSizeBytes']``.
 If omitted, the defaults are inherited from the ``Bucket`` instance option.
 
+The functions `rename` and `unlink` will rename or remove all revisions of a
+filename. If the filename does not exist, these functions throw a ``FileNotFoundException``.
+
 Example
 -------
 
@@ -86,6 +90,8 @@ Each call to these functions makes a request to the server.
 
    echo file_get_contents('gridfs://mybucket/hello.txt');
 
+   unlink('gridfs://mybucket/hello.txt');
+
 The output would then resemble:
 
 .. code-block:: none
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -85,20 +85,34 @@
       <code>$context</code>
     </MixedArgumentTypeCoercion>
   </file>
+  <file src="src/GridFS/CollectionWrapper.php">
+    <MixedAssignment>
+      <code>$ids[]</code>
+    </MixedAssignment>
+  </file>
   <file src="src/GridFS/ReadableStream.php">
     <MixedArgument>
       <code><![CDATA[$currentChunk->n]]></code>
       <code><![CDATA[$this->file->length]]></code>
     </MixedArgument>
   </file>
   <file src="src/GridFS/StreamWrapper.php">
-    <InvalidArgument>
-      <code>$context</code>
-      <code>$context</code>
-    </InvalidArgument>
+    <InvalidDocblock>
+      <code>private function getContext(string $path, string $mode): array</code>
+    </InvalidDocblock>
+    <MixedArgumentTypeCoercion>
+      <code><![CDATA[$this->getContext($path, $mode)]]></code>
+      <code><![CDATA[$this->getContext($path, $mode)]]></code>
+    </MixedArgumentTypeCoercion>
     <MixedAssignment>
       <code>$context</code>
+      <code>$count</code>
+      <code>$count</code>
     </MixedAssignment>
+    <MixedMethodCall>
+      <code>deleteFileAndChunksByFilename</code>
+      <code>updateFilenameForFilename</code>
+    </MixedMethodCall>
   </file>
   <file src="src/Model/BSONArray.php">
     <MixedAssignment>
diff --git a/src/GridFS/CollectionWrapper.php b/src/GridFS/CollectionWrapper.php
@@ -80,6 +80,29 @@ public function deleteChunksByFilesId($id): void
         $this->chunksCollection->deleteMany(['files_id' => $id]);
     }
 
+    /**
+     * Delete all GridFS files and chunks for a given filename.
+     */
+    public function deleteFileAndChunksByFilename(string $filename): ?int
+    {
+        /** @var iterable<array{_id: mixed}> $files */
+        $files = $this->findFiles(['filename' => $filename], [
+            'typeMap' => ['root' => 'array'],
+            'projection' => ['_id' => 1],
+        ]);
+
+        /** @var list<mixed> $ids */
+        $ids = [];
+        foreach ($files as $file) {
+            $ids[] = $file['_id'];
+        }
+
+        $count = $this->filesCollection->deleteMany(['_id' => ['$in' => $ids]])->getDeletedCount();
+        $this->chunksCollection->deleteMany(['files_id' => ['$in' => $ids]]);
+
+        return $count;
+    }
+
     /**
      * Deletes a GridFS file and related chunks by ID.
      *
@@ -256,12 +279,12 @@ public function insertFile($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): UpdateResult
+    public function updateFilenameForFilename(string $filename, string $newFilename): ?int
     {
         return $this->filesCollection->updateMany(
             ['filename' => $filename],
             ['$set' => ['filename' => $newFilename]],
-        );
+        )->getMatchedCount();
     }
 
     /**
diff --git a/src/GridFS/Exception/FileNotFoundException.php b/src/GridFS/Exception/FileNotFoundException.php
@@ -25,6 +25,17 @@
 
 class FileNotFoundException extends RuntimeException
 {
+    /**
+     * Thrown when a file cannot be found by its filename.
+     *
+     * @param string $filename Filename
+     * @return self
+     */
+    public static function byFilename(string $filename)
+    {
+        return new self(sprintf('File with name "%s" not found', $filename));
+    }
+
     /**
      * Thrown when a file cannot be found by its filename and revision.
      *
diff --git a/src/GridFS/ReadableStream.php b/src/GridFS/ReadableStream.php
@@ -22,14 +22,12 @@
 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;
@@ -178,20 +176,6 @@ public function readBytes(int $length): string
         return $data;
     }
 
-    /**
-     * Rename all revisions of the file.
-     */
-    public function rename(string $newFilename): bool
-    {
-        if (! isset($this->file->filename) || ! is_string($this->file->filename)) {
-            throw new LogicException('Cannot rename file without a filename');
-        }
-
-        $this->collectionWrapper->updateFilenameForFilename($this->file->filename, $newFilename);
-
-        return true;
-    }
-
     /**
      * Seeks the chunk and buffer offsets for the next read operation.
      *
diff --git a/src/GridFS/StreamWrapper.php b/src/GridFS/StreamWrapper.php
@@ -96,7 +96,8 @@ public static function register(string $protocol = 'gridfs'): void
     /**
      * Rename all revisions of a filename.
      *
-     * @return bool True on success or false on failure.
+     * @return true
+     * @throws FileNotFoundException
      */
     public function rename(string $fromPath, string $toPath): bool
     {
@@ -105,16 +106,17 @@ public function rename(string $fromPath, string $toPath): bool
             throw LogicException::renamePathMismatch($fromPath, $toPath);
         }
 
-        try {
-            $this->stream_open($fromPath, 'r', 0, $openedPath);
-        } catch (FileNotFoundException $e) {
-            return false;
-        }
+        $context = $this->getContext($fromPath, 'w');
+
+        $newFilename = explode('/', $toPath, 4)[3] ?? '';
+        $count = $context['collectionWrapper']->updateFilenameForFilename($context['filename'], $newFilename);
 
-        $newName = explode('/', $toPath, 4)[3] ?? '';
-        assert($this->stream instanceof ReadableStream);
+        if ($count === 0) {
+            throw FileNotFoundException::byFilename($fromPath);
+        }
 
-        return $this->stream->rename($newName);
+        // If $count is null, the update is unacknowledged, the operation is considered successful.
+        return true;
     }
 
     /**
@@ -170,41 +172,12 @@ public function stream_eof(): bool
      */
     public function stream_open(string $path, string $mode, int $options, ?string &$openedPath): bool
     {
-        $context = [];
-
-        /**
-         * The Bucket methods { @see Bucket::openUploadStream() } and { @see Bucket::openDownloadStreamByFile() }
-         * always set an internal context. But the context can also be set by the user.
-         */
-        if (is_resource($this->context)) {
-            $context = stream_context_get_options($this->context)['gridfs'] ?? [];
-
-            if (! is_array($context)) {
-                throw LogicException::invalidContext($context);
-            }
-        }
-
-        // When the stream is opened using fopen(), the context is not required, it can contain only options.
-        if (! isset($context['collectionWrapper'])) {
-            $bucketAlias = explode('/', $path, 4)[2] ?? '';
-
-            if (! isset(self::$contextResolvers[$bucketAlias])) {
-                throw LogicException::bucketAliasNotRegistered($bucketAlias);
-            }
-
-            $context = self::$contextResolvers[$bucketAlias]($path, $mode, $context);
-        }
-
-        if (! $context['collectionWrapper'] instanceof CollectionWrapper) {
-            throw LogicException::invalidContextCollectionWrapper($context['collectionWrapper']);
-        }
-
         if ($mode === 'r' || $mode === 'rb') {
-            return $this->initReadableStream($context);
+            return $this->initReadableStream($this->getContext($path, $mode));
         }
 
         if ($mode === 'w' || $mode === 'wb') {
-            return $this->initWritableStream($context);
+            return $this->initWritableStream($this->getContext($path, $mode));
         }
 
         throw LogicException::openModeNotSupported($mode);
@@ -324,6 +297,25 @@ public function stream_write(string $data): int
         return $this->stream->writeBytes($data);
     }
 
+    /**
+     * Remove all revisions of a filename.
+     *
+     * @return true
+     * @throws FileNotFoundException
+     */
+    public function unlink(string $path): bool
+    {
+        $context = $this->getContext($path, 'w');
+        $count = $context['collectionWrapper']->deleteFileAndChunksByFilename($context['filename']);
+
+        if ($count === 0) {
+            throw FileNotFoundException::byFilename($path);
+        }
+
+        // If $count is null, the update is unacknowledged, the operation is considered successful.
+        return true;
+    }
+
     /** @return false|array */
     public function url_stat(string $path, int $flags)
     {
@@ -338,6 +330,45 @@ public function url_stat(string $path, int $flags)
         return $this->stream_stat();
     }
 
+    /**
+     * @return array{collectionWrapper: CollectionWrapper, file: object}|array{collectionWrapper: CollectionWrapper, filename: string, options: array}
+     * @psalm-return ($mode == 'r' or $mode == 'rb' ? array{collectionWrapper: CollectionWrapper, file: object} : array{collectionWrapper: CollectionWrapper, filename: string, options: array})
+     */
+    private function getContext(string $path, string $mode): array
+    {
+        $context = [];
+
+        /**
+         * The Bucket methods { @see Bucket::openUploadStream() } and { @see Bucket::openDownloadStreamByFile() }
+         * always set an internal context. But the context can also be set by the user.
+         */
+        if (is_resource($this->context)) {
+            $context = stream_context_get_options($this->context)['gridfs'] ?? [];
+
+            if (! is_array($context)) {
+                throw LogicException::invalidContext($context);
+            }
+        }
+
+        // When the stream is opened using fopen(), the context is not required, it can contain only options.
+        if (! isset($context['collectionWrapper'])) {
+            $bucketAlias = explode('/', $path, 4)[2] ?? '';
+
+            if (! isset(self::$contextResolvers[$bucketAlias])) {
+                throw LogicException::bucketAliasNotRegistered($bucketAlias);
+            }
+
+            /** @see Bucket::resolveStreamContext() */
+            $context = self::$contextResolvers[$bucketAlias]($path, $mode, $context);
+        }
+
+        if (! $context['collectionWrapper'] instanceof CollectionWrapper) {
+            throw LogicException::invalidContextCollectionWrapper($context['collectionWrapper']);
+        }
+
+        return $context;
+    }
+
     /**
      * Returns a stat template with default values.
      */
diff --git a/tests/GridFS/StreamWrapperFunctionalTest.php b/tests/GridFS/StreamWrapperFunctionalTest.php
@@ -29,6 +29,7 @@
 use function stream_context_create;
 use function stream_get_contents;
 use function time;
+use function unlink;
 use function usleep;
 
 use const SEEK_CUR;
@@ -374,10 +375,33 @@ public function testRenameAllRevisions(): void
         $this->assertSame(6, file_put_contents($path, 'foobar'));
         $this->assertSame(6, file_put_contents($path, 'foobar'));
 
-        $this->assertTrue(rename($path, $path . '.renamed'));
+        $result = rename($path, $path . '.renamed');
+        $this->assertTrue($result);
         $this->assertTrue(file_exists($path . '.renamed'));
         $this->assertFalse(file_exists($path));
         $this->assertSame('foobar', file_get_contents($path . '.renamed'));
+
+        $this->expectException(FileNotFoundException::class);
+        $this->expectExceptionMessage('File with name "gridfs://bucket/filename" not found');
+        rename($path, $path . '.renamed');
+    }
+
+    public function testRenameSameFilename(): void
+    {
+        $this->bucket->registerGlobalStreamWrapperAlias('bucket');
+        $path = 'gridfs://bucket/filename';
+
+        $this->assertSame(6, file_put_contents($path, 'foobar'));
+
+        $result = rename($path, $path);
+        $this->assertTrue($result);
+        $this->assertTrue(file_exists($path));
+        $this->assertSame('foobar', file_get_contents($path));
+
+        $path = 'gridfs://bucket/missing';
+        $this->expectException(FileNotFoundException::class);
+        $this->expectExceptionMessage('File with name "gridfs://bucket/missing" not found');
+        rename($path, $path);
     }
 
     public function testRenamePathMismatch(): void
@@ -387,4 +411,22 @@ public function testRenamePathMismatch(): void
 
         rename('gridfs://bucket/filename', 'gridfs://other/newname');
     }
+
+    public function testUnlinkAllRevisions(): void
+    {
+        $this->bucket->registerGlobalStreamWrapperAlias('bucket');
+        $path = 'gridfs://bucket/path/to/filename';
+
+        file_put_contents($path, 'version 0');
+        file_put_contents($path, 'version 1');
+
+        $result = unlink($path);
+
+        $this->assertTrue($result);
+        $this->assertFalse(file_exists($path));
+
+        $this->expectException(FileNotFoundException::class);
+        $this->expectExceptionMessage('File with name "gridfs://bucket/path/to/filename" not found');
+        unlink($path);
+    }
 }
