From c1a0765644bbc709c42c63ca1a893474946271dc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20L=C3=BCck?= <christian@lueck.tv>
Date: Thu, 27 Apr 2017 09:41:18 +0200
Subject: [PATCH 1/2] Use constructor parameter instead of public $bufferSize
 property

---
 README.md                                     | 19 +++++++++++--------
 src/DuplexResourceStream.php                  | 11 ++++++-----
 src/ReadableResourceStream.php                | 11 ++++++-----
 tests/DuplexResourceStreamIntegrationTest.php |  9 +++------
 tests/DuplexResourceStreamTest.php            | 11 +++++------
 tests/ReadableResourceStreamTest.php          |  7 +++----
 6 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/README.md b/README.md
index 0ed0885..e1d3f3a 100644
--- a/README.md
+++ b/README.md
@@ -753,21 +753,22 @@ take care of the underlying stream resource.
 You SHOULD only use its public API and SHOULD NOT interfere with the underlying
 stream resource manually.
 
-The `$bufferSize` property controls the maximum buffer size in bytes to read
-at once from the stream.
+This class takes an optional `int|null $readChunkSize` parameter that controls
+the maximum buffer size in bytes to read at once from the stream.
+You can use a `null` value here in order to apply its default value.
 This value SHOULD NOT be changed unless you know what you're doing.
 This can be a positive number which means that up to X bytes will be read
 at once from the underlying stream resource. Note that the actual number
 of bytes read may be lower if the stream resource has less than X bytes
 currently available.
-This can be `null` which means "read everything available" from the
+This can be `-1` which means "read everything available" from the
 underlying stream resource.
 This should read until the stream resource is not readable anymore
 (i.e. underlying buffer drained), note that this does not neccessarily
 mean it reached EOF.
 
 ```php
-$stream->bufferSize = 8192;
+$stream = new ReadableResourceStream(STDIN, $loop, 8192);
 ```
 
 ### WritableResourceStream
@@ -873,21 +874,23 @@ take care of the underlying stream resource.
 You SHOULD only use its public API and SHOULD NOT interfere with the underlying
 stream resource manually.
 
-The `$bufferSize` property controls the maximum buffer size in bytes to read
-at once from the stream.
+This class takes an optional `int|null $readChunkSize` parameter that controls
+the maximum buffer size in bytes to read at once from the stream.
+You can use a `null` value here in order to apply its default value.
 This value SHOULD NOT be changed unless you know what you're doing.
 This can be a positive number which means that up to X bytes will be read
 at once from the underlying stream resource. Note that the actual number
 of bytes read may be lower if the stream resource has less than X bytes
 currently available.
-This can be `null` which means "read everything available" from the
+This can be `-1` which means "read everything available" from the
 underlying stream resource.
 This should read until the stream resource is not readable anymore
 (i.e. underlying buffer drained), note that this does not neccessarily
 mean it reached EOF.
 
 ```php
-$stream->bufferSize = 8192;
+$conn = stream_socket_client('tcp://google.com:80');
+$stream = new DuplexResourceStream($conn, $loop, 8192);
 ```
 
 Any `write()` calls to this class will not be performaned instantly, but will
diff --git a/src/DuplexResourceStream.php b/src/DuplexResourceStream.php
index 5258793..a93ca64 100644
--- a/src/DuplexResourceStream.php
+++ b/src/DuplexResourceStream.php
@@ -16,15 +16,15 @@ class DuplexResourceStream extends EventEmitter implements DuplexStreamInterface
      * of bytes read may be lower if the stream resource has less than X bytes
      * currently available.
      *
-     * This can be `null` which means read everything available from the
+     * This can be `-1` which means read everything available from the
      * underlying stream resource.
      * This should read until the stream resource is not readable anymore
      * (i.e. underlying buffer drained), note that this does not neccessarily
      * mean it reached EOF.
      *
-     * @var int|null
+     * @var int
      */
-    public $bufferSize = 65536;
+    private $bufferSize;
 
     private $stream;
     protected $readable = true;
@@ -33,7 +33,7 @@ class DuplexResourceStream extends EventEmitter implements DuplexStreamInterface
     protected $loop;
     protected $buffer;
 
-    public function __construct($stream, LoopInterface $loop, WritableStreamInterface $buffer = null)
+    public function __construct($stream, LoopInterface $loop, $readChunkSize = null, WritableStreamInterface $buffer = null)
     {
         if (!is_resource($stream) || get_resource_type($stream) !== "stream") {
              throw new InvalidArgumentException('First parameter must be a valid stream resource');
@@ -69,6 +69,7 @@ public function __construct($stream, LoopInterface $loop, WritableStreamInterfac
 
         $this->stream = $stream;
         $this->loop = $loop;
+        $this->bufferSize = ($readChunkSize === null) ? 65536 : (int)$readChunkSize;
         $this->buffer = $buffer;
 
         $that = $this;
@@ -169,7 +170,7 @@ public function handleData($stream)
             );
         });
 
-        $data = stream_get_contents($stream, $this->bufferSize === null ? -1 : $this->bufferSize);
+        $data = stream_get_contents($stream, $this->bufferSize);
 
         restore_error_handler();
 
diff --git a/src/ReadableResourceStream.php b/src/ReadableResourceStream.php
index 6be9572..88d42ec 100644
--- a/src/ReadableResourceStream.php
+++ b/src/ReadableResourceStream.php
@@ -18,15 +18,15 @@ class ReadableResourceStream extends EventEmitter implements ReadableStreamInter
      * of bytes read may be lower if the stream resource has less than X bytes
      * currently available.
      *
-     * This can be `null` which means read everything available from the
+     * This can be `-1` which means read everything available from the
      * underlying stream resource.
      * This should read until the stream resource is not readable anymore
      * (i.e. underlying buffer drained), note that this does not neccessarily
      * mean it reached EOF.
      *
-     * @var int|null
+     * @var int
      */
-    public $bufferSize = 65536;
+    private $bufferSize;
 
     /**
      * @var resource
@@ -36,7 +36,7 @@ class ReadableResourceStream extends EventEmitter implements ReadableStreamInter
     private $closed = false;
     private $loop;
 
-    public function __construct($stream, LoopInterface $loop)
+    public function __construct($stream, LoopInterface $loop, $readChunkSize = null)
     {
         if (!is_resource($stream) || get_resource_type($stream) !== "stream") {
              throw new InvalidArgumentException('First parameter must be a valid stream resource');
@@ -68,6 +68,7 @@ public function __construct($stream, LoopInterface $loop)
 
         $this->stream = $stream;
         $this->loop = $loop;
+        $this->bufferSize = ($readChunkSize === null) ? 65536 : (int)$readChunkSize;
 
         $this->resume();
     }
@@ -123,7 +124,7 @@ public function handleData()
             );
         });
 
-        $data = stream_get_contents($this->stream, $this->bufferSize === null ? -1 : $this->bufferSize);
+        $data = stream_get_contents($this->stream, $this->bufferSize);
 
         restore_error_handler();
 
diff --git a/tests/DuplexResourceStreamIntegrationTest.php b/tests/DuplexResourceStreamIntegrationTest.php
index 139a39f..37d05da 100644
--- a/tests/DuplexResourceStreamIntegrationTest.php
+++ b/tests/DuplexResourceStreamIntegrationTest.php
@@ -31,14 +31,11 @@ public function testBufferReadsLargeChunks($condition, $loopFactory)
 
         list($sockA, $sockB) = stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, 0);
 
-        $streamA = new DuplexResourceStream($sockA, $loop);
-        $streamB = new DuplexResourceStream($sockB, $loop);
-
         $bufferSize = 4096;
-        $streamA->bufferSize = $bufferSize;
-        $streamB->bufferSize = $bufferSize;
+        $streamA = new DuplexResourceStream($sockA, $loop, $bufferSize);
+        $streamB = new DuplexResourceStream($sockB, $loop, $bufferSize);
 
-        $testString = str_repeat("*", $streamA->bufferSize + 1);
+        $testString = str_repeat("*", $bufferSize + 1);
 
         $buffer = "";
         $streamB->on('data', function ($data) use (&$buffer) {
diff --git a/tests/DuplexResourceStreamTest.php b/tests/DuplexResourceStreamTest.php
index b4096b5..2fe9c86 100644
--- a/tests/DuplexResourceStreamTest.php
+++ b/tests/DuplexResourceStreamTest.php
@@ -70,7 +70,7 @@ public function testConstructorAcceptsBuffer()
 
         $buffer = $this->getMockBuilder('React\Stream\WritableStreamInterface')->getMock();
 
-        $conn = new DuplexResourceStream($stream, $loop, $buffer);
+        $conn = new DuplexResourceStream($stream, $loop, null, $buffer);
 
         $this->assertSame($buffer, $conn->getBuffer());
     }
@@ -97,7 +97,7 @@ public function testEndShouldEndBuffer()
         $buffer = $this->getMockBuilder('React\Stream\WritableStreamInterface')->getMock();
         $buffer->expects($this->once())->method('end')->with('foo');
 
-        $conn = new DuplexResourceStream($stream, $loop, $buffer);
+        $conn = new DuplexResourceStream($stream, $loop, null, $buffer);
         $conn->end('foo');
     }
 
@@ -149,7 +149,7 @@ public function testDataEventDoesEmitOneChunkMatchingBufferSize()
 
         $capturedData = null;
 
-        $conn = new DuplexResourceStream($stream, $loop);
+        $conn = new DuplexResourceStream($stream, $loop, 4321);
         $conn->on('data', function ($data) use (&$capturedData) {
             $capturedData = $data;
         });
@@ -160,7 +160,7 @@ public function testDataEventDoesEmitOneChunkMatchingBufferSize()
         $conn->handleData($stream);
 
         $this->assertTrue($conn->isReadable());
-        $this->assertEquals($conn->bufferSize, strlen($capturedData));
+        $this->assertEquals(4321, strlen($capturedData));
     }
 
     /**
@@ -174,8 +174,7 @@ public function testDataEventDoesEmitOneChunkUntilStreamEndsWhenBufferSizeIsInfi
 
         $capturedData = null;
 
-        $conn = new DuplexResourceStream($stream, $loop);
-        $conn->bufferSize = null;
+        $conn = new DuplexResourceStream($stream, $loop, -1);
 
         $conn->on('data', function ($data) use (&$capturedData) {
             $capturedData = $data;
diff --git a/tests/ReadableResourceStreamTest.php b/tests/ReadableResourceStreamTest.php
index 4609e70..a6909ba 100644
--- a/tests/ReadableResourceStreamTest.php
+++ b/tests/ReadableResourceStreamTest.php
@@ -120,7 +120,7 @@ public function testDataEventDoesEmitOneChunkMatchingBufferSize()
 
         $capturedData = null;
 
-        $conn = new ReadableResourceStream($stream, $loop);
+        $conn = new ReadableResourceStream($stream, $loop, 4321);
         $conn->on('data', function ($data) use (&$capturedData) {
             $capturedData = $data;
         });
@@ -131,7 +131,7 @@ public function testDataEventDoesEmitOneChunkMatchingBufferSize()
         $conn->handleData($stream);
 
         $this->assertTrue($conn->isReadable());
-        $this->assertEquals($conn->bufferSize, strlen($capturedData));
+        $this->assertEquals(4321, strlen($capturedData));
     }
 
     /**
@@ -145,8 +145,7 @@ public function testDataEventDoesEmitOneChunkUntilStreamEndsWhenBufferSizeIsInfi
 
         $capturedData = null;
 
-        $conn = new ReadableResourceStream($stream, $loop);
-        $conn->bufferSize = null;
+        $conn = new ReadableResourceStream($stream, $loop, -1);
 
         $conn->on('data', function ($data) use (&$capturedData) {
             $capturedData = $data;

From d69ad7bce57639e8189ffaa3c6ff543eba4e5750 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20L=C3=BCck?= <christian@lueck.tv>
Date: Thu, 27 Apr 2017 11:25:31 +0200
Subject: [PATCH 2/2] Use constructor parameter instead of public $softLimit
 property

---
 README.md                            | 26 ++++++++++++++++++--------
 src/DuplexResourceStream.php         |  8 --------
 src/WritableResourceStream.php       |  7 ++++---
 tests/DuplexResourceStreamTest.php   |  7 +++----
 tests/WritableStreamResourceTest.php | 24 ++++++++----------------
 5 files changed, 33 insertions(+), 39 deletions(-)

diff --git a/README.md b/README.md
index e1d3f3a..86a6a28 100644
--- a/README.md
+++ b/README.md
@@ -820,11 +820,14 @@ ready to accept data.
 For this, it uses an in-memory buffer string to collect all outstanding writes.
 This buffer has a soft-limit applied which defines how much data it is willing
 to accept before the caller SHOULD stop sending further data.
-It currently defaults to 64 KiB and can be controlled through the public
-`$softLimit` property like this:
+
+This class takes an optional `int|null $writeBufferSoftLimit` parameter that controls
+this maximum buffer size in bytes.
+You can use a `null` value here in order to apply its default value.
+This value SHOULD NOT be changed unless you know what you're doing.
 
 ```php
-$stream->softLimit = 8192;
+$stream = new WritableResourceStream(STDOUT, $loop, 8192);
 ```
 
 See also [`write()`](#write) for more details.
@@ -899,15 +902,22 @@ ready to accept data.
 For this, it uses an in-memory buffer string to collect all outstanding writes.
 This buffer has a soft-limit applied which defines how much data it is willing
 to accept before the caller SHOULD stop sending further data.
-It currently defaults to 64 KiB and can be controlled through the public
-`$softLimit` property like this:
+
+This class takes another optional `WritableStreamInterface|null $buffer` parameter
+that controls this write behavior of this stream.
+You can use a `null` value here in order to apply its default value.
+This value SHOULD NOT be changed unless you know what you're doing.
+
+If you want to change the write buffer soft limit, you can pass an instance of
+[`WritableResourceStream`](#writableresourcestream) like this:
 
 ```php
-$buffer = $stream->getBuffer();
-$buffer->softLimit = 8192;
+$conn = stream_socket_client('tcp://google.com:80');
+$buffer = new WritableResourceStream($conn, $loop, 8192);
+$stream = new DuplexResourceStream($conn, $loop, null, $buffer);
 ```
 
-See also [`write()`](#write) for more details.
+See also [`WritableResourceStream`](#writableresourcestream) for more details.
 
 ### ThroughStream
 
diff --git a/src/DuplexResourceStream.php b/src/DuplexResourceStream.php
index a93ca64..8a9f070 100644
--- a/src/DuplexResourceStream.php
+++ b/src/DuplexResourceStream.php
@@ -196,14 +196,6 @@ public function handleClose()
         }
     }
 
-    /**
-     * @return WritableStreamInterface
-     */
-    public function getBuffer()
-    {
-        return $this->buffer;
-    }
-
     /**
      * Returns whether this is a pipe resource in a legacy environment
      *
diff --git a/src/WritableResourceStream.php b/src/WritableResourceStream.php
index 2140232..47883e2 100644
--- a/src/WritableResourceStream.php
+++ b/src/WritableResourceStream.php
@@ -8,15 +8,15 @@
 class WritableResourceStream extends EventEmitter implements WritableStreamInterface
 {
     private $stream;
-    public $softLimit = 65536;
+    private $loop;
+    private $softLimit;
 
     private $listening = false;
     private $writable = true;
     private $closed = false;
-    private $loop;
     private $data = '';
 
-    public function __construct($stream, LoopInterface $loop)
+    public function __construct($stream, LoopInterface $loop, $writeBufferSoftLimit = null)
     {
         if (!is_resource($stream) || get_resource_type($stream) !== "stream") {
             throw new \InvalidArgumentException('First parameter must be a valid stream resource');
@@ -35,6 +35,7 @@ public function __construct($stream, LoopInterface $loop)
 
         $this->stream = $stream;
         $this->loop = $loop;
+        $this->softLimit = ($writeBufferSoftLimit === null) ? 65536 : (int)$writeBufferSoftLimit;
     }
 
     public function isWritable()
diff --git a/tests/DuplexResourceStreamTest.php b/tests/DuplexResourceStreamTest.php
index 2fe9c86..9ea43f9 100644
--- a/tests/DuplexResourceStreamTest.php
+++ b/tests/DuplexResourceStreamTest.php
@@ -4,6 +4,7 @@
 
 use React\Stream\DuplexResourceStream;
 use Clue\StreamFilter as Filter;
+use React\Stream\WritableResourceStream;
 
 class DuplexResourceStreamTest extends TestCase
 {
@@ -71,8 +72,6 @@ public function testConstructorAcceptsBuffer()
         $buffer = $this->getMockBuilder('React\Stream\WritableStreamInterface')->getMock();
 
         $conn = new DuplexResourceStream($stream, $loop, null, $buffer);
-
-        $this->assertSame($buffer, $conn->getBuffer());
     }
 
     public function testCloseShouldEmitCloseEvent()
@@ -284,12 +283,12 @@ public function testBufferEventsShouldBubbleUp()
         $stream = fopen('php://temp', 'r+');
         $loop = $this->createLoopMock();
 
-        $conn = new DuplexResourceStream($stream, $loop);
+        $buffer = new WritableResourceStream($stream, $loop);
+        $conn = new DuplexResourceStream($stream, $loop, null, $buffer);
 
         $conn->on('drain', $this->expectCallableOnce());
         $conn->on('error', $this->expectCallableOnce());
 
-        $buffer = $conn->getBuffer();
         $buffer->emit('drain');
         $buffer->emit('error', array(new \RuntimeException('Whoops')));
     }
diff --git a/tests/WritableStreamResourceTest.php b/tests/WritableStreamResourceTest.php
index 606a684..80ce1b1 100644
--- a/tests/WritableStreamResourceTest.php
+++ b/tests/WritableStreamResourceTest.php
@@ -115,8 +115,7 @@ public function testWriteReturnsFalseWhenWritableResourceStreamIsFull()
         $loop = $this->createWriteableLoopMock();
         $loop->preventWrites = true;
 
-        $buffer = new WritableResourceStream($stream, $loop);
-        $buffer->softLimit = 4;
+        $buffer = new WritableResourceStream($stream, $loop, 4);
         $buffer->on('error', $this->expectCallableNever());
 
         $this->assertTrue($buffer->write("foo"));
@@ -132,8 +131,7 @@ public function testWriteReturnsFalseWhenWritableResourceStreamIsExactlyFull()
         $stream = fopen('php://temp', 'r+');
         $loop = $this->createLoopMock();
 
-        $buffer = new WritableResourceStream($stream, $loop);
-        $buffer->softLimit = 3;
+        $buffer = new WritableResourceStream($stream, $loop, 3);
 
         $this->assertFalse($buffer->write("foo"));
     }
@@ -148,8 +146,7 @@ public function testWriteDetectsWhenOtherSideIsClosed()
 
         $loop = $this->createWriteableLoopMock();
 
-        $buffer = new WritableResourceStream($a, $loop);
-        $buffer->softLimit = 4;
+        $buffer = new WritableResourceStream($a, $loop, 4);
         $buffer->on('error', $this->expectCallableOnce());
 
         fclose($b);
@@ -166,8 +163,7 @@ public function testEmitsDrainAfterWriteWhichExceedsBuffer()
         $stream = fopen('php://temp', 'r+');
         $loop = $this->createLoopMock();
 
-        $buffer = new WritableResourceStream($stream, $loop);
-        $buffer->softLimit = 2;
+        $buffer = new WritableResourceStream($stream, $loop, 2);
         $buffer->on('error', $this->expectCallableNever());
         $buffer->on('drain', $this->expectCallableOnce());
 
@@ -184,8 +180,7 @@ public function testWriteInDrain()
         $stream = fopen('php://temp', 'r+');
         $loop = $this->createLoopMock();
 
-        $buffer = new WritableResourceStream($stream, $loop);
-        $buffer->softLimit = 2;
+        $buffer = new WritableResourceStream($stream, $loop, 2);
         $buffer->on('error', $this->expectCallableNever());
 
         $buffer->once('drain', function () use ($buffer) {
@@ -209,8 +204,7 @@ public function testDrainAfterWrite()
         $stream = fopen('php://temp', 'r+');
         $loop = $this->createLoopMock();
 
-        $buffer = new WritableResourceStream($stream, $loop);
-        $buffer->softLimit = 2;
+        $buffer = new WritableResourceStream($stream, $loop, 2);
 
         $buffer->on('drain', $this->expectCallableOnce());
 
@@ -227,8 +221,7 @@ public function testDrainAfterWriteWillRemoveResourceFromLoopWithoutClosing()
         $loop = $this->createLoopMock();
         $loop->expects($this->once())->method('removeWriteStream')->with($stream);
 
-        $buffer = new WritableResourceStream($stream, $loop);
-        $buffer->softLimit = 2;
+        $buffer = new WritableResourceStream($stream, $loop, 2);
 
         $buffer->on('drain', $this->expectCallableOnce());
 
@@ -247,8 +240,7 @@ public function testClosingDuringDrainAfterWriteWillRemoveResourceFromLoopOnceAn
         $loop = $this->createLoopMock();
         $loop->expects($this->once())->method('removeWriteStream')->with($stream);
 
-        $buffer = new WritableResourceStream($stream, $loop);
-        $buffer->softLimit = 2;
+        $buffer = new WritableResourceStream($stream, $loop, 2);
 
         $buffer->on('drain', function () use ($buffer) {
             $buffer->close();