From 4619ebd0145e508ff8ba75a08d05601eef4317df Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 14 Nov 2024 08:20:20 +0100 Subject: [PATCH] After tracking a visit, set its location in the request as attribute --- module/Core/src/Action/AbstractTrackingAction.php | 8 ++++++-- module/Core/src/Action/RedirectAction.php | 3 +-- .../src/RedirectRule/Entity/RedirectCondition.php | 4 +++- .../Middleware/ExtraPathRedirectMiddleware.php | 9 +++++++-- .../RedirectRule/Entity/RedirectConditionTest.php | 13 ++++++++++++- 5 files changed, 29 insertions(+), 8 deletions(-) diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 78eebc05..b7ddb69a 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -15,6 +15,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; +use Shlinkio\Shlink\IpGeolocation\Model\Location; abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMethodInterface { @@ -30,9 +31,12 @@ abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMet try { $shortUrl = $this->urlResolver->resolveEnabledShortUrl($identifier); - $this->requestTracker->trackIfApplicable($shortUrl, $request); + $visit = $this->requestTracker->trackIfApplicable($shortUrl, $request); - return $this->createSuccessResp($shortUrl, $request); + return $this->createSuccessResp( + $shortUrl, + $request->withAttribute(Location::class, $visit?->getVisitLocation()), + ); } catch (ShortUrlNotFoundException) { return $this->createErrorResp($request, $handler); } diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 942cf550..a929f290 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Action; -use Fig\Http\Message\StatusCodeInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; @@ -13,7 +12,7 @@ use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; -class RedirectAction extends AbstractTrackingAction implements StatusCodeInterface +class RedirectAction extends AbstractTrackingAction { public function __construct( ShortUrlResolverInterface $urlResolver, diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index affa994a..3782f0ef 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -9,6 +9,7 @@ use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; use Shlinkio\Shlink\Core\RedirectRule\Model\Validation\RedirectRulesInputFilter; use Shlinkio\Shlink\Core\Util\IpAddressUtils; +use Shlinkio\Shlink\Core\Visit\Entity\VisitLocation; use Shlinkio\Shlink\IpGeolocation\Model\Location; use function Shlinkio\Shlink\Core\acceptLanguageToLocales; @@ -128,7 +129,8 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable private function matchesGeolocationCountryCode(ServerRequestInterface $request): bool { $geolocation = $request->getAttribute(Location::class); - if (!($geolocation instanceof Location)) { + // TODO We should eventually rely on `Location` type only + if (! ($geolocation instanceof Location) && ! ($geolocation instanceof VisitLocation)) { return false; } diff --git a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php index 4a02f6e9..2b4ac7cc 100644 --- a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php @@ -17,6 +17,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; +use Shlinkio\Shlink\IpGeolocation\Model\Location; use function array_slice; use function count; @@ -73,9 +74,13 @@ readonly class ExtraPathRedirectMiddleware implements MiddlewareInterface try { $shortUrl = $this->resolver->resolveEnabledShortUrl($identifier); - $this->requestTracker->trackIfApplicable($shortUrl, $request); + $visit = $this->requestTracker->trackIfApplicable($shortUrl, $request); - $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request, $extraPath); + $longUrl = $this->redirectionBuilder->buildShortUrlRedirect( + $shortUrl, + $request->withAttribute(Location::class, $visit?->getVisitLocation()), + $extraPath, + ); return $this->redirectResponseHelper->buildRedirectResponse($longUrl); } catch (ShortUrlNotFoundException) { if ($extraPath === null || ! $this->urlShortenerOptions->multiSegmentSlugsEnabled) { diff --git a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php index 895b8236..179d35e9 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -10,6 +10,7 @@ use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Middleware\IpAddressMiddlewareFactory; use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Entity\RedirectCondition; +use Shlinkio\Shlink\Core\Visit\Entity\VisitLocation; use Shlinkio\Shlink\IpGeolocation\Model\Location; use const ShlinkioTest\Shlink\ANDROID_USER_AGENT; @@ -98,7 +99,7 @@ class RedirectConditionTest extends TestCase #[Test, DataProvider('provideVisits')] public function matchesGeolocationCountryCode( - Location|null $location, + Location|VisitLocation|null $location, string $countryCodeToMatch, bool $expected, ): void { @@ -113,5 +114,15 @@ class RedirectConditionTest extends TestCase yield 'non-matching location' => [new Location(countryCode: 'ES'), 'US', false]; yield 'matching location' => [new Location(countryCode: 'US'), 'US', true]; yield 'matching case-insensitive' => [new Location(countryCode: 'US'), 'us', true]; + yield 'matching visit location' => [ + VisitLocation::fromGeolocation(new Location(countryCode: 'US')), + 'US', + true, + ]; + yield 'matching visit case-insensitive' => [ + VisitLocation::fromGeolocation(new Location(countryCode: 'es')), + 'ES', + true, + ]; } }