Merge pull request #175 from clue-labs/container-handlers

Support loading `AccessLogHandler` and `ErrorHandler` from `Container`

Author: SimonFrings

docs/api/app.md

@@ -194,25 +194,60 @@ the [`ErrorHandler`](middleware.md#errorhandler) to the list of middleware used.
 You may also explicitly pass an [`ErrorHandler`](middleware.md#errorhandler)
 middleware to the `App` like this:
 
+=== "Using middleware instances"
+
+    ```php title="public/index.php"
+    <?php
+
+    require __DIR__ . '/../vendor/autoload.php';
+
+    $app = new FrameworkX\App(
+        new FrameworkX\ErrorHandler()
+    );
+
+    // Register routes here, see routing…
+
+    $app->run();
+    ```
+
+=== "Using middleware names"
+
+    ```php title="public/index.php"
+    <?php
+
+    require __DIR__ . '/../vendor/autoload.php';
+
+    $app = new FrameworkX\App(
+        FrameworkX\ErrorHandler::class
+    );
+
+    // Register routes here, see routing…
+
+    $app->run();
+    ```
+
+If you do not explicitly pass an [`ErrorHandler`](middleware.md#errorhandler) or
+if you pass another middleware before an [`ErrorHandler`](middleware.md#errorhandler)
+to the `App`, a default error handler will be added as a first handler automatically.
+You may use the [DI container configuration](../best-practices/controllers.md#container-configuration)
+to configure the default error handler like this:
+
 ```php title="public/index.php"
 <?php
 
 require __DIR__ . '/../vendor/autoload.php';
 
-$app = new FrameworkX\App(
-    new FrameworkX\ErrorHandler()
-);
+$container = new FrameworkX\Container([
+    FrameworkX\ErrorHandler::class => fn () => new FrameworkX\ErrorHandler()
+]);
+
+$app = new FrameworkX\App($container);
 
 // Register routes here, see routing…
 
 $app->run();
 ```
 
-> ⚠️ **Feature preview**
->
-> Note that the [`ErrorHandler`](middleware.md#errorhandler) may currently only
-> be passed as a middleware instance and not as a middleware name to the `App`.
-
 By default, this error message contains only few details to the client to avoid
 leaking too much internal information.
 If you want to implement custom error handling, you're recommended to either
@@ -245,28 +280,68 @@ adding the [`AccessLogHandler`](middleware.md#accessloghandler) to the list of
 middleware used. You may also explicitly pass an [`AccessLogHandler`](middleware.md#accessloghandler)
 middleware to the `App` like this:
 
-```php title="public/index.php"
-<?php
+=== "Using middleware instances"
 
-require __DIR__ . '/../vendor/autoload.php';
+    ```php title="public/index.php"
+    <?php
 
-$app = new FrameworkX\App(
-    new FrameworkX\AccessLogHandler(),
-    new FrameworkX\ErrorHandler()
-);
+    require __DIR__ . '/../vendor/autoload.php';
 
-// Register routes here, see routing…
+    $app = new FrameworkX\App(
+        new FrameworkX\AccessLogHandler(),
+        new FrameworkX\ErrorHandler()
+    );
 
-$app->run();
-```
+    // Register routes here, see routing…
+
+    $app->run();
+    ```
+
+=== "Using middleware names"
+
+    ```php title="public/index.php"
+    <?php
+
+    require __DIR__ . '/../vendor/autoload.php';
+
+    $app = new FrameworkX\App(
+        FrameworkX\AccessLogHandler::class,
+        FrameworkX\ErrorHandler::class
+    );
+
+    // Register routes here, see routing…
+
+    $app->run();
+    ```
 
 > ⚠️ **Feature preview**
 >
 > Note that the [`AccessLogHandler`](middleware.md#accessloghandler) may
-> currently only be passed as a global middleware instance and not as a global
-> middleware name to the `App` and may not be used for individual routes.
+> currently only be passed as a global middleware to the `App` and may not be
+> used for individual routes.
 
 If you pass an [`AccessLogHandler`](middleware.md#accessloghandler) to the `App`,
 it must be followed by an [`ErrorHandler`](middleware.md#errorhandler) like in
 the previous example. See also [error handling](#error-handling) for more
 details.
+
+If you do not explicitly pass an [`AccessLogHandler`](middleware.md#accessloghandler)
+to the `App`, a default access log handler will be added as a first handler automatically.
+You may use the [DI container configuration](../best-practices/controllers.md#container-configuration)
+to configure the default access log handler like this:
+
+```php title="public/index.php"
+<?php
+
+require __DIR__ . '/../vendor/autoload.php';
+
+$container = new FrameworkX\Container([
+    FrameworkX\AccessLogHandler::class => fn () => new FrameworkX\AccessLogHandler()
+]);
+
+$app = new FrameworkX\App($container);
+
+// Register routes here, see routing…
+
+$app->run();
+```

src/App.php

@@ -41,28 +41,45 @@ public function __construct(...$middleware)
         // new MiddlewareHandler([$fiberHandler, $accessLogHandler, $errorHandler, ...$middleware, $routeHandler])
         $handlers = [];
 
+        $container = $needsErrorHandler = new Container();
+
         // only log for built-in webserver and PHP development webserver by default, others have their own access log
-        $needsAccessLog = (\PHP_SAPI === 'cli' || \PHP_SAPI === 'cli-server');
+        $needsAccessLog = (\PHP_SAPI === 'cli' || \PHP_SAPI === 'cli-server') ? $container : null;
 
-        $container = new Container();
         if ($middleware) {
             $needsErrorHandlerNext = false;
             foreach ($middleware as $handler) {
+                // load AccessLogHandler and ErrorHandler instance from last Container
+                if ($handler === AccessLogHandler::class) {
+                    $handler = $container->getAccessLogHandler();
+                } elseif ($handler === ErrorHandler::class) {
+                    $handler = $container->getErrorHandler();
+                }
+
+                // ensure AccessLogHandler is always followed by ErrorHandler
                 if ($needsErrorHandlerNext && !$handler instanceof ErrorHandler) {
                     break;
                 }
                 $needsErrorHandlerNext = false;
 
                 if ($handler instanceof Container) {
+                    // remember last Container to load any following class names
                     $container = $handler;
-                } elseif ($handler === ErrorHandler::class || $handler === AccessLogHandler::class) {
-                    throw new \TypeError($handler . ' may currently only be passed as a middleware instance');
+
+                    // add default ErrorHandler from last Container before adding any other handlers, may be followed by other Container instances (unlikely)
+                    if (!$handlers) {
+                        $needsErrorHandler = $needsAccessLog = $container;
+                    }
                 } elseif (!\is_callable($handler)) {
                     $handlers[] = $container->callable($handler);
                 } else {
+                    // don't need a default ErrorHandler if we're adding one as first handler or AccessLogHandler as first followed by one
+                    if ($needsErrorHandler && ($handler instanceof ErrorHandler || $handler instanceof AccessLogHandler) && !$handlers) {
+                        $needsErrorHandler = null;
+                    }
                     $handlers[] = $handler;
                     if ($handler instanceof AccessLogHandler) {
-                        $needsAccessLog = false;
+                        $needsAccessLog = null;
                         $needsErrorHandlerNext = true;
                     }
                 }
@@ -73,13 +90,13 @@ public function __construct(...$middleware)
         }
 
         // add default ErrorHandler as first handler unless it is already added explicitly
-        if (!($handlers[0] ?? null) instanceof ErrorHandler && !($handlers[0] ?? null) instanceof AccessLogHandler) {
-            \array_unshift($handlers, new ErrorHandler());
+        if ($needsErrorHandler instanceof Container) {
+            \array_unshift($handlers, $needsErrorHandler->getErrorHandler());
         }
 
         // only log for built-in webserver and PHP development webserver by default, others have their own access log
-        if ($needsAccessLog) {
-            \array_unshift($handlers, new AccessLogHandler());
+        if ($needsAccessLog instanceof Container) {
+            \array_unshift($handlers, $needsAccessLog->getAccessLogHandler());
         }
 
         // automatically start new fiber for each request on PHP 8.1+

src/Container.php

@@ -92,6 +92,32 @@ public function callable(string $class): callable
         };
     }
 
+    /** @internal */
+    public function getAccessLogHandler(): AccessLogHandler
+    {
+        if ($this->container instanceof ContainerInterface) {
+            if ($this->container->has(AccessLogHandler::class)) {
+                return $this->container->get(AccessLogHandler::class);
+            } else {
+                return new AccessLogHandler();
+            }
+        }
+        return $this->load(AccessLogHandler::class);
+    }
+
+    /** @internal */
+    public function getErrorHandler(): ErrorHandler
+    {
+        if ($this->container instanceof ContainerInterface) {
+            if ($this->container->has(ErrorHandler::class)) {
+                return $this->container->get(ErrorHandler::class);
+            } else {
+                return new ErrorHandler();
+            }
+        }
+        return $this->load(ErrorHandler::class);
+    }
+
     /**
      * @param class-string $name
      * @return object

src/RouteHandler.php

@@ -56,7 +56,7 @@ public function map(array $methods, string $route, $handler, ...$handlers): void
                 $container = $handler;
                 unset($handlers[$i]);
             } elseif ($handler instanceof AccessLogHandler || $handler === AccessLogHandler::class) {
-                throw new \TypeError('AccessLogHandler may currently only be passed as a global middleware instance');
+                throw new \TypeError('AccessLogHandler may currently only be passed as a global middleware');
             } elseif (!\is_callable($handler)) {
                 $handlers[$i] = $container->callable($handler);
             }