From a9259c8f318d137ac20e891f21fc210204250467 Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Mon, 20 Apr 2026 20:45:03 +0200 Subject: [PATCH 1/9] Have Buffer class implement the Seekable interface --- src/main/php/io/streams/Buffer.class.php | 128 +++++++++----- src/test/php/io/unittest/BufferTest.class.php | 160 +++++++++++++++--- 2 files changed, 221 insertions(+), 67 deletions(-) diff --git a/src/main/php/io/streams/Buffer.class.php b/src/main/php/io/streams/Buffer.class.php index 5e76a7bef..d685513c5 100755 --- a/src/main/php/io/streams/Buffer.class.php +++ b/src/main/php/io/streams/Buffer.class.php @@ -1,7 +1,7 @@ = 0'); } - - $this->files= $files instanceof Folder ? $files->getURI() : (string)$files; $this->threshold= $threshold; + $this->persist= $persist; + + if ($files instanceof File) { + $this->files= fn() => $files; + } else if ($files instanceof Path && $files->isFile()) { + $this->files= fn() => $files->asFile(); + } else if ($files instanceof Folder) { + $this->files= fn() => new File(tempnam($files->getURI(), "b{$this->threshold}")); + } else { + $this->files= fn() => new File(tempnam((string)$files, "b{$this->threshold}")); + } } /** Returns buffer size */ @@ -41,31 +50,41 @@ public function size(): int { return $this->size; } public function file(): ?File { return $this->file; } /** Returns whether this buffer is draining */ - public function draining(): bool { return $this->draining; } + public function draining(): bool { return ($this->file ? $this->file->tell() : $this->pointer) > 0; } /** * Write a string * - * @param var $arg + * @param string $bytes * @return void - * @throws lang.IllegalStateException */ public function write($bytes) { - if ($this->draining) throw new IllegalStateException('Started draining buffer'); - - $this->size+= strlen($bytes); - if ($this->size <= $this->threshold) { - $this->memory.= $bytes; - return; - } - - if (null === $this->file) { - $this->file= new File(tempnam($this->files, "b{$this->threshold}")); - $this->file->open(File::READWRITE); - $this->file->write($this->memory); - $this->memory= null; + $length= strlen($bytes); + + if ($this->size + $length <= $this->threshold) { + $tail= strlen($this->memory); + if ($this->pointer < $tail) { + $this->memory= substr_replace($this->memory, $bytes, $this->pointer, $length); + } else if ($this->pointer > $tail) { + $this->memory.= str_repeat("\x00", $this->pointer - $tail).$bytes; + } else { + $this->memory.= $bytes; + } + + $this->pointer+= $length; + $this->size= strlen($this->memory); + } else { + if (null === $this->file) { + $this->file= ($this->files)(); + $this->file->open(File::REWRITE); + $this->file->write($this->memory); + $this->file->seek($this->pointer, SEEK_SET); + $this->memory= null; + } + + $this->file->write($bytes); + $this->size= $this->file->size(); } - $this->file->write($bytes); } /** @return void */ @@ -73,22 +92,9 @@ public function flush() { $this->file && $this->file->flush(); } - /** - * Resets buffer to be able to read from the beginning - * - * @return void - */ - public function reset() { - $this->file ? $this->file->seek(0, SEEK_SET) : $this->pointer= 0; - $this->draining= true; - } - /** @return int */ public function available() { - return $this->draining - ? $this->size - ($this->file ? $this->file->tell() : $this->pointer) - : $this->size - ; + return $this->size - ($this->file ? $this->file->tell() : $this->pointer); } /** @@ -99,22 +105,58 @@ public function available() { */ public function read($limit= 8192) { if ($this->file) { - $this->draining || $this->file->seek(0, SEEK_SET) && $this->draining= true; return (string)$this->file->read($limit); } else { - $this->draining= true; $chunk= substr($this->memory, $this->pointer, $limit); $this->pointer+= strlen($chunk); return $chunk; } } + /** + * Resets buffer to be able to read from the beginning. Optimized + * form of calling `seek(0, SEEK_SET)`. + * + * @return void + */ + public function reset() { + $this->file ? $this->file->seek(0, SEEK_SET) : $this->pointer= 0; + } + + /** + * Seeks to a given offset. + * + * @param int $offset + * @param int $whence SEEK_SET, SEEK_CUR or SEEK_END + * @return void + * @throws io.IOException + */ + public function seek($offset, $whence= SEEK_SET) { + switch ($whence) { + case SEEK_SET: $position= $offset; break; + case SEEK_CUR: $position= ($this->file ? $this->file->tell() : $this->pointer) + $offset; break; + case SEEK_END: $position= $this->size + $offset; break; + default: $position= -1; break; + } + + if ($position < 0) { + throw new IOException("Seek error, position {$offset} in mode {$whence}"); + } + + $this->file ? $this->file->seek($position, SEEK_SET) : $this->pointer= $position; + } + + /** @return int */ + public function tell() { + return $this->file ? $this->file->tell() : $this->pointer; + } + /** @return void */ public function close() { if (null === $this->file || !$this->file->isOpen()) return; $this->file->close(); - $this->file->unlink(); + $this->persist || $this->file->unlink(); } /** Ensure the file (if any) is closed */ diff --git a/src/test/php/io/unittest/BufferTest.class.php b/src/test/php/io/unittest/BufferTest.class.php index 6ce774141..4defe5af3 100755 --- a/src/test/php/io/unittest/BufferTest.class.php +++ b/src/test/php/io/unittest/BufferTest.class.php @@ -1,35 +1,62 @@ temp= Environment::tempDir(); + /** Creates a new fixture */ + private function newFixture(int $threshold= self::THRESHOLD, bool $persist= false): Buffer { + return new Buffer(Environment::tempDir(), $threshold, $persist); + } + + /** @return iterable */ + private function directories() { + $temp= Environment::tempDir(); + + yield [$temp]; + yield [new Path($temp)]; + yield [new Folder($temp)]; + } + + #[Test, Values(from: 'directories')] + public function with_directory($files) { + new Buffer($files, self::THRESHOLD); + } + + #[Test] + public function with_temp_file() { + $t= new TempFile(); + $fixture= new Buffer($t, 0); + $fixture->write('Test'); + + Assert::equals($t, $fixture->file()); } #[Test] - public function can_create() { - new Buffer($this->temp, self::THRESHOLD); + public function with_temp_path() { + $t= new TempFile(); + $fixture= new Buffer(new Path($t), 0); + $fixture->write('Test'); + + Assert::equals($t, $fixture->file()); } #[Test] public function threshold_must_be_larger_than_zero() { - Assert::throws(IllegalArgumentException::class, fn() => new Buffer($this->temp, -1)); + Assert::throws(IllegalArgumentException::class, fn() => $this->newFixture(-1)); } #[Test, Values([1, 127, 128])] public function uses_memory_under_threshold($length) { $bytes= str_repeat('*', $length); - $fixture= new Buffer($this->temp, self::THRESHOLD); + $fixture= $this->newFixture(); $fixture->write($bytes); + $fixture->reset(); Assert::null($fixture->file()); Assert::equals($length, $fixture->size()); @@ -40,8 +67,9 @@ public function uses_memory_under_threshold($length) { public function stores_file_when_exceeding_threshold($length) { $bytes= str_repeat('*', $length); - $fixture= new Buffer($this->temp, self::THRESHOLD); + $fixture= $this->newFixture(); $fixture->write($bytes); + $fixture->reset(); Assert::instance(File::class, $fixture->file()); Assert::equals($length, $fixture->size()); @@ -52,8 +80,9 @@ public function stores_file_when_exceeding_threshold($length) { public function read_after_eof($length) { $bytes= str_repeat('*', $length); - $fixture= new Buffer($this->temp, self::THRESHOLD); + $fixture= $this->newFixture(); $fixture->write($bytes); + $fixture->reset(); Assert::equals($length, $fixture->available()); Assert::equals($bytes, $fixture->read()); @@ -65,41 +94,124 @@ public function read_after_eof($length) { public function reset($length) { $bytes= str_repeat('*', $length); - $fixture= new Buffer($this->temp, self::THRESHOLD); + $fixture= $this->newFixture(); $fixture->write($bytes); - Assert::equals($length, $fixture->available()); - Assert::equals($bytes, $fixture->read()); + // At EOF, there is nothing to read + Assert::equals(0, $fixture->available()); + Assert::equals('', $fixture->read()); $fixture->reset(); + // Back at the beginning of the file Assert::equals($length, $fixture->available()); Assert::equals($bytes, $fixture->read()); } + #[Test, Values([3, 4, 5, 6, 7])] + public function write_after_read_with($threshold) { + $fixture= $this->newFixture($threshold); + + $fixture->write('Test'); + $fixture->seek(-4, SEEK_CUR); + Assert::equals('Test', $fixture->read()); + + $fixture->write('ed'); + $fixture->seek(-2, SEEK_CUR); + Assert::equals('ed', $fixture->read()); + } + #[Test] - public function cannot_write_after_draining_started() { - $fixture= new Buffer($this->temp, self::THRESHOLD); + public function file_created_on_write() { + $fixture= $this->newFixture(0); $fixture->write('Test'); - Assert::false($fixture->draining()); - $fixture->read(); - Assert::true($fixture->draining()); - Assert::throws(IllegalStateException::class, fn() => $fixture->write('Test')); + Assert::true($fixture->file()->exists()); } #[Test] public function file_deleted_on_close() { - $fixture= new Buffer($this->temp, 0); + $fixture= $this->newFixture(0); $fixture->write('Test'); + Assert::true($fixture->file()->exists()); $fixture->close(); Assert::false($fixture->file()->exists()); } + #[Test] + public function file_kept_on_close_with_persist() { + $fixture= $this->newFixture(0, true); + $fixture->write('Test'); + Assert::true($fixture->file()->exists()); + + $fixture->close(); + Assert::true($fixture->file()->exists()); + } + + #[Test, Values([0, 128])] + public function tell_after_write_with($threshold) { + $fixture= $this->newFixture($threshold); + $fixture->write('Test success'); + + Assert::equals(12, $fixture->tell()); + Assert::equals('', $fixture->read()); + } + + #[Test, Values([0, 128])] + public function tell_after_read_with($threshold) { + $fixture= $this->newFixture($threshold); + $fixture->write('Test success'); + $fixture->reset(); + $fixture->read(5); + + Assert::equals(5, $fixture->tell()); + Assert::equals('success', $fixture->read()); + } + + #[Test, Values([0, 128])] + public function seek_set_with($threshold) { + $fixture= $this->newFixture($threshold); + $fixture->write('Test success'); + $fixture->seek(5, SEEK_SET); + + Assert::equals(5, $fixture->tell()); + Assert::equals('success', $fixture->read()); + } + + #[Test, Values([0, 128])] + public function seek_cur_with($threshold) { + $fixture= $this->newFixture($threshold); + $fixture->write('Test success'); + $fixture->seek(-7, SEEK_CUR); + + Assert::equals(5, $fixture->tell()); + Assert::equals('success', $fixture->read()); + } + + #[Test, Values([0, 128])] + public function seek_end_with($threshold) { + $fixture= $this->newFixture($threshold); + $fixture->write('Test success'); + + $fixture->seek(-7, SEEK_END); + Assert::equals(5, $fixture->tell()); + Assert::equals('success', $fixture->read()); + } + + #[Test] + public function cannot_seek_before_start() { + Assert::throws(IOException::class, fn() => $this->newFixture()->seek(-1)); + } + + #[Test] + public function cannot_seek_invalid_whence() { + Assert::throws(IOException::class, fn() => $this->newFixture()->seek(0, 6100)); + } + #[Test] public function double_close() { - $fixture= new Buffer($this->temp, 0); + $fixture= $this->newFixture(0); $fixture->write('Test'); $fixture->close(); From e7e21b8663e64339038daa2b4ffa8c0e3016ceae Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Mon, 20 Apr 2026 20:54:22 +0200 Subject: [PATCH 2/9] Check if file exists before deleting it If an `io.TempFile` instance is passed, its destructor may have deleted the file itself --- src/main/php/io/streams/Buffer.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/php/io/streams/Buffer.class.php b/src/main/php/io/streams/Buffer.class.php index d685513c5..238e2c1df 100755 --- a/src/main/php/io/streams/Buffer.class.php +++ b/src/main/php/io/streams/Buffer.class.php @@ -156,7 +156,7 @@ public function close() { if (null === $this->file || !$this->file->isOpen()) return; $this->file->close(); - $this->persist || $this->file->unlink(); + $this->persist || $this->file->exists() && $this->file->unlink(); } /** Ensure the file (if any) is closed */ From 984310dde6ec40d196ef8ea2c549af169440589f Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Mon, 20 Apr 2026 21:13:07 +0200 Subject: [PATCH 3/9] Extend io.TempFile's constructor to accept location --- src/main/php/io/TempFile.class.php | 26 ++++++++++++++----- .../php/io/unittest/TempFileTest.class.php | 17 ++++++++++-- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/main/php/io/TempFile.class.php b/src/main/php/io/TempFile.class.php index c5a09de0a..f110f27ef 100755 --- a/src/main/php/io/TempFile.class.php +++ b/src/main/php/io/TempFile.class.php @@ -16,9 +16,9 @@ * printf('Created temporary file "%s"', $f->getURI()); * ``` * - * Note: The temporary file is not deleted when the file - * handle is closed (e.g., a call to close()), this will have - * to be done manually. + * The temporary file is deleted when the object representing + * it goes of out scope and is garbage-collected. To keep the + * file, use the `persistent()` method. * * Note: A possible race condition exists: From the time the * file name string is created (when the constructor is called) @@ -34,9 +34,23 @@ class TempFile extends File { private $persistent= false; - /** @param string $prefix default "tmp" */ - public function __construct($prefix= 'tmp') { - parent::__construct(tempnam(Environment::tempDir(), $prefix.uniqid(microtime(true)))); + /** + * Creates a new temporary file with a given prefix. Uses the environment's + * temporary directory (typically `$TEMP`) if no other location is supplied. + * + * @param string $prefix + * @param ?string|io.Path|io.Folder $location + */ + public function __construct($prefix= 'tmp', $location= null) { + if (null === $location) { + $directory= Environment::tempDir(); + } else if ($location instanceof Folder) { + $directory= $location->getURI(); + } else { + $directory= (string)$location; + } + + parent::__construct(tempnam($directory, $prefix.uniqid(microtime(true)))); } /** diff --git a/src/test/php/io/unittest/TempFileTest.class.php b/src/test/php/io/unittest/TempFileTest.class.php index 727b921c5..a0d7e0b3d 100755 --- a/src/test/php/io/unittest/TempFileTest.class.php +++ b/src/test/php/io/unittest/TempFileTest.class.php @@ -1,11 +1,18 @@ getPath())); } + #[Test, Values(from: 'directories')] + public function supply_tempdir($location) { + $t= new TempFile('tmp', $location); + Assert::equals(realpath('.'), realpath($t->getPath())); + } + #[Test] public function filename_begins_with_prefix() { $t= new TempFile('pre'); From bfbb539a635afe2f12224f46de0880f82814ae70 Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Mon, 20 Apr 2026 21:16:55 +0200 Subject: [PATCH 4/9] Simplify code inside constructor --- src/main/php/io/streams/Buffer.class.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/php/io/streams/Buffer.class.php b/src/main/php/io/streams/Buffer.class.php index 238e2c1df..3897d5407 100755 --- a/src/main/php/io/streams/Buffer.class.php +++ b/src/main/php/io/streams/Buffer.class.php @@ -1,6 +1,6 @@ files= fn() => $files; } else if ($files instanceof Path && $files->isFile()) { $this->files= fn() => $files->asFile(); - } else if ($files instanceof Folder) { - $this->files= fn() => new File(tempnam($files->getURI(), "b{$this->threshold}")); } else { - $this->files= fn() => new File(tempnam((string)$files, "b{$this->threshold}")); + $this->files= fn() => new TempFile("b{$this->threshold}", $files); } } From ec086eaaba85b07ae08ea84610f7d754b09939da Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Mon, 20 Apr 2026 21:18:28 +0200 Subject: [PATCH 5/9] Remove draining() method --- src/main/php/io/streams/Buffer.class.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/php/io/streams/Buffer.class.php b/src/main/php/io/streams/Buffer.class.php index 3897d5407..7250207cd 100755 --- a/src/main/php/io/streams/Buffer.class.php +++ b/src/main/php/io/streams/Buffer.class.php @@ -47,9 +47,6 @@ public function size(): int { return $this->size; } /** Returns the underlying file, if any */ public function file(): ?File { return $this->file; } - /** Returns whether this buffer is draining */ - public function draining(): bool { return ($this->file ? $this->file->tell() : $this->pointer) > 0; } - /** * Write a string * From 17036fd0406f96adfd212d3d8f148f0ddbca45e1 Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Mon, 20 Apr 2026 21:26:13 +0200 Subject: [PATCH 6/9] Test integration with io.Blob --- src/test/php/io/unittest/BufferTest.class.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/test/php/io/unittest/BufferTest.class.php b/src/test/php/io/unittest/BufferTest.class.php index 4defe5af3..868413dd4 100755 --- a/src/test/php/io/unittest/BufferTest.class.php +++ b/src/test/php/io/unittest/BufferTest.class.php @@ -1,7 +1,7 @@ close(); $fixture->close(); } + + #[Test] + public function integrated_with_blob_api() { + $fixture= $this->newFixture(); + $fixture->write('Test'); + $fixture->reset(); + + Assert::equals('Test', (string)(new Blob($fixture))->bytes()); + } } \ No newline at end of file From af95afd0abe1da0f31467c6a46886f890e2b80ae Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Mon, 20 Apr 2026 21:29:22 +0200 Subject: [PATCH 7/9] As buffers are seekable, test accessing bytes twice --- src/test/php/io/unittest/BufferTest.class.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/php/io/unittest/BufferTest.class.php b/src/test/php/io/unittest/BufferTest.class.php index 868413dd4..4f1cd6a11 100755 --- a/src/test/php/io/unittest/BufferTest.class.php +++ b/src/test/php/io/unittest/BufferTest.class.php @@ -224,6 +224,8 @@ public function integrated_with_blob_api() { $fixture->write('Test'); $fixture->reset(); - Assert::equals('Test', (string)(new Blob($fixture))->bytes()); + $blob= new Blob($fixture); + Assert::equals('Test', (string)$blob->bytes()); + Assert::equals('Test', (string)$blob->bytes()); } } \ No newline at end of file From 55972b3a4971f06111001c06cdb97775e566400b Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Mon, 20 Apr 2026 22:00:22 +0200 Subject: [PATCH 8/9] QA: Add braces for clarity Visible instead of implicit operator precedence --- src/main/php/io/TempFile.class.php | 2 +- src/main/php/io/streams/Buffer.class.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/php/io/TempFile.class.php b/src/main/php/io/TempFile.class.php index f110f27ef..0efc43ff3 100755 --- a/src/main/php/io/TempFile.class.php +++ b/src/main/php/io/TempFile.class.php @@ -89,6 +89,6 @@ public function persistent() { /** Ensures file is closed and deleted */ public function __destruct() { parent::__destruct(); - $this->persistent || file_exists($this->uri) && unlink($this->uri); + $this->persistent || (file_exists($this->uri) && unlink($this->uri)); } } diff --git a/src/main/php/io/streams/Buffer.class.php b/src/main/php/io/streams/Buffer.class.php index 7250207cd..ddd8613a9 100755 --- a/src/main/php/io/streams/Buffer.class.php +++ b/src/main/php/io/streams/Buffer.class.php @@ -151,7 +151,7 @@ public function close() { if (null === $this->file || !$this->file->isOpen()) return; $this->file->close(); - $this->persist || $this->file->exists() && $this->file->unlink(); + $this->persist || ($this->file->exists() && $this->file->unlink()); } /** Ensure the file (if any) is closed */ From 5971b15b3784c0b44d051f2b4e01d9067084893e Mon Sep 17 00:00:00 2001 From: Timm Friebe Date: Wed, 22 Apr 2026 19:54:22 +0200 Subject: [PATCH 9/9] Default threshold to 0 (write to file immediately) --- src/main/php/io/streams/Buffer.class.php | 2 +- src/test/php/io/unittest/BufferTest.class.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/php/io/streams/Buffer.class.php b/src/main/php/io/streams/Buffer.class.php index ddd8613a9..092c81a97 100755 --- a/src/main/php/io/streams/Buffer.class.php +++ b/src/main/php/io/streams/Buffer.class.php @@ -25,7 +25,7 @@ class Buffer implements InputStream, OutputStream, Seekable { * @param bool $persist * @throws lang.IllegalArgumentException */ - public function __construct($files, int $threshold, bool $persist= false) { + public function __construct($files, int $threshold= 0, bool $persist= false) { if ($threshold < 0) { throw new IllegalArgumentException('Threshold must be >= 0'); } diff --git a/src/test/php/io/unittest/BufferTest.class.php b/src/test/php/io/unittest/BufferTest.class.php index 4f1cd6a11..3b7d7b2ac 100755 --- a/src/test/php/io/unittest/BufferTest.class.php +++ b/src/test/php/io/unittest/BufferTest.class.php @@ -30,7 +30,7 @@ public function with_directory($files) { #[Test] public function with_temp_file() { $t= new TempFile(); - $fixture= new Buffer($t, 0); + $fixture= new Buffer($t); $fixture->write('Test'); Assert::equals($t, $fixture->file()); @@ -39,7 +39,7 @@ public function with_temp_file() { #[Test] public function with_temp_path() { $t= new TempFile(); - $fixture= new Buffer(new Path($t), 0); + $fixture= new Buffer(new Path($t)); $fixture->write('Test'); Assert::equals($t, $fixture->file());