Internal refactoring, remove unneeded MessageFactory helper class

This is an internal preparation only and does not affect any public
APIs. Some internal logic has been refactored and moved to classes with
better cohesion. This is done in preparation for upcoming improvements
to the `Transfer-Encoding: chunked` response header.

src/Browser.php

@@ -3,9 +3,10 @@
 namespace React\Http;
 
 use Psr\Http\Message\ResponseInterface;
+use RingCentral\Psr7\Request;
 use RingCentral\Psr7\Uri;
 use React\EventLoop\LoopInterface;
-use React\Http\Io\MessageFactory;
+use React\Http\Io\ReadableBodyStream;
 use React\Http\Io\Sender;
 use React\Http\Io\Transaction;
 use React\Promise\PromiseInterface;
@@ -19,7 +20,6 @@
 class Browser
 {
     private $transaction;
-    private $messageFactory;
     private $baseUrl;
     private $protocolVersion = '1.1';
 
@@ -59,10 +59,8 @@ class Browser
      */
     public function __construct(LoopInterface $loop, ConnectorInterface $connector = null)
     {
-        $this->messageFactory = new MessageFactory();
         $this->transaction = new Transaction(
-            Sender::createFromLoop($loop, $connector, $this->messageFactory),
-            $this->messageFactory,
+            Sender::createFromLoop($loop, $connector),
             $loop
         );
     }
@@ -721,18 +719,22 @@ private function withOptions(array $options)
      * @param string                         $method
      * @param string                         $url
      * @param array                          $headers
-     * @param string|ReadableStreamInterface $contents
+     * @param string|ReadableStreamInterface $body
      * @return PromiseInterface<ResponseInterface,Exception>
      */
-    private function requestMayBeStreaming($method, $url, array $headers = array(), $contents = '')
+    private function requestMayBeStreaming($method, $url, array $headers = array(), $body = '')
     {
         if ($this->baseUrl !== null) {
             // ensure we're actually below the base URL
             $url = Uri::resolve($this->baseUrl, $url);
         }
 
-        $request = $this->messageFactory->request($method, $url, $headers, $contents, $this->protocolVersion);
+        if ($body instanceof ReadableStreamInterface) {
+            $body = new ReadableBodyStream($body);
+        }
 
-        return $this->transaction->send($request);
+        return $this->transaction->send(
+            new Request($method, $url, $headers, $body, $this->protocolVersion)
+        );
     }
 }

src/Client/Response.php

@@ -86,11 +86,24 @@ private function getHeader($name)
         return isset($normalized[$name]) ? (array)$normalized[$name] : array();
     }
 
-    private function getHeaderLine($name)
+    /**
+     * @param string $name
+     * @return string
+     */
+    public function getHeaderLine($name)
     {
         return implode(', ' , $this->getHeader($name));
     }
 
+    /**
+     * @param string $name
+     * @return bool
+     */
+    public function hasHeader($name)
+    {
+        return $this->getHeader($name) !== array();
+    }
+
     /** @internal */
     public function handleData($data)
     {

src/Io/Sender.php

@@ -6,6 +6,7 @@
 use React\EventLoop\LoopInterface;
 use React\Http\Client\Client as HttpClient;
 use React\Http\Client\Response as ResponseStream;
+use React\Http\Message\Response;
 use React\Promise\PromiseInterface;
 use React\Promise\Deferred;
 use React\Socket\ConnectorInterface;
@@ -47,24 +48,22 @@ class Sender
      * @param ConnectorInterface|null $connector
      * @return self
      */
-    public static function createFromLoop(LoopInterface $loop, ConnectorInterface $connector = null, MessageFactory $messageFactory)
+    public static function createFromLoop(LoopInterface $loop, ConnectorInterface $connector = null)
     {
-        return new self(new HttpClient($loop, $connector), $messageFactory);
+        return new self(new HttpClient($loop, $connector));
     }
 
     private $http;
-    private $messageFactory;
 
     /**
      * [internal] Instantiate Sender
      *
      * @param HttpClient $http
      * @internal
      */
-    public function __construct(HttpClient $http, MessageFactory $messageFactory)
+    public function __construct(HttpClient $http)
     {
         $this->http = $http;
-        $this->messageFactory = $messageFactory;
     }
 
     /**
@@ -109,16 +108,22 @@ public function send(RequestInterface $request)
             $deferred->reject($error);
         });
 
-        $messageFactory = $this->messageFactory;
-        $requestStream->on('response', function (ResponseStream $responseStream) use ($deferred, $messageFactory, $request) {
+        $requestStream->on('response', function (ResponseStream $responseStream) use ($deferred, $request) {
+            $length = null;
+            $code = $responseStream->getCode();
+            if ($request->getMethod() === 'HEAD' || ($code >= 100 && $code < 200) || $code == 204 || $code == 304) {
+                $length = 0;
+            } elseif ($responseStream->hasHeader('Content-Length')) {
+                $length = (int) $responseStream->getHeaderLine('Content-Length');
+            }
+
             // apply response header values from response stream
-            $deferred->resolve($messageFactory->response(
-                $responseStream->getVersion(),
+            $deferred->resolve(new Response(
                 $responseStream->getCode(),
-                $responseStream->getReasonPhrase(),
                 $responseStream->getHeaders(),
-                $responseStream,
-                $request->getMethod()
+                new ReadableBodyStream($responseStream, $length),
+                $responseStream->getVersion(),
+                $responseStream->getReasonPhrase()
             ));
         });
 

src/Io/Transaction.php

@@ -5,6 +5,7 @@
 use Psr\Http\Message\RequestInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\UriInterface;
+use RingCentral\Psr7\Request;
 use RingCentral\Psr7\Uri;
 use React\EventLoop\LoopInterface;
 use React\Http\Message\ResponseException;
@@ -18,7 +19,6 @@
 class Transaction
 {
     private $sender;
-    private $messageFactory;
     private $loop;
 
     // context: http.timeout (ini_get('default_socket_timeout'): 60)
@@ -37,10 +37,9 @@ class Transaction
 
     private $maximumSize = 16777216; // 16 MiB = 2^24 bytes
 
-    public function __construct(Sender $sender, MessageFactory $messageFactory, LoopInterface $loop)
+    public function __construct(Sender $sender, LoopInterface $loop)
     {
         $this->sender = $sender;
-        $this->messageFactory = $messageFactory;
         $this->loop = $loop;
     }
 
@@ -55,7 +54,7 @@ public function withOptions(array $options)
             if (property_exists($transaction, $name)) {
                 // restore default value if null is given
                 if ($value === null) {
-                    $default = new self($this->sender, $this->messageFactory, $this->loop);
+                    $default = new self($this->sender, $this->loop);
                     $value = $default->$name;
                 }
 
@@ -186,11 +185,10 @@ public function bufferResponse(ResponseInterface $response, $deferred)
         }
 
         // buffer stream and resolve with buffered body
-        $messageFactory = $this->messageFactory;
         $maximumSize = $this->maximumSize;
         $promise = \React\Promise\Stream\buffer($stream, $maximumSize)->then(
-            function ($body) use ($response, $messageFactory) {
-                return $response->withBody($messageFactory->body($body));
+            function ($body) use ($response) {
+                return $response->withBody(\RingCentral\Psr7\stream_for($body));
             },
             function ($e) use ($stream, $maximumSize) {
                 // try to close stream if buffering fails (or is cancelled)
@@ -280,7 +278,7 @@ private function makeRedirectRequest(RequestInterface $request, UriInterface $lo
         // naïve approach..
         $method = ($request->getMethod() === 'HEAD') ? 'HEAD' : 'GET';
 
-        return $this->messageFactory->request($method, $location, $request->getHeaders());
+        return new Request($method, $location, $request->getHeaders());
     }
 
     private function progress($name, array $args = array())

tests/FunctionalBrowserTest.php

@@ -64,14 +64,22 @@ public function setUpBrowserAndServer()
                 );
             }
 
-            if ($path === '/status/300') {
+            if ($path === '/status/204') {
                 return new Response(
-                    300,
+                    204,
                     array(),
                     ''
                 );
             }
 
+            if ($path === '/status/304') {
+                return new Response(
+                    304,
+                    array(),
+                    'Not modified'
+                );
+            }
+
             if ($path === '/status/404') {
                 return new Response(
                     404,
@@ -308,12 +316,24 @@ public function testFollowRedirectsZeroRejectsOnRedirect()
         Block\await($browser->get($this->base . 'redirect-to?url=get'), $this->loop);
     }
 
-    /**
-     * @doesNotPerformAssertions
-     */
-    public function testResponseStatus300WithoutLocationShouldResolveWithoutFollowingRedirect()
+    public function testResponseStatus204ShouldResolveWithEmptyBody()
     {
-        Block\await($this->browser->get($this->base . 'status/300'), $this->loop);
+        $response = Block\await($this->browser->get($this->base . 'status/204'), $this->loop);
+        $this->assertFalse($response->hasHeader('Content-Length'));
+
+        $body = $response->getBody();
+        $this->assertEquals(0, $body->getSize());
+        $this->assertEquals('', (string) $body);
+    }
+
+    public function testResponseStatus304ShouldResolveWithEmptyBodyButContentLengthResponseHeader()
+    {
+        $response = Block\await($this->browser->get($this->base . 'status/304'), $this->loop);
+        $this->assertEquals('12', $response->getHeaderLine('Content-Length'));
+
+        $body = $response->getBody();
+        $this->assertEquals(0, $body->getSize());
+        $this->assertEquals('', (string) $body);
     }
 
     /**
@@ -595,9 +615,33 @@ public function testSendsExplicitHttp10Request()
     public function testHeadRequestReceivesResponseWithEmptyBodyButWithContentLengthResponseHeader()
     {
         $response = Block\await($this->browser->head($this->base . 'get'), $this->loop);
-        $this->assertEquals('', (string)$response->getBody());
-        $this->assertEquals(0, $response->getBody()->getSize());
         $this->assertEquals('5', $response->getHeaderLine('Content-Length'));
+
+        $body = $response->getBody();
+        $this->assertEquals(0, $body->getSize());
+        $this->assertEquals('', (string) $body);
+    }
+
+    public function testRequestStreamingGetReceivesResponseWithStreamingBodyAndKnownSize()
+    {
+        $response = Block\await($this->browser->requestStreaming('GET', $this->base . 'get'), $this->loop);
+        $this->assertEquals('5', $response->getHeaderLine('Content-Length'));
+
+        $body = $response->getBody();
+        $this->assertEquals(5, $body->getSize());
+        $this->assertEquals('', (string) $body);
+        $this->assertInstanceOf('React\Stream\ReadableStreamInterface', $body);
+    }
+
+    public function testRequestStreamingGetReceivesResponseWithStreamingBodyAndUnknownSizeFromStreamingEndpoint()
+    {
+        $response = Block\await($this->browser->requestStreaming('GET', $this->base . 'stream/1'), $this->loop);
+        $this->assertFalse($response->hasHeader('Content-Length'));
+
+        $body = $response->getBody();
+        $this->assertNull($body->getSize());
+        $this->assertEquals('', (string) $body);
+        $this->assertInstanceOf('React\Stream\ReadableStreamInterface', $body);
     }
 
     public function testRequestStreamingGetReceivesStreamingResponseBody()