Skip to content

PHPLIB-1323 Implement unlink for GridFS stream wrapper #1206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +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)

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
Expand Down Expand Up @@ -86,6 +87,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
Expand Down
10 changes: 10 additions & 0 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@
<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>
Expand All @@ -100,6 +105,11 @@
<code>$context</code>
</MixedAssignment>
</file>
<file src="src/GridFS/WritableStream.php">
<MixedArgument>
<code><![CDATA[$this->file['filename']]]></code>
</MixedArgument>
</file>
<file src="src/Model/BSONArray.php">
<MixedAssignment>
<code>$this[$key]</code>
Expand Down
24 changes: 24 additions & 0 deletions src/GridFS/CollectionWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,30 @@ 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], [
'codec' => null,
'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 ?? 0;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is zero in case of WriteConcern=0 or not file to update.

}

/**
* Deletes a GridFS file and related chunks by ID.
*
Expand Down
13 changes: 13 additions & 0 deletions src/GridFS/StreamWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,19 @@ public function stream_write(string $data): int
return $this->stream->writeBytes($data);
}

public function unlink(string $path): bool
{
try {
$this->stream_open($path, 'w', 0, $openedPath);
} catch (FileNotFoundException $e) {
return false;
}

assert($this->stream instanceof WritableStream);

return $this->stream->delete() > 0;
}

/** @return false|array */
public function url_stat(string $path, int $flags)
{
Expand Down
15 changes: 15 additions & 0 deletions src/GridFS/WritableStream.php
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,21 @@ 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']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize this is an internal class, so we're not actually introducing a public API here, but a delete() method that affects multiple revisions seems odd, which is otherwise an abstraction for writing a single revision.

In the PR description, you said:

The context resolver mechanism introduced in #1138 doesn't give direct access to the CollectionWrapper which is required for this feature.

If we look at stream_open, it accesses a CollectionWrapper after resolving the path to an array of context options. Couldn't the same approach be used here to access a CollectionWrapper directly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I felt deep down that using ReadableStream and WritableStream wasn't correct. I refactored by adding a private getContext method to use CollectionWrapper directly for rename and unlink.

} finally {
// Prevent further operations on this stream
$this->abort();
}
}

/**
* Return the stream's file document.
*/
Expand Down
15 changes: 15 additions & 0 deletions tests/GridFS/StreamWrapperFunctionalTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -387,4 +388,18 @@ 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));
}
}