8000 bug #50884 [Finder] Fix initial directory is opened twice (mvorisek) · symfony/symfony@99fa865 · GitHub
[go: up one dir, main page]
Skip to content

Commit 99fa865

Browse files
nicolas-grekasnicolas-grekas
committed
bug #50884 [Finder] Fix initial directory is opened twice (mvorisek)
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [Finder] Fix initial directory is opened twice | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #50851 | License | MIT | Doc PR | no The issue was introduced in #8120 which was coded wrongly for several reasons: - `seekable` support is about file/stream content seeking, not about rewind support, non-seekable stream handler can support rewind - it worked by coincidence - the original code involved additional directory open (syscall), which is/was slow - 1st rewind of DirectoryIterator is implicit, ie. it is not needed - again, skipping the rewind worked by coincidence - 2nd+ rewind must fail if it is not supported, the original code silently skipped it The test from #8120 is kept and is passing. I also improved the ftp support detection to make the test stricter. To support rewind for ftp I opened - php/php-src#11597 - but this Symfony PR does not need it. Commits ------- d3f03be [Finder] Fix initial directory is opened twice
2 parents 9f300e9 + d3f03be commit 99fa865

File tree

2 files changed

+22
-34
lines changed

2 files changed

+22
-34
lines changed

‎src/Symfony/Component/Finder/Iterator/RecursiveDirectoryIterator.php‎

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class RecursiveDirectoryIterator extends \RecursiveDirectoryIterator
2929
/**
3030
* @var bool
3131
*/
32-
private $rewindable;
32+
private $ignoreFirstRewind = true;
3333

3434
// these 3 properties take part of the performance optimization to avoid redoing the same work in all iterations
3535
private $rootPath;
@@ -118,7 +118,6 @@ public function getChildren()
118118
$children->ignoreUnreadableDirs = $this->ignoreUnreadableDirs;
119119

120120
// performance optimization to avoid redoing the same work in all children
121-
$children->rewindable = &$this->rewindable;
122121
$children->rootPath = $this->rootPath;
123122
}
124123

@@ -129,40 +128,30 @@ public function getChildren()
129128
}
130129

131130
/**
132-
* Do nothing for non rewindable stream.
133-
*
134131
* @return void
135132
*/
136133
#[\ReturnTypeWillChange]
137-
public function rewind()
134+
public function next()
138135
{
139-
if (false === $this->isRewindable()) {
140-
return;
141-
}
136+
$this->ignoreFirstRewind = false;
142137

143-
parent::rewind();
138+
parent::next();
144139
}
145140

146141
/**
147-
* Checks if the stream is rewindable.
148-
*
149-
* @return bool
142+
* @return void
150143
*/
151-
public function isRewindable()
144+
#[\ReturnTypeWillChange]
145+
public function rewind()
152146
{
153-
if (null !== $this->rewindable) {
154-
return $this->rewindable;
155-
}
156-
157-
if (false !== $stream = @opendir($this->getPath())) {
158-
$infos = stream_get_meta_data($stream);
159-
closedir($stream);
147+
// some streams like FTP are not rewindable, ignore the first rewind after creation,
148+
// as newly created DirectoryIterator does not need to be rewound
149+
if ($this->ignoreFirstRewind) {
150+
$this->ignoreFirstRewind = false;
160151

161-
if ($infos['seekable']) {
162-
return $this->rewindable = true;
163-
}
152+
return;
164153
}
165154

166-
return $this->rewindable = false;
155+
parent::rewind();
167156
}
168157
}

‎src/Symfony/Component/Finder/Tests/Iterator/RecursiveDirectoryIteratorTest.php‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,19 @@
1515

1616
class RecursiveDirectoryIteratorTest extends IteratorTestCase
1717
{
18+
protected function setUp(): void
19+
{
20+
if (!\in_array('ftp', stream_get_wrappers(), true) || !\ini_get('allow_url_fopen')) {
21+
$this->markTestSkipped('Unsupported stream "ftp".');
22+
}
23+
}
24+
1825
/**
1926
* @group network
2027
*/
2128
public function testRewindOnFtp()
2229
{
23-
try {
24-
$i = new RecursiveDirectoryIterator('ftp://speedtest:speedtest@ftp.otenet.gr/', \RecursiveDirectoryIterator::SKIP_DOTS);
25-
} catch (\UnexpectedValueException $e) {
26-
$this->markTestSkipped('Unsupported stream "ftp".');
27-
}
30+
$i = new RecursiveDirectoryIterator('ftp://speedtest:speedtest@ftp.otenet.gr/', \RecursiveDirectoryIterator::SKIP_DOTS);
2831

2932
$i->rewind();
3033

@@ -36,11 +39,7 @@ public function testRewindOnFtp()
3639
*/
3740
public function testSeekOnFtp()
3841
{
39-
try {
40-
$i = new RecursiveDirectoryIterator('ftp://speedtest:speedtest@ftp.otenet.gr/', \RecursiveDirectoryIterator::SKIP_DOTS);
41-
} catch (\UnexpectedValueException $e) {
42-
$this->markTestSkipped('Unsupported stream "ftp".');
43-
}
42+
$i = new RecursiveDirectoryIterator('ftp://speedtest:speedtest@ftp.otenet.gr/', \RecursiveDirectoryIterator::SKIP_DOTS);
4443

4544
$contains = [
4645
'ftp://speedtest:speedtest@ftp.otenet.gr'.\DIRECTORY_SEPARATOR.'test100Mb.db',

0 commit comments

Comments
 (0)
0