From 1bafe54a756754e164cff6a207292a368bcffbb3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 23 Nov 2019 10:25:12 +0100 Subject: [PATCH] Split NotFoundHandler into two different middlewares --- .../autoload/middleware-pipeline.global.php | 3 +- module/Core/config/dependencies.config.php | 12 ++-- ...andler.php => NotFoundRedirectHandler.php} | 41 ++----------- .../ErrorHandler/NotFoundTemplateHandler.php | 46 +++++++++++++++ ...st.php => NotFoundRedirectHandlerTest.php} | 50 +++------------- .../NotFoundTemplateHandlerTest.php | 59 +++++++++++++++++++ 6 files changed, 125 insertions(+), 86 deletions(-) rename module/Core/src/ErrorHandler/{NotFoundHandler.php => NotFoundRedirectHandler.php} (57%) create mode 100644 module/Core/src/ErrorHandler/NotFoundTemplateHandler.php rename module/Core/test/ErrorHandler/{NotFoundHandlerTest.php => NotFoundRedirectHandlerTest.php} (59%) create mode 100644 module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index d43eed5b..6013f56a 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/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, ], ], ], diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 55391fa8..0ebdae7d 100644 --- a/module/Core/config/dependencies.config.php +++ b/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'], diff --git a/module/Core/src/ErrorHandler/NotFoundHandler.php b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php similarity index 57% rename from module/Core/src/ErrorHandler/NotFoundHandler.php rename to module/Core/src/ErrorHandler/NotFoundRedirectHandler.php index e409c906..f6a03395 100644 --- a/module/Core/src/ErrorHandler/NotFoundHandler.php +++ b/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 diff --git a/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php b/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php new file mode 100644 index 00000000..7b84043d --- /dev/null +++ b/module/Core/src/ErrorHandler/NotFoundTemplateHandler.php @@ -0,0 +1,46 @@ +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); + } +} diff --git a/module/Core/test/ErrorHandler/NotFoundHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php similarity index 59% rename from module/Core/test/ErrorHandler/NotFoundHandlerTest.php rename to module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php index 5806e5ec..d2776179 100644 --- a/module/Core/test/ErrorHandler/NotFoundHandlerTest.php +++ b/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, - ]; - } } diff --git a/module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php new file mode 100644 index 00000000..7d763448 --- /dev/null +++ b/module/Core/test/ErrorHandler/NotFoundTemplateHandlerTest.php @@ -0,0 +1,59 @@ +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, + ]; + } +}