Skip to content

Simplify App by always using default loop, drop optional loop instance #88

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SimonFrings merged 1 commit into from
Nov 30, 2021
Merged

Simplify App by always using default loop, drop optional loop instance #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 4 additions & 26 deletions src/App.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,13 @@
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use React\EventLoop\Loop;
use React\EventLoop\LoopInterface;
use React\Http\HttpServer;
use React\Promise\Deferred;
use React\Promise\PromiseInterface;
use React\Socket\SocketServer;

class App
{
private $loop;

/** @var MiddlewareHandler */
private $handler;

Expand All @@ -34,32 +31,13 @@ class App
* // instantiate with global middleware
* $app = new App($middleware);
* $app = new App($middleware1, $middleware2);
*
* // instantiate with optional $loop
* $app = new App($loop);
* $app = new App($loop, $middleware);
* $app = new App($loop, $middleware1, $middleware2);
*
* // invalid $loop argument
* $app = new App(null);
* $app = new App(null, $middleware);
* ```
*
* @param callable|LoopInterface|null $loop
* @param callable ...$middleware
* @throws \TypeError if given $loop argument is invalid
*/
public function __construct($loop = null, callable ...$middleware)
public function __construct(callable ...$middleware)
{
$errorHandler = new ErrorHandler();
if (\is_callable($loop)) {
\array_unshift($middleware, $loop);
$loop = null;
} elseif (\func_num_args() !== 0 && !$loop instanceof LoopInterface) {
throw new \TypeError('Argument 1 ($loop) must be callable|' . LoopInterface::class . ', ' . $errorHandler->describeType($loop) . ' given');
}

$this->loop = $loop ?? Loop::get();
$this->router = new RouteHandler();

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

$this->loop->run();
Loop::run();
}

private function runLoop()
{
$http = new HttpServer($this->loop, function (ServerRequestInterface $request) {
$http = new HttpServer(function (ServerRequestInterface $request) {
return $this->handleRequest($request);
});

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

$socket = new SocketServer($listen, [], $this->loop);
$socket = new SocketServer($listen);
$http->listen($socket);

$this->sapi->log('Listening on ' . \str_replace('tcp:', 'http:', $socket->getAddress()));
Expand Down
107 changes: 40 additions & 67 deletions tests/AppTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
use PHPUnit\Framework\TestCase;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use React\EventLoop\Loop;
use React\EventLoop\LoopInterface;
use React\Http\Message\Response;
use React\Http\Message\ServerRequest;
Expand All @@ -21,43 +20,14 @@
use ReflectionProperty;
use function React\Promise\reject;
use function React\Promise\resolve;
use React\EventLoop\Loop;

class AppTest extends TestCase
{
public function testConstructWithLoopAssignsGivenLoopInstance()
{
$loop = $this->createMock(LoopInterface::class);
$app = new App($loop);

$ref = new ReflectionProperty($app, 'loop');
$ref->setAccessible(true);
$ret = $ref->getValue($app);

$this->assertSame($loop, $ret);
}

public function testConstructWithoutLoopAssignsGlobalLoopInstance()
public function testConstructWithMiddlewareAssignsGivenMiddleware()
{
$app = new App();

$ref = new ReflectionProperty($app, 'loop');
$ref->setAccessible(true);
$ret = $ref->getValue($app);

$this->assertSame(Loop::get(), $ret);
}

public function testConstructWithLoopAndMiddlewareAssignsGivenLoopInstanceAndMiddleware()
{
$loop = $this->createMock(LoopInterface::class);
$middleware = function () { };
$app = new App($loop, $middleware);

$ref = new ReflectionProperty($app, 'loop');
$ref->setAccessible(true);
$ret = $ref->getValue($app);

$this->assertSame($loop, $ret);
$app = new App($middleware);

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

public function testConstructWithInvalidLoopThrows()
{
$this->expectException(\TypeError::class);
$this->expectExceptionMessage('Argument 1 ($loop) must be callable|React\EventLoop\LoopInterface, stdClass given');
new App((object)[]);
}

public function testConstructWithNullLoopButMiddlwareThrows()
{
$this->expectException(\TypeError::class);
$this->expectExceptionMessage('Argument 1 ($loop) must be callable|React\EventLoop\LoopInterface, null given');
new App(null, function () { });
}

public function testRunWillRunGivenLoopInstanceAndReportListeningAddress()
public function testRunWillReportListeningAddressAndRunLoopWithSocketServer()
{
$socket = @stream_socket_server('127.0.0.1:8080');
if ($socket === false) {
$this->markTestSkipped('Listen address :8080 already in use');
}
fclose($socket);

$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->once())->method('run');
$app = new App($loop);
$app = new App();

// lovely: remove socket server on next tick to terminate loop
Loop::futureTick(function () {
$resources = get_resources();
$socket = end($resources);

Loop::removeReadStream($socket);
fclose($socket);
});

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

public function testRunWillRunGivenLoopInstanceAndReportListeningAddressFromEnvironment()
public function testRunWillReportListeningAddressFromEnvironmentAndRunLoopWithSocketServer()
{
$socket = @stream_socket_server('127.0.0.1:0');
$addr = stream_socket_get_name($socket, false);
fclose($socket);

putenv('X_LISTEN=' . $addr);
$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->once())->method('run');
$app = new App($loop);
$app = new App();

// lovely: remove socket server on next tick to terminate loop
Loop::futureTick(function () {
$resources = get_resources();
$socket = end($resources);

Loop::removeReadStream($socket);
fclose($socket);
});

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

public function testRunWillRunGivenLoopInstanceAndReportListeningAddressFromEnvironmentWithRandomPort()
public function testRunWillReportListeningAddressFromEnvironmentWithRandomPortAndRunLoopWithSocketServer()
{
putenv('X_LISTEN=127.0.0.1:0');
$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->once())->method('run');
$app = new App($loop);
$app = new App();

// lovely: remove socket server on next tick to terminate loop
Loop::futureTick(function () {
$resources = get_resources();
$socket = end($resources);

Loop::removeReadStream($socket);
fclose($socket);
});

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

public function testRunAppWithEmptyAddressThrowsWithoutRunningLoop()
public function testRunAppWithEmptyAddressThrows()
{
putenv('X_LISTEN=');
$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->never())->method('run');
$app = new App($loop);
$app = new App();

$this->expectException(\InvalidArgumentException::class);
$app->run();
}

public function testRunAppWithBusyPortThrowsWithoutRunningLoop()
public function testRunAppWithBusyPortThrows()
{
$socket = @stream_socket_server('127.0.0.1:0');
$addr = stream_socket_get_name($socket, false);
Expand All @@ -152,9 +127,7 @@ public function testRunAppWithBusyPortThrowsWithoutRunningLoop()
}

putenv('X_LISTEN=' . $addr);
$loop = $this->createMock(LoopInterface::class);
$loop->expects($this->never())->method('run');
$app = new App($loop);
$app = new App();

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