From 9be281d3f3eb979445d21fac8a8785b26e461c6c Mon Sep 17 00:00:00 2001 From: Rowan Tommins Date: Tue, 25 Feb 2020 22:33:42 +0000 Subject: [PATCH 1/3] Add some (failing) tests for Issue 222 (writing streams and feof()) --- tests/phpunit/vfsStreamWrapperTestCase.php | 94 ++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/tests/phpunit/vfsStreamWrapperTestCase.php b/tests/phpunit/vfsStreamWrapperTestCase.php index e2fa370b..b2b4805a 100644 --- a/tests/phpunit/vfsStreamWrapperTestCase.php +++ b/tests/phpunit/vfsStreamWrapperTestCase.php @@ -904,4 +904,98 @@ public function multipleWritesOnSameFileHaveDifferentPointers(): void assertThat(file_get_contents($url), equals($contentB . $contentA)); } + + /** + * @test + */ + public function readsAndWritesOnSameFileHaveDifferentPointers(): void + { + $contentA = uniqid('a'); + $contentB = uniqid('b'); + $url = $this->fileInSubdir->url(); + + $fp1 = fopen($url, 'wb'); + $fp2 = fopen($url, 'rb'); + + fwrite($fp1, $contentA); + $contentBeforeWrite = fread($fp2, strlen($contentA)); + + fwrite($fp1, $contentB); + $contentAfterWrite = fread($fp2, strlen($contentB)); + + fclose($fp1); + fclose($fp2); + + assertThat($contentBeforeWrite, equals($contentA)); + assertThat($contentAfterWrite, equals($contentB)); + } + + /** + * @test + */ + public function feofIsFalseWhenEmptyFileOpened(): void + { + $this->fileInSubdir->setContent(''); + + $stream = fopen($this->fileInSubdir->url(), 'r'); + + assertFalse(feof($stream)); + } + + /** + * @test + */ + public function feofIsTrueAfterEmptyFileRead(): void + { + $this->fileInSubdir->setContent(''); + + $stream = fopen($this->fileInSubdir->url(), 'r'); + + fgets($stream); + + assertTrue(feof($stream)); + } + + /** + * @test + */ + public function feofIsFalseWhenEmptyStreamRewound(): void + { + $this->fileInSubdir->setContent(''); + + $stream = fopen($this->fileInSubdir->url(), 'r'); + + fgets($stream); + rewind($stream); + assertFalse(feof($stream)); + } + + /** + * @test + */ + public function feofIsFalseAfterReadingLastLine(): void + { + $this->fileInSubdir->setContent("Line 1\n"); + + $stream = fopen($this->fileInSubdir->url(), 'r'); + + fgets($stream); + + assertFalse(feof($stream)); + } + + /** + * @test + */ + public function feofIsTrueAfterReadingBeyondLastLine(): void + { + $this->fileInSubdir->setContent("Line 1\n"); + + $stream = fopen($this->fileInSubdir->url(), 'r'); + + fgets($stream); + fgets($stream); + + assertTrue(feof($stream)); + } } From e4a597210fb876c1ca090f4a6e40057b5761b7ca Mon Sep 17 00:00:00 2001 From: Rowan Tommins Date: Wed, 26 Feb 2020 12:35:42 +0000 Subject: [PATCH 2/3] Fix for Issue #222: handle EOF and offset correctly on read The EOF flag on native file streams is only set after reading *past* the end of the buffer, never on open or seek. I believe all the failing tests here are asserting behaviour that doesn't match the native file handler. --- src/OpenedFile.php | 2 +- src/content/FileContent.php | 2 +- src/content/SeekableFileContent.php | 25 +++++++++++++++++++++---- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/OpenedFile.php b/src/OpenedFile.php index f92eea72..68b12805 100644 --- a/src/OpenedFile.php +++ b/src/OpenedFile.php @@ -261,7 +261,7 @@ public function getGroup(): int private function restorePosition(): void { - $this->base->getContentObject()->seek($this->position, SEEK_SET); + $this->base->getContentObject()->seek($this->position, SEEK_SET, false); } private function savePosition(): void diff --git a/src/content/FileContent.php b/src/content/FileContent.php index 900e4448..b30e1c59 100644 --- a/src/content/FileContent.php +++ b/src/content/FileContent.php @@ -38,7 +38,7 @@ public function read(int $count): string; /** * seeks to the given offset */ - public function seek(int $offset, int $whence): bool; + public function seek(int $offset, int $whence, bool $resetEof = true): bool; /** * checks whether pointer is at end of file diff --git a/src/content/SeekableFileContent.php b/src/content/SeekableFileContent.php index 0597d3a9..1a22942d 100644 --- a/src/content/SeekableFileContent.php +++ b/src/content/SeekableFileContent.php @@ -32,13 +32,26 @@ abstract class SeekableFileContent implements FileContent */ private $offset = 0; - /** + /** + * has the stream reached EOF; this happens on the first read *after* seeking to the end of the file + * + * @var bool + */ + private $eof=false; + + /** * reads the given amount of bytes from content */ public function read(int $count): string { + // If offset is already at or past end of file, set EOF flag + if ( $this->offset >= $this->size() ) { + $this->eof = true; + return ''; + } + $data = $this->doRead($this->offset, $count); - $this->offset += $count; + $this->offset += strlen($data); return $data; } @@ -51,7 +64,7 @@ abstract protected function doRead(int $offset, int $count): string; /** * seeks to the given offset */ - public function seek(int $offset, int $whence): bool + public function seek(int $offset, int $whence, bool $resetEof = true): bool { $newOffset = $this->offset; switch ($whence) { @@ -76,6 +89,10 @@ public function seek(int $offset, int $whence): bool } $this->offset = $newOffset; + // EOF is always false after a manual seek + if ( $resetEof ) { + $this->eof = false; + } return true; } @@ -85,7 +102,7 @@ public function seek(int $offset, int $whence): bool */ public function eof(): bool { - return $this->size() <= $this->offset; + return $this->eof; } /** From 28c71a792b95a00d8774f82f62542b1e8274931e Mon Sep 17 00:00:00 2001 From: Rowan Tommins Date: Wed, 26 Feb 2020 13:16:44 +0000 Subject: [PATCH 3/3] Issue #222: Fix tests which incorrectly modelled EOF & offset Testing with normal files show that fread() does not progress the pointer beyond the end of the file, and a read *beyond* the file size is required before feof() returns true. --- .../StringBasedFileContentTestCase.php | 24 +++++++++++++++++-- tests/phpunit/vfsStreamFileTestCase.php | 21 +++++++--------- .../phpunit/vfsStreamWrapperFileTestCase.php | 1 + 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/tests/phpunit/content/StringBasedFileContentTestCase.php b/tests/phpunit/content/StringBasedFileContentTestCase.php index c1075913..a548116f 100644 --- a/tests/phpunit/content/StringBasedFileContentTestCase.php +++ b/tests/phpunit/content/StringBasedFileContentTestCase.php @@ -125,18 +125,38 @@ public function readLessThenSizeDoesNotReachEof(): void /** * @test */ - public function readSizeReachesEof(): void + public function readSizeReachesDoesNotReachEof(): void { $this->stringBasedFileContent->read(9); + assertFalse($this->stringBasedFileContent->eof()); + } + + /** + * @test + */ + public function readSizeReachesEofOnNextRead(): void + { + $this->stringBasedFileContent->read(9); + $this->stringBasedFileContent->read(1); assertTrue($this->stringBasedFileContent->eof()); } /** * @test */ - public function readMoreThanSizeReachesEof(): void + public function readMoreThanSizeDoesNotReachEof(): void + { + $this->stringBasedFileContent->read(10); + assertFalse($this->stringBasedFileContent->eof()); + } + + /** + * @test + */ + public function readMoreThanSizeReachesEofOnNextRead(): void { $this->stringBasedFileContent->read(10); + $this->stringBasedFileContent->read(1); assertTrue($this->stringBasedFileContent->eof()); } diff --git a/tests/phpunit/vfsStreamFileTestCase.php b/tests/phpunit/vfsStreamFileTestCase.php index 31d555ec..23f33f69 100644 --- a/tests/phpunit/vfsStreamFileTestCase.php +++ b/tests/phpunit/vfsStreamFileTestCase.php @@ -145,9 +145,9 @@ public function contentCanBeChanged(): void /** * @test */ - public function isAtEofWhenEmpty(): void + public function isNotAtEofWhenEmpty(): void { - assertTrue($this->file->eof()); + assertFalse($this->file->eof()); } /** @@ -169,19 +169,10 @@ public function readFromEmptyFileReturnsEmptyString(): void /** * @test */ - public function readFromEmptyFileMovesPointer(): void + public function readFromEmptyFileDoesNotMovePointer(): void { $this->file->read(5); - assertThat($this->file->getBytesRead(), equals(5)); - } - - /** - * @test - */ - public function reportsAmountOfBytesReadEvenWhenEmpty(): void - { - $this->file->read(5); - assertThat($this->file->getBytesRead(), equals(5)); + assertThat($this->file->getBytesRead(), equals(0)); } /** @@ -228,6 +219,10 @@ public function partialReads(): void assertThat($this->file->read(3), equals('baz')); assertThat($this->file->getBytesRead(), equals(9)); + assertFalse($this->file->eof()); + + assertThat($this->file->read(1), equals('')); + assertThat($this->file->getBytesRead(), equals(9)); assertTrue($this->file->eof()); } diff --git a/tests/phpunit/vfsStreamWrapperFileTestCase.php b/tests/phpunit/vfsStreamWrapperFileTestCase.php index f213cde8..efaaca17 100644 --- a/tests/phpunit/vfsStreamWrapperFileTestCase.php +++ b/tests/phpunit/vfsStreamWrapperFileTestCase.php @@ -172,6 +172,7 @@ public function recognizesEof(): void { $fp = fopen($this->fileInSubdir->url(), 'r'); fseek($fp, 1, SEEK_END); + fread($fp, 1); assertTrue(feof($fp)); fclose($fp); }