Simplify logic in RedirectRule when checking geolocation conditions

module/Core/functions/functions.php

@@ -293,5 +293,10 @@ function ipAddressFromRequest(ServerRequestInterface $request): string|null
 
 function geolocationFromRequest(ServerRequestInterface $request): Location|null
 {
-    return $request->getAttribute(Location::class);
+    $geolocation = $request->getAttribute(Location::class);
+    if ($geolocation !== null && ! $geolocation instanceof Location) {
+        // TODO Throw exception
+    }
+
+    return $geolocation;
 }

module/Core/src/RedirectRule/Entity/RedirectCondition.php

@@ -9,11 +9,10 @@ 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;
 use function Shlinkio\Shlink\Core\ArrayUtils\some;
+use function Shlinkio\Shlink\Core\geolocationFromRequest;
 use function Shlinkio\Shlink\Core\ipAddressFromRequest;
 use function Shlinkio\Shlink\Core\normalizeLocale;
 use function Shlinkio\Shlink\Core\splitLocale;
@@ -134,9 +133,8 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable
 
     private function matchesGeolocationCountryCode(ServerRequestInterface $request): bool
     {
-        $geolocation = $request->getAttribute(Location::class);
+        $geolocation = geolocationFromRequest($request);
-        // TODO We should eventually rely on `Location` type only
+        if ($geolocation === null) {
-        if (! ($geolocation instanceof Location) && ! ($geolocation instanceof VisitLocation)) {
             return false;
         }
 
@@ -145,14 +143,12 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable
 
     private function matchesGeolocationCityName(ServerRequestInterface $request): bool
     {
-        $geolocation = $request->getAttribute(Location::class);
+        $geolocation = geolocationFromRequest($request);
-        // TODO We should eventually rely on `Location` type only
+        if ($geolocation === null) {
-        if (! ($geolocation instanceof Location) && ! ($geolocation instanceof VisitLocation)) {
             return false;
         }
 
-        $cityName = $geolocation instanceof Location ? $geolocation->city : $geolocation->cityName;
+        return strcasecmp($geolocation->city, $this->matchValue) === 0;
-        return strcasecmp($cityName, $this->matchValue) === 0;
     }
 
     public function jsonSerialize(): array

module/Core/test/RedirectRule/Entity/RedirectConditionTest.php

@@ -10,7 +10,6 @@ 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;
@@ -99,7 +98,7 @@ class RedirectConditionTest extends TestCase
 
     #[Test, DataProvider('provideVisitsWithCountry')]
     public function matchesGeolocationCountryCode(
-        Location|VisitLocation|null $location,
+        Location|null $location,
         string $countryCodeToMatch,
         bool $expected,
     ): void {
@@ -114,21 +113,11 @@ 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,
-        ];
     }
 
     #[Test, DataProvider('provideVisitsWithCity')]
     public function matchesGeolocationCityName(
-        Location|VisitLocation|null $location,
+        Location|null $location,
         string $cityNameToMatch,
         bool $expected,
     ): void {
@@ -143,15 +132,5 @@ class RedirectConditionTest extends TestCase
         yield 'non-matching location' => [new Location(city: 'Los Angeles'), 'New York', false];
         yield 'matching location' => [new Location(city: 'Madrid'), 'Madrid', true];
         yield 'matching case-insensitive' => [new Location(city: 'Los Angeles'), 'los angeles', true];
-        yield 'matching visit location' => [
-            VisitLocation::fromGeolocation(new Location(city: 'New York')),
-            'New York',
-            true,
-        ];
-        yield 'matching visit case-insensitive' => [
-            VisitLocation::fromGeolocation(new Location(city: 'barcelona')),
-            'Barcelona',
-            true,
-        ];
     }
 }