From 1437ff48ce6799112706f2bfd87551c3bd85bb9f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 7 May 2018 10:58:49 +0200 Subject: [PATCH] Ensured all core actions log errors --- config/autoload/middleware-pipeline.global.php | 4 ++-- module/Core/config/dependencies.config.php | 14 ++++++-------- module/Core/src/Action/AbstractTrackingAction.php | 11 ++++++++++- module/Core/src/Action/PreviewAction.php | 15 +++++++++++++-- module/Core/src/Action/QrCodeAction.php | 10 +++++----- .../src/Action/Util/ErrorResponseBuilderTrait.php | 4 ++-- .../{NotFoundDelegate.php => NotFoundHandler.php} | 6 +++--- ...ndDelegateTest.php => NotFoundHandlerTest.php} | 8 ++++---- 8 files changed, 45 insertions(+), 27 deletions(-) rename module/Core/src/Response/{NotFoundDelegate.php => NotFoundHandler.php} (90%) rename module/Core/test/Response/{NotFoundDelegateTest.php => NotFoundHandlerTest.php} (88%) diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index 90587cd3..dc17bd53 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -2,7 +2,7 @@ declare(strict_types=1); use Shlinkio\Shlink\Common\Middleware\LocaleMiddleware; -use Shlinkio\Shlink\Core\Response\NotFoundDelegate; +use Shlinkio\Shlink\Core\Response\NotFoundHandler; use Shlinkio\Shlink\Rest\Middleware\BodyParserMiddleware; use Shlinkio\Shlink\Rest\Middleware\CheckAuthenticationMiddleware; use Shlinkio\Shlink\Rest\Middleware\CrossDomainMiddleware; @@ -50,7 +50,7 @@ return [ 'post-routing' => [ 'middleware' => [ Expressive\Router\Middleware\DispatchMiddleware::class, - NotFoundDelegate::class, + NotFoundHandler::class, ], 'priority' => 1, ], diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 9061a3f5..58def9ec 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -6,7 +6,7 @@ use Shlinkio\Shlink\Common\Service\PreviewGenerator; use Shlinkio\Shlink\Core\Action; use Shlinkio\Shlink\Core\Middleware; use Shlinkio\Shlink\Core\Options; -use Shlinkio\Shlink\Core\Response\NotFoundDelegate; +use Shlinkio\Shlink\Core\Response\NotFoundHandler; use Shlinkio\Shlink\Core\Service; use Zend\Expressive\Router\RouterInterface; use Zend\Expressive\Template\TemplateRendererInterface; @@ -17,7 +17,7 @@ return [ 'dependencies' => [ 'factories' => [ Options\AppOptions::class => Options\AppOptionsFactory::class, - NotFoundDelegate::class => ConfigAbstractFactory::class, + NotFoundHandler::class => ConfigAbstractFactory::class, // Services Service\UrlShortener::class => ConfigAbstractFactory::class, @@ -33,14 +33,10 @@ return [ Action\PreviewAction::class => ConfigAbstractFactory::class, Middleware\QrCodeCacheMiddleware::class => ConfigAbstractFactory::class, ], - - 'aliases' => [ - 'Zend\Expressive\Delegate\DefaultDelegate' => NotFoundDelegate::class, - ], ], ConfigAbstractFactory::class => [ - NotFoundDelegate::class => [TemplateRendererInterface::class], + NotFoundHandler::class => [TemplateRendererInterface::class], // Services Service\UrlShortener::class => [ @@ -60,14 +56,16 @@ return [ Service\UrlShortener::class, Service\VisitsTracker::class, Options\AppOptions::class, + 'Logger_Shlink', ], Action\PixelAction::class => [ Service\UrlShortener::class, Service\VisitsTracker::class, Options\AppOptions::class, + 'Logger_Shlink', ], Action\QrCodeAction::class => [RouterInterface::class, Service\UrlShortener::class, 'Logger_Shlink'], - Action\PreviewAction::class => [PreviewGenerator::class, Service\UrlShortener::class], + Action\PreviewAction::class => [PreviewGenerator::class, Service\UrlShortener::class, 'Logger_Shlink'], Middleware\QrCodeCacheMiddleware::class => [Cache::class], ], diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 0ee475db..147e31b6 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -7,6 +7,8 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; @@ -30,15 +32,21 @@ abstract class AbstractTrackingAction implements MiddlewareInterface * @var AppOptions */ private $appOptions; + /** + * @var LoggerInterface + */ + private $logger; public function __construct( UrlShortenerInterface $urlShortener, VisitsTrackerInterface $visitTracker, - AppOptions $appOptions + AppOptions $appOptions, + LoggerInterface $logger = null ) { $this->urlShortener = $urlShortener; $this->visitTracker = $visitTracker; $this->appOptions = $appOptions; + $this->logger = $logger ?: new NullLogger(); } /** @@ -66,6 +74,7 @@ abstract class AbstractTrackingAction implements MiddlewareInterface return $this->createResp($longUrl); } catch (InvalidShortCodeException | EntityDoesNotExistException $e) { + $this->logger->warning('An error occurred while tracking short code.' . PHP_EOL . $e); return $this->buildErrorResponse($request, $handler); } } diff --git a/module/Core/src/Action/PreviewAction.php b/module/Core/src/Action/PreviewAction.php index 3dd3ba43..83748a3b 100644 --- a/module/Core/src/Action/PreviewAction.php +++ b/module/Core/src/Action/PreviewAction.php @@ -7,6 +7,8 @@ use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Shlinkio\Shlink\Common\Exception\PreviewGenerationException; use Shlinkio\Shlink\Common\Service\PreviewGeneratorInterface; use Shlinkio\Shlink\Common\Util\ResponseUtilsTrait; @@ -28,11 +30,19 @@ class PreviewAction implements MiddlewareInterface * @var UrlShortenerInterface */ private $urlShortener; + /** + * @var LoggerInterface + */ + private $logger; - public function __construct(PreviewGeneratorInterface $previewGenerator, UrlShortenerInterface $urlShortener) - { + public function __construct( + PreviewGeneratorInterface $previewGenerator, + UrlShortenerInterface $urlShortener, + LoggerInterface $logger = null + ) { $this->previewGenerator = $previewGenerator; $this->urlShortener = $urlShortener; + $this->logger = $logger ?: new NullLogger(); } /** @@ -53,6 +63,7 @@ class PreviewAction implements MiddlewareInterface $imagePath = $this->previewGenerator->generatePreview($url); return $this->generateImageResponse($imagePath); } catch (InvalidShortCodeException | EntityDoesNotExistException | PreviewGenerationException $e) { + $this->logger->warning('An error occurred while generating preview image.' . PHP_EOL . $e); return $this->buildErrorResponse($request, $handler); } } diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index 3e5aeaf9..81b1d326 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -15,6 +15,7 @@ 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; +use Zend\Expressive\Router\Exception\RuntimeException; use Zend\Expressive\Router\RouterInterface; class QrCodeAction implements MiddlewareInterface @@ -52,6 +53,8 @@ class QrCodeAction implements MiddlewareInterface * @param RequestHandlerInterface $handler * * @return Response + * @throws \InvalidArgumentException + * @throws RuntimeException */ public function process(Request $request, RequestHandlerInterface $handler): Response { @@ -59,11 +62,8 @@ class QrCodeAction implements MiddlewareInterface $shortCode = $request->getAttribute('shortCode'); try { $this->urlShortener->shortCodeToUrl($shortCode); - } catch (InvalidShortCodeException $e) { - $this->logger->warning('Tried to create a QR code with an invalid short code' . PHP_EOL . $e); - return $this->buildErrorResponse($request, $handler); - } catch (EntityDoesNotExistException $e) { - $this->logger->warning('Tried to create a QR code with a not found short code' . PHP_EOL . $e); + } catch (InvalidShortCodeException | EntityDoesNotExistException $e) { + $this->logger->warning('An error occurred while creating QR code' . PHP_EOL . $e); return $this->buildErrorResponse($request, $handler); } diff --git a/module/Core/src/Action/Util/ErrorResponseBuilderTrait.php b/module/Core/src/Action/Util/ErrorResponseBuilderTrait.php index efd0bc2a..89a88f47 100644 --- a/module/Core/src/Action/Util/ErrorResponseBuilderTrait.php +++ b/module/Core/src/Action/Util/ErrorResponseBuilderTrait.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core\Action\Util; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; -use Shlinkio\Shlink\Core\Response\NotFoundDelegate; +use Shlinkio\Shlink\Core\Response\NotFoundHandler; trait ErrorResponseBuilderTrait { @@ -14,7 +14,7 @@ trait ErrorResponseBuilderTrait ServerRequestInterface $request, RequestHandlerInterface $handler ): ResponseInterface { - $request = $request->withAttribute(NotFoundDelegate::NOT_FOUND_TEMPLATE, 'ShlinkCore::invalid-short-code'); + $request = $request->withAttribute(NotFoundHandler::NOT_FOUND_TEMPLATE, 'ShlinkCore::invalid-short-code'); return $handler->handle($request); } } diff --git a/module/Core/src/Response/NotFoundDelegate.php b/module/Core/src/Response/NotFoundHandler.php similarity index 90% rename from module/Core/src/Response/NotFoundDelegate.php rename to module/Core/src/Response/NotFoundHandler.php index db5ab040..fe055311 100644 --- a/module/Core/src/Response/NotFoundDelegate.php +++ b/module/Core/src/Response/NotFoundHandler.php @@ -6,13 +6,13 @@ namespace Shlinkio\Shlink\Core\Response; use Fig\Http\Message\StatusCodeInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Psr\Http\Server\RequestHandlerInterface as DelegateInterface; +use Psr\Http\Server\RequestHandlerInterface; use Zend\Diactoros\Response; use Zend\Expressive\Template\TemplateRendererInterface; -class NotFoundDelegate implements DelegateInterface +class NotFoundHandler implements RequestHandlerInterface { - const NOT_FOUND_TEMPLATE = 'notFoundTemplate'; + public const NOT_FOUND_TEMPLATE = 'notFoundTemplate'; /** * @var TemplateRendererInterface diff --git a/module/Core/test/Response/NotFoundDelegateTest.php b/module/Core/test/Response/NotFoundHandlerTest.php similarity index 88% rename from module/Core/test/Response/NotFoundDelegateTest.php rename to module/Core/test/Response/NotFoundHandlerTest.php index b3ce2e66..80b6171e 100644 --- a/module/Core/test/Response/NotFoundDelegateTest.php +++ b/module/Core/test/Response/NotFoundHandlerTest.php @@ -7,15 +7,15 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Core\Response\NotFoundDelegate; +use Shlinkio\Shlink\Core\Response\NotFoundHandler; use Zend\Diactoros\Response; use Zend\Diactoros\ServerRequestFactory; use Zend\Expressive\Template\TemplateRendererInterface; -class NotFoundDelegateTest extends TestCase +class NotFoundHandlerTest extends TestCase { /** - * @var NotFoundDelegate + * @var NotFoundHandler */ private $delegate; /** @@ -26,7 +26,7 @@ class NotFoundDelegateTest extends TestCase public function setUp() { $this->renderer = $this->prophesize(TemplateRendererInterface::class); - $this->delegate = new NotFoundDelegate($this->renderer->reveal()); + $this->delegate = new NotFoundHandler($this->renderer->reveal()); } /**