Refactor rename and unlink to use the context directly

mongodb · Jan 9, 2024 · 1d8e3c4 · 1d8e3c4
1 parent fc32d0a
commit 1d8e3c4
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 83 deletions.
diff --git a/docs/reference/method/MongoDBGridFSBucket-registerGlobalStreamWrapperAlias.txt b/docs/reference/method/MongoDBGridFSBucket-registerGlobalStreamWrapperAlias.txt
@@ -49,7 +49,7 @@ Supported stream functions:
 - :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:`unlink() <unlink>` (remove all revisions of a file)
+- :php:`unlink() <unlink>` remove 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/psalm-baseline.xml b/psalm-baseline.xml
@@ -98,17 +98,26 @@
   </file>
   <file src="src/GridFS/StreamWrapper.php">
     <InvalidArgument>
-      <code>$context</code>
-      <code>$context</code>
+      <code><![CDATA[$this->getContext($path, $mode)]]></code>
+      <code><![CDATA[$this->getContext($path, $mode)]]></code>
     </InvalidArgument>
-    <MixedAssignment>
+    <InvalidReturnStatement>
       <code>$context</code>
-    </MixedAssignment>
-  </file>
-  <file src="src/GridFS/WritableStream.php">
+    </InvalidReturnStatement>
+    <InvalidReturnType>
+      <code>array{collectionWrapper: CollectionWrapper, file: object}|array{collectionWrapper: CollectionWrapper, filename: string, options: array</code>
+    </InvalidReturnType>
     <MixedArgument>
-      <code><![CDATA[$this->file['filename']]]></code>
+      <code><![CDATA[$context['file']->filename]]></code>
+      <code><![CDATA[$context['file']->filename]]></code>
     </MixedArgument>
+    <MixedAssignment>
+      <code>$context</code>
+    </MixedAssignment>
+    <PossiblyUndefinedArrayOffset>
+      <code><![CDATA[$context['file']]]></code>
+      <code><![CDATA[$context['file']]]></code>
+    </PossiblyUndefinedArrayOffset>
   </file>
   <file src="src/Model/BSONArray.php">
     <MixedAssignment>

diff --git a/src/GridFS/CollectionWrapper.php b/src/GridFS/CollectionWrapper.php
@@ -87,7 +87,6 @@ public function deleteFileAndChunksByFilename(string $filename): int
     {
         /** @var iterable<array{_id: mixed}> $files */
         $files = $this->findFiles(['filename' => $filename], [
-            'codec' => null,
             'typeMap' => ['root' => 'array'],
             'projection' => ['_id' => 1],
         ]);

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
@@ -106,15 +106,15 @@ public function rename(string $fromPath, string $toPath): bool
         }
 
         try {
-            $this->stream_open($fromPath, 'r', 0, $openedPath);
+            $context = $this->getContext($fromPath, 'r');
         } catch (FileNotFoundException $e) {
             return false;
         }
 
-        $newName = explode('/', $toPath, 4)[3] ?? '';
-        assert($this->stream instanceof ReadableStream);
+        $newFilename = explode('/', $toPath, 4)[3] ?? '';
+        $context['collectionWrapper']->updateFilenameForFilename($context['file']->filename, $newFilename);
 
-        return $this->stream->rename($newName);
+        return true;
     }
 
     /**
@@ -170,41 +170,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);
@@ -326,15 +297,10 @@ public function stream_write(string $data): int
 
     public function unlink(string $path): bool
     {
-        try {
-            $this->stream_open($path, 'w', 0, $openedPath);
-        } catch (FileNotFoundException $e) {
-            return false;
-        }
+        $context = $this->getContext($path, 'r');
+        $count = $context['collectionWrapper']->deleteFileAndChunksByFilename($context['file']->filename);
 
-        assert($this->stream instanceof WritableStream);
-
-        return $this->stream->delete() > 0;
+        return $count > 0;
     }
 
     /** @return false|array */
@@ -405,4 +371,39 @@ private function initWritableStream(array $contextOptions): bool
 
         return true;
     }
+
+    /** @return 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);
+            }
+
+            $context = self::$contextResolvers[$bucketAlias]($path, $mode, $context);
+        }
+
+        if (! $context['collectionWrapper'] instanceof CollectionWrapper) {
+            throw LogicException::invalidContextCollectionWrapper($context['collectionWrapper']);
+        }
+
+        return $context;
+    }
 }
diff --git a/src/GridFS/WritableStream.php b/src/GridFS/WritableStream.php
@@ -173,21 +173,6 @@ public function close(): void
         $this->isClosed = true;
     }
 
-    /**
-     * Delete all files and chunks associated with this stream filename.
-     *
-     * @return int The number of deleted files
-     */
-    public function delete(): int
-    {
-        try {
-            return $this->collectionWrapper->deleteFileAndChunksByFilename($this->file['filename']);
-        } finally {
-            // Prevent further operations on this stream
-            $this->abort();
-        }
-    }
-
     /**
      * Return the stream's file document.
      */