Split NotFoundHandler into two different middlewares

config/autoload/middleware-pipeline.global.php

@@ -67,7 +67,8 @@ return [
         ],
         'not-found' => [
             'middleware' => [
-                Core\ErrorHandler\NotFoundHandler::class,
+                Core\ErrorHandler\NotFoundRedirectHandler::class,
+                Core\ErrorHandler\NotFoundTemplateHandler::class,
             ],
         ],
     ],

module/Core/config/dependencies.config.php

@@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core;
 
 use Doctrine\Common\Cache\Cache;
 use Psr\EventDispatcher\EventDispatcherInterface;
-use Shlinkio\Shlink\Core\ErrorHandler\NotFoundHandler;
+use Shlinkio\Shlink\Core\ErrorHandler;
 use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions;
 use Shlinkio\Shlink\PreviewGenerator\Service\PreviewGenerator;
 use Zend\Expressive\Router\RouterInterface;
@@ -17,7 +17,8 @@ return [
 
     'dependencies' => [
         'factories' => [
-            NotFoundHandler::class => ConfigAbstractFactory::class,
+            ErrorHandler\NotFoundRedirectHandler::class => ConfigAbstractFactory::class,
+            ErrorHandler\NotFoundTemplateHandler::class => ConfigAbstractFactory::class,
 
             Options\AppOptions::class => ConfigAbstractFactory::class,
             Options\DeleteShortUrlsOptions::class => ConfigAbstractFactory::class,
@@ -43,11 +44,8 @@ return [
     ],
 
     ConfigAbstractFactory::class => [
-        NotFoundHandler::class => [
-            TemplateRendererInterface::class,
-            NotFoundRedirectOptions::class,
-            'config.router.base_path',
-        ],
+        ErrorHandler\NotFoundRedirectHandler::class => [NotFoundRedirectOptions::class, 'config.router.base_path'],
+        ErrorHandler\NotFoundTemplateHandler::class => [TemplateRendererInterface::class],
 
         Options\AppOptions::class => ['config.app_options'],
         Options\DeleteShortUrlsOptions::class => ['config.delete_short_urls'],

module/Core/src/ErrorHandler/NotFoundRedirectHandler.php

@@ -4,67 +4,38 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core\ErrorHandler;
 
-use Fig\Http\Message\StatusCodeInterface;
-use InvalidArgumentException;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Message\UriInterface;
+use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 use Shlinkio\Shlink\Core\Action\RedirectAction;
 use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions;
 use Zend\Diactoros\Response;
 use Zend\Expressive\Router\RouteResult;
-use Zend\Expressive\Template\TemplateRendererInterface;
 
-use function array_shift;
-use function explode;
 use function rtrim;
 
-class NotFoundHandler implements RequestHandlerInterface
+class NotFoundRedirectHandler implements MiddlewareInterface
 {
-    public const NOT_FOUND_TEMPLATE = 'ShlinkCore::error/404';
-    public const INVALID_SHORT_CODE_TEMPLATE = 'ShlinkCore::invalid-short-code';
-
-    /** @var TemplateRendererInterface */
-    private $renderer;
     /** @var NotFoundRedirectOptions */
     private $redirectOptions;
     /** @var string */
     private $shlinkBasePath;
 
-    public function __construct(
-        TemplateRendererInterface $renderer,
-        NotFoundRedirectOptions $redirectOptions,
-        string $shlinkBasePath
-    ) {
-        $this->renderer = $renderer;
+    public function __construct(NotFoundRedirectOptions $redirectOptions, string $shlinkBasePath)
+    {
         $this->redirectOptions = $redirectOptions;
         $this->shlinkBasePath = $shlinkBasePath;
     }
 
-    /**
-     * Dispatch the next available middleware and return the response.
-     *
-     * @param ServerRequestInterface $request
-     *
-     * @return ResponseInterface
-     * @throws InvalidArgumentException
-     */
-    public function handle(ServerRequestInterface $request): ResponseInterface
+    public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
         /** @var RouteResult $routeResult */
         $routeResult = $request->getAttribute(RouteResult::class, RouteResult::fromRouteFailure(null));
         $redirectResponse = $this->createRedirectResponse($routeResult, $request->getUri());
-        if ($redirectResponse !== null) {
-            return $redirectResponse;
-        }
 
-        $accepts = explode(',', $request->getHeaderLine('Accept'));
-        $accept = array_shift($accepts);
-        $status = StatusCodeInterface::STATUS_NOT_FOUND;
-
-        $template = $routeResult->isFailure() ? self::NOT_FOUND_TEMPLATE : self::INVALID_SHORT_CODE_TEMPLATE;
-        return new Response\HtmlResponse($this->renderer->render($template), $status);
+        return $redirectResponse ?? $handler->handle($request);
     }
 
     private function createRedirectResponse(RouteResult $routeResult, UriInterface $uri): ?ResponseInterface

module/Core/src/ErrorHandler/NotFoundTemplateHandler.php

@@ -0,0 +1,46 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\ErrorHandler;
+
+use Fig\Http\Message\StatusCodeInterface;
+use InvalidArgumentException;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Server\RequestHandlerInterface;
+use Zend\Diactoros\Response;
+use Zend\Expressive\Router\RouteResult;
+use Zend\Expressive\Template\TemplateRendererInterface;
+
+class NotFoundTemplateHandler implements RequestHandlerInterface
+{
+    public const NOT_FOUND_TEMPLATE = 'ShlinkCore::error/404';
+    public const INVALID_SHORT_CODE_TEMPLATE = 'ShlinkCore::invalid-short-code';
+
+    /** @var TemplateRendererInterface */
+    private $renderer;
+
+    public function __construct(TemplateRendererInterface $renderer)
+    {
+        $this->renderer = $renderer;
+    }
+
+    /**
+     * Dispatch the next available middleware and return the response.
+     *
+     * @param ServerRequestInterface $request
+     *
+     * @return ResponseInterface
+     * @throws InvalidArgumentException
+     */
+    public function handle(ServerRequestInterface $request): ResponseInterface
+    {
+        /** @var RouteResult $routeResult */
+        $routeResult = $request->getAttribute(RouteResult::class, RouteResult::fromRouteFailure(null));
+        $status = StatusCodeInterface::STATUS_NOT_FOUND;
+
+        $template = $routeResult->isFailure() ? self::NOT_FOUND_TEMPLATE : self::INVALID_SHORT_CODE_TEMPLATE;
+        return new Response\HtmlResponse($this->renderer->render($template), $status);
+    }
+}

module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php

@@ -5,35 +5,29 @@ declare(strict_types=1);
 namespace ShlinkioTest\Shlink\Core\ErrorHandler;
 
 use PHPUnit\Framework\TestCase;
-use Prophecy\Argument;
-use Prophecy\Prophecy\ObjectProphecy;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\MiddlewareInterface;
+use Psr\Http\Server\RequestHandlerInterface;
 use Shlinkio\Shlink\Core\Action\RedirectAction;
-use Shlinkio\Shlink\Core\ErrorHandler\NotFoundHandler;
+use Shlinkio\Shlink\Core\ErrorHandler\NotFoundRedirectHandler;
 use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions;
 use Zend\Diactoros\Response;
 use Zend\Diactoros\ServerRequestFactory;
 use Zend\Diactoros\Uri;
 use Zend\Expressive\Router\Route;
 use Zend\Expressive\Router\RouteResult;
-use Zend\Expressive\Template\TemplateRendererInterface;
 
-class NotFoundHandlerTest extends TestCase
+class NotFoundRedirectHandlerTest extends TestCase
 {
-    /** @var NotFoundHandler */
-    private $delegate;
-    /** @var ObjectProphecy */
-    private $renderer;
+    /** @var NotFoundRedirectHandler */
+    private $middleware;
     /** @var NotFoundRedirectOptions */
     private $redirectOptions;
 
     public function setUp(): void
     {
-        $this->renderer = $this->prophesize(TemplateRendererInterface::class);
         $this->redirectOptions = new NotFoundRedirectOptions();
-
-        $this->delegate = new NotFoundHandler($this->renderer->reveal(), $this->redirectOptions, '');
+        $this->middleware = new NotFoundRedirectHandler($this->redirectOptions, '');
     }
 
     /**
@@ -48,11 +42,10 @@ class NotFoundHandlerTest extends TestCase
         $this->redirectOptions->regular404 = 'regular404';
         $this->redirectOptions->baseUrl = 'baseUrl';
 
-        $resp = $this->delegate->handle($request);
+        $resp = $this->middleware->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal());
 
         $this->assertInstanceOf(Response\RedirectResponse::class, $resp);
         $this->assertEquals($expectedRedirectTo, $resp->getHeaderLine('Location'));
-        $this->renderer->render(Argument::cetera())->shouldNotHaveBeenCalled();
     }
 
     public function provideRedirects(): iterable
@@ -86,33 +79,4 @@ class NotFoundHandlerTest extends TestCase
             'invalidShortUrl',
         ];
     }
-
-    /**
-     * @test
-     * @dataProvider provideTemplates
-     */
-    public function properErrorTemplateIsRendered(ServerRequestInterface $request, string $expectedTemplate): void
-    {
-        $request = $request->withHeader('Accept', 'text/html');
-        $render = $this->renderer->render($expectedTemplate)->willReturn('');
-
-        $resp = $this->delegate->handle($request);
-
-        $this->assertInstanceOf(Response\HtmlResponse::class, $resp);
-        $render->shouldHaveBeenCalledOnce();
-    }
-
-    public function provideTemplates(): iterable
-    {
-        $request = ServerRequestFactory::fromGlobals();
-
-        yield [$request, NotFoundHandler::NOT_FOUND_TEMPLATE];
-        yield [
-            $request->withAttribute(
-                RouteResult::class,
-                RouteResult::fromRoute(new Route('', $this->prophesize(MiddlewareInterface::class)->reveal()))
-            ),
-            NotFoundHandler::INVALID_SHORT_CODE_TEMPLATE,
-        ];
-    }
 }

module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php

@@ -0,0 +1,59 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Core\ErrorHandler;
+
+use PHPUnit\Framework\TestCase;
+use Prophecy\Prophecy\ObjectProphecy;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Server\MiddlewareInterface;
+use Shlinkio\Shlink\Core\ErrorHandler\NotFoundTemplateHandler;
+use Zend\Diactoros\Response;
+use Zend\Diactoros\ServerRequestFactory;
+use Zend\Expressive\Router\Route;
+use Zend\Expressive\Router\RouteResult;
+use Zend\Expressive\Template\TemplateRendererInterface;
+
+class NotFoundTemplateHandlerTest extends TestCase
+{
+    /** @var NotFoundTemplateHandler */
+    private $handler;
+    /** @var ObjectProphecy */
+    private $renderer;
+
+    public function setUp(): void
+    {
+        $this->renderer = $this->prophesize(TemplateRendererInterface::class);
+        $this->handler = new NotFoundTemplateHandler($this->renderer->reveal());
+    }
+
+    /**
+     * @test
+     * @dataProvider provideTemplates
+     */
+    public function properErrorTemplateIsRendered(ServerRequestInterface $request, string $expectedTemplate): void
+    {
+        $request = $request->withHeader('Accept', 'text/html');
+        $render = $this->renderer->render($expectedTemplate)->willReturn('');
+
+        $resp = $this->handler->handle($request);
+
+        $this->assertInstanceOf(Response\HtmlResponse::class, $resp);
+        $render->shouldHaveBeenCalledOnce();
+    }
+
+    public function provideTemplates(): iterable
+    {
+        $request = ServerRequestFactory::fromGlobals();
+
+        yield [$request, NotFoundTemplateHandler::NOT_FOUND_TEMPLATE];
+        yield [
+            $request->withAttribute(
+                RouteResult::class,
+                RouteResult::fromRoute(new Route('', $this->prophesize(MiddlewareInterface::class)->reveal()))
+            ),
+            NotFoundTemplateHandler::INVALID_SHORT_CODE_TEMPLATE,
+        ];
+    }
+}