Skip to content

Commit 981ede2

Browse files
GromNaNGromNaN
committed
Remove exception from rename and unlink. Return false
1 parent c2bcab1 commit 981ede2

File tree

5 files changed

+38
-34
lines changed

5 files changed

+38
-34
lines changed

‎docs/reference/method/MongoDBGridFSBucket-registerGlobalStreamWrapperAlias.txt

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ Supported stream functions:
4848
- :php:`filesize() <filesize>`
4949
- :php:`file() <file>`
5050
- :php:`fopen() <fopen>` with "r", "rb", "w", and "wb" modes
51-
- :php:`rename() <rename>` rename all revisions of a file in the same bucket
52-
- :php:`unlink() <unlink>` remove all revisions of a file in the same bucket
51+
- :php:`rename() <rename>`
52+
- :php:`unlink() <unlink>`
5353

5454
In read mode, the stream context can contain the option ``gridfs['revision']``
5555
to specify the revision number of the file to read. If omitted, the most recent
@@ -58,6 +58,9 @@ revision is read (revision ``-1``).
5858
In write mode, the stream context can contain the option ``gridfs['chunkSizeBytes']``.
5959
If omitted, the defaults are inherited from the ``Bucket`` instance option.
6060

61+
The functions `rename` and `unlink` will rename or remove all revisions of a
62+
filename. If the filename does not exist, these functions will return ``false``.
63+
6164
Example
6265
-------
6366

‎psalm-baseline.xml

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -97,27 +97,22 @@
9797
</MixedArgument>
9898
</file>
9999
<file src="src/GridFS/StreamWrapper.php">
100-
<InvalidArgument>
100+
<InvalidDocblock>
101+
<code>private function getContext(string $path, string $mode): array</code>
102+
</InvalidDocblock>
103+
<MixedArgumentTypeCoercion>
101104
<code><![CDATA[$this->getContext($path, $mode)]]></code>
102105
<code><![CDATA[$this->getContext($path, $mode)]]></code>
103-
</InvalidArgument>
104-
<InvalidReturnStatement>
105-
<code>$context</code>
106-
</InvalidReturnStatement>
107-
<InvalidReturnType>
108-
<code>array{collectionWrapper: CollectionWrapper, file: object}|array{collectionWrapper: CollectionWrapper, filename: string, options: array</code>
109-
</InvalidReturnType>
110-
<MixedArgument>
111-
<code><![CDATA[$context['file']->filename]]></code>
112-
<code><![CDATA[$context['file']->filename]]></code>
113-
</MixedArgument>
106+
</MixedArgumentTypeCoercion>
114107
<MixedAssignment>
115108
<code>$context</code>
109+
<code>$count</code>
110+
<code>$count</code>
116111
</MixedAssignment>
117-
<PossiblyUndefinedArrayOffset>
118-
<code><![CDATA[$context['file']]]></code>
119-
<code><![CDATA[$context['file']]]></code>
120-
</PossiblyUndefinedArrayOffset>
112+
<MixedMethodCall>
113+
<code>deleteFileAndChunksByFilename</code>
114+
<code>updateFilenameForFilename</code>
115+
</MixedMethodCall>
121116
</file>
122117
<file src="src/Model/BSONArray.php">
123118
<MixedAssignment>

‎src/GridFS/CollectionWrapper.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public function deleteChunksByFilesId($id): void
8383
/**
8484
* Delete all GridFS files and chunks for a given filename.
8585
*/
86-
public function deleteFileAndChunksByFilename(string $filename): int
86+
public function deleteFileAndChunksByFilename(string $filename): ?int
8787
{
8888
/** @var iterable<array{_id: mixed}> $files */
8989
$files = $this->findFiles(['filename' => $filename], [
@@ -100,7 +100,7 @@ public function deleteFileAndChunksByFilename(string $filename): int
100100
$count = $this->filesCollection->deleteMany(['_id' => ['$in' => $ids]])->getDeletedCount();
101101
$this->chunksCollection->deleteMany(['files_id' => ['$in' => $ids]]);
102102

103-
return $count ?? 0;
103+
return $count;
104104
}
105105

106106
/**
@@ -279,12 +279,12 @@ public function insertFile($file): void
279279
/**
280280
* Updates the filename field in the file document for all the files with a given filename.
281281
*/
282-
public function updateFilenameForFilename(string $filename, string $newFilename): UpdateResult
282+
public function updateFilenameForFilename(string $filename, string $newFilename): ?int
283283
{
284284
return $this->filesCollection->updateMany(
285285
['filename' => $filename],
286286
['$set' => ['filename' => $newFilename]],
287-
);
287+
)->getModifiedCount();
288288
}
289289

290290
/**

‎src/GridFS/StreamWrapper.php

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -105,16 +105,14 @@ public function rename(string $fromPath, string $toPath): bool
105105
throw LogicException::renamePathMismatch($fromPath, $toPath);
106106
}
107107

108-
try {
109-
$context = $this->getContext($fromPath, 'r');
110-
} catch (FileNotFoundException $e) {
111-
return false;
112-
}
108+
$context = $this->getContext($fromPath, 'w');
113109

114110
$newFilename = explode('/', $toPath, 4)[3] ?? '';
115-
$context['collectionWrapper']->updateFilenameForFilename($context['file']->filename, $newFilename);
111+
$count = $context['collectionWrapper']->updateFilenameForFilename($context['filename'], $newFilename);
116112

117-
return true;
113+
// If $count === 0, the file does not exist.
114+
// If $count is null, the update is unacknowledged, the operation is considered successful.
115+
return $count !== 0;
118116
}
119117

120118
/**
@@ -297,10 +295,10 @@ public function stream_write(string $data): int
297295

298296
public function unlink(string $path): bool
299297
{
300-
$context = $this->getContext($path, 'r');
301-
$count = $context['collectionWrapper']->deleteFileAndChunksByFilename($context['file']->filename);
298+
$context = $this->getContext($path, 'w');
299+
$count = $context['collectionWrapper']->deleteFileAndChunksByFilename($context['filename']);
302300

303-
return $count > 0;
301+
return $count !== 0;
304302
}
305303

306304
/** @return false|array */
@@ -317,7 +315,7 @@ public function url_stat(string $path, int $flags)
317315
return $this->stream_stat();
318316
}
319317

320-
/** @return array{collectionWrapper: CollectionWrapper, file: object}|array{collectionWrapper: CollectionWrapper, filename: string, options: array} */
318+
/** @psalm-return ($mode == 'r' ? array{collectionWrapper: CollectionWrapper, file: object} : array{collectionWrapper: CollectionWrapper, filename: string, options: array}) */
321319
private function getContext(string $path, string $mode): array
322320
{
323321
$context = [];
@@ -342,6 +340,7 @@ private function getContext(string $path, string $mode): array
342340
throw LogicException::bucketAliasNotRegistered($bucketAlias);
343341
}
344342

343+
/** @see Bucket::resolveStreamContext() */
345344
$context = self::$contextResolvers[$bucketAlias]($path, $mode, $context);
346345
}
347346

‎tests/GridFS/StreamWrapperFunctionalTest.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,10 +375,14 @@ public function testRenameAllRevisions(): void
375375
$this->assertSame(6, file_put_contents($path, 'foobar'));
376376
$this->assertSame(6, file_put_contents($path, 'foobar'));
377377

378-
$this->assertTrue(rename($path, $path . '.renamed'));
378+
$result = rename($path, $path . '.renamed');
379+
$this->assertTrue($result);
379380
$this->assertTrue(file_exists($path . '.renamed'));
380381
$this->assertFalse(file_exists($path));
381382
$this->assertSame('foobar', file_get_contents($path . '.renamed'));
383+
384+
$result = rename($path, $path . '.renamed');
385+
$this->assertFalse($result, 'File does not exist anymore');
382386
}
383387

384388
public function testRenamePathMismatch(): void
@@ -401,5 +405,8 @@ public function testUnlinkAllRevisions(): void
401405

402406
$this->assertTrue($result);
403407
$this->assertFalse(file_exists($path));
408+
409+
$result = unlink($path);
410+
$this->assertFalse($result, 'File does not exist anymore');
404411
}
405412
}

0 commit comments

Comments
 (0)