From 5d2d64bf0affce53ab74b80bc482ee8a91bb0aad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 1 Sep 2023 15:24:56 +0200 Subject: [PATCH 1/3] Refactor internal `ReactiveHandler` to inject `LogStreamHandler` object --- src/App.php | 3 +- src/Io/ReactiveHandler.php | 6 +- tests/Io/ReactiveHandlerTest.php | 100 ++++++++----------------------- 3 files changed, 31 insertions(+), 78 deletions(-) diff --git a/src/App.php b/src/App.php index 173d2fb..8ccfd9c 100644 --- a/src/App.php +++ b/src/App.php @@ -2,6 +2,7 @@ namespace FrameworkX; +use FrameworkX\Io\LogStreamHandler; use FrameworkX\Io\MiddlewareHandler; use FrameworkX\Io\ReactiveHandler; use FrameworkX\Io\RedirectHandler; @@ -126,7 +127,7 @@ public function __construct(...$middleware) $this->router = $router; $this->handler = new MiddlewareHandler($handlers); - $this->sapi = \PHP_SAPI === 'cli' ? new ReactiveHandler($container->getEnv('X_LISTEN')) : new SapiHandler(); + $this->sapi = \PHP_SAPI === 'cli' ? new ReactiveHandler(new LogStreamHandler('php://output'), $container->getEnv('X_LISTEN')) : new SapiHandler(); } /** diff --git a/src/Io/ReactiveHandler.php b/src/Io/ReactiveHandler.php index 579eb8a..2f26939 100644 --- a/src/Io/ReactiveHandler.php +++ b/src/Io/ReactiveHandler.php @@ -29,10 +29,10 @@ class ReactiveHandler /** @var string */ private $listenAddress; - public function __construct(?string $listenAddress) + /** @throws void */ + public function __construct(LogStreamHandler $logger, ?string $listenAddress) { - /** @throws void */ - $this->logger = new LogStreamHandler('php://output'); + $this->logger = $logger; $this->listenAddress = $listenAddress ?? '127.0.0.1:8080'; } diff --git a/tests/Io/ReactiveHandlerTest.php b/tests/Io/ReactiveHandlerTest.php index e4a2fbb..2e4833f 100644 --- a/tests/Io/ReactiveHandlerTest.php +++ b/tests/Io/ReactiveHandlerTest.php @@ -23,17 +23,11 @@ public function testRunWillReportDefaultListeningAddressAndRunLoop(): void assert(is_resource($socket)); fclose($socket); - $handler = new ReactiveHandler(null); - $logger = $this->createMock(LogStreamHandler::class); $logger->expects($this->atLeastOnce())->method('log')->withConsecutive(['Listening on http://127.0.0.1:8080']); + assert($logger instanceof LogStreamHandler); - // $handler->logger = $logger; - $ref = new \ReflectionProperty($handler, 'logger'); - if (PHP_VERSION_ID < 80100) { - $ref->setAccessible(true); - } - $ref->setValue($handler, $logger); + $handler = new ReactiveHandler($logger, null); // lovely: remove socket server on next tick to terminate loop Loop::futureTick(function () { @@ -58,17 +52,11 @@ public function testRunWillReportGivenListeningAddressAndRunLoop(): void assert(is_string($addr)); fclose($socket); - $handler = new ReactiveHandler($addr); - $logger = $this->createMock(LogStreamHandler::class); $logger->expects($this->atLeastOnce())->method('log')->withConsecutive(['Listening on http://' . $addr]); + assert($logger instanceof LogStreamHandler); - // $handler->logger = $logger; - $ref = new \ReflectionProperty($handler, 'logger'); - if (PHP_VERSION_ID < 80100) { - $ref->setAccessible(true); - } - $ref->setValue($handler, $logger); + $handler = new ReactiveHandler($logger, $addr); // lovely: remove socket server on next tick to terminate loop Loop::futureTick(function () { @@ -87,17 +75,11 @@ public function testRunWillReportGivenListeningAddressAndRunLoop(): void public function testRunWillReportGivenListeningAddressWithRandomPortAndRunLoop(): void { - $handler = new ReactiveHandler('127.0.0.1:0'); - $logger = $this->createMock(LogStreamHandler::class); $logger->expects($this->atLeastOnce())->method('log')->withConsecutive([$this->matches('Listening on http://127.0.0.1:%d')]); + assert($logger instanceof LogStreamHandler); - // $handler->logger = $logger; - $ref = new \ReflectionProperty($handler, 'logger'); - if (PHP_VERSION_ID < 80100) { - $ref->setAccessible(true); - } - $ref->setValue($handler, $logger); + $handler = new ReactiveHandler($logger, '127.0.0.1:0'); // lovely: remove socket server on next tick to terminate loop Loop::futureTick(function () { @@ -116,16 +98,10 @@ public function testRunWillReportGivenListeningAddressWithRandomPortAndRunLoop() public function testRunWillRestartLoopUntilSocketIsClosed(): void { - $handler = new ReactiveHandler('127.0.0.1:0'); - $logger = $this->createMock(LogStreamHandler::class); + assert($logger instanceof LogStreamHandler); - // $handler->logger = $logger; - $ref = new \ReflectionProperty($handler, 'logger'); - if (PHP_VERSION_ID < 80100) { - $ref->setAccessible(true); - } - $ref->setValue($handler, $logger); + $handler = new ReactiveHandler($logger, '127.0.0.1:0'); // lovely: remove socket server on next tick to terminate loop Loop::futureTick(function () use ($logger) { @@ -155,16 +131,10 @@ public function testRunWillListenForHttpRequestAndSendBackHttpResponseOverSocket assert(is_string($addr)); fclose($socket); - $handler = new ReactiveHandler($addr); - $logger = $this->createMock(LogStreamHandler::class); + assert($logger instanceof LogStreamHandler); - // $handler->logger = $logger; - $ref = new \ReflectionProperty($handler, 'logger'); - if (PHP_VERSION_ID < 80100) { - $ref->setAccessible(true); - } - $ref->setValue($handler, $logger); + $handler = new ReactiveHandler($logger, $addr); Loop::futureTick(function () use ($addr): void { $connector = new Connector(); @@ -206,16 +176,10 @@ public function testRunWillOnlyRestartLoopAfterAwaitingWhenFibersAreNotAvailable assert(is_string($addr)); fclose($socket); - $handler = new ReactiveHandler($addr); - $logger = $this->createMock(LogStreamHandler::class); + assert($logger instanceof LogStreamHandler); - // $handler->logger = $logger; - $ref = new \ReflectionProperty($handler, 'logger'); - if (PHP_VERSION_ID < 80100) { - $ref->setAccessible(true); - } - $ref->setValue($handler, $logger); + $handler = new ReactiveHandler($logger, $addr); Loop::futureTick(function () use ($addr, $logger): void { $connector = new Connector(); @@ -276,16 +240,10 @@ public function testRunWillReportHttpErrorForInvalidClientRequest(): void assert(is_string($addr)); fclose($socket); - $handler = new ReactiveHandler($addr); - $logger = $this->createMock(LogStreamHandler::class); + assert($logger instanceof LogStreamHandler); - // $handler->logger = $logger; - $ref = new \ReflectionProperty($handler, 'logger'); - if (PHP_VERSION_ID < 80100) { - $ref->setAccessible(true); - } - $ref->setValue($handler, $logger); + $handler = new ReactiveHandler($logger, $addr); Loop::futureTick(function () use ($addr, $logger): void { $connector = new Connector(); @@ -320,17 +278,11 @@ public function testRunWillReportHttpErrorForInvalidClientRequest(): void */ public function testRunWillStopWhenReceivingSigint(): void { - $handler = new ReactiveHandler('127.0.0.1:0'); - $logger = $this->createMock(LogStreamHandler::class); $logger->expects($this->exactly(2))->method('log'); + assert($logger instanceof LogStreamHandler); - // $handler->logger = $logger; - $ref = new \ReflectionProperty($handler, 'logger'); - if (PHP_VERSION_ID < 80100) { - $ref->setAccessible(true); - } - $ref->setValue($handler, $logger); + $handler = new ReactiveHandler($logger, '127.0.0.1:0'); Loop::futureTick(function () use ($logger) { $logger->expects($this->once())->method('log')->with('Received SIGINT, stopping loop'); @@ -350,16 +302,10 @@ public function testRunWillStopWhenReceivingSigint(): void */ public function testRunWillStopWhenReceivingSigterm(): void { - $handler = new ReactiveHandler('127.0.0.1:0'); - $logger = $this->createMock(LogStreamHandler::class); + assert($logger instanceof LogStreamHandler); - // $handler->logger = $logger; - $ref = new \ReflectionProperty($handler, 'logger'); - if (PHP_VERSION_ID < 80100) { - $ref->setAccessible(true); - } - $ref->setValue($handler, $logger); + $handler = new ReactiveHandler($logger, '127.0.0.1:0'); Loop::futureTick(function () use ($logger) { $logger->expects($this->once())->method('log')->with('Received SIGTERM, stopping loop'); @@ -374,7 +320,10 @@ public function testRunWillStopWhenReceivingSigterm(): void public function testRunWithEmptyAddressThrows(): void { - $handler = new ReactiveHandler(''); + $logger = $this->createMock(LogStreamHandler::class); + assert($logger instanceof LogStreamHandler); + + $handler = new ReactiveHandler($logger, ''); $this->expectException(\InvalidArgumentException::class); $handler->run(function (): void { }); @@ -391,7 +340,10 @@ public function testRunWithBusyPortThrows(): void $this->markTestSkipped('System does not prevent listening on same address twice'); } - $handler = new ReactiveHandler($addr); + $logger = $this->createMock(LogStreamHandler::class); + assert($logger instanceof LogStreamHandler); + + $handler = new ReactiveHandler($logger, $addr); $this->expectException(\RuntimeException::class); $this->expectExceptionMessage('Failed to listen on'); From 7e23d1066f9367e5f98a625ccd350d71eb0981d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Sun, 3 Sep 2023 22:08:52 +0200 Subject: [PATCH 2/3] Refactor and simplify `App` with new `Container::getSapi()` helper --- src/App.php | 2 +- src/Container.php | 25 ++++++++ tests/AppTest.php | 36 +++++++---- tests/ContainerTest.php | 130 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 180 insertions(+), 13 deletions(-) diff --git a/src/App.php b/src/App.php index 8ccfd9c..3b7f3c1 100644 --- a/src/App.php +++ b/src/App.php @@ -127,7 +127,7 @@ public function __construct(...$middleware) $this->router = $router; $this->handler = new MiddlewareHandler($handlers); - $this->sapi = \PHP_SAPI === 'cli' ? new ReactiveHandler(new LogStreamHandler('php://output'), $container->getEnv('X_LISTEN')) : new SapiHandler(); + $this->sapi = $container->getSapi(); } /** diff --git a/src/Container.php b/src/Container.php index a144dc9..507a20f 100644 --- a/src/Container.php +++ b/src/Container.php @@ -2,7 +2,10 @@ namespace FrameworkX; +use FrameworkX\Io\LogStreamHandler; +use FrameworkX\Io\ReactiveHandler; use FrameworkX\Io\RouteHandler; +use FrameworkX\Io\SapiHandler; use Psr\Container\ContainerInterface; use Psr\Http\Message\ServerRequestInterface; @@ -166,6 +169,8 @@ public function getObject(string $class) /*: object (PHP 7.2+) */ return $this; // @phpstan-ignore-line returns instanceof `T` } elseif ($class === RouteHandler::class) { return new RouteHandler($this); // @phpstan-ignore-line returns instanceof `T` + } elseif ($class === ReactiveHandler::class) { + return new ReactiveHandler(new LogStreamHandler('php://output'), $this->getEnv('X_LISTEN')); // @phpstan-ignore-line returns instanceof `T` } return new $class(); } @@ -173,6 +178,19 @@ public function getObject(string $class) /*: object (PHP 7.2+) */ return $this->loadObject($class); } + /** + * [Internal] Get SAPI handler from container + * + * @return ReactiveHandler|SapiHandler + * @throws \TypeError if container config or factory returns an unexpected type + * @throws \Throwable if container factory function throws unexpected exception + * @internal + */ + public function getSapi() /*: ReactiveHandler|SapiHandler (PHP 8.0+) */ + { + return $this->getObject(\PHP_SAPI === 'cli' ? ReactiveHandler::class : SapiHandler::class); + } + /** * @template T of object * @param class-string $name @@ -186,6 +204,13 @@ private function loadObject(string $name, int $depth = 64) /*: object (PHP 7.2+) { \assert(\is_array($this->container)); + if ($name === ReactiveHandler::class && !\array_key_exists(ReactiveHandler::class, $this->container)) { + // special case: create ReactiveHandler with X_LISTEN environment variable + $this->container[ReactiveHandler::class] = static function (?string $X_LISTEN = null): ReactiveHandler { + return new ReactiveHandler(new LogStreamHandler('php://output'), $X_LISTEN); + }; + } + if (\array_key_exists($name, $this->container)) { if (\is_string($this->container[$name])) { if ($depth < 1) { diff --git a/tests/AppTest.php b/tests/AppTest.php index 70b2190..b46f840 100644 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -92,16 +92,27 @@ public function testConstructWithContainerMockAssignsDefaultHandlersFromContaine $errorHandler = new ErrorHandler(); $routeHandler = $this->createMock(RouteHandler::class); + $sapi = $this->createMock(ReactiveHandler::class); + $container = $this->createMock(Container::class); $container->expects($this->exactly(3))->method('getObject')->willReturnMap([ [AccessLogHandler::class, $accessLogHandler], [ErrorHandler::class, $errorHandler], [RouteHandler::class, $routeHandler], ]); - + $container->expects($this->once())->method('getSapi')->willReturn($sapi); assert($container instanceof Container); + $app = new App($container); + $ref = new \ReflectionProperty($app, 'sapi'); + if (PHP_VERSION_ID < 80100) { + $ref->setAccessible(true); + } + $ret = $ref->getValue($app); + + $this->assertSame($sapi, $ret); + $ref = new ReflectionProperty($app, 'handler'); if (PHP_VERSION_ID < 80100) { $ref->setAccessible(true); @@ -127,6 +138,14 @@ public function testConstructWithContainerInstanceAssignsDefaultHandlersAndConta $container = new Container([]); $app = new App($container); + $ref = new \ReflectionProperty($app, 'sapi'); + if (PHP_VERSION_ID < 80100) { + $ref->setAccessible(true); + } + $ret = $ref->getValue($app); + + $this->assertSame($container->getSapi(), $ret); + $ref = new ReflectionProperty($app, 'handler'); if (PHP_VERSION_ID < 80100) { $ref->setAccessible(true); @@ -900,19 +919,16 @@ public function testConstructWithContainerWithListenAddressWillPassListenAddress $this->assertEquals('0.0.0.0:8081', $listenAddress); } - public function testRunWillExecuteRunOnSapiHandler(): void + public function testRunWillExecuteRunOnSapiHandlerFromContainer(): void { - $app = new App(); - $sapi = $this->createMock(ReactiveHandler::class); $sapi->expects($this->once())->method('run'); - // $app->sapi = $sapi; - $ref = new \ReflectionProperty($app, 'sapi'); - if (PHP_VERSION_ID < 80100) { - $ref->setAccessible(true); - } - $ref->setValue($app, $sapi); + $container = new Container([ + ReactiveHandler::class => $sapi + ]); + + $app = new App($container); $app->run(); } diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php index 19c4644..f4b01a4 100644 --- a/tests/ContainerTest.php +++ b/tests/ContainerTest.php @@ -5,6 +5,8 @@ use FrameworkX\AccessLogHandler; use FrameworkX\Container; use FrameworkX\ErrorHandler; +use FrameworkX\Io\LogStreamHandler; +use FrameworkX\Io\ReactiveHandler; use FrameworkX\Io\RouteHandler; use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; @@ -2636,7 +2638,7 @@ public function testGetObjectReturnsDefaultRouteHandlerInstance(): void $this->assertInstanceOf(RouteHandler::class, $router); $ref = new \ReflectionProperty($router, 'container'); - if (\PHP_VERSION_ID < 801000) { + if (PHP_VERSION_ID < 80100) { $ref->setAccessible(true); } $ret = $ref->getValue($router); @@ -2702,7 +2704,7 @@ public function testGetObjectReturnsDefaultRouteHandlerInstanceIfPsrContainerHas $this->assertInstanceOf(RouteHandler::class, $router); $ref = new \ReflectionProperty($router, 'container'); - if (\PHP_VERSION_ID < 801000) { + if (PHP_VERSION_ID < 80100) { $ref->setAccessible(true); } $ret = $ref->getValue($router); @@ -2823,6 +2825,130 @@ public function testGetObjectThrowsIfPsrContainerReturnsWrongType(): void $container->getObject(AccessLogHandler::class); } + public function testGetSapiReturnsDefaultReactiveHandlerInstance(): void + { + $container = new Container([]); + + $sapi = $container->getSapi(); + + $this->assertInstanceOf(ReactiveHandler::class, $sapi); + + $ref = new \ReflectionProperty($sapi, 'listenAddress'); + if (PHP_VERSION_ID < 80100) { + $ref->setAccessible(true); + } + $listenAddress = $ref->getValue($sapi); + + $this->assertEquals('127.0.0.1:8080', $listenAddress); + } + + public function testGetSapiReturnsDefaultReactiveHandlerInstanceWithCustomListenAddress(): void + { + $container = new Container([ + 'X_LISTEN' => '127.0.0.1:8081' + ]); + + $sapi = $container->getSapi(); + + $this->assertInstanceOf(ReactiveHandler::class, $sapi); + + $ref = new \ReflectionProperty($sapi, 'listenAddress'); + if (PHP_VERSION_ID < 80100) { + $ref->setAccessible(true); + } + $listenAddress = $ref->getValue($sapi); + + $this->assertEquals('127.0.0.1:8081', $listenAddress); + } + + public function testGetSapiTwiceReturnsSameReactiveHandlerInstance(): void + { + $container = new Container([]); + + $sapi = $container->getSapi(); + + $this->assertSame($sapi, $container->getSapi()); + } + + public function testGetSapiReturnsReactiveHandlerInstanceFromConfig(): void + { + $sapi = new ReactiveHandler(new LogStreamHandler('php://output'), null); + + $container = new Container([ + ReactiveHandler::class => $sapi + ]); + + $ret = $container->getSapi(); + + $this->assertSame($sapi, $ret); + } + + public function testGetSapiReturnsReactiveHandlerInstanceFromPsrContainer(): void + { + $sapi = new ReactiveHandler(new LogStreamHandler('php://output'), null); + + $psr = $this->createMock(ContainerInterface::class); + $psr->expects($this->once())->method('has')->with(ReactiveHandler::class)->willReturn(true); + $psr->expects($this->once())->method('get')->with(ReactiveHandler::class)->willReturn($sapi); + + assert($psr instanceof ContainerInterface); + $container = new Container($psr); + + $ret = $container->getSapi(); + + $this->assertSame($sapi, $ret); + } + + public function testGetSapiReturnsDefaultReactiveHandlerInstanceWithDefaultListenAddressIfPsrContainerHasNoEntry(): void + { + $psr = $this->createMock(ContainerInterface::class); + $psr->expects($this->exactly(2))->method('has')->willReturnMap([ + [ReactiveHandler::class, false], + ['X_LISTEN', false], + ]); + $psr->expects($this->never())->method('get'); + + assert($psr instanceof ContainerInterface); + $container = new Container($psr); + + $sapi = $container->getSapi(); + + $this->assertInstanceOf(ReactiveHandler::class, $sapi); + + $ref = new \ReflectionProperty($sapi, 'listenAddress'); + if (PHP_VERSION_ID < 80100) { + $ref->setAccessible(true); + } + $listenAddress = $ref->getValue($sapi); + + $this->assertEquals('127.0.0.1:8080', $listenAddress); + } + + public function testGetSapiReturnsDefaultReactiveHandlerInstanceWithCustomListenAddressIfPsrContainerHasNoEntryButCustomListenAddress(): void + { + $psr = $this->createMock(ContainerInterface::class); + $psr->expects($this->exactly(2))->method('has')->willReturnMap([ + [ReactiveHandler::class, false], + ['X_LISTEN', true], + ]); + $psr->expects($this->once())->method('get')->with('X_LISTEN')->willReturn('127.0.0.1:8081'); + + assert($psr instanceof ContainerInterface); + $container = new Container($psr); + + $sapi = $container->getSapi(); + + $this->assertInstanceOf(ReactiveHandler::class, $sapi); + + $ref = new \ReflectionProperty($sapi, 'listenAddress'); + if (PHP_VERSION_ID < 80100) { + $ref->setAccessible(true); + } + $listenAddress = $ref->getValue($sapi); + + $this->assertEquals('127.0.0.1:8081', $listenAddress); + } + public function testInvokeContainerAsMiddlewareReturnsFromNextRequestHandler(): void { $request = new ServerRequest('GET', 'http://example.com/'); From f70c5672f3869a439af7860f4a633c821065d68b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Wed, 10 Dec 2025 19:04:30 +0100 Subject: [PATCH 3/3] Fix test error for old reactphp/async v3 on PHP 8.1+ --- tests/Io/ReactiveHandlerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Io/ReactiveHandlerTest.php b/tests/Io/ReactiveHandlerTest.php index 2e4833f..b63da1e 100644 --- a/tests/Io/ReactiveHandlerTest.php +++ b/tests/Io/ReactiveHandlerTest.php @@ -186,7 +186,7 @@ public function testRunWillOnlyRestartLoopAfterAwaitingWhenFibersAreNotAvailable $promise = $connector->connect($addr); // the loop will only need to be restarted if fibers are not available (PHP < 8.1) - if (PHP_VERSION_ID < 80100) { + if (!function_exists('React\Async\async')) { $logger->expects($this->once())->method('log')->with('Warning: Loop restarted. Upgrade to react/async v4 recommended for production use.'); } else { $logger->expects($this->never())->method('log');