-
Notifications
You must be signed in to change notification settings - Fork 264
PHPLIB-1568 Add GridFS\Bucket::deleteByName(filename)
and renameByName(filename, newFilename)
#1504
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
Conversation
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class is internal, we can change the return type. This is always an int
, an exception is thrown in case of error.
['filename' => $filename], | ||
['$set' => ['filename' => $newFilename]], | ||
)->getMatchedCount()]]></code> | ||
</NullableReturnStatement> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Boths additions will be removed in v2.x, thanks to #1454
$filename = $filename; | ||
$revision = $revision; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought I had already removed that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted that this remained when 1b29a6d introduced type hints on the method.
GridFS\Bucket::deleteByName(filename)
GridFS\Bucket::deleteByName(filename)
and renameByName(filename, newFilename)
$filename = $filename; | ||
$revision = $revision; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted that this remained when 1b29a6d introduced type hints on the method.
@@ -160,6 +160,41 @@ public function testDeleteStillRemovesChunksIfFileDoesNotExist($input, $expected | |||
$this->assertCollectionCount($this->chunksCollection, 0); | |||
} | |||
|
|||
public function testDeleteByName(): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this Pr block on spec changes for DRIVERS-2807 and DRIVERS-2808? Also, will the PR(s) for those tickets include spec tests to be synced?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spec tests added in mongodb/specifications#1702
b9d5dc0
to
a955d57
Compare
Fix PHPLIB-1568
GridFS\Bucket::deleteByName($filename)
Use the
CollectionWrapper::deleteFileAndChunksByFilename()
introduced for the need of theStreamWrapper::unlink()
.unlink
for GridFS stream wrapper #1206Implementation:
If a DB error occurs during 1, nothing happened.
If a DB error occurs during 2 or 3, some chunks may become orphan and we cannot retry as the mapping chunks-id-to-file-name is missing.
There could be an issue if the list of ids is too big. In that case, we would have to work by batches.
GridFS\Bucket::renameByName($filename)
Use the
CollectionWrapper::updateFilenameForFilename()
introduced for the need ofStreamWrapper::rename()
rename
for GridFS stream wrapper #1207Implementation: