Refactor to move access logging to new AccessLogHandler

src/AccessLogHandler.php

@@ -0,0 +1,54 @@
+<?php
+
+namespace FrameworkX;
+
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+use React\Promise\PromiseInterface;
+
+/**
+ * @internal
+ */
+class AccessLogHandler
+{
+    /** @var SapiHandler */
+    private $sapi;
+
+    public function __construct()
+    {
+        $this->sapi = new SapiHandler();
+    }
+
+    /**
+     * @return ResponseInterface|PromiseInterface<ResponseInterface>|\Generator
+     */
+    public function __invoke(ServerRequestInterface $request, callable $next)
+    {
+        $response = $next($request);
+
+        if ($response instanceof PromiseInterface) {
+            return $response->then(function (ResponseInterface $response) use ($request) {
+                $this->log($request, $response);
+                return $response;
+            });
+        } elseif ($response instanceof \Generator) {
+            return (function (\Generator $generator) use ($request) {
+                $response = yield from $generator;
+                $this->log($request, $response);
+                return $response;
+            })($response);
+        } else {
+            $this->log($request, $response);
+            return $response;
+        }
+    }
+
+    private function log(ServerRequestInterface $request, ResponseInterface $response): void
+    {
+        $this->sapi->log(
+            ($request->getServerParams()['REMOTE_ADDR'] ?? '-') . ' ' .
+            '"' . $request->getMethod() . ' ' . $request->getUri()->getPath() . ' HTTP/' . $request->getProtocolVersion() . '" ' .
+            $response->getStatusCode() . ' ' . $response->getBody()->getSize()
+        );
+    }
+}

src/App.php

@@ -62,8 +62,14 @@ public function __construct($loop = null, callable ...$middleware)
         $this->loop = $loop ?? Loop::get();
         $this->router = new RouteHandler();
 
-        // new MiddlewareHandler([$errorHandler, ...$middleware, $routeHandler])
+        // new MiddlewareHandler([$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
+        if (\PHP_SAPI === 'cli' || \PHP_SAPI === 'cli-server') {
+            \array_unshift($middleware, new AccessLogHandler());
+        }
+
         $middleware[] = $this->router;
         $this->handler = new MiddlewareHandler($middleware);
         $this->sapi = new SapiHandler();
@@ -133,17 +139,7 @@ public function run()
     private function runLoop()
     {
         $http = new HttpServer($this->loop, function (ServerRequestInterface $request) {
-            $response = $this->handleRequest($request);
-
-            if ($response instanceof ResponseInterface) {
-                $this->sapi->logRequestResponse($request, $response);
-            } elseif ($response instanceof PromiseInterface) {
-                $response->then(function (ResponseInterface $response) use ($request) {
-                    $this->sapi->logRequestResponse($request, $response);
-                });
-            }
-
-            return $response;
+            return $this->handleRequest($request);
         });
 
         $listen = \getenv('X_LISTEN');
@@ -176,11 +172,9 @@ private function runOnce()
         $response = $this->handleRequest($request);
 
         if ($response instanceof ResponseInterface) {
-            $this->sapi->logRequestResponse($request, $response);
             $this->sapi->sendResponse($response);
         } elseif ($response instanceof PromiseInterface) {
-            $response->then(function (ResponseInterface $response) use ($request) {
-                $this->sapi->logRequestResponse($request, $response);
+            $response->then(function (ResponseInterface $response) {
                 $this->sapi->sendResponse($response);
             });
         }

src/SapiHandler.php

@@ -15,16 +15,9 @@ class SapiHandler
     /** @var resource */
     private $logStream;
 
-    /** @var bool */
-    private $shouldLogRequest;
-
     public function __construct()
     {
         $this->logStream = PHP_SAPI === 'cli' ? \fopen('php://output', 'a') : (\defined('STDERR') ? \STDERR : \fopen('php://stderr', 'a'));
-
-        // Only log for built-in webserver and PHP development webserver, others have their own access log.
-        // Yes, this should be moved out of this class.
-        $this->shouldLogRequest = PHP_SAPI === 'cli' || PHP_SAPI === 'cli-server';
     }
 
     public function requestFromGlobals(): ServerRequestInterface
@@ -126,19 +119,6 @@ public function sendResponse(ResponseInterface $response): void
         }
     }
 
-    public function logRequestResponse(ServerRequestInterface $request, ResponseInterface $response): void
-    {
-        if (!$this->shouldLogRequest) {
-            return;
-        }
-
-        $this->log(
-            ($request->getServerParams()['REMOTE_ADDR'] ?? '-') . ' ' .
-            '"' . $request->getMethod() . ' ' . $request->getUri()->getPath() . ' HTTP/' . $request->getProtocolVersion() . '" ' .
-            $response->getStatusCode() . ' ' . $response->getBody()->getSize()
-        );
-    }
-
     public function log(string $message): void
     {
         $time = microtime(true);

tests/AccessLogHandlerTest.php

@@ -0,0 +1,70 @@
+<?php
+
+namespace FrameworkX\Tests;
+
+use FrameworkX\AccessLogHandler;
+use PHPUnit\Framework\TestCase;
+use React\Http\Message\Response;
+use React\Http\Message\ServerRequest;
+use function React\Promise\resolve;
+
+class AccessLogHandlerTest extends TestCase
+{
+    public function testInvokePrintsRequestLogWithCurrentDateAndTime()
+    {
+        $handler = new AccessLogHandler();
+
+        $request = new ServerRequest('GET', 'http://localhost:8080/users', [], '', '1.1', ['REMOTE_ADDR' => '127.0.0.1']);
+        $response = new Response(200, [], "Hello\n");
+
+        // 2021-01-29 12:22:01.717 127.0.0.1 "GET /users HTTP/1.1" 200 6\n
+        $this->expectOutputRegex("/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3} 127\.0\.0\.1 \"GET \/users HTTP\/1\.1\" 200 6" . PHP_EOL . "$/");
+        $handler($request, function () use ($response) { return $response; });
+    }
+
+    public function testInvokeWithDeferredNextPrintsRequestLogWithCurrentDateAndTime()
+    {
+        $handler = new AccessLogHandler();
+
+        $request = new ServerRequest('GET', 'http://localhost:8080/users', [], '', '1.1', ['REMOTE_ADDR' => '127.0.0.1']);
+        $response = new Response(200, [], "Hello\n");
+
+        // 2021-01-29 12:22:01.717 127.0.0.1 "GET /users HTTP/1.1" 200 6\n
+        $this->expectOutputRegex("/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3} 127\.0\.0\.1 \"GET \/users HTTP\/1\.1\" 200 6" . PHP_EOL . "$/");
+        $handler($request, function () use ($response) { return resolve($response); });
+    }
+
+    public function testInvokeWithCoroutineNextPrintsRequestLogWithCurrentDateAndTime()
+    {
+        $handler = new AccessLogHandler();
+
+        $request = new ServerRequest('GET', 'http://localhost:8080/users', [], '', '1.1', ['REMOTE_ADDR' => '127.0.0.1']);
+        $response = new Response(200, [], "Hello\n");
+
+        $generator = $handler($request, function () use ($response) {
+            if (false) {
+                yield;
+            }
+            return $response;
+        });
+
+        /** @var \Generator $generator */
+        $this->assertInstanceOf(\Generator::class, $generator);
+
+        // 2021-01-29 12:22:01.717 127.0.0.1 "GET /users HTTP/1.1" 200 6\n
+        $this->expectOutputRegex("/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3} 127\.0\.0\.1 \"GET \/users HTTP\/1\.1\" 200 6" . PHP_EOL . "$/");
+        $generator->next();
+    }
+
+    public function testInvokeWithoutRemoteAddressPrintsRequestLogWithDashAsPlaceholder()
+    {
+        $handler = new AccessLogHandler();
+
+        $request = new ServerRequest('GET', 'http://localhost:8080/users');
+        $response = new Response(200, [], "Hello\n");
+
+        // 2021-01-29 12:22:01.717 - "GET /users HTTP/1.1" 200 6\n
+        $this->expectOutputRegex("/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3} - \"GET \/users HTTP\/1\.1\" 200 6" . PHP_EOL . "$/");
+        $handler($request, function () use ($response) { return $response; });
+    }
+}