Merge pull request #380 from clue-labs/no-messagefactory

Internal refactoring, remove unneeded MessageFactory helper class

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()