From 1e79969c3bb7329456ed6f5c8e947c0a2d903a43 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 Jan 2018 09:24:33 +0100 Subject: [PATCH] Made visits not to be tracked if query param has been provided --- module/Core/config/dependencies.config.php | 6 ++- module/Core/src/Action/RedirectAction.php | 19 +++++++-- .../Core/test/Action/RedirectActionTest.php | 42 +++++++++++++++++-- 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 31a4619a..11038e4b 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -55,7 +55,11 @@ return [ Service\Tag\TagService::class => ['em'], // Middleware - Action\RedirectAction::class => [Service\UrlShortener::class, Service\VisitsTracker::class], + Action\RedirectAction::class => [ + Service\UrlShortener::class, + Service\VisitsTracker::class, + Options\AppOptions::class, + ], Action\QrCodeAction::class => [RouterInterface::class, Service\UrlShortener::class, 'Logger_Shlink'], Action\PreviewAction::class => [PreviewGenerator::class, Service\UrlShortener::class], Middleware\QrCodeCacheMiddleware::class => [Cache::class], diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 066e9508..e7702dde 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -10,6 +10,7 @@ use Psr\Http\Message\ServerRequestInterface as Request; use Shlinkio\Shlink\Core\Action\Util\ErrorResponseBuilderTrait; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; +use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use Zend\Diactoros\Response\RedirectResponse; @@ -26,11 +27,19 @@ class RedirectAction implements MiddlewareInterface * @var VisitsTrackerInterface */ private $visitTracker; + /** + * @var AppOptions + */ + private $appOptions; - public function __construct(UrlShortenerInterface $urlShortener, VisitsTrackerInterface $visitTracker) - { + public function __construct( + UrlShortenerInterface $urlShortener, + VisitsTrackerInterface $visitTracker, + AppOptions $appOptions + ) { $this->urlShortener = $urlShortener; $this->visitTracker = $visitTracker; + $this->appOptions = $appOptions; } /** @@ -45,12 +54,16 @@ class RedirectAction implements MiddlewareInterface public function process(Request $request, DelegateInterface $delegate) { $shortCode = $request->getAttribute('shortCode', ''); + $query = $request->getQueryParams(); + $disableTrackParam = $this->appOptions->getDisableTrackParam(); try { $longUrl = $this->urlShortener->shortCodeToUrl($shortCode); // Track visit to this short code - $this->visitTracker->track($shortCode, $request); + if ($disableTrackParam === null || ! \array_key_exists($disableTrackParam, $query)) { + $this->visitTracker->track($shortCode, $request); + } // Return a redirect response to the long URL. // Use a temporary redirect to make sure browsers always hit the server for analytics purposes diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index a429549f..ed37baae 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -10,6 +10,7 @@ use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; +use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Service\VisitsTracker; use ShlinkioTest\Shlink\Common\Util\TestUtils; @@ -26,13 +27,21 @@ class RedirectActionTest extends TestCase * @var ObjectProphecy */ protected $urlShortener; + /** + * @var ObjectProphecy + */ + protected $visitTracker; public function setUp() { $this->urlShortener = $this->prophesize(UrlShortener::class); - $visitTracker = $this->prophesize(VisitsTracker::class); - $visitTracker->track(Argument::any()); - $this->action = new RedirectAction($this->urlShortener->reveal(), $visitTracker->reveal()); + $this->visitTracker = $this->prophesize(VisitsTracker::class); + + $this->action = new RedirectAction( + $this->urlShortener->reveal(), + $this->visitTracker->reveal(), + new AppOptions(['disableTrackParam' => 'foobar']) + ); } /** @@ -44,6 +53,8 @@ class RedirectActionTest extends TestCase $expectedUrl = 'http://domain.com/foo/bar'; $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($expectedUrl) ->shouldBeCalledTimes(1); + $this->visitTracker->track(Argument::cetera())->willReturn(null) + ->shouldBeCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); $response = $this->action->process($request, TestUtils::createDelegateMock()->reveal()); @@ -62,6 +73,9 @@ class RedirectActionTest extends TestCase $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class) ->shouldBeCalledTimes(1); + $this->visitTracker->track(Argument::cetera())->willReturn(null) + ->shouldNotBeCalled(); + $delegate = $this->prophesize(DelegateInterface::class); /** @var MethodProphecy $process */ $process = $delegate->process(Argument::any())->willReturn(new Response()); @@ -71,4 +85,26 @@ class RedirectActionTest extends TestCase $process->shouldHaveBeenCalledTimes(1); } + + /** + * @test + */ + public function visitIsNotTrackedIfDisableParamIsProvided() + { + $shortCode = 'abc123'; + $expectedUrl = 'http://domain.com/foo/bar'; + $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($expectedUrl) + ->shouldBeCalledTimes(1); + $this->visitTracker->track(Argument::cetera())->willReturn(null) + ->shouldNotBeCalled(); + + $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode) + ->withQueryParams(['foobar' => true]); + $response = $this->action->process($request, TestUtils::createDelegateMock()->reveal()); + + $this->assertInstanceOf(Response\RedirectResponse::class, $response); + $this->assertEquals(302, $response->getStatusCode()); + $this->assertTrue($response->hasHeader('Location')); + $this->assertEquals($expectedUrl, $response->getHeaderLine('Location')); + } }