Skip to content

Commit f674d1b

Browse files
WyriHaximusWyriHaximus
authored
Merge pull request #380 from clue-labs/no-messagefactory
Internal refactoring, remove unneeded MessageFactory helper class
2 parents 7873a87 + 788526e commit f674d1b

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)