Skip to content

Commit 1c911d2

Browse files
clueclue
committed
Refactor to add new ClientConnectionManager to manage HTTP connections
1 parent 28b598a commit 1c911d2

9 files changed

Lines changed: 276 additions & 194 deletions

‎src/Client/Client.php‎

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

55
use Psr\Http\Message\RequestInterface;
6-
use React\EventLoop\LoopInterface;
6+
use React\Http\Io\ClientConnectionManager;
77
use React\Http\Io\ClientRequestStream;
8-
use React\Socket\Connector;
9-
use React\Socket\ConnectorInterface;
108

119
/**
1210
* @internal
1311
*/
1412
class Client
1513
{
16-
private $connector;
14+
/** @var ClientConnectionManager */
15+
private $connectionManager;
1716

18-
public function __construct(LoopInterface $loop, ConnectorInterface $connector = null)
17+
public function __construct(ClientConnectionManager $connectionManager)
1918
{
20-
if ($connector === null) {
21-
$connector = new Connector(array(), $loop);
22-
}
23-
24-
$this->connector = $connector;
19+
$this->connectionManager = $connectionManager;
2520
}
2621

2722
/** @return ClientRequestStream */
2823
public function request(RequestInterface $request)
2924
{
30-
return new ClientRequestStream($this->connector, $request);
25+
return new ClientRequestStream($this->connectionManager, $request);
3126
}
3227
}

‎src/Io/ClientConnectionManager.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
namespace React\Http\Io;
4+
5+
use Psr\Http\Message\UriInterface;
6+
use React\Promise\PromiseInterface;
7+
use React\Socket\ConnectionInterface;
8+
use React\Socket\ConnectorInterface;
9+
10+
/**
11+
* [Internal] Manages outgoing HTTP connections for the HTTP client
12+
*
13+
* @internal
14+
* @final
15+
*/
16+
class ClientConnectionManager
17+
{
18+
/** @var ConnectorInterface */
19+
private $connector;
20+
21+
public function __construct(ConnectorInterface $connector)
22+
{
23+
$this->connector = $connector;
24+
}
25+
26+
/**
27+
* @return PromiseInterface<ConnectionInterface>
28+
*/
29+
public function connect(UriInterface $uri)
30+
{
31+
$scheme = $uri->getScheme();
32+
if ($scheme !== 'https' && $scheme !== 'http') {
33+
return \React\Promise\reject(new \InvalidArgumentException(
34+
'Invalid request URL given'
35+
));
36+
}
37+
38+
$port = $uri->getPort();
39+
if ($port === null) {
40+
$port = $scheme === 'https' ? 443 : 80;
41+
}
42+
43+
return $this->connector->connect(($scheme === 'https' ? 'tls://' : '') . $uri->getHost() . ':' . $port);
44+
}
45+
}

‎src/Io/ClientRequestStream.php‎

Lines changed: 5 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,8 @@
44

55
use Evenement\EventEmitter;
66
use Psr\Http\Message\RequestInterface;
7-
use Psr\Http\Message\ResponseInterface;
87
use React\Http\Message\Response;
9-
use React\Promise;
108
use React\Socket\ConnectionInterface;
11-
use React\Socket\ConnectorInterface;
129
use React\Stream\WritableStreamInterface;
1310
use RingCentral\Psr7 as gPsr;
1411

@@ -26,8 +23,8 @@ class ClientRequestStream extends EventEmitter implements WritableStreamInterfac
2623
const STATE_HEAD_WRITTEN = 2;
2724
const STATE_END = 3;
2825

29-
/** @var ConnectorInterface */
30-
private $connector;
26+
/** @var ClientConnectionManager */
27+
private $connectionManager;
3128

3229
/** @var RequestInterface */
3330
private $request;
@@ -44,9 +41,9 @@ class ClientRequestStream extends EventEmitter implements WritableStreamInterfac
4441

4542
private $pendingWrites = '';
4643

47-
public function __construct(ConnectorInterface $connector, RequestInterface $request)
44+
public function __construct(ClientConnectionManager $connectionManager, RequestInterface $request)
4845
{
49-
$this->connector = $connector;
46+
$this->connectionManager = $connectionManager;
5047
$this->request = $request;
5148
}
5249

@@ -65,7 +62,7 @@ private function writeHead()
6562
$pendingWrites = &$this->pendingWrites;
6663
$that = $this;
6764

68-
$promise = $this->connect();
65+
$promise = $this->connectionManager->connect($this->request->getUri());
6966
$promise->then(
7067
function (ConnectionInterface $connection) use ($request, &$connectionRef, &$stateRef, &$pendingWrites, $that) {
7168
$connectionRef = $connection;
@@ -252,28 +249,4 @@ public function close()
252249
$this->emit('close');
253250
$this->removeAllListeners();
254251
}
255-
256-
protected function connect()
257-
{
258-
$scheme = $this->request->getUri()->getScheme();
259-
if ($scheme !== 'https' && $scheme !== 'http') {
260-
return Promise\reject(
261-
new \InvalidArgumentException('Invalid request URL given')
262-
);
263-
}
264-
265-
$host = $this->request->getUri()->getHost();
266-
$port = $this->request->getUri()->getPort();
267-
268-
if ($scheme === 'https') {
269-
$host = 'tls://' . $host;
270-
}
271-
272-
if ($port === null) {
273-
$port = $scheme === 'https' ? 443 : 80;
274-
}
275-
276-
return $this->connector
277-
->connect($host . ':' . $port);
278-
}
279252
}

‎src/Io/Sender.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use React\Http\Client\Client as HttpClient;
99
use React\Promise\PromiseInterface;
1010
use React\Promise\Deferred;
11+
use React\Socket\Connector;
1112
use React\Socket\ConnectorInterface;
1213
use React\Stream\ReadableStreamInterface;
1314

@@ -49,7 +50,11 @@ class Sender
4950
*/
5051
public static function createFromLoop(LoopInterface $loop, ConnectorInterface $connector = null)
5152
{
52-
return new self(new HttpClient($loop, $connector));
53+
if ($connector === null) {
54+
$connector = new Connector(array(), $loop);
55+
}
56+
57+
return new self(new HttpClient(new ClientConnectionManager($connector)));
5358
}
5459

5560
private $http;

‎tests/BrowserTest.php‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,13 @@ public function testConstructWithConnectorAssignsGivenConnector()
6060
$ref->setAccessible(true);
6161
$client = $ref->getValue($sender);
6262

63-
$ref = new \ReflectionProperty($client, 'connector');
63+
$ref = new \ReflectionProperty($client, 'connectionManager');
6464
$ref->setAccessible(true);
65-
$ret = $ref->getValue($client);
65+
$connectionManager = $ref->getValue($client);
66+
67+
$ref = new \ReflectionProperty($connectionManager, 'connector');
68+
$ref->setAccessible(true);
69+
$ret = $ref->getValue($connectionManager);
6670

6771
$this->assertSame($connector, $ret);
6872
}
@@ -85,9 +89,13 @@ public function testConstructWithConnectorWithLegacySignatureAssignsGivenConnect
8589
$ref->setAccessible(true);
8690
$client = $ref->getValue($sender);
8791

88-
$ref = new \ReflectionProperty($client, 'connector');
92+
$ref = new \ReflectionProperty($client, 'connectionManager');
93+
$ref->setAccessible(true);
94+
$connectionManager = $ref->getValue($client);
95+
96+
$ref = new \ReflectionProperty($connectionManager, 'connector');
8997
$ref->setAccessible(true);
90-
$ret = $ref->getValue($client);
98+
$ret = $ref->getValue($connectionManager);
9199

92100
$this->assertSame($connector, $ret);
93101
}

‎tests/Client/FunctionalIntegrationTest.php‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@
33
namespace React\Tests\Http\Client;
44

55
use Psr\Http\Message\ResponseInterface;
6-
use React\EventLoop\Loop;
76
use React\Http\Client\Client;
7+
use React\Http\Io\ClientConnectionManager;
88
use React\Http\Message\Request;
99
use React\Promise\Deferred;
1010
use React\Promise\Stream;
1111
use React\Socket\ConnectionInterface;
12+
use React\Socket\Connector;
1213
use React\Socket\SocketServer;
1314
use React\Stream\ReadableStreamInterface;
1415
use React\Tests\Http\TestCase;
@@ -45,7 +46,7 @@ public function testRequestToLocalhostEmitsSingleRemoteConnection()
4546
});
4647
$port = parse_url($socket->getAddress(), PHP_URL_PORT);
4748

48-
$client = new Client(Loop::get());
49+
$client = new Client(new ClientConnectionManager(new Connector()));
4950
$request = $client->request(new Request('GET', 'http://localhost:' . $port, array(), '', '1.0'));
5051

5152
$promise = Stream\first($request, 'close');
@@ -62,7 +63,7 @@ public function testRequestLegacyHttpServerWithOnlyLineFeedReturnsSuccessfulResp
6263
$socket->close();
6364
});
6465

65-
$client = new Client(Loop::get());
66+
$client = new Client(new ClientConnectionManager(new Connector()));
6667
$request = $client->request(new Request('GET', str_replace('tcp:', 'http:', $socket->getAddress()), array(), '', '1.0'));
6768

6869
$once = $this->expectCallableOnceWith('body');
@@ -82,7 +83,7 @@ public function testSuccessfulResponseEmitsEnd()
8283
// max_nesting_level was set to 100 for PHP Versions < 5.4 which resulted in failing test for legacy PHP
8384
ini_set('xdebug.max_nesting_level', 256);
8485

85-
$client = new Client(Loop::get());
86+
$client = new Client(new ClientConnectionManager(new Connector()));
8687

8788
$request = $client->request(new Request('GET', 'http://www.google.com/', array(), '', '1.0'));
8889

@@ -107,7 +108,7 @@ public function testPostDataReturnsData()
107108
// max_nesting_level was set to 100 for PHP Versions < 5.4 which resulted in failing test for legacy PHP
108109
ini_set('xdebug.max_nesting_level', 256);
109110

110-
$client = new Client(Loop::get());
111+
$client = new Client(new ClientConnectionManager(new Connector()));
111112

112113
$data = str_repeat('.', 33000);
113114
$request = $client->request(new Request('POST', 'https://' . (mt_rand(0, 1) === 0 ? 'eu.' : '') . 'httpbin.org/post', array('Content-Length' => strlen($data)), '', '1.0'));
@@ -139,7 +140,7 @@ public function testPostJsonReturnsData()
139140
$this->markTestSkipped('Not supported on HHVM');
140141
}
141142

142-
$client = new Client(Loop::get());
143+
$client = new Client(new ClientConnectionManager(new Connector()));
143144

144145
$data = json_encode(array('numbers' => range(1, 50)));
145146
$request = $client->request(new Request('POST', 'https://httpbin.org/post', array('Content-Length' => strlen($data), 'Content-Type' => 'application/json'), '', '1.0'));
@@ -169,7 +170,7 @@ public function testCancelPendingConnectionEmitsClose()
169170
// max_nesting_level was set to 100 for PHP Versions < 5.4 which resulted in failing test for legacy PHP
170171
ini_set('xdebug.max_nesting_level', 256);
171172

172-
$client = new Client(Loop::get());
173+
$client = new Client(new ClientConnectionManager(new Connector()));
173174

174175
$request = $client->request(new Request('GET', 'http://www.google.com/', array(), '', '1.0'));
175176
$request->on('error', $this->expectCallableNever());

‎tests/Io/ClientConnectionManagerTest.php‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
<?php
2+
3+
namespace React\Tests\Http\Io;
4+
5+
use RingCentral\Psr7\Uri;
6+
use React\Http\Io\ClientConnectionManager;
7+
use React\Promise\Promise;
8+
use React\Tests\Http\TestCase;
9+
10+
class ClientConnectionManagerTest extends TestCase
11+
{
12+
public function testConnectWithHttpsUriShouldConnectToTlsWithDefaultPort()
13+
{
14+
$promise = new Promise(function () { });
15+
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
16+
$connector->expects($this->once())->method('connect')->with('tls://reactphp.org:443')->willReturn($promise);
17+
18+
$connectionManager = new ClientConnectionManager($connector);
19+
20+
$ret = $connectionManager->connect(new Uri('https://reactphp.org/'));
21+
$this->assertSame($promise, $ret);
22+
}
23+
24+
public function testConnectWithHttpUriShouldConnectToTcpWithDefaultPort()
25+
{
26+
$promise = new Promise(function () { });
27+
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
28+
$connector->expects($this->once())->method('connect')->with('reactphp.org:80')->willReturn($promise);
29+
30+
$connectionManager = new ClientConnectionManager($connector);
31+
32+
$ret = $connectionManager->connect(new Uri('http://reactphp.org/'));
33+
$this->assertSame($promise, $ret);
34+
}
35+
36+
public function testConnectWithExplicitPortShouldConnectWithGivenPort()
37+
{
38+
$promise = new Promise(function () { });
39+
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
40+
$connector->expects($this->once())->method('connect')->with('reactphp.org:8080')->willReturn($promise);
41+
42+
$connectionManager = new ClientConnectionManager($connector);
43+
44+
$ret = $connectionManager->connect(new Uri('http://reactphp.org:8080/'));
45+
$this->assertSame($promise, $ret);
46+
}
47+
48+
public function testConnectWithInvalidSchemeShouldRejectWithException()
49+
{
50+
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
51+
$connector->expects($this->never())->method('connect');
52+
53+
$connectionManager = new ClientConnectionManager($connector);
54+
55+
$promise = $connectionManager->connect(new Uri('ftp://reactphp.org/'));
56+
57+
$exception = null;
58+
$promise->then(null, function ($reason) use (&$exception) {
59+
$exception = $reason;
60+
});
61+
62+
$this->assertInstanceOf('InvalidArgumentException', $exception);
63+
$this->assertEquals('Invalid request URL given', $exception->getMessage());
64+
}
65+
66+
public function testConnectWithoutSchemeShouldRejectWithException()
67+
{
68+
$connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
69+
$connector->expects($this->never())->method('connect');
70+
71+
$connectionManager = new ClientConnectionManager($connector);
72+
73+
$promise = $connectionManager->connect(new Uri('reactphp.org'));
74+
75+
$exception = null;
76+
$promise->then(null, function ($reason) use (&$exception) {
77+
$exception = $reason;
78+
});
79+
80+
$this->assertInstanceOf('InvalidArgumentException', $exception);
81+
$this->assertEquals('Invalid request URL given', $exception->getMessage());
82+
}
83+
}

0 commit comments

Comments
 (0)