Skip to content

Commit 788526e

Browse files
clueclue
committed
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.
1 parent 4ce74d2 commit 788526e

9 files changed

Lines changed: 199 additions & 394 deletions

File tree

‎src/Browser.php‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@
33
namespace React\Http;
44

55
use Psr\Http\Message\ResponseInterface;
6+
use RingCentral\Psr7\Request;
67
use RingCentral\Psr7\Uri;
78
use React\EventLoop\LoopInterface;
8-
use React\Http\Io\MessageFactory;
9+
use React\Http\Io\ReadableBodyStream;
910
use React\Http\Io\Sender;
1011
use React\Http\Io\Transaction;
1112
use React\Promise\PromiseInterface;
@@ -19,7 +20,6 @@
1920
class Browser
2021
{
2122
private $transaction;
22-
private $messageFactory;
2323
private $baseUrl;
2424
private $protocolVersion = '1.1';
2525

@@ -59,10 +59,8 @@ class Browser
5959
*/
6060
public function __construct(LoopInterface $loop, ConnectorInterface $connector = null)
6161
{
62-
$this->messageFactory = new MessageFactory();
6362
$this->transaction = new Transaction(
64-
Sender::createFromLoop($loop, $connector, $this->messageFactory),
65-
$this->messageFactory,
63+
Sender::createFromLoop($loop, $connector),
6664
$loop
6765
);
6866
}
@@ -721,18 +719,22 @@ private function withOptions(array $options)
721719
* @param string $method
722720
* @param string $url
723721
* @param array $headers
724-
* @param string|ReadableStreamInterface $contents
722+
* @param string|ReadableStreamInterface $body
725723
* @return PromiseInterface<ResponseInterface,Exception>
726724
*/
727-
private function requestMayBeStreaming($method, $url, array $headers = array(), $contents = '')
725+
private function requestMayBeStreaming($method, $url, array $headers = array(), $body = '')
728726
{
729727
if ($this->baseUrl !== null) {
730728
// ensure we're actually below the base URL
731729
$url = Uri::resolve($this->baseUrl, $url);
732730
}
733731

734-
$request = $this->messageFactory->request($method, $url, $headers, $contents, $this->protocolVersion);
732+
if ($body instanceof ReadableStreamInterface) {
733+
$body = new ReadableBodyStream($body);
734+
}
735735

736-
return $this->transaction->send($request);
736+
return $this->transaction->send(
737+
new Request($method, $url, $headers, $body, $this->protocolVersion)
738+
);
737739
}
738740
}

‎src/Client/Response.php‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,24 @@ private function getHeader($name)
8686
return isset($normalized[$name]) ? (array)$normalized[$name] : array();
8787
}
8888

89-
private function getHeaderLine($name)
89+
/**
90+
* @param string $name
91+
* @return string
92+
*/
93+
public function getHeaderLine($name)
9094
{
9195
return implode(', ' , $this->getHeader($name));
9296
}
9397

98+
/**
99+
* @param string $name
100+
* @return bool
101+
*/
102+
public function hasHeader($name)
103+
{
104+
return $this->getHeader($name) !== array();
105+
}
106+
94107
/** @internal */
95108
public function handleData($data)
96109
{

‎src/Io/MessageFactory.php‎

Lines changed: 0 additions & 78 deletions
This file was deleted.

‎src/Io/Sender.php‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use React\EventLoop\LoopInterface;
77
use React\Http\Client\Client as HttpClient;
88
use React\Http\Client\Response as ResponseStream;
9+
use React\Http\Message\Response;
910
use React\Promise\PromiseInterface;
1011
use React\Promise\Deferred;
1112
use React\Socket\ConnectorInterface;
@@ -47,24 +48,22 @@ class Sender
4748
* @param ConnectorInterface|null $connector
4849
* @return self
4950
*/
50-
public static function createFromLoop(LoopInterface $loop, ConnectorInterface $connector = null, MessageFactory $messageFactory)
51+
public static function createFromLoop(LoopInterface $loop, ConnectorInterface $connector = null)
5152
{
52-
return new self(new HttpClient($loop, $connector), $messageFactory);
53+
return new self(new HttpClient($loop, $connector));
5354
}
5455

5556
private $http;
56-
private $messageFactory;
5757

5858
/**
5959
* [internal] Instantiate Sender
6060
*
6161
* @param HttpClient $http
6262
* @internal
6363
*/
64-
public function __construct(HttpClient $http, MessageFactory $messageFactory)
64+
public function __construct(HttpClient $http)
6565
{
6666
$this->http = $http;
67-
$this->messageFactory = $messageFactory;
6867
}
6968

7069
/**
@@ -109,16 +108,22 @@ public function send(RequestInterface $request)
109108
$deferred->reject($error);
110109
});
111110

112-
$messageFactory = $this->messageFactory;
113-
$requestStream->on('response', function (ResponseStream $responseStream) use ($deferred, $messageFactory, $request) {
111+
$requestStream->on('response', function (ResponseStream $responseStream) use ($deferred, $request) {
112+
$length = null;
113+
$code = $responseStream->getCode();
114+
if ($request->getMethod() === 'HEAD' || ($code >= 100 && $code < 200) || $code == 204 || $code == 304) {
115+
$length = 0;
116+
} elseif ($responseStream->hasHeader('Content-Length')) {
117+
$length = (int) $responseStream->getHeaderLine('Content-Length');
118+
}
119+
114120
// apply response header values from response stream
115-
$deferred->resolve($messageFactory->response(
116-
$responseStream->getVersion(),
121+
$deferred->resolve(new Response(
117122
$responseStream->getCode(),
118-
$responseStream->getReasonPhrase(),
119123
$responseStream->getHeaders(),
120-
$responseStream,
121-
$request->getMethod()
124+
new ReadableBodyStream($responseStream, $length),
125+
$responseStream->getVersion(),
126+
$responseStream->getReasonPhrase()
122127
));
123128
});
124129

‎src/Io/Transaction.php‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Psr\Http\Message\RequestInterface;
66
use Psr\Http\Message\ResponseInterface;
77
use Psr\Http\Message\UriInterface;
8+
use RingCentral\Psr7\Request;
89
use RingCentral\Psr7\Uri;
910
use React\EventLoop\LoopInterface;
1011
use React\Http\Message\ResponseException;
@@ -18,7 +19,6 @@
1819
class Transaction
1920
{
2021
private $sender;
21-
private $messageFactory;
2222
private $loop;
2323

2424
// context: http.timeout (ini_get('default_socket_timeout'): 60)
@@ -37,10 +37,9 @@ class Transaction
3737

3838
private $maximumSize = 16777216; // 16 MiB = 2^24 bytes
3939

40-
public function __construct(Sender $sender, MessageFactory $messageFactory, LoopInterface $loop)
40+
public function __construct(Sender $sender, LoopInterface $loop)
4141
{
4242
$this->sender = $sender;
43-
$this->messageFactory = $messageFactory;
4443
$this->loop = $loop;
4544
}
4645

@@ -55,7 +54,7 @@ public function withOptions(array $options)
5554
if (property_exists($transaction, $name)) {
5655
// restore default value if null is given
5756
if ($value === null) {
58-
$default = new self($this->sender, $this->messageFactory, $this->loop);
57+
$default = new self($this->sender, $this->loop);
5958
$value = $default->$name;
6059
}
6160

@@ -186,11 +185,10 @@ public function bufferResponse(ResponseInterface $response, $deferred)
186185
}
187186

188187
// buffer stream and resolve with buffered body
189-
$messageFactory = $this->messageFactory;
190188
$maximumSize = $this->maximumSize;
191189
$promise = \React\Promise\Stream\buffer($stream, $maximumSize)->then(
192-
function ($body) use ($response, $messageFactory) {
193-
return $response->withBody($messageFactory->body($body));
190+
function ($body) use ($response) {
191+
return $response->withBody(\RingCentral\Psr7\stream_for($body));
194192
},
195193
function ($e) use ($stream, $maximumSize) {
196194
// try to close stream if buffering fails (or is cancelled)
@@ -280,7 +278,7 @@ private function makeRedirectRequest(RequestInterface $request, UriInterface $lo
280278
// naïve approach..
281279
$method = ($request->getMethod() === 'HEAD') ? 'HEAD' : 'GET';
282280

283-
return $this->messageFactory->request($method, $location, $request->getHeaders());
281+
return new Request($method, $location, $request->getHeaders());
284282
}
285283

286284
private function progress($name, array $args = array())

‎tests/FunctionalBrowserTest.php‎

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,14 +64,22 @@ public function setUpBrowserAndServer()
6464
);
6565
}
6666

67-
if ($path === '/status/300') {
67+
if ($path === '/status/204') {
6868
return new Response(
69-
300,
69+
204,
7070
array(),
7171
''
7272
);
7373
}
7474

75+
if ($path === '/status/304') {
76+
return new Response(
77+
304,
78+
array(),
79+
'Not modified'
80+
);
81+
}
82+
7583
if ($path === '/status/404') {
7684
return new Response(
7785
404,
@@ -308,12 +316,24 @@ public function testFollowRedirectsZeroRejectsOnRedirect()
308316
Block\await($browser->get($this->base . 'redirect-to?url=get'), $this->loop);
309317
}
310318

311-
/**
312-
* @doesNotPerformAssertions
313-
*/
314-
public function testResponseStatus300WithoutLocationShouldResolveWithoutFollowingRedirect()
319+
public function testResponseStatus204ShouldResolveWithEmptyBody()
315320
{
316-
Block\await($this->browser->get($this->base . 'status/300'), $this->loop);
321+
$response = Block\await($this->browser->get($this->base . 'status/204'), $this->loop);
322+
$this->assertFalse($response->hasHeader('Content-Length'));
323+
324+
$body = $response->getBody();
325+
$this->assertEquals(0, $body->getSize());
326+
$this->assertEquals('', (string) $body);
327+
}
328+
329+
public function testResponseStatus304ShouldResolveWithEmptyBodyButContentLengthResponseHeader()
330+
{
331+
$response = Block\await($this->browser->get($this->base . 'status/304'), $this->loop);
332+
$this->assertEquals('12', $response->getHeaderLine('Content-Length'));
333+
334+
$body = $response->getBody();
335+
$this->assertEquals(0, $body->getSize());
336+
$this->assertEquals('', (string) $body);
317337
}
318338

319339
/**
@@ -595,9 +615,33 @@ public function testSendsExplicitHttp10Request()
595615
public function testHeadRequestReceivesResponseWithEmptyBodyButWithContentLengthResponseHeader()
596616
{
597617
$response = Block\await($this->browser->head($this->base . 'get'), $this->loop);
598-
$this->assertEquals('', (string)$response->getBody());
599-
$this->assertEquals(0, $response->getBody()->getSize());
600618
$this->assertEquals('5', $response->getHeaderLine('Content-Length'));
619+
620+
$body = $response->getBody();
621+
$this->assertEquals(0, $body->getSize());
622+
$this->assertEquals('', (string) $body);
623+
}
624+
625+
public function testRequestStreamingGetReceivesResponseWithStreamingBodyAndKnownSize()
626+
{
627+
$response = Block\await($this->browser->requestStreaming('GET', $this->base . 'get'), $this->loop);
628+
$this->assertEquals('5', $response->getHeaderLine('Content-Length'));
629+
630+
$body = $response->getBody();
631+
$this->assertEquals(5, $body->getSize());
632+
$this->assertEquals('', (string) $body);
633+
$this->assertInstanceOf('React\Stream\ReadableStreamInterface', $body);
634+
}
635+
636+
public function testRequestStreamingGetReceivesResponseWithStreamingBodyAndUnknownSizeFromStreamingEndpoint()
637+
{
638+
$response = Block\await($this->browser->requestStreaming('GET', $this->base . 'stream/1'), $this->loop);
639+
$this->assertFalse($response->hasHeader('Content-Length'));
640+
641+
$body = $response->getBody();
642+
$this->assertNull($body->getSize());
643+
$this->assertEquals('', (string) $body);
644+
$this->assertInstanceOf('React\Stream\ReadableStreamInterface', $body);
601645
}
602646

603647
public function testRequestStreamingGetReceivesStreamingResponseBody()

0 commit comments

Comments
 (0)