diff --git a/module/Core/src/Action/PreviewAction.php b/module/Core/src/Action/PreviewAction.php index e732f6ef..4ac2a50c 100644 --- a/module/Core/src/Action/PreviewAction.php +++ b/module/Core/src/Action/PreviewAction.php @@ -11,7 +11,6 @@ use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Shlinkio\Shlink\Common\Response\ResponseUtilsTrait; -use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; @@ -22,7 +21,6 @@ use Shlinkio\Shlink\PreviewGenerator\Service\PreviewGeneratorInterface; class PreviewAction implements MiddlewareInterface { use ResponseUtilsTrait; - use ErrorResponseBuilderTrait; /** @var PreviewGeneratorInterface */ private $previewGenerator; @@ -60,7 +58,7 @@ class PreviewAction implements MiddlewareInterface return $this->generateImageResponse($imagePath); } catch (InvalidShortCodeException | EntityDoesNotExistException | PreviewGenerationException $e) { $this->logger->warning('An error occurred while generating preview image. {e}', ['e' => $e]); - return $this->buildErrorResponse($request, $handler); + return $handler->handle($request); } } } diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index ecc258eb..a1fdae5b 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -12,7 +12,6 @@ use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Shlinkio\Shlink\Common\Response\QrCodeResponse; -use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; @@ -21,8 +20,6 @@ use Zend\Expressive\Router\RouterInterface; class QrCodeAction implements MiddlewareInterface { - use ErrorResponseBuilderTrait; - private const DEFAULT_SIZE = 300; private const MIN_SIZE = 50; private const MAX_SIZE = 1000; @@ -65,7 +62,7 @@ class QrCodeAction implements MiddlewareInterface $this->urlShortener->shortCodeToUrl($shortCode, $domain); } catch (InvalidShortCodeException | EntityDoesNotExistException $e) { $this->logger->warning('An error occurred while creating QR code. {e}', ['e' => $e]); - return $this->buildErrorResponse($request, $handler); + return $handler->handle($request); } $path = $this->router->generateUri(RedirectAction::class, ['shortCode' => $shortCode]); diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 0ae94235..d27bc0ad 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -7,14 +7,11 @@ namespace Shlinkio\Shlink\Core\Action; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; -use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait; use Shlinkio\Shlink\Core\Options; use Zend\Diactoros\Response\RedirectResponse; class RedirectAction extends AbstractTrackingAction { - use ErrorResponseBuilderTrait; - /** @var Options\NotFoundRedirectOptions */ private $redirectOptions; @@ -27,6 +24,6 @@ class RedirectAction extends AbstractTrackingAction protected function createErrorResp(ServerRequestInterface $request, RequestHandlerInterface $handler): Response { - return $this->buildErrorResponse($request, $handler); + return $handler->handle($request); } } diff --git a/module/Core/src/Action/Util/ErrorResponseBuilderTrait.php b/module/Core/src/Action/Util/ErrorResponseBuilderTrait.php deleted file mode 100644 index b9046006..00000000 --- a/module/Core/src/Action/Util/ErrorResponseBuilderTrait.php +++ /dev/null @@ -1,21 +0,0 @@ -withAttribute(NotFoundHandler::NOT_FOUND_TEMPLATE, 'ShlinkCore::invalid-short-code'); - return $handler->handle($request); - } -} diff --git a/module/Core/src/Response/NotFoundHandler.php b/module/Core/src/Response/NotFoundHandler.php index 2791da68..57e9fef4 100644 --- a/module/Core/src/Response/NotFoundHandler.php +++ b/module/Core/src/Response/NotFoundHandler.php @@ -8,6 +8,7 @@ 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\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; @@ -22,12 +23,11 @@ use function rtrim; class NotFoundHandler implements RequestHandlerInterface { - public const NOT_FOUND_TEMPLATE = 'notFoundTemplate'; + public const NOT_FOUND_ERROR_TEMPLATE = 'ShlinkCore::error/404'; + public const INVALID_SHORT_CODE_ERROR_TEMPLATE = 'ShlinkCore::invalid-short-code'; /** @var TemplateRendererInterface */ private $renderer; - /** @var string */ - private $defaultTemplate; /** @var NotFoundRedirectOptions */ private $redirectOptions; /** @var string */ @@ -36,11 +36,9 @@ class NotFoundHandler implements RequestHandlerInterface public function __construct( TemplateRendererInterface $renderer, NotFoundRedirectOptions $redirectOptions, - string $shlinkBasePath, - string $defaultTemplate = 'ShlinkCore::error/404' + string $shlinkBasePath ) { $this->renderer = $renderer; - $this->defaultTemplate = $defaultTemplate; $this->redirectOptions = $redirectOptions; $this->shlinkBasePath = $shlinkBasePath; } @@ -55,7 +53,9 @@ class NotFoundHandler implements RequestHandlerInterface */ public function handle(ServerRequestInterface $request): ResponseInterface { - $redirectResponse = $this->createRedirectResponse($request); + /** @var RouteResult $routeResult */ + $routeResult = $request->getAttribute(RouteResult::class, RouteResult::fromRouteFailure(null)); + $redirectResponse = $this->createRedirectResponse($routeResult, $request->getUri()); if ($redirectResponse !== null) { return $redirectResponse; } @@ -72,15 +72,15 @@ class NotFoundHandler implements RequestHandlerInterface ], $status); } - $notFoundTemplate = $request->getAttribute(self::NOT_FOUND_TEMPLATE, $this->defaultTemplate); + $notFoundTemplate = $routeResult->isFailure() + ? self::NOT_FOUND_ERROR_TEMPLATE + : self::INVALID_SHORT_CODE_ERROR_TEMPLATE; return new Response\HtmlResponse($this->renderer->render($notFoundTemplate), $status); } - private function createRedirectResponse(ServerRequestInterface $request): ?ResponseInterface + private function createRedirectResponse(RouteResult $routeResult, UriInterface $uri): ?ResponseInterface { - /** @var RouteResult $routeResult */ - $routeResult = $request->getAttribute(RouteResult::class, RouteResult::fromRouteFailure(null)); - $isBaseUrl = rtrim($request->getUri()->getPath(), '/') === $this->shlinkBasePath; + $isBaseUrl = rtrim($uri->getPath(), '/') === $this->shlinkBasePath; if ($isBaseUrl && $this->redirectOptions->hasBaseUrlRedirect()) { return new Response\RedirectResponse($this->redirectOptions->getBaseUrlRedirect()); diff --git a/module/Core/test/Response/NotFoundHandlerTest.php b/module/Core/test/Response/NotFoundHandlerTest.php index e2cf3667..de15e81e 100644 --- a/module/Core/test/Response/NotFoundHandlerTest.php +++ b/module/Core/test/Response/NotFoundHandlerTest.php @@ -110,4 +110,33 @@ 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_ERROR_TEMPLATE]; + yield [ + $request->withAttribute( + RouteResult::class, + RouteResult::fromRoute(new Route('', $this->prophesize(MiddlewareInterface::class)->reveal())) + ), + NotFoundHandler::INVALID_SHORT_CODE_ERROR_TEMPLATE, + ]; + } }