Skip to content

Commit 5e86f28

Browse files
SimonFringsSimonFrings
authored
Merge pull request #88 from clue-labs/default-loop
Simplify `App` by always using default loop, drop optional loop instance
2 parents 7e919a1 + c1aadaa commit 5e86f28

2 files changed

Lines changed: 44 additions & 93 deletions

File tree

‎src/App.php‎

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,13 @@
55
use Psr\Http\Message\ResponseInterface;
66
use Psr\Http\Message\ServerRequestInterface;
77
use React\EventLoop\Loop;
8-
use React\EventLoop\LoopInterface;
98
use React\Http\HttpServer;
109
use React\Promise\Deferred;
1110
use React\Promise\PromiseInterface;
1211
use React\Socket\SocketServer;
1312

1413
class App
1514
{
16-
private $loop;
17-
1815
/** @var MiddlewareHandler */
1916
private $handler;
2017

@@ -34,32 +31,13 @@ class App
3431
* // instantiate with global middleware
3532
* $app = new App($middleware);
3633
* $app = new App($middleware1, $middleware2);
37-
*
38-
* // instantiate with optional $loop
39-
* $app = new App($loop);
40-
* $app = new App($loop, $middleware);
41-
* $app = new App($loop, $middleware1, $middleware2);
42-
*
43-
* // invalid $loop argument
44-
* $app = new App(null);
45-
* $app = new App(null, $middleware);
4634
* ```
4735
*
48-
* @param callable|LoopInterface|null $loop
4936
* @param callable ...$middleware
50-
* @throws \TypeError if given $loop argument is invalid
5137
*/
52-
public function __construct($loop = null, callable ...$middleware)
38+
public function __construct(callable ...$middleware)
5339
{
5440
$errorHandler = new ErrorHandler();
55-
if (\is_callable($loop)) {
56-
\array_unshift($middleware, $loop);
57-
$loop = null;
58-
} elseif (\func_num_args() !== 0 && !$loop instanceof LoopInterface) {
59-
throw new \TypeError('Argument 1 ($loop) must be callable|' . LoopInterface::class . ', ' . $errorHandler->describeType($loop) . ' given');
60-
}
61-
62-
$this->loop = $loop ?? Loop::get();
6341
$this->router = new RouteHandler();
6442

6543
// new MiddlewareHandler([$accessLogHandler, $errorHandler, ...$middleware, $routeHandler])
@@ -133,12 +111,12 @@ public function run()
133111
$this->runOnce(); // @codeCoverageIgnore
134112
}
135113

136-
$this->loop->run();
114+
Loop::run();
137115
}
138116

139117
private function runLoop()
140118
{
141-
$http = new HttpServer($this->loop, function (ServerRequestInterface $request) {
119+
$http = new HttpServer(function (ServerRequestInterface $request) {
142120
return $this->handleRequest($request);
143121
});
144122

@@ -147,7 +125,7 @@ private function runLoop()
147125
$listen = '127.0.0.1:8080';
148126
}
149127

150-
$socket = new SocketServer($listen, [], $this->loop);
128+
$socket = new SocketServer($listen);
151129
$http->listen($socket);
152130

153131
$this->sapi->log('Listening on ' . \str_replace('tcp:', 'http:', $socket->getAddress()));

‎tests/AppTest.php‎

Lines changed: 40 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use PHPUnit\Framework\TestCase;
1212
use Psr\Http\Message\ResponseInterface;
1313
use Psr\Http\Message\ServerRequestInterface;
14-
use React\EventLoop\Loop;
1514
use React\EventLoop\LoopInterface;
1615
use React\Http\Message\Response;
1716
use React\Http\Message\ServerRequest;
@@ -21,43 +20,14 @@
2120
use ReflectionProperty;
2221
use function React\Promise\reject;
2322
use function React\Promise\resolve;
23+
use React\EventLoop\Loop;
2424

2525
class AppTest extends TestCase
2626
{
27-
public function testConstructWithLoopAssignsGivenLoopInstance()
28-
{
29-
$loop = $this->createMock(LoopInterface::class);
30-
$app = new App($loop);
31-
32-
$ref = new ReflectionProperty($app, 'loop');
33-
$ref->setAccessible(true);
34-
$ret = $ref->getValue($app);
35-
36-
$this->assertSame($loop, $ret);
37-
}
38-
39-
public function testConstructWithoutLoopAssignsGlobalLoopInstance()
27+
public function testConstructWithMiddlewareAssignsGivenMiddleware()
4028
{
41-
$app = new App();
42-
43-
$ref = new ReflectionProperty($app, 'loop');
44-
$ref->setAccessible(true);
45-
$ret = $ref->getValue($app);
46-
47-
$this->assertSame(Loop::get(), $ret);
48-
}
49-
50-
public function testConstructWithLoopAndMiddlewareAssignsGivenLoopInstanceAndMiddleware()
51-
{
52-
$loop = $this->createMock(LoopInterface::class);
5329
$middleware = function () { };
54-
$app = new App($loop, $middleware);
55-
56-
$ref = new ReflectionProperty($app, 'loop');
57-
$ref->setAccessible(true);
58-
$ret = $ref->getValue($app);
59-
60-
$this->assertSame($loop, $ret);
30+
$app = new App($middleware);
6131

6232
$ref = new ReflectionProperty($app, 'handler');
6333
$ref->setAccessible(true);
@@ -75,74 +45,79 @@ public function testConstructWithLoopAndMiddlewareAssignsGivenLoopInstanceAndMid
7545
$this->assertInstanceOf(RouteHandler::class, $handlers[3]);
7646
}
7747

78-
public function testConstructWithInvalidLoopThrows()
79-
{
80-
$this->expectException(\TypeError::class);
81-
$this->expectExceptionMessage('Argument 1 ($loop) must be callable|React\EventLoop\LoopInterface, stdClass given');
82-
new App((object)[]);
83-
}
84-
85-
public function testConstructWithNullLoopButMiddlwareThrows()
86-
{
87-
$this->expectException(\TypeError::class);
88-
$this->expectExceptionMessage('Argument 1 ($loop) must be callable|React\EventLoop\LoopInterface, null given');
89-
new App(null, function () { });
90-
}
91-
92-
public function testRunWillRunGivenLoopInstanceAndReportListeningAddress()
48+
public function testRunWillReportListeningAddressAndRunLoopWithSocketServer()
9349
{
9450
$socket = @stream_socket_server('127.0.0.1:8080');
9551
if ($socket === false) {
9652
$this->markTestSkipped('Listen address :8080 already in use');
9753
}
9854
fclose($socket);
9955

100-
$loop = $this->createMock(LoopInterface::class);
101-
$loop->expects($this->once())->method('run');
102-
$app = new App($loop);
56+
$app = new App();
57+
58+
// lovely: remove socket server on next tick to terminate loop
59+
Loop::futureTick(function () {
60+
$resources = get_resources();
61+
$socket = end($resources);
62+
63+
Loop::removeReadStream($socket);
64+
fclose($socket);
65+
});
10366

10467
$this->expectOutputRegex('/' . preg_quote('Listening on http://127.0.0.1:8080' . PHP_EOL, '/') . '$/');
10568
$app->run();
10669
}
10770

108-
public function testRunWillRunGivenLoopInstanceAndReportListeningAddressFromEnvironment()
71+
public function testRunWillReportListeningAddressFromEnvironmentAndRunLoopWithSocketServer()
10972
{
11073
$socket = @stream_socket_server('127.0.0.1:0');
11174
$addr = stream_socket_get_name($socket, false);
11275
fclose($socket);
11376

11477
putenv('X_LISTEN=' . $addr);
115-
$loop = $this->createMock(LoopInterface::class);
116-
$loop->expects($this->once())->method('run');
117-
$app = new App($loop);
78+
$app = new App();
79+
80+
// lovely: remove socket server on next tick to terminate loop
81+
Loop::futureTick(function () {
82+
$resources = get_resources();
83+
$socket = end($resources);
84+
85+
Loop::removeReadStream($socket);
86+
fclose($socket);
87+
});
11888

11989
$this->expectOutputRegex('/' . preg_quote('Listening on http://' . $addr . PHP_EOL, '/') . '$/');
12090
$app->run();
12191
}
12292

123-
public function testRunWillRunGivenLoopInstanceAndReportListeningAddressFromEnvironmentWithRandomPort()
93+
public function testRunWillReportListeningAddressFromEnvironmentWithRandomPortAndRunLoopWithSocketServer()
12494
{
12595
putenv('X_LISTEN=127.0.0.1:0');
126-
$loop = $this->createMock(LoopInterface::class);
127-
$loop->expects($this->once())->method('run');
128-
$app = new App($loop);
96+
$app = new App();
97+
98+
// lovely: remove socket server on next tick to terminate loop
99+
Loop::futureTick(function () {
100+
$resources = get_resources();
101+
$socket = end($resources);
102+
103+
Loop::removeReadStream($socket);
104+
fclose($socket);
105+
});
129106

130107
$this->expectOutputRegex('/' . preg_quote('Listening on http://127.0.0.1:', '/') . '\d+' . PHP_EOL . '$/');
131108
$app->run();
132109
}
133110

134-
public function testRunAppWithEmptyAddressThrowsWithoutRunningLoop()
111+
public function testRunAppWithEmptyAddressThrows()
135112
{
136113
putenv('X_LISTEN=');
137-
$loop = $this->createMock(LoopInterface::class);
138-
$loop->expects($this->never())->method('run');
139-
$app = new App($loop);
114+
$app = new App();
140115

141116
$this->expectException(\InvalidArgumentException::class);
142117
$app->run();
143118
}
144119

145-
public function testRunAppWithBusyPortThrowsWithoutRunningLoop()
120+
public function testRunAppWithBusyPortThrows()
146121
{
147122
$socket = @stream_socket_server('127.0.0.1:0');
148123
$addr = stream_socket_get_name($socket, false);
@@ -152,9 +127,7 @@ public function testRunAppWithBusyPortThrowsWithoutRunningLoop()
152127
}
153128

154129
putenv('X_LISTEN=' . $addr);
155-
$loop = $this->createMock(LoopInterface::class);
156-
$loop->expects($this->never())->method('run');
157-
$app = new App($loop);
130+
$app = new App();
158131

159132
$this->expectException(\RuntimeException::class);
160133
$this->expectExceptionMessage('Failed to listen on');

0 commit comments

Comments
 (0)