Skip to content

Commit f7ec6f8

Browse files
authored
fix: bug in recursive delete based on glob (#2332)
* fix: bug in recursive delete based on glob * fix: rm on windows"
1 parent 23e7da3 commit f7ec6f8

2 files changed

Lines changed: 82 additions & 5 deletions

File tree

src/lib/filesystem/src/Flow/Filesystem/Local/NativeLocalFilesystem.php

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@
1010
use Flow\Filesystem\Exception\{InvalidArgumentException, InvalidSchemeException, RuntimeException};
1111
use Flow\Filesystem\Path\Filter\OnlyFiles;
1212
use Flow\Filesystem\Stream\{NativeLocalDestinationStream, NativeLocalSourceStream};
13-
use Webmozart\Glob\Iterator\GlobIterator;
13+
use Webmozart\Glob\Glob;
14+
use Webmozart\Glob\Iterator\{GlobFilterIterator, GlobIterator};
1415

1516
/**
1617
* This implementation is based on the native PHP filesystem functions documented here: https://www.php.net/manual/en/book.filesystem.php
@@ -129,7 +130,7 @@ public function rm(Path $path) : bool
129130

130131
$deletedCount = 0;
131132

132-
foreach (new GlobIterator($path->path()) as $filePath) {
133+
foreach ($this->matchChildFirst($path->path()) as $filePath) {
133134
$filePath = type_string()->assert($filePath);
134135

135136
if (\is_dir($filePath)) {
@@ -188,6 +189,38 @@ public function writeTo(Path $path) : DestinationStream
188189
return NativeLocalDestinationStream::openBlank($path);
189190
}
190191

192+
/**
193+
* Lazy iterator over glob matches in CHILD_FIRST order so callers can safely delete each match
194+
* without confusing webmozart/glob's internal RecursiveIteratorIterator (which descends with SELF_FIRST).
195+
*/
196+
private function matchChildFirst(string $glob) : \Iterator
197+
{
198+
$glob = self::canonicalizePath($glob);
199+
$basePath = Glob::getBasePath($glob);
200+
201+
if (!\is_dir($basePath)) {
202+
return new \EmptyIterator();
203+
}
204+
205+
$recursive = new \RecursiveIteratorIterator(
206+
new \RecursiveDirectoryIterator(
207+
$basePath,
208+
\RecursiveDirectoryIterator::CURRENT_AS_PATHNAME | \RecursiveDirectoryIterator::SKIP_DOTS
209+
),
210+
\RecursiveIteratorIterator::CHILD_FIRST
211+
);
212+
213+
return new GlobFilterIterator(
214+
$glob,
215+
(static function () use ($recursive) {
216+
foreach ($recursive as $path) {
217+
yield self::canonicalizePath(type_string()->assert($path));
218+
}
219+
})(),
220+
GlobFilterIterator::FILTER_VALUE
221+
);
222+
}
223+
191224
private function rmdir(string $dirPath) : void
192225
{
193226
if (!\is_dir($dirPath)) {
@@ -221,13 +254,21 @@ private function rmdir(string $dirPath) : void
221254
\rmdir($dirPath);
222255
}
223256

257+
private static function canonicalizePath(string $path) : string
258+
{
259+
return type_string()->cast(\preg_replace('#/+#', '/', \str_replace('\\', '/', $path)));
260+
}
261+
224262
private static function statFor(Path $path, string $absolutePath) : FileStatus
225263
{
226264
$isFile = \is_file($absolutePath);
227-
$size = $isFile ? (\filesize($absolutePath) ?: null) : null;
228265
$mtime = \filemtime($absolutePath);
229-
$lastModifiedAt = $mtime !== false ? new \DateTimeImmutable('@' . $mtime) : null;
230266

231-
return new FileStatus($path, $isFile, $size, $lastModifiedAt);
267+
return new FileStatus(
268+
$path,
269+
$isFile,
270+
$isFile ? (\filesize($absolutePath) ?: null) : null,
271+
$mtime !== false ? new \DateTimeImmutable('@' . $mtime) : null
272+
);
232273
}
233274
}

src/lib/filesystem/tests/Flow/Filesystem/Tests/Integration/NativeLocalFilesystemTest.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,42 @@ public function test_removing_folder_pattern() : void
451451
self::assertNull($fs->status(path(__DIR__ . '/var/nested/orders/orders_01.csv')));
452452
}
453453

454+
public function test_removing_pattern_with_redundant_slashes() : void
455+
{
456+
$fs = native_local_filesystem();
457+
458+
$resource = \fopen(__DIR__ . '/Fixtures/orders.csv', 'rb');
459+
self::assertIsResource($resource);
460+
$fs->writeTo(path(__DIR__ . '/var/redundant_slash/file.txt'))->fromResource($resource);
461+
462+
$fs->rm(path(__DIR__ . '/var/redundant_slash//*.txt'));
463+
464+
self::assertNull($fs->status(path(__DIR__ . '/var/redundant_slash/file.txt')));
465+
}
466+
467+
public function test_removing_recursive_pattern_with_nested_directories() : void
468+
{
469+
$fs = native_local_filesystem();
470+
471+
$resource1 = \fopen(__DIR__ . '/Fixtures/orders.csv', 'rb');
472+
self::assertIsResource($resource1);
473+
$fs->writeTo(path(__DIR__ . '/var/recursive_rm/dir_a/file1.txt'))->fromResource($resource1);
474+
475+
$resource2 = \fopen(__DIR__ . '/Fixtures/orders.csv', 'rb');
476+
self::assertIsResource($resource2);
477+
$fs->writeTo(path(__DIR__ . '/var/recursive_rm/dir_a/nested/file2.txt'))->fromResource($resource2);
478+
479+
$resource3 = \fopen(__DIR__ . '/Fixtures/orders.csv', 'rb');
480+
self::assertIsResource($resource3);
481+
$fs->writeTo(path(__DIR__ . '/var/recursive_rm/dir_b/file3.txt'))->fromResource($resource3);
482+
483+
$fs->rm(path(__DIR__ . '/var/recursive_rm/**/*'));
484+
485+
self::assertNull($fs->status(path(__DIR__ . '/var/recursive_rm/dir_a/file1.txt')));
486+
self::assertNull($fs->status(path(__DIR__ . '/var/recursive_rm/dir_a/nested/file2.txt')));
487+
self::assertNull($fs->status(path(__DIR__ . '/var/recursive_rm/dir_b/file3.txt')));
488+
}
489+
454490
public function test_rm_rejects_mismatched_scheme() : void
455491
{
456492
$this->expectException(InvalidSchemeException::class);

0 commit comments

Comments
 (0)