From 40c6d6efa8459102463793be2e3377f8f0717953 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Tue, 15 Feb 2022 16:12:52 +0100 Subject: [PATCH 1/2] Automatically start new fiber for each request on PHP 8.1+ --- composer.json | 3 +- examples/index.php | 16 +++ src/App.php | 7 +- src/FiberHandler.php | 71 ++++++++++++ tests/AppMiddlewareTest.php | 68 +++++++++--- tests/AppTest.php | 213 ++++++++++++++++++++++++++++++++---- tests/FiberHandlerTest.php | 193 ++++++++++++++++++++++++++++++++ tests/acceptance.sh | 4 + 8 files changed, 539 insertions(+), 36 deletions(-) create mode 100644 src/FiberHandler.php create mode 100644 tests/FiberHandlerTest.php diff --git a/composer.json b/composer.json index c18e466..09420da 100644 --- a/composer.json +++ b/composer.json @@ -17,7 +17,8 @@ "react/promise": "^2.7" }, "require-dev": { - "phpunit/phpunit": "^9.5 || ^7.5" + "phpunit/phpunit": "^9.5 || ^7.5", + "react/async": "^4@dev || ^3@dev" }, "autoload": { "psr-4": { diff --git a/examples/index.php b/examples/index.php index 5f87b49..b16a3b8 100644 --- a/examples/index.php +++ b/examples/index.php @@ -24,6 +24,22 @@ ); }); +$app->get('/sleep/promise', function () { + return React\Promise\Timer\sleep(0.1)->then(function () { + return React\Http\Message\Response::plaintext("OK\n"); + }); +}); +$app->get('/sleep/coroutine', function () { + yield React\Promise\Timer\sleep(0.1); + return React\Http\Message\Response::plaintext("OK\n"); +}); +if (PHP_VERSION_ID >= 80100 && function_exists('React\Async\async')) { // requires PHP 8.1+ with react/async 4+ + $app->get('/sleep/fiber', function () { + React\Async\await(React\Promise\Timer\sleep(0.1)); + return React\Http\Message\Response::plaintext("OK\n"); + }); +} + $app->get('/uri[/{path:.*}]', function (ServerRequestInterface $request) { return React\Http\Message\Response::plaintext( (string) $request->getUri() . "\n" diff --git a/src/App.php b/src/App.php index cf8ed4e..bb95ebc 100644 --- a/src/App.php +++ b/src/App.php @@ -52,7 +52,7 @@ public function __construct(...$middleware) } } - // new MiddlewareHandler([$accessLogHandler, $errorHandler, ...$middleware, $routeHandler]) + // new MiddlewareHandler([$fiberHandler, $accessLogHandler, $errorHandler, ...$middleware, $routeHandler]) \array_unshift($middleware, $errorHandler); // only log for built-in webserver and PHP development webserver by default, others have their own access log @@ -60,6 +60,11 @@ public function __construct(...$middleware) \array_unshift($middleware, new AccessLogHandler()); } + // automatically start new fiber for each request on PHP 8.1+ + if (\PHP_VERSION_ID >= 80100) { + \array_unshift($middleware, new FiberHandler()); // @codeCoverageIgnore + } + $this->router = new RouteHandler($container); $middleware[] = $this->router; $this->handler = new MiddlewareHandler($middleware); diff --git a/src/FiberHandler.php b/src/FiberHandler.php new file mode 100644 index 0000000..38d7ab8 --- /dev/null +++ b/src/FiberHandler.php @@ -0,0 +1,71 @@ + + * Returns a promise that is fulfilled with a `ResponseInterface` on + * success. This method never throws or resolves a rejected promise. + * If the request can not be routed or the handler fails, it will be + * turned into a valid error response before returning. + * @throws void + */ + public function __invoke(ServerRequestInterface $request, callable $next): PromiseInterface + { + return new Promise(function ($resolve) use ($next, $request) { + $fiber = new \Fiber(function () use ($resolve, $next, $request) { + $response = $next($request); + if ($response instanceof \Generator) { + $response = $this->coroutine($response); + } + + $resolve($response); + }); + $fiber->start(); + }); + } + + private function coroutine(\Generator $generator): PromiseInterface + { + $next = null; + $deferred = new Deferred(); + $next = function () use ($generator, &$next, $deferred) { + if (!$generator->valid()) { + $deferred->resolve($generator->getReturn()); + return; + } + + $promise = $generator->current(); + $promise->then(function ($value) use ($generator, $next) { + $generator->send($value); + $next(); + }, function ($reason) use ($generator, $next) { + $generator->throw($reason); + $next(); + }); + }; + + $next(); + + return $deferred->promise(); + } +} diff --git a/tests/AppMiddlewareTest.php b/tests/AppMiddlewareTest.php index d891851..cc6f804 100644 --- a/tests/AppMiddlewareTest.php +++ b/tests/AppMiddlewareTest.php @@ -2,7 +2,9 @@ namespace FrameworkX\Tests; +use FrameworkX\AccessLogHandler; use FrameworkX\App; +use FrameworkX\FiberHandler; use FrameworkX\RouteHandler; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ResponseInterface; @@ -169,7 +171,7 @@ public function testMapMethodWithMiddlewareAddsGivenMethodsOnRouter() public function testMiddlewareCallsNextReturnsResponseFromRouter() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $middleware = function (ServerRequestInterface $request, callable $next) { return $next($request); @@ -203,7 +205,7 @@ public function testMiddlewareCallsNextReturnsResponseFromRouter() public function testMiddlewareCallsNextWithModifiedRequestReturnsResponseFromRouter() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $middleware = function (ServerRequestInterface $request, callable $next) { return $next($request->withAttribute('name', 'Alice')); @@ -237,7 +239,7 @@ public function testMiddlewareCallsNextWithModifiedRequestReturnsResponseFromRou public function testMiddlewareCallsNextReturnsResponseModifiedInMiddlewareFromRouter() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $middleware = function (ServerRequestInterface $request, callable $next) { $response = $next($request); @@ -364,7 +366,7 @@ public function testMiddlewareCallsNextReturnsCoroutineResponseModifiedInMiddlew public function testMiddlewareCallsNextWhichThrowsExceptionReturnsInternalServerErrorResponse() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $middleware = function (ServerRequestInterface $request, callable $next) { return $next($request); @@ -395,7 +397,7 @@ public function testMiddlewareCallsNextWhichThrowsExceptionReturnsInternalServer public function testMiddlewareWhichThrowsExceptionReturnsInternalServerErrorResponse() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $line = __LINE__ + 2; $middleware = function (ServerRequestInterface $request, callable $next) { @@ -424,7 +426,7 @@ public function testMiddlewareWhichThrowsExceptionReturnsInternalServerErrorResp public function testGlobalMiddlewareCallsNextReturnsResponseFromController() { - $app = $this->createAppWithoutLogger(function (ServerRequestInterface $request, callable $next) { + $app = $this->createAppWithoutFibersOrLogger(function (ServerRequestInterface $request, callable $next) { return $next($request); }); @@ -461,7 +463,7 @@ public function __invoke(ServerRequestInterface $request, callable $next) } }; - $app = $this->createAppWithoutLogger($middleware); + $app = $this->createAppWithoutFibersOrLogger($middleware); $app->get('/', function () { return new Response( @@ -496,7 +498,7 @@ public function __invoke(ServerRequestInterface $request, callable $next) } }; - $app = $this->createAppWithoutLogger(get_class($middleware)); + $app = $this->createAppWithoutFibersOrLogger(get_class($middleware)); $app->get('/', function () { return new Response( @@ -532,7 +534,7 @@ public function __invoke(ServerRequestInterface $request, callable $next) } }; - $app = $this->createAppWithoutLogger(get_class($middleware)); + $app = $this->createAppWithoutFibersOrLogger(get_class($middleware)); $app->get('/', get_class($middleware), function (ServerRequestInterface $request) { return new Response( @@ -560,7 +562,7 @@ public function __invoke(ServerRequestInterface $request, callable $next) public function testGlobalMiddlewareCallsNextWithModifiedRequestWillBeUsedForRouting() { - $app = $this->createAppWithoutLogger(function (ServerRequestInterface $request, callable $next) { + $app = $this->createAppWithoutFibersOrLogger(function (ServerRequestInterface $request, callable $next) { return $next($request->withUri($request->getUri()->withPath('/users'))); }); @@ -590,7 +592,7 @@ public function testGlobalMiddlewareCallsNextWithModifiedRequestWillBeUsedForRou public function testGlobalMiddlewareCallsNextReturnsModifiedResponseWhenModifyingResponseFromRouter() { - $app = $this->createAppWithoutLogger(function (ServerRequestInterface $request, callable $next) { + $app = $this->createAppWithoutFibersOrLogger(function (ServerRequestInterface $request, callable $next) { $response = $next($request); assert($response instanceof ResponseInterface); @@ -621,7 +623,7 @@ public function testGlobalMiddlewareCallsNextReturnsModifiedResponseWhenModifyin public function testGlobalMiddlewareReturnsResponseWithoutCallingNextReturnsResponseWithoutCallingRouter() { - $app = $this->createAppWithoutLogger(function () { + $app = $this->createAppWithoutFibersOrLogger(function () { return new Response( 200, [ @@ -788,8 +790,46 @@ private function createAppWithoutLogger(...$middleware): App $ref->setAccessible(true); $handlers = $ref->getValue($middleware); - unset($handlers[0]); - $ref->setValue($middleware, array_values($handlers)); + if (PHP_VERSION_ID >= 80100) { + $first = array_shift($handlers); + $this->assertInstanceOf(FiberHandler::class, $first); + + $next = array_shift($handlers); + $this->assertInstanceOf(AccessLogHandler::class, $next); + + array_unshift($handlers, $next, $first); + } + + $first = array_shift($handlers); + $this->assertInstanceOf(AccessLogHandler::class, $first); + + $ref->setValue($middleware, $handlers); + + return $app; + } + + /** @param callable|class-string ...$middleware */ + private function createAppWithoutFibersOrLogger(...$middleware): App + { + $app = new App(...$middleware); + + $ref = new \ReflectionProperty($app, 'handler'); + $ref->setAccessible(true); + $middleware = $ref->getValue($app); + + $ref = new \ReflectionProperty($middleware, 'handlers'); + $ref->setAccessible(true); + $handlers = $ref->getValue($middleware); + + if (PHP_VERSION_ID >= 80100) { + $first = array_shift($handlers); + $this->assertInstanceOf(FiberHandler::class, $first); + } + + $first = array_shift($handlers); + $this->assertInstanceOf(AccessLogHandler::class, $first); + + $ref->setValue($middleware, $handlers); return $app; } diff --git a/tests/AppTest.php b/tests/AppTest.php index 7733ff5..8261cdf 100644 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -6,6 +6,7 @@ use FrameworkX\App; use FrameworkX\Container; use FrameworkX\ErrorHandler; +use FrameworkX\FiberHandler; use FrameworkX\MiddlewareHandler; use FrameworkX\RouteHandler; use FrameworkX\SapiHandler; @@ -30,6 +31,7 @@ use React\Promise\PromiseInterface; use ReflectionMethod; use ReflectionProperty; +use function React\Async\await; use function React\Promise\reject; use function React\Promise\resolve; @@ -72,6 +74,11 @@ public function testConstructWithMiddlewareAssignsGivenMiddleware() $ref->setAccessible(true); $handlers = $ref->getValue($handler); + if (PHP_VERSION_ID >= 80100) { + $first = array_shift($handlers); + $this->assertInstanceOf(FiberHandler::class, $first); + } + $this->assertCount(4, $handlers); $this->assertInstanceOf(AccessLogHandler::class, $handlers[0]); $this->assertInstanceOf(ErrorHandler::class, $handlers[1]); @@ -93,6 +100,11 @@ public function testConstructWithContainerAssignsContainerForRouteHandlerOnly() $ref->setAccessible(true); $handlers = $ref->getValue($handler); + if (PHP_VERSION_ID >= 80100) { + $first = array_shift($handlers); + $this->assertInstanceOf(FiberHandler::class, $first); + } + $this->assertCount(3, $handlers); $this->assertInstanceOf(AccessLogHandler::class, $handlers[0]); $this->assertInstanceOf(ErrorHandler::class, $handlers[1]); @@ -122,6 +134,11 @@ public function testConstructWithContainerAndMiddlewareClassNameAssignsCallableF $ref->setAccessible(true); $handlers = $ref->getValue($handler); + if (PHP_VERSION_ID >= 80100) { + $first = array_shift($handlers); + $this->assertInstanceOf(FiberHandler::class, $first); + } + $this->assertCount(4, $handlers); $this->assertInstanceOf(AccessLogHandler::class, $handlers[0]); $this->assertInstanceOf(ErrorHandler::class, $handlers[1]); @@ -225,7 +242,7 @@ public function testRunAppWithBusyPortThrows() public function testRunOnceWillCreateRequestFromSapiThenRouteRequestAndThenSendResponseFromHandler() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $response = new Response(); $app->get('/', function () use ($response) { @@ -251,7 +268,7 @@ public function testRunOnceWillCreateRequestFromSapiThenRouteRequestAndThenSendR public function testRunOnceWillCreateRequestFromSapiThenRouteRequestAndThenSendResponseFromDeferredHandler() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $response = new Response(); $app->get('/', function () use ($response) { @@ -467,7 +484,7 @@ public function testRedirectMethodWithCustomRedirectCodeAddsAnyRouteOnRouterWhic public function testHandleRequestWithProxyRequestReturnsResponseWithMessageThatProxyRequestsAreNotAllowed() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $request = new ServerRequest('GET', 'http://google.com/'); $request = $request->withRequestTarget('http://google.com/'); @@ -489,7 +506,7 @@ public function testHandleRequestWithProxyRequestReturnsResponseWithMessageThatP public function testHandleRequestWithUnknownRouteReturnsResponseWithFileNotFoundMessage() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $request = new ServerRequest('GET', 'http://localhost/invalid'); @@ -510,7 +527,7 @@ public function testHandleRequestWithUnknownRouteReturnsResponseWithFileNotFound public function testHandleRequestWithInvalidRequestMethodReturnsResponseWithSingleMethodNotAllowedMessage() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $app->get('/users', function () { }); @@ -534,7 +551,7 @@ public function testHandleRequestWithInvalidRequestMethodReturnsResponseWithSing public function testHandleRequestWithInvalidRequestMethodReturnsResponseWithMultipleMethodNotAllowedMessage() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $app->get('/users', function () { }); $app->head('/users', function () { }); @@ -560,7 +577,7 @@ public function testHandleRequestWithInvalidRequestMethodReturnsResponseWithMult public function testHandleRequestWithMatchingRouteReturnsResponseFromMatchingRouteHandler() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $app->get('/users', function () { return new Response( @@ -588,7 +605,7 @@ public function testHandleRequestWithMatchingRouteReturnsResponseFromMatchingRou public function testHandleRequestWithOptionsAsteriskRequestReturnsResponseFromMatchingEmptyRouteHandler() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $app->options('', function () { return new Response( @@ -681,7 +698,7 @@ public function testHandleRequestWithMatchingRouteReturnsPendingPromiseWhenHandl public function testHandleRequestWithMatchingRouteReturnsResponseWhenHandlerReturnsCoroutineWhichReturnsResponseWithoutYielding() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $app->get('/users', function () { if (false) { @@ -820,10 +837,90 @@ public function testHandleRequestWithMatchingRouteReturnsPendingPromiseWhenHandl $this->assertFalse($resolved); } - public function testHandleRequestWithMatchingRouteAndRouteVariablesReturnsResponseFromHandlerWithRouteVariablesAssignedAsRequestAttributes() + public function testHandleRequestWithMatchingRouteReturnsPromiseResolvingWithResponseWhenHandlerReturnsResponse() + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped('Requires PHP 8.1+'); + } + + $app = $this->createAppWithoutLogger(); + + $app->get('/users', function () { + return new Response( + 200, + [ + 'Content-Type' => 'text/html' + ], + "OK\n" + ); + }); + + $request = new ServerRequest('GET', 'http://localhost/users'); + + // $promise = $app->handleRequest($request); + $ref = new ReflectionMethod($app, 'handleRequest'); + $ref->setAccessible(true); + $promise = $ref->invoke($app, $request); + + /** @var PromiseInterface $promise */ + $this->assertInstanceOf(PromiseInterface::class, $promise); + + $response = null; + $promise->then(function ($value) use (&$response) { + $response = $value; + }); + + /** @var ResponseInterface $response */ + $this->assertInstanceOf(ResponseInterface::class, $response); + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals('text/html', $response->getHeaderLine('Content-Type')); + $this->assertEquals("OK\n", (string) $response->getBody()); + } + + public function testHandleRequestWithMatchingRouteReturnsPromiseResolvingWithResponseWhenHandlerReturnsResponseAfterAwaitingPromiseResolvingWithResponse() { + if (PHP_VERSION_ID < 80100 || !function_exists('React\Async\async')) { + $this->markTestSkipped('Requires PHP 8.1+ with react/async 4+'); + } + $app = $this->createAppWithoutLogger(); + $app->get('/users', function () { + return await(resolve(new Response( + 200, + [ + 'Content-Type' => 'text/html' + ], + "OK\n" + ))); + }); + + $request = new ServerRequest('GET', 'http://localhost/users'); + + // $promise = $app->handleRequest($request); + $ref = new ReflectionMethod($app, 'handleRequest'); + $ref->setAccessible(true); + $promise = $ref->invoke($app, $request); + + /** @var PromiseInterface $promise */ + $this->assertInstanceOf(PromiseInterface::class, $promise); + + $response = null; + $promise->then(function ($value) use (&$response) { + $response = $value; + }); + + /** @var ResponseInterface $response */ + $this->assertInstanceOf(ResponseInterface::class, $response); + $this->assertEquals(200, $response->getStatusCode()); + $this->assertEquals('text/html', $response->getHeaderLine('Content-Type')); + $this->assertEquals("OK\n", (string) $response->getBody()); + } + + public function testHandleRequestWithMatchingRouteAndRouteVariablesReturnsResponseFromHandlerWithRouteVariablesAssignedAsRequestAttributes() + { + $app = $this->createAppWithoutFibersOrLogger(); + $app->get('/users/{name}', function (ServerRequestInterface $request) { $name = $request->getAttribute('name'); @@ -852,7 +949,7 @@ public function testHandleRequestWithMatchingRouteAndRouteVariablesReturnsRespon public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerThrowsException() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $line = __LINE__ + 2; $app->get('/users', function () { @@ -983,7 +1080,7 @@ public function testHandleRequestWithMatchingRouteReturnsPromiseWhichFulfillsWit public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerReturnsCoroutineWhichThrowsExceptionWithoutYielding() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $line = __LINE__ + 5; $app->get('/users', function () { @@ -1047,6 +1144,45 @@ public function testHandleRequestWithMatchingRouteReturnsPromiseWhichFulfillsWit $this->assertStringContainsString("

Expected request handler to return Psr\Http\Message\ResponseInterface but got uncaught RuntimeException with message Foo in AppTest.php:$line.

\n", (string) $response->getBody()); } + public function testHandleRequestWithMatchingRouteReturnsPromiseWhichFulfillsWithInternalServerErrorResponseWhenHandlerThrowsAfterAwaitingPromiseRejectingWithException() + { + if (PHP_VERSION_ID < 80100 || !function_exists('React\Async\async')) { + $this->markTestSkipped('Requires PHP 8.1+ with react/async 4+'); + } + + $app = $this->createAppWithoutLogger(); + + $line = __LINE__ + 2; + $app->get('/users', function () { + return await(reject(new \RuntimeException('Foo'))); + }); + + $request = new ServerRequest('GET', 'http://localhost/users'); + + // $promise = $app->handleRequest($request); + $ref = new ReflectionMethod($app, 'handleRequest'); + $ref->setAccessible(true); + $promise = $ref->invoke($app, $request); + + /** @var PromiseInterface $promise */ + $this->assertInstanceOf(PromiseInterface::class, $promise); + + $response = null; + $promise->then(function ($value) use (&$response) { + $response = $value; + }); + + /** @var ResponseInterface $response */ + $this->assertInstanceOf(ResponseInterface::class, $response); + $this->assertEquals(500, $response->getStatusCode()); + $this->assertEquals('text/html; charset=utf-8', $response->getHeaderLine('Content-Type')); + $this->assertStringMatchesFormat("\n%a\n", (string) $response->getBody()); + + $this->assertStringContainsString("Error 500: Internal Server Error\n", (string) $response->getBody()); + $this->assertStringContainsString("

The requested page failed to load, please try again later.

\n", (string) $response->getBody()); + $this->assertStringContainsString("

Expected request handler to return Psr\Http\Message\ResponseInterface but got uncaught RuntimeException with message Foo in AppTest.php:$line.

\n", (string) $response->getBody()); + } + public function testHandleRequestWithMatchingRouteReturnsPromiseWhichFulfillsWithInternalServerErrorResponseWhenHandlerReturnsCoroutineWhichReturnsNull() { $app = $this->createAppWithoutLogger(); @@ -1084,7 +1220,7 @@ public function testHandleRequestWithMatchingRouteReturnsPromiseWhichFulfillsWit public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerReturnsCoroutineWhichYieldsNullImmediately() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $line = __LINE__ + 3; $app->get('/users', function () { @@ -1111,7 +1247,7 @@ public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResp public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerReturnsWrongValue() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $app->get('/users', function () { return null; @@ -1137,7 +1273,7 @@ public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResp public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerClassDoesNotExist() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $app->get('/users', 'UnknownClass'); @@ -1228,7 +1364,7 @@ public function provideInvalidClasses() */ public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerClassIsInvalid(string $class, string $error) { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $app->get('/users', $class); @@ -1252,7 +1388,7 @@ public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResp public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerClassRequiresUnexpectedCallableParameter() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $line = __LINE__ + 2; $controller = new class { @@ -1281,7 +1417,7 @@ public function __invoke(int $value) { } public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerClassHasNoInvokeMethod() { - $app = $this->createAppWithoutLogger(); + $app = $this->createAppWithoutFibersOrLogger(); $controller = new class { }; @@ -1386,8 +1522,45 @@ private function createAppWithoutLogger(): App $ref->setAccessible(true); $handlers = $ref->getValue($middleware); - unset($handlers[0]); - $ref->setValue($middleware, array_values($handlers)); + if (PHP_VERSION_ID >= 80100) { + $first = array_shift($handlers); + $this->assertInstanceOf(FiberHandler::class, $first); + + $next = array_shift($handlers); + $this->assertInstanceOf(AccessLogHandler::class, $next); + + array_unshift($handlers, $next, $first); + } + + $first = array_shift($handlers); + $this->assertInstanceOf(AccessLogHandler::class, $first); + + $ref->setValue($middleware, $handlers); + + return $app; + } + + private function createAppWithoutFibersOrLogger(): App + { + $app = new App(); + + $ref = new \ReflectionProperty($app, 'handler'); + $ref->setAccessible(true); + $middleware = $ref->getValue($app); + + $ref = new \ReflectionProperty($middleware, 'handlers'); + $ref->setAccessible(true); + $handlers = $ref->getValue($middleware); + + if (PHP_VERSION_ID >= 80100) { + $first = array_shift($handlers); + $this->assertInstanceOf(FiberHandler::class, $first); + } + + $first = array_shift($handlers); + $this->assertInstanceOf(AccessLogHandler::class, $first); + + $ref->setValue($middleware, $handlers); return $app; } diff --git a/tests/FiberHandlerTest.php b/tests/FiberHandlerTest.php new file mode 100644 index 0000000..8525c30 --- /dev/null +++ b/tests/FiberHandlerTest.php @@ -0,0 +1,193 @@ +markTestSkipped('Requires PHP 8.1+ with react/async 4+'); + } + } + + public function testInvokeWithHandlerReturningResponseReturnsPromiseResolvingWithSameResponse() + { + $handler = new FiberHandler(); + + $request = new ServerRequest('GET', 'http://example.com/'); + $response = new Response(); + + $promise = $handler($request, function () use ($response) { return $response; }); + + /** @var PromiseInterface $promise */ + $this->assertInstanceOf(PromiseInterface::class, $promise); + + $ret = null; + $promise->then(function ($value) use (&$ret) { + $ret = $value; + }); + + $this->assertSame($response, $ret); + } + + public function testInvokeWithHandlerReturningPromiseResolvingWithResponseReturnsPromiseResolvingWithSameResponse() + { + $handler = new FiberHandler(); + + $request = new ServerRequest('GET', 'http://example.com/'); + $response = new Response(); + + $promise = $handler($request, function () use ($response) { return resolve($response); }); + + /** @var PromiseInterface $promise */ + $this->assertInstanceOf(PromiseInterface::class, $promise); + + $ret = null; + $promise->then(function ($value) use (&$ret) { + $ret = $value; + }); + + $this->assertSame($response, $ret); + } + + public function testInvokeWithHandlerReturningGeneratorReturningResponseReturnsPromiseResolvingWithSameResponse() + { + $handler = new FiberHandler(); + + $request = new ServerRequest('GET', 'http://example.com/'); + $response = new Response(); + + $promise = $handler($request, function () use ($response) { + if (false) { + yield; + } + return $response; + }); + + /** @var PromiseInterface $promise */ + $this->assertInstanceOf(PromiseInterface::class, $promise); + + $ret = null; + $promise->then(function ($value) use (&$ret) { + $ret = $value; + }); + + $this->assertSame($response, $ret); + } + + public function testInvokeWithHandlerReturningGeneratorReturningResponseAfterYieldingResolvedPromiseReturnsPromiseResolvingWithSameResponse() + { + $handler = new FiberHandler(); + + $request = new ServerRequest('GET', 'http://example.com/'); + $response = new Response(); + + $promise = $handler($request, function () use ($response) { + return yield resolve($response); + }); + + /** @var PromiseInterface $promise */ + $this->assertInstanceOf(PromiseInterface::class, $promise); + + $ret = null; + $promise->then(function ($value) use (&$ret) { + $ret = $value; + }); + + $this->assertSame($response, $ret); + } + + public function testInvokeWithHandlerReturningGeneratorReturningResponseAfterYieldingRejectedPromiseReturnsPromiseResolvingWithSameResponse() + { + $handler = new FiberHandler(); + + $request = new ServerRequest('GET', 'http://example.com/'); + $response = new Response(); + + $promise = $handler($request, function () use ($response) { + try { + yield reject(new \RuntimeException('Foo')); + } catch (\RuntimeException $e) { + return $response; + } + }); + + /** @var PromiseInterface $promise */ + $this->assertInstanceOf(PromiseInterface::class, $promise); + + $ret = null; + $promise->then(function ($value) use (&$ret) { + $ret = $value; + }); + + $this->assertSame($response, $ret); + } + + public function testInvokeWithHandlerReturningResponseAfterAwaitingResolvedPromiseReturnsPromiseResolvingWithSameResponse() + { + $handler = new FiberHandler(); + + $request = new ServerRequest('GET', 'http://example.com/'); + $response = new Response(); + + $promise = $handler($request, function () use ($response) { + return await(resolve($response)); + }); + + /** @var PromiseInterface $promise */ + $this->assertInstanceOf(PromiseInterface::class, $promise); + + $ret = null; + $promise->then(function ($value) use (&$ret) { + $ret = $value; + }); + + $this->assertSame($response, $ret); + } + + public function testInvokeWithHandlerReturningResponseAfterAwaitingPendingPromiseReturnsPromiseResolvingWithSameResponse() + { + $handler = new FiberHandler(); + + $request = new ServerRequest('GET', 'http://example.com/'); + $response = new Response(); + + $deferred = new Deferred(); + + $promise = $handler($request, function () use ($deferred) { + return await($deferred->promise()); + }); + + /** @var PromiseInterface $promise */ + $this->assertInstanceOf(PromiseInterface::class, $promise); + + $ret = null; + $promise->then(function ($value) use (&$ret) { + $ret = $value; + }); + + $this->assertNull($ret); + + $deferred->resolve($response); + + // await next tick: https://github.com/reactphp/async/issues/27 + await(new Promise(function ($resolve) { + Loop::futureTick($resolve); + })); + + $this->assertSame($response, $ret); + } +} diff --git a/tests/acceptance.sh b/tests/acceptance.sh index 5ecb1fd..dec09b7 100755 --- a/tests/acceptance.sh +++ b/tests/acceptance.sh @@ -26,6 +26,10 @@ out=$(curl -v $base/ 2>&1 -X POST); match "HTTP/.* 405" out=$(curl -v $base/error 2>&1); match "HTTP/.* 500" && match -iP "Content-Type: text/html; charset=utf-8[\r\n]" && match "Unable to load error" out=$(curl -v $base/error/null 2>&1); match "HTTP/.* 500" && match -iP "Content-Type: text/html; charset=utf-8[\r\n]" +out=$(curl -v $base/sleep/promise 2>&1); match "HTTP/.* 200" && match -iP "Content-Type: text/plain; charset=utf-8[\r\n]" +out=$(curl -v $base/sleep/coroutine 2>&1); match "HTTP/.* 200" && match -iP "Content-Type: text/plain; charset=utf-8[\r\n]" +out=$(curl -v $base/sleep/fiber 2>&1); skipif "HTTP/.* 404" && match "HTTP/.* 200" && match -iP "Content-Type: text/plain; charset=utf-8[\r\n]" # skip PHP < 8.1 + out=$(curl -v $base/uri 2>&1); match "HTTP/.* 200" && match "$base/uri" out=$(curl -v $base/uri/ 2>&1); match "HTTP/.* 200" && match "$base/uri/" out=$(curl -v $base/uri/foo 2>&1); match "HTTP/.* 200" && match "$base/uri/foo" From f61d0e3e3579da092b8cfda88022c082e9d14d29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Tue, 15 Feb 2022 20:21:05 +0100 Subject: [PATCH 2/2] Improve `FiberHandler` to only wrap in promise when using `await()` --- src/FiberHandler.php | 63 +++++++-------- tests/AppMiddlewareTest.php | 50 +++--------- tests/AppTest.php | 148 +++++++++++++----------------------- tests/FiberHandlerTest.php | 68 +++++------------ 4 files changed, 113 insertions(+), 216 deletions(-) diff --git a/src/FiberHandler.php b/src/FiberHandler.php index 38d7ab8..bc432dd 100644 --- a/src/FiberHandler.php +++ b/src/FiberHandler.php @@ -5,7 +5,6 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use React\Promise\Deferred; -use React\Promise\Promise; use React\Promise\PromiseInterface; /** @@ -22,50 +21,40 @@ class FiberHandler { /** - * @return PromiseInterface - * Returns a promise that is fulfilled with a `ResponseInterface` on - * success. This method never throws or resolves a rejected promise. - * If the request can not be routed or the handler fails, it will be - * turned into a valid error response before returning. + * @return ResponseInterface|PromiseInterface|\Generator + * Returns a `ResponseInterface` from the next request handler in the + * chain. If the next request handler returns immediately, this method + * will return immediately. If the next request handler suspends the + * fiber (see `await()`), this method will return a `PromiseInterface` + * that is fulfilled with a `ResponseInterface` when the fiber is + * terminated successfully. If the next request handler returns a + * promise, this method will return a promise that follows its + * resolution. If the next request handler returns a Generator-based + * coroutine, this method returns a `Generator`. This method never + * throws or resolves a rejected promise. If the handler fails, it will + * be turned into a valid error response before returning. * @throws void */ - public function __invoke(ServerRequestInterface $request, callable $next): PromiseInterface + public function __invoke(ServerRequestInterface $request, callable $next): mixed { - return new Promise(function ($resolve) use ($next, $request) { - $fiber = new \Fiber(function () use ($resolve, $next, $request) { - $response = $next($request); - if ($response instanceof \Generator) { - $response = $this->coroutine($response); - } + $deferred = null; + $fiber = new \Fiber(function () use ($request, $next, &$deferred) { + $response = $next($request); + assert($response instanceof ResponseInterface || $response instanceof PromiseInterface || $response instanceof \Generator); - $resolve($response); - }); - $fiber->start(); - }); - } - - private function coroutine(\Generator $generator): PromiseInterface - { - $next = null; - $deferred = new Deferred(); - $next = function () use ($generator, &$next, $deferred) { - if (!$generator->valid()) { - $deferred->resolve($generator->getReturn()); - return; + if ($deferred !== null) { + $deferred->resolve($response); } - $promise = $generator->current(); - $promise->then(function ($value) use ($generator, $next) { - $generator->send($value); - $next(); - }, function ($reason) use ($generator, $next) { - $generator->throw($reason); - $next(); - }); - }; + return $response; + }); - $next(); + $fiber->start(); + if ($fiber->isTerminated()) { + return $fiber->getReturn(); + } + $deferred = new Deferred(); return $deferred->promise(); } } diff --git a/tests/AppMiddlewareTest.php b/tests/AppMiddlewareTest.php index cc6f804..35fec4d 100644 --- a/tests/AppMiddlewareTest.php +++ b/tests/AppMiddlewareTest.php @@ -171,7 +171,7 @@ public function testMapMethodWithMiddlewareAddsGivenMethodsOnRouter() public function testMiddlewareCallsNextReturnsResponseFromRouter() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $middleware = function (ServerRequestInterface $request, callable $next) { return $next($request); @@ -205,7 +205,7 @@ public function testMiddlewareCallsNextReturnsResponseFromRouter() public function testMiddlewareCallsNextWithModifiedRequestReturnsResponseFromRouter() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $middleware = function (ServerRequestInterface $request, callable $next) { return $next($request->withAttribute('name', 'Alice')); @@ -239,7 +239,7 @@ public function testMiddlewareCallsNextWithModifiedRequestReturnsResponseFromRou public function testMiddlewareCallsNextReturnsResponseModifiedInMiddlewareFromRouter() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $middleware = function (ServerRequestInterface $request, callable $next) { $response = $next($request); @@ -366,7 +366,7 @@ public function testMiddlewareCallsNextReturnsCoroutineResponseModifiedInMiddlew public function testMiddlewareCallsNextWhichThrowsExceptionReturnsInternalServerErrorResponse() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $middleware = function (ServerRequestInterface $request, callable $next) { return $next($request); @@ -397,7 +397,7 @@ public function testMiddlewareCallsNextWhichThrowsExceptionReturnsInternalServer public function testMiddlewareWhichThrowsExceptionReturnsInternalServerErrorResponse() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $line = __LINE__ + 2; $middleware = function (ServerRequestInterface $request, callable $next) { @@ -426,7 +426,7 @@ public function testMiddlewareWhichThrowsExceptionReturnsInternalServerErrorResp public function testGlobalMiddlewareCallsNextReturnsResponseFromController() { - $app = $this->createAppWithoutFibersOrLogger(function (ServerRequestInterface $request, callable $next) { + $app = $this->createAppWithoutLogger(function (ServerRequestInterface $request, callable $next) { return $next($request); }); @@ -463,7 +463,7 @@ public function __invoke(ServerRequestInterface $request, callable $next) } }; - $app = $this->createAppWithoutFibersOrLogger($middleware); + $app = $this->createAppWithoutLogger($middleware); $app->get('/', function () { return new Response( @@ -498,7 +498,7 @@ public function __invoke(ServerRequestInterface $request, callable $next) } }; - $app = $this->createAppWithoutFibersOrLogger(get_class($middleware)); + $app = $this->createAppWithoutLogger(get_class($middleware)); $app->get('/', function () { return new Response( @@ -534,7 +534,7 @@ public function __invoke(ServerRequestInterface $request, callable $next) } }; - $app = $this->createAppWithoutFibersOrLogger(get_class($middleware)); + $app = $this->createAppWithoutLogger(get_class($middleware)); $app->get('/', get_class($middleware), function (ServerRequestInterface $request) { return new Response( @@ -562,7 +562,7 @@ public function __invoke(ServerRequestInterface $request, callable $next) public function testGlobalMiddlewareCallsNextWithModifiedRequestWillBeUsedForRouting() { - $app = $this->createAppWithoutFibersOrLogger(function (ServerRequestInterface $request, callable $next) { + $app = $this->createAppWithoutLogger(function (ServerRequestInterface $request, callable $next) { return $next($request->withUri($request->getUri()->withPath('/users'))); }); @@ -592,7 +592,7 @@ public function testGlobalMiddlewareCallsNextWithModifiedRequestWillBeUsedForRou public function testGlobalMiddlewareCallsNextReturnsModifiedResponseWhenModifyingResponseFromRouter() { - $app = $this->createAppWithoutFibersOrLogger(function (ServerRequestInterface $request, callable $next) { + $app = $this->createAppWithoutLogger(function (ServerRequestInterface $request, callable $next) { $response = $next($request); assert($response instanceof ResponseInterface); @@ -623,7 +623,7 @@ public function testGlobalMiddlewareCallsNextReturnsModifiedResponseWhenModifyin public function testGlobalMiddlewareReturnsResponseWithoutCallingNextReturnsResponseWithoutCallingRouter() { - $app = $this->createAppWithoutFibersOrLogger(function () { + $app = $this->createAppWithoutLogger(function () { return new Response( 200, [ @@ -807,30 +807,4 @@ private function createAppWithoutLogger(...$middleware): App return $app; } - - /** @param callable|class-string ...$middleware */ - private function createAppWithoutFibersOrLogger(...$middleware): App - { - $app = new App(...$middleware); - - $ref = new \ReflectionProperty($app, 'handler'); - $ref->setAccessible(true); - $middleware = $ref->getValue($app); - - $ref = new \ReflectionProperty($middleware, 'handlers'); - $ref->setAccessible(true); - $handlers = $ref->getValue($middleware); - - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - - $first = array_shift($handlers); - $this->assertInstanceOf(AccessLogHandler::class, $first); - - $ref->setValue($middleware, $handlers); - - return $app; - } } diff --git a/tests/AppTest.php b/tests/AppTest.php index 8261cdf..e527e02 100644 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -27,6 +27,7 @@ use React\EventLoop\Loop; use React\Http\Message\Response; use React\Http\Message\ServerRequest; +use React\Promise\Deferred; use React\Promise\Promise; use React\Promise\PromiseInterface; use ReflectionMethod; @@ -242,7 +243,7 @@ public function testRunAppWithBusyPortThrows() public function testRunOnceWillCreateRequestFromSapiThenRouteRequestAndThenSendResponseFromHandler() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $response = new Response(); $app->get('/', function () use ($response) { @@ -268,7 +269,7 @@ public function testRunOnceWillCreateRequestFromSapiThenRouteRequestAndThenSendR public function testRunOnceWillCreateRequestFromSapiThenRouteRequestAndThenSendResponseFromDeferredHandler() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $response = new Response(); $app->get('/', function () use ($response) { @@ -484,7 +485,7 @@ public function testRedirectMethodWithCustomRedirectCodeAddsAnyRouteOnRouterWhic public function testHandleRequestWithProxyRequestReturnsResponseWithMessageThatProxyRequestsAreNotAllowed() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $request = new ServerRequest('GET', 'http://google.com/'); $request = $request->withRequestTarget('http://google.com/'); @@ -506,7 +507,7 @@ public function testHandleRequestWithProxyRequestReturnsResponseWithMessageThatP public function testHandleRequestWithUnknownRouteReturnsResponseWithFileNotFoundMessage() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $request = new ServerRequest('GET', 'http://localhost/invalid'); @@ -527,7 +528,7 @@ public function testHandleRequestWithUnknownRouteReturnsResponseWithFileNotFound public function testHandleRequestWithInvalidRequestMethodReturnsResponseWithSingleMethodNotAllowedMessage() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $app->get('/users', function () { }); @@ -551,7 +552,7 @@ public function testHandleRequestWithInvalidRequestMethodReturnsResponseWithSing public function testHandleRequestWithInvalidRequestMethodReturnsResponseWithMultipleMethodNotAllowedMessage() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $app->get('/users', function () { }); $app->head('/users', function () { }); @@ -577,7 +578,7 @@ public function testHandleRequestWithInvalidRequestMethodReturnsResponseWithMult public function testHandleRequestWithMatchingRouteReturnsResponseFromMatchingRouteHandler() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $app->get('/users', function () { return new Response( @@ -605,7 +606,7 @@ public function testHandleRequestWithMatchingRouteReturnsResponseFromMatchingRou public function testHandleRequestWithOptionsAsteriskRequestReturnsResponseFromMatchingEmptyRouteHandler() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $app->options('', function () { return new Response( @@ -698,7 +699,7 @@ public function testHandleRequestWithMatchingRouteReturnsPendingPromiseWhenHandl public function testHandleRequestWithMatchingRouteReturnsResponseWhenHandlerReturnsCoroutineWhichReturnsResponseWithoutYielding() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $app->get('/users', function () { if (false) { @@ -837,46 +838,6 @@ public function testHandleRequestWithMatchingRouteReturnsPendingPromiseWhenHandl $this->assertFalse($resolved); } - public function testHandleRequestWithMatchingRouteReturnsPromiseResolvingWithResponseWhenHandlerReturnsResponse() - { - if (PHP_VERSION_ID < 80100) { - $this->markTestSkipped('Requires PHP 8.1+'); - } - - $app = $this->createAppWithoutLogger(); - - $app->get('/users', function () { - return new Response( - 200, - [ - 'Content-Type' => 'text/html' - ], - "OK\n" - ); - }); - - $request = new ServerRequest('GET', 'http://localhost/users'); - - // $promise = $app->handleRequest($request); - $ref = new ReflectionMethod($app, 'handleRequest'); - $ref->setAccessible(true); - $promise = $ref->invoke($app, $request); - - /** @var PromiseInterface $promise */ - $this->assertInstanceOf(PromiseInterface::class, $promise); - - $response = null; - $promise->then(function ($value) use (&$response) { - $response = $value; - }); - - /** @var ResponseInterface $response */ - $this->assertInstanceOf(ResponseInterface::class, $response); - $this->assertEquals(200, $response->getStatusCode()); - $this->assertEquals('text/html', $response->getHeaderLine('Content-Type')); - $this->assertEquals("OK\n", (string) $response->getBody()); - } - public function testHandleRequestWithMatchingRouteReturnsPromiseResolvingWithResponseWhenHandlerReturnsResponseAfterAwaitingPromiseResolvingWithResponse() { if (PHP_VERSION_ID < 80100 || !function_exists('React\Async\async')) { @@ -885,14 +846,10 @@ public function testHandleRequestWithMatchingRouteReturnsPromiseResolvingWithRes $app = $this->createAppWithoutLogger(); - $app->get('/users', function () { - return await(resolve(new Response( - 200, - [ - 'Content-Type' => 'text/html' - ], - "OK\n" - ))); + $deferred = new Deferred(); + + $app->get('/users', function () use ($deferred) { + return await($deferred->promise()); }); $request = new ServerRequest('GET', 'http://localhost/users'); @@ -910,6 +867,21 @@ public function testHandleRequestWithMatchingRouteReturnsPromiseResolvingWithRes $response = $value; }); + $this->assertNull($response); + + $deferred->resolve(new Response( + 200, + [ + 'Content-Type' => 'text/html' + ], + "OK\n" + )); + + // await next tick: https://github.com/reactphp/async/issues/27 + await(new Promise(function ($resolve) { + Loop::futureTick($resolve); + })); + /** @var ResponseInterface $response */ $this->assertInstanceOf(ResponseInterface::class, $response); $this->assertEquals(200, $response->getStatusCode()); @@ -919,7 +891,7 @@ public function testHandleRequestWithMatchingRouteReturnsPromiseResolvingWithRes public function testHandleRequestWithMatchingRouteAndRouteVariablesReturnsResponseFromHandlerWithRouteVariablesAssignedAsRequestAttributes() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $app->get('/users/{name}', function (ServerRequestInterface $request) { $name = $request->getAttribute('name'); @@ -949,7 +921,7 @@ public function testHandleRequestWithMatchingRouteAndRouteVariablesReturnsRespon public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerThrowsException() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $line = __LINE__ + 2; $app->get('/users', function () { @@ -1080,7 +1052,7 @@ public function testHandleRequestWithMatchingRouteReturnsPromiseWhichFulfillsWit public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerReturnsCoroutineWhichThrowsExceptionWithoutYielding() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $line = __LINE__ + 5; $app->get('/users', function () { @@ -1152,9 +1124,13 @@ public function testHandleRequestWithMatchingRouteReturnsPromiseWhichFulfillsWit $app = $this->createAppWithoutLogger(); - $line = __LINE__ + 2; - $app->get('/users', function () { - return await(reject(new \RuntimeException('Foo'))); + $deferred = new Deferred(); + + $line = __LINE__ + 1; + $exception = new \RuntimeException('Foo'); + + $app->get('/users', function () use ($deferred) { + return await($deferred->promise()); }); $request = new ServerRequest('GET', 'http://localhost/users'); @@ -1172,6 +1148,15 @@ public function testHandleRequestWithMatchingRouteReturnsPromiseWhichFulfillsWit $response = $value; }); + $this->assertNull($response); + + $deferred->reject($exception); + + // await next tick: https://github.com/reactphp/async/issues/27 + await(new Promise(function ($resolve) { + Loop::futureTick($resolve); + })); + /** @var ResponseInterface $response */ $this->assertInstanceOf(ResponseInterface::class, $response); $this->assertEquals(500, $response->getStatusCode()); @@ -1220,7 +1205,7 @@ public function testHandleRequestWithMatchingRouteReturnsPromiseWhichFulfillsWit public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerReturnsCoroutineWhichYieldsNullImmediately() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $line = __LINE__ + 3; $app->get('/users', function () { @@ -1247,7 +1232,7 @@ public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResp public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerReturnsWrongValue() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $app->get('/users', function () { return null; @@ -1273,7 +1258,7 @@ public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResp public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerClassDoesNotExist() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $app->get('/users', 'UnknownClass'); @@ -1364,7 +1349,7 @@ public function provideInvalidClasses() */ public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerClassIsInvalid(string $class, string $error) { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $app->get('/users', $class); @@ -1388,7 +1373,7 @@ public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResp public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerClassRequiresUnexpectedCallableParameter() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $line = __LINE__ + 2; $controller = new class { @@ -1417,7 +1402,7 @@ public function __invoke(int $value) { } public function testHandleRequestWithMatchingRouteReturnsInternalServerErrorResponseWhenHandlerClassHasNoInvokeMethod() { - $app = $this->createAppWithoutFibersOrLogger(); + $app = $this->createAppWithoutLogger(); $controller = new class { }; @@ -1539,29 +1524,4 @@ private function createAppWithoutLogger(): App return $app; } - - private function createAppWithoutFibersOrLogger(): App - { - $app = new App(); - - $ref = new \ReflectionProperty($app, 'handler'); - $ref->setAccessible(true); - $middleware = $ref->getValue($app); - - $ref = new \ReflectionProperty($middleware, 'handlers'); - $ref->setAccessible(true); - $handlers = $ref->getValue($middleware); - - if (PHP_VERSION_ID >= 80100) { - $first = array_shift($handlers); - $this->assertInstanceOf(FiberHandler::class, $first); - } - - $first = array_shift($handlers); - $this->assertInstanceOf(AccessLogHandler::class, $first); - - $ref->setValue($middleware, $handlers); - - return $app; - } } diff --git a/tests/FiberHandlerTest.php b/tests/FiberHandlerTest.php index 8525c30..d388eec 100644 --- a/tests/FiberHandlerTest.php +++ b/tests/FiberHandlerTest.php @@ -23,22 +23,14 @@ public function setUp(): void } } - public function testInvokeWithHandlerReturningResponseReturnsPromiseResolvingWithSameResponse() + public function testInvokeWithHandlerReturningResponseReturnsSameResponse() { $handler = new FiberHandler(); $request = new ServerRequest('GET', 'http://example.com/'); $response = new Response(); - $promise = $handler($request, function () use ($response) { return $response; }); - - /** @var PromiseInterface $promise */ - $this->assertInstanceOf(PromiseInterface::class, $promise); - - $ret = null; - $promise->then(function ($value) use (&$ret) { - $ret = $value; - }); + $ret = $handler($request, function () use ($response) { return $response; }); $this->assertSame($response, $ret); } @@ -63,61 +55,54 @@ public function testInvokeWithHandlerReturningPromiseResolvingWithResponseReturn $this->assertSame($response, $ret); } - public function testInvokeWithHandlerReturningGeneratorReturningResponseReturnsPromiseResolvingWithSameResponse() + public function testInvokeWithHandlerReturningGeneratorReturningResponseReturnsGeneratorReturningSameResponse() { $handler = new FiberHandler(); $request = new ServerRequest('GET', 'http://example.com/'); $response = new Response(); - $promise = $handler($request, function () use ($response) { + $generator = $handler($request, function () use ($response) { if (false) { yield; } return $response; }); - /** @var PromiseInterface $promise */ - $this->assertInstanceOf(PromiseInterface::class, $promise); - - $ret = null; - $promise->then(function ($value) use (&$ret) { - $ret = $value; - }); + /** @var \Generator $generator */ + $this->assertInstanceOf(\Generator::class, $generator); + $ret = $generator->getReturn(); $this->assertSame($response, $ret); } - public function testInvokeWithHandlerReturningGeneratorReturningResponseAfterYieldingResolvedPromiseReturnsPromiseResolvingWithSameResponse() + public function testInvokeWithHandlerReturningGeneratorReturningResponseAfterYieldingResolvedPromiseReturnsGeneratorReturningSameResponse() { $handler = new FiberHandler(); $request = new ServerRequest('GET', 'http://example.com/'); $response = new Response(); - $promise = $handler($request, function () use ($response) { + $generator = $handler($request, function () use ($response) { return yield resolve($response); }); - /** @var PromiseInterface $promise */ - $this->assertInstanceOf(PromiseInterface::class, $promise); - - $ret = null; - $promise->then(function ($value) use (&$ret) { - $ret = $value; - }); + /** @var \Generator $generator */ + $this->assertInstanceOf(\Generator::class, $generator); + $generator->send($response); + $ret = $generator->getReturn(); $this->assertSame($response, $ret); } - public function testInvokeWithHandlerReturningGeneratorReturningResponseAfterYieldingRejectedPromiseReturnsPromiseResolvingWithSameResponse() + public function testInvokeWithHandlerReturningGeneratorReturningResponseAfterYieldingRejectedPromiseReturnsGeneratorReturningSameResponse() { $handler = new FiberHandler(); $request = new ServerRequest('GET', 'http://example.com/'); $response = new Response(); - $promise = $handler($request, function () use ($response) { + $generator = $handler($request, function () use ($response) { try { yield reject(new \RuntimeException('Foo')); } catch (\RuntimeException $e) { @@ -125,36 +110,25 @@ public function testInvokeWithHandlerReturningGeneratorReturningResponseAfterYie } }); - /** @var PromiseInterface $promise */ - $this->assertInstanceOf(PromiseInterface::class, $promise); - - $ret = null; - $promise->then(function ($value) use (&$ret) { - $ret = $value; - }); + /** @var \Generator $generator */ + $this->assertInstanceOf(\Generator::class, $generator); + $generator->throw(new \RuntimeException('Foo')); + $ret = $generator->getReturn(); $this->assertSame($response, $ret); } - public function testInvokeWithHandlerReturningResponseAfterAwaitingResolvedPromiseReturnsPromiseResolvingWithSameResponse() + public function testInvokeWithHandlerReturningResponseAfterAwaitingResolvedPromiseReturnsSameResponse() { $handler = new FiberHandler(); $request = new ServerRequest('GET', 'http://example.com/'); $response = new Response(); - $promise = $handler($request, function () use ($response) { + $ret = $handler($request, function () use ($response) { return await(resolve($response)); }); - /** @var PromiseInterface $promise */ - $this->assertInstanceOf(PromiseInterface::class, $promise); - - $ret = null; - $promise->then(function ($value) use (&$ret) { - $ret = $value; - }); - $this->assertSame($response, $ret); }