Merge pull request #2282 from acelaya-forks/feature/track-redirect-url

Add redirect_url field to track where a visitor is redirected for a visit
shlinkio · Nov 24, 2024 · 7634f55 · 7634f55
2 parents fef512a + 571a464
commit 7634f55
Show file tree

Hide file tree

Showing 18 changed files with 152 additions and 19 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
     * `geolocation-country-code`: Allows to perform redirections based on the ISO 3166-1 alpha-2 two-letter country code resolved while geolocating the visitor.
     * `geolocation-city-name`: Allows to perform redirections based on the city name resolved while geolocating the visitor.
 
+* [#2032](https://github.com/shlinkio/shlink/issues/2032) Save the URL to which a visitor is redirected when a visit is tracked.
+
+    The value is exposed in the API as a new `redirectUrl` field for visit objects.
+
+    This is useful to know where a visitor was redirected for a short URL with dynamic redirect rules, for special redirects, or simply in case the long URL was changed over time, and you still want to know where visitors were redirected originally.
+
+    Some visits may not have a redirect URL if a redirect didn't happen, like for orphan visits when no special redirects are configured, or when a visit is tracked as part of the pixel action.
+
 ### Changed
 * [#2193](https://github.com/shlinkio/shlink/issues/2193) API keys are now hashed using SHA256, instead of being saved in plain text.
 

diff --git a/config/constants.php b/config/constants.php
@@ -22,3 +22,4 @@
 const DEFAULT_QR_CODE_COLOR = '#000000'; // Black
 const DEFAULT_QR_CODE_BG_COLOR = '#ffffff'; // White
 const IP_ADDRESS_REQUEST_ATTRIBUTE = 'remote_address';
+const REDIRECT_URL_REQUEST_ATTRIBUTE = 'redirect_url';
diff --git a/docs/async-api/async-api.json b/docs/async-api/async-api.json
@@ -247,6 +247,11 @@
                         "type": "string",
                         "nullable": true,
                         "description": "The originally visited URL that triggered the tracking of this visit"
+                    },
+                    "redirectUrl": {
+                        "type": "string",
+                        "nullable":  true,
+                        "description": "The URL to which the visitor was redirected, or null if a redirect did not occur, like for 404 requests or pixel tracking"
                     }
                 },
                 "example": {

diff --git a/docs/swagger/definitions/Visit.json b/docs/swagger/definitions/Visit.json
@@ -25,6 +25,10 @@
         "visitedUrl": {
             "type": ["string", "null"],
             "description": "The originally visited URL that triggered the tracking of this visit"
+        },
+        "redirectUrl": {
+            "type": ["string", "null"],
+            "description": "The URL to which the visitor was redirected, or null if a redirect did not occur, like for 404 requests or pixel tracking"
         }
     }
 }
diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.Visit.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.Visit.php
@@ -75,4 +75,10 @@
             ->columnName('potential_bot')
             ->option('default', false)
             ->build();
+
+    fieldWithUtf8Charset($builder->createField('redirectUrl', Types::STRING), $emConfig)
+        ->columnName('redirect_url')
+        ->length(Visitor::REDIRECT_URL_MAX_LENGTH)
+        ->nullable()
+        ->build();
 };
diff --git a/module/Core/migrations/Version20241124112257.php b/module/Core/migrations/Version20241124112257.php
@@ -0,0 +1,39 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkMigrations;
+
+use Doctrine\DBAL\Platforms\MySQLPlatform;
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\DBAL\Types\Types;
+use Doctrine\Migrations\AbstractMigration;
+
+final class Version20241124112257 extends AbstractMigration
+{
+    private const COLUMN_NAME = 'redirect_url';
+
+    public function up(Schema $schema): void
+    {
+        $visits = $schema->getTable('visits');
+        $this->skipIf($visits->hasColumn(self::COLUMN_NAME));
+
+        $visits->addColumn(self::COLUMN_NAME, Types::STRING, [
+            'length' => 2048,
+            'notnull' => false,
+            'default' => null,
+        ]);
+    }
+
+    public function down(Schema $schema): void
+    {
+        $visits = $schema->getTable('visits');
+        $this->skipIf(! $visits->hasColumn(self::COLUMN_NAME));
+        $visits->dropColumn(self::COLUMN_NAME);
+    }
+
+    public function isTransactional(): bool
+    {
+        return ! ($this->connection->getDatabasePlatform() instanceof MySQLPlatform);
+    }
+}
diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php
@@ -16,6 +16,8 @@
 use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface;
 use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface;
 
+use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE;
+
 abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMethodInterface
 {
     public function __construct(
@@ -30,9 +32,13 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
 
         try {
             $shortUrl = $this->urlResolver->resolveEnabledShortUrl($identifier);
-            $this->requestTracker->trackIfApplicable($shortUrl, $request);
+            $response = $this->createSuccessResp($shortUrl, $request);
+            $this->requestTracker->trackIfApplicable($shortUrl, $request->withAttribute(
+                REDIRECT_URL_REQUEST_ATTRIBUTE,
+                $response->hasHeader('Location') ? $response->getHeaderLine('Location') : null,
+            ));
 
-            return $this->createSuccessResp($shortUrl, $request);
+            return $response;
         } catch (ShortUrlNotFoundException) {
             return $this->createErrorResp($request, $handler);
         }

diff --git a/module/Core/src/ErrorHandler/NotFoundTrackerMiddleware.php b/module/Core/src/ErrorHandler/NotFoundTrackerMiddleware.php
@@ -10,15 +10,22 @@
 use Psr\Http\Server\RequestHandlerInterface;
 use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface;
 
-class NotFoundTrackerMiddleware implements MiddlewareInterface
+use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE;
+
+readonly class NotFoundTrackerMiddleware implements MiddlewareInterface
 {
     public function __construct(private RequestTrackerInterface $requestTracker)
     {
     }
 
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
-        $this->requestTracker->trackNotFoundIfApplicable($request);
-        return $handler->handle($request);
+        $response = $handler->handle($request);
+        $this->requestTracker->trackNotFoundIfApplicable($request->withAttribute(
+            REDIRECT_URL_REQUEST_ATTRIBUTE,
+            $response->hasHeader('Location') ? $response->getHeaderLine('Location') : null,
+        ));
+
+        return $response;
     }
 }
diff --git a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php
@@ -25,6 +25,8 @@
 use function sprintf;
 use function trim;
 
+use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE;
+
 readonly class ExtraPathRedirectMiddleware implements MiddlewareInterface
 {
     public function __construct(
@@ -73,9 +75,12 @@ private function tryToResolveRedirect(
 
         try {
             $shortUrl = $this->resolver->resolveEnabledShortUrl($identifier);
-            $this->requestTracker->trackIfApplicable($shortUrl, $request);
-
             $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request, $extraPath);
+            $this->requestTracker->trackIfApplicable(
+                $shortUrl,
+                $request->withAttribute(REDIRECT_URL_REQUEST_ATTRIBUTE, $longUrl),
+            );
+
             return $this->redirectResponseHelper->buildRedirectResponse($longUrl);
         } catch (ShortUrlNotFoundException) {
             if ($extraPath === null || ! $this->urlShortenerOptions->multiSegmentSlugsEnabled) {

diff --git a/module/Core/src/Visit/Entity/Visit.php b/module/Core/src/Visit/Entity/Visit.php
@@ -28,6 +28,7 @@ private function __construct(
         public readonly bool $potentialBot,
         public readonly string|null $remoteAddr = null,
         public readonly string|null $visitedUrl = null,
+        public readonly string|null $redirectUrl = null,
         private VisitLocation|null $visitLocation = null,
         public readonly Chronos $date = new Chronos(),
     ) {
@@ -68,6 +69,7 @@ private static function fromVisitor(
             potentialBot: $visitor->potentialBot,
             remoteAddr: self::processAddress($visitor->remoteAddress, $anonymize),
             visitedUrl: $visitor->visitedUrl,
+            redirectUrl: $visitor->redirectUrl,
             visitLocation: $geolocation !== null ? VisitLocation::fromGeolocation($geolocation) : null,
         );
     }
@@ -156,6 +158,7 @@ public function jsonSerialize(): array
             'visitLocation' => $this->visitLocation,
             'potentialBot' => $this->potentialBot,
             'visitedUrl' => $this->visitedUrl,
+            'redirectUrl' => $this->redirectUrl,
         ];
         if (! $this->isOrphan()) {
             return $base;

diff --git a/module/Core/src/Visit/Model/Visitor.php b/module/Core/src/Visit/Model/Visitor.php
@@ -13,12 +13,15 @@
 use function Shlinkio\Shlink\Core\isCrawler;
 use function substr;
 
+use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE;
+
 final readonly class Visitor
 {
     public const USER_AGENT_MAX_LENGTH = 512;
     public const REFERER_MAX_LENGTH = 1024;
     public const REMOTE_ADDRESS_MAX_LENGTH = 256;
     public const VISITED_URL_MAX_LENGTH = 2048;
+    public const REDIRECT_URL_MAX_LENGTH = 2048;
 
     private function __construct(
         public string $userAgent,
@@ -27,6 +30,7 @@ private function __construct(
         public string $visitedUrl,
         public bool $potentialBot,
         public Location|null $geolocation,
+        public string|null $redirectUrl,
     ) {
     }
 
@@ -36,6 +40,7 @@ public static function fromParams(
         string|null $remoteAddress = null,
         string $visitedUrl = '',
         Location|null $geolocation = null,
+        string|null $redirectUrl = null,
     ): self {
         return new self(
             userAgent: self::cropToLength($userAgent, self::USER_AGENT_MAX_LENGTH),
@@ -46,6 +51,7 @@ public static function fromParams(
             visitedUrl: self::cropToLength($visitedUrl, self::VISITED_URL_MAX_LENGTH),
             potentialBot: isCrawler($userAgent),
             geolocation: $geolocation,
+            redirectUrl: $redirectUrl === null ? null : self::cropToLength($redirectUrl, self::REDIRECT_URL_MAX_LENGTH),
         );
     }
 
@@ -62,6 +68,7 @@ public static function fromRequest(ServerRequestInterface $request): self
             remoteAddress: ipAddressFromRequest($request),
             visitedUrl: $request->getUri()->__toString(),
             geolocation: geolocationFromRequest($request),
+            redirectUrl: $request->getAttribute(REDIRECT_URL_REQUEST_ATTRIBUTE),
         );
     }
 
@@ -85,6 +92,7 @@ public function normalizeForTrackingOptions(TrackingOptions $options): self
             // Keep the fact that the visit was a potential bot, even if we no longer save the user agent
             potentialBot: $this->potentialBot,
             geolocation: $this->geolocation,
+            redirectUrl: $this->redirectUrl,
         );
     }
 }
diff --git a/module/Core/test/Action/PixelActionTest.php b/module/Core/test/Action/PixelActionTest.php
@@ -16,6 +16,8 @@
 use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface;
 use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface;
 
+use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE;
+
 class PixelActionTest extends TestCase
 {
     private PixelAction $action;
@@ -34,12 +36,17 @@ protected function setUp(): void
     public function imageIsReturned(): void
     {
         $shortCode = 'abc123';
+        $shortUrl = ShortUrl::withLongUrl('http://domain.com/foo/bar');
+        $request = (new ServerRequest())->withAttribute('shortCode', $shortCode);
+
         $this->urlResolver->expects($this->once())->method('resolveEnabledShortUrl')->with(
             ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, ''),
-        )->willReturn(ShortUrl::withLongUrl('http://domain.com/foo/bar'));
-        $this->requestTracker->expects($this->once())->method('trackIfApplicable')->withAnyParameters();
+        )->willReturn($shortUrl);
+        $this->requestTracker->expects($this->once())->method('trackIfApplicable')->with(
+            $shortUrl,
+            $request->withAttribute(REDIRECT_URL_REQUEST_ATTRIBUTE, null),
+        );
 
-        $request = (new ServerRequest())->withAttribute('shortCode', $shortCode);
         $response = $this->action->process($request, $this->createMock(RequestHandlerInterface::class));
 
         self::assertInstanceOf(PixelResponse::class, $response);

diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php
@@ -19,6 +19,8 @@
 use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface;
 use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface;
 
+use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE;
+
 class RedirectActionTest extends TestCase
 {
     private const LONG_URL = 'https://domain.com/foo/bar?some=thing';
@@ -50,16 +52,20 @@ public function redirectionIsPerformedToLongUrl(): void
     {
         $shortCode = 'abc123';
         $shortUrl = ShortUrl::withLongUrl(self::LONG_URL);
+        $expectedResp = new Response\RedirectResponse(self::LONG_URL);
+        $request = (new ServerRequest())->withAttribute('shortCode', $shortCode);
+
         $this->urlResolver->expects($this->once())->method('resolveEnabledShortUrl')->with(
             ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, ''),
         )->willReturn($shortUrl);
-        $this->requestTracker->expects($this->once())->method('trackIfApplicable');
-        $expectedResp = new Response\RedirectResponse(self::LONG_URL);
+        $this->requestTracker->expects($this->once())->method('trackIfApplicable')->with(
+            $shortUrl,
+            $request->withAttribute(REDIRECT_URL_REQUEST_ATTRIBUTE, self::LONG_URL),
+        );
         $this->redirectRespHelper->expects($this->once())->method('buildRedirectResponse')->with(
             self::LONG_URL,
         )->willReturn($expectedResp);
 
-        $request = (new ServerRequest())->withAttribute('shortCode', $shortCode);
         $response = $this->action->process($request, $this->createMock(RequestHandlerInterface::class));
 
         self::assertSame($expectedResp, $response);

diff --git a/module/Core/test/ErrorHandler/NotFoundTrackerMiddlewareTest.php b/module/Core/test/ErrorHandler/NotFoundTrackerMiddlewareTest.php
@@ -4,7 +4,9 @@
 
 namespace ShlinkioTest\Shlink\Core\ErrorHandler;
 
+use Laminas\Diactoros\Response;
 use Laminas\Diactoros\ServerRequestFactory;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
@@ -14,6 +16,8 @@
 use Shlinkio\Shlink\Core\ErrorHandler\NotFoundTrackerMiddleware;
 use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface;
 
+use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE;
+
 class NotFoundTrackerMiddlewareTest extends TestCase
 {
     private NotFoundTrackerMiddleware $middleware;
@@ -33,12 +37,22 @@ protected function setUp(): void
         );
     }
 
-    #[Test]
-    public function delegatesIntoRequestTracker(): void
+    #[Test, DataProvider('provideResponses')]
+    public function delegatesIntoRequestTracker(Response $resp, string|null $expectedRedirectUrl): void
     {
-        $this->handler->expects($this->once())->method('handle')->with($this->request);
-        $this->requestTracker->expects($this->once())->method('trackNotFoundIfApplicable')->with($this->request);
+        $this->handler->expects($this->once())->method('handle')->with($this->request)->willReturn($resp);
+        $this->requestTracker->expects($this->once())->method('trackNotFoundIfApplicable')->with(
+            $this->request->withAttribute(REDIRECT_URL_REQUEST_ATTRIBUTE, $expectedRedirectUrl),
+        );
+
+        $result = $this->middleware->process($this->request, $this->handler);
 
-        $this->middleware->process($this->request, $this->handler);
+        self::assertSame($resp, $result);
+    }
+
+    public static function provideResponses(): iterable
+    {
+        yield 'no location response' => [new Response(), null];
+        yield 'location response' => [new Response\RedirectResponse('the_location'), 'the_location'];
     }
 }
diff --git a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php
@@ -80,6 +80,7 @@ public function visitIsProperlySerializedIntoUpdate(string $method, string $expe
                 'date' => $visit->date->toAtomString(),
                 'potentialBot' => false,
                 'visitedUrl' => '',
+                'redirectUrl' => null,
             ],
         ], $update->payload);
     }
@@ -105,6 +106,7 @@ public function orphanVisitIsProperlySerializedIntoUpdate(Visit $orphanVisit): v
                 'potentialBot' => false,
                 'visitedUrl' => $orphanVisit->visitedUrl,
                 'type' => $orphanVisit->type->value,
+                'redirectUrl' => null,
             ],
         ], $update->payload);
     }

diff --git a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php
@@ -30,6 +30,8 @@
 use function Laminas\Stratigility\middleware;
 use function str_starts_with;
 
+use const Shlinkio\Shlink\REDIRECT_URL_REQUEST_ATTRIBUTE;
+
 class ExtraPathRedirectMiddlewareTest extends TestCase
 {
     private MockObject & ShortUrlResolverInterface $resolver;
@@ -159,7 +161,10 @@ function () use ($shortUrl, &$currentIteration, $expectedResolveCalls): ShortUrl
         $this->redirectResponseHelper->expects($this->once())->method('buildRedirectResponse')->with(
             'the_built_long_url',
         )->willReturn(new RedirectResponse(''));
-        $this->requestTracker->expects($this->once())->method('trackIfApplicable')->with($shortUrl, $request);
+        $this->requestTracker->expects($this->once())->method('trackIfApplicable')->with(
+            $shortUrl,
+            $request->withAttribute(REDIRECT_URL_REQUEST_ATTRIBUTE, 'the_built_long_url'),
+        );
 
         $this->middleware($options)->process($request, $this->handler);
     }