From 265e8cdeaf4dc27e4fa15c5262b158de35a2bccf Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 15 Jul 2021 13:28:31 +0200 Subject: [PATCH] Refactored tracking actions --- config/autoload/url-shortener.global.php | 5 +++- module/Core/config/dependencies.config.php | 5 +--- .../src/Action/AbstractTrackingAction.php | 24 ++++++++----------- module/Core/src/Action/PixelAction.php | 3 ++- module/Core/src/Action/RedirectAction.php | 16 ++++--------- .../Core/src/Options/UrlShortenerOptions.php | 11 +++++++++ module/Core/test/Action/PixelActionTest.php | 5 ---- .../Core/test/Action/RedirectActionTest.php | 2 +- 8 files changed, 34 insertions(+), 37 deletions(-) diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index d7cd8b02..4a6afbc2 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -16,9 +16,12 @@ return [ 'validate_url' => false, // Deprecated 'visits_webhooks' => [], 'default_short_codes_length' => DEFAULT_SHORT_CODES_LENGTH, + 'auto_resolve_titles' => false, + 'append_extra_path' => false, + + // TODO Move these two options to their own config namespace. Maybe "redirects". 'redirect_status_code' => DEFAULT_REDIRECT_STATUS_CODE, 'redirect_cache_lifetime' => DEFAULT_REDIRECT_CACHE_LIFETIME, - 'auto_resolve_titles' => false, ], ]; diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 7a53f343..34de226d 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -118,17 +118,14 @@ return [ Action\RedirectAction::class => [ Service\ShortUrl\ShortUrlResolver::class, Visit\VisitsTracker::class, - ShortUrl\Helper\ShortUrlRedirectionBuilder::class, Options\TrackingOptions::class, + ShortUrl\Helper\ShortUrlRedirectionBuilder::class, Util\RedirectResponseHelper::class, - 'Logger_Shlink', ], Action\PixelAction::class => [ Service\ShortUrl\ShortUrlResolver::class, Visit\VisitsTracker::class, - ShortUrl\Helper\ShortUrlRedirectionBuilder::class, Options\TrackingOptions::class, - 'Logger_Shlink', ], Action\QrCodeAction::class => [ Service\ShortUrl\ShortUrlResolver::class, diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index c6b2279d..b0f3d6ee 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -7,33 +7,27 @@ namespace Shlinkio\Shlink\Core\Action; use Fig\Http\Message\RequestMethodInterface; use Mezzio\Router\Middleware\ImplicitHeadMiddleware; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\ResponseInterface as Response; 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\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Options\TrackingOptions; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; -use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilderInterface; use Shlinkio\Shlink\Core\Visit\VisitsTrackerInterface; use function array_key_exists; abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMethodInterface { - private LoggerInterface $logger; - public function __construct( private ShortUrlResolverInterface $urlResolver, private VisitsTrackerInterface $visitTracker, - private ShortUrlRedirectionBuilderInterface $redirectionBuilder, private TrackingOptions $trackingOptions, - ?LoggerInterface $logger = null ) { - $this->logger = $logger ?? new NullLogger(); } public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface @@ -48,9 +42,8 @@ abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMet $this->visitTracker->track($shortUrl, Visitor::fromRequest($request)); } - return $this->createSuccessResp($this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $query)); + return $this->createSuccessResp($shortUrl, $request); } catch (ShortUrlNotFoundException $e) { - $this->logger->warning('An error occurred while tracking short code. {e}', ['e' => $e]); return $this->createErrorResp($request, $handler); } } @@ -66,10 +59,13 @@ abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMet return $disableTrackParam === null || ! array_key_exists($disableTrackParam, $query); } - abstract protected function createSuccessResp(string $longUrl): ResponseInterface; - - abstract protected function createErrorResp( + abstract protected function createSuccessResp( + ShortUrl $shortUrl, ServerRequestInterface $request, - RequestHandlerInterface $handler, ): ResponseInterface; + + protected function createErrorResp(ServerRequestInterface $request, RequestHandlerInterface $handler): Response + { + return $handler->handle($request); + } } diff --git a/module/Core/src/Action/PixelAction.php b/module/Core/src/Action/PixelAction.php index 3f67bdec..0cf2a801 100644 --- a/module/Core/src/Action/PixelAction.php +++ b/module/Core/src/Action/PixelAction.php @@ -8,10 +8,11 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Common\Response\PixelResponse; +use Shlinkio\Shlink\Core\Entity\ShortUrl; class PixelAction extends AbstractTrackingAction { - protected function createSuccessResp(string $longUrl): ResponseInterface + protected function createSuccessResp(ShortUrl $shortUrl, ServerRequestInterface $request): ResponseInterface { return new PixelResponse(); } diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 7c313c53..2711f5bc 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -7,8 +7,7 @@ namespace Shlinkio\Shlink\Core\Action; use Fig\Http\Message\StatusCodeInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface; -use Psr\Http\Server\RequestHandlerInterface; -use Psr\Log\LoggerInterface; +use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Options; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilderInterface; @@ -20,21 +19,16 @@ class RedirectAction extends AbstractTrackingAction implements StatusCodeInterfa public function __construct( ShortUrlResolverInterface $urlResolver, VisitsTrackerInterface $visitTracker, - ShortUrlRedirectionBuilderInterface $redirectionBuilder, Options\TrackingOptions $trackingOptions, + private ShortUrlRedirectionBuilderInterface $redirectionBuilder, private RedirectResponseHelperInterface $redirectResponseHelper, - ?LoggerInterface $logger = null ) { - parent::__construct($urlResolver, $visitTracker, $redirectionBuilder, $trackingOptions, $logger); + parent::__construct($urlResolver, $visitTracker, $trackingOptions); } - protected function createSuccessResp(string $longUrl): Response + protected function createSuccessResp(ShortUrl $shortUrl, ServerRequestInterface $request): Response { + $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request->getQueryParams()); return $this->redirectResponseHelper->buildRedirectResponse($longUrl); } - - protected function createErrorResp(ServerRequestInterface $request, RequestHandlerInterface $handler): Response - { - return $handler->handle($request); - } } diff --git a/module/Core/src/Options/UrlShortenerOptions.php b/module/Core/src/Options/UrlShortenerOptions.php index a0005da2..31ecc137 100644 --- a/module/Core/src/Options/UrlShortenerOptions.php +++ b/module/Core/src/Options/UrlShortenerOptions.php @@ -19,6 +19,7 @@ class UrlShortenerOptions extends AbstractOptions private int $redirectStatusCode = DEFAULT_REDIRECT_STATUS_CODE; private int $redirectCacheLifetime = DEFAULT_REDIRECT_CACHE_LIFETIME; private bool $autoResolveTitles = false; + private bool $appendExtraPath = false; public function isUrlValidationEnabled(): bool { @@ -67,6 +68,16 @@ class UrlShortenerOptions extends AbstractOptions $this->autoResolveTitles = $autoResolveTitles; } + public function appendExtraPath(): bool + { + return $this->appendExtraPath; + } + + protected function setAppendExtraPath(bool $appendExtraPath): void + { + $this->appendExtraPath = $appendExtraPath; + } + /** @deprecated */ protected function setAnonymizeRemoteAddr(bool $anonymizeRemoteAddr): void { diff --git a/module/Core/test/Action/PixelActionTest.php b/module/Core/test/Action/PixelActionTest.php index 2436ac27..6df2498a 100644 --- a/module/Core/test/Action/PixelActionTest.php +++ b/module/Core/test/Action/PixelActionTest.php @@ -16,7 +16,6 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Options\TrackingOptions; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; -use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilderInterface; use Shlinkio\Shlink\Core\Visit\VisitsTracker; class PixelActionTest extends TestCase @@ -32,13 +31,9 @@ class PixelActionTest extends TestCase $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); $this->visitTracker = $this->prophesize(VisitsTracker::class); - $redirectBuilder = $this->prophesize(ShortUrlRedirectionBuilderInterface::class); - $redirectBuilder->buildShortUrlRedirect(Argument::cetera())->willReturn('http://domain.com/foo/bar'); - $this->action = new PixelAction( $this->urlResolver->reveal(), $this->visitTracker->reveal(), - $redirectBuilder->reveal(), new TrackingOptions(), ); } diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index e3c4694a..cc5690d0 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -48,8 +48,8 @@ class RedirectActionTest extends TestCase $this->action = new RedirectAction( $this->urlResolver->reveal(), $this->visitTracker->reveal(), - $redirectBuilder->reveal(), new Options\TrackingOptions(['disableTrackParam' => 'foobar']), + $redirectBuilder->reveal(), $this->redirectRespHelper->reveal(), ); }