diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php
index a9de1f0c..47a47735 100644
--- a/module/CLI/config/dependencies.config.php
+++ b/module/CLI/config/dependencies.config.php
@@ -20,7 +20,6 @@ use Shlinkio\Shlink\Core\Visit;
use Shlinkio\Shlink\Installer\Factory\ProcessHelperFactory;
use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater;
use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2Options;
-use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
use Shlinkio\Shlink\Rest\Service\ApiKeyService;
use Symfony\Component\Console as SymfonyCli;
use Symfony\Component\Lock\LockFactory;
@@ -97,7 +96,7 @@ return [
Command\Visit\DownloadGeoLiteDbCommand::class => [GeoLite\GeolocationDbUpdater::class],
Command\Visit\LocateVisitsCommand::class => [
Visit\VisitLocator::class,
- IpLocationResolverInterface::class,
+ Visit\VisitToLocationHelper::class,
LockFactory::class,
],
Command\Visit\GetOrphanVisitsCommand::class => [Visit\VisitsStatsHelper::class],
diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php
index 47aa50d1..59db9367 100644
--- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php
+++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php
@@ -11,11 +11,11 @@ use Shlinkio\Shlink\Common\Util\IpAddress;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
+use Shlinkio\Shlink\Core\Visit\Model\UnlocatableIpType;
use Shlinkio\Shlink\Core\Visit\VisitGeolocationHelperInterface;
use Shlinkio\Shlink\Core\Visit\VisitLocatorInterface;
-use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
+use Shlinkio\Shlink\Core\Visit\VisitToLocationHelperInterface;
use Shlinkio\Shlink\IpGeolocation\Model\Location;
-use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
use Symfony\Component\Console\Exception\RuntimeException;
use Symfony\Component\Console\Input\ArrayInput;
use Symfony\Component\Console\Input\InputInterface;
@@ -35,7 +35,7 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat
public function __construct(
private readonly VisitLocatorInterface $visitLocator,
- private readonly IpLocationResolverInterface $ipLocationResolver,
+ private readonly VisitToLocationHelperInterface $visitToLocation,
LockFactory $locker,
) {
parent::__construct($locker);
@@ -132,39 +132,33 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat
*/
public function geolocateVisit(Visit $visit): Location
{
- if (! $visit->hasRemoteAddr()) {
- $this->io->writeln(
- 'Ignored visit with no IP address',
- OutputInterface::VERBOSITY_VERBOSE,
- );
- throw IpCannotBeLocatedException::forEmptyAddress();
- }
-
- $ipAddr = $visit->getRemoteAddr() ?? '';
+ $ipAddr = $visit->getRemoteAddr() ?? '?';
$this->io->write(sprintf('Processing IP %s>', $ipAddr));
- if ($ipAddr === IpAddress::LOCALHOST) {
- $this->io->writeln(' [Ignored localhost address]');
- throw IpCannotBeLocatedException::forLocalhost();
- }
try {
- return $this->ipLocationResolver->resolveIpLocation($ipAddr);
- } catch (WrongIpException $e) {
- $this->io->writeln(' [An error occurred while locating IP. Skipped>]');
- if ($this->io->isVerbose()) {
+ return $this->visitToLocation->resolveVisitLocation($visit);
+ } catch (IpCannotBeLocatedException $e) {
+ $this->io->writeln(match ($e->type) {
+ UnlocatableIpType::EMPTY_ADDRESS => ' [Ignored visit with no IP address]',
+ UnlocatableIpType::LOCALHOST => ' [Ignored localhost address]',
+ UnlocatableIpType::ERROR => ' [An error occurred while locating IP. Skipped>]',
+ });
+
+ if ($e->type === UnlocatableIpType::ERROR && $this->io->isVerbose()) {
$this->getApplication()?->renderThrowable($e, $this->io);
}
- throw IpCannotBeLocatedException::forError($e);
+ throw $e;
}
}
public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void
{
- $message = ! $visitLocation->isEmpty()
- ? sprintf(' [Address located in "%s"]', $visitLocation->getCountryName())
- : ' [Address not found]';
- $this->io->writeln($message);
+ if (! $visitLocation->isEmpty()) {
+ $this->io->writeln(sprintf(' [Address located in "%s"]', $visitLocation->getCountryName()));
+ } elseif ($visit->hasRemoteAddr() && $visit->getRemoteAddr() !== IpAddress::LOCALHOST) {
+ $this->io->writeln(' [Could not locate address]');
+ }
}
private function checkDbUpdate(): void
diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
index 4111c1dc..63ad3e52 100644
--- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
+++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
@@ -10,16 +10,16 @@ use Prophecy\Prophecy\ObjectProphecy;
use Shlinkio\Shlink\CLI\Command\Visit\DownloadGeoLiteDbCommand;
use Shlinkio\Shlink\CLI\Command\Visit\LocateVisitsCommand;
use Shlinkio\Shlink\CLI\Util\ExitCodes;
-use Shlinkio\Shlink\Common\Util\IpAddress;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
+use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
use Shlinkio\Shlink\Core\Model\Visitor;
use Shlinkio\Shlink\Core\Visit\VisitGeolocationHelperInterface;
use Shlinkio\Shlink\Core\Visit\VisitLocator;
+use Shlinkio\Shlink\Core\Visit\VisitToLocationHelperInterface;
use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
use Shlinkio\Shlink\IpGeolocation\Model\Location;
-use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
use ShlinkioTest\Shlink\CLI\CliTestUtilsTrait;
use Symfony\Component\Console\Exception\RuntimeException;
use Symfony\Component\Console\Output\OutputInterface;
@@ -36,14 +36,14 @@ class LocateVisitsCommandTest extends TestCase
private CommandTester $commandTester;
private ObjectProphecy $visitService;
- private ObjectProphecy $ipResolver;
+ private ObjectProphecy $visitToLocation;
private ObjectProphecy $lock;
private ObjectProphecy $downloadDbCommand;
protected function setUp(): void
{
$this->visitService = $this->prophesize(VisitLocator::class);
- $this->ipResolver = $this->prophesize(IpLocationResolverInterface::class);
+ $this->visitToLocation = $this->prophesize(VisitToLocationHelperInterface::class);
$locker = $this->prophesize(Lock\LockFactory::class);
$this->lock = $this->prophesize(Lock\LockInterface::class);
@@ -54,7 +54,7 @@ class LocateVisitsCommandTest extends TestCase
$command = new LocateVisitsCommand(
$this->visitService->reveal(),
- $this->ipResolver->reveal(),
+ $this->visitToLocation->reveal(),
$locker->reveal(),
);
@@ -84,7 +84,7 @@ class LocateVisitsCommandTest extends TestCase
$mockMethodBehavior,
);
$locateAllVisits = $this->visitService->locateAllVisits(Argument::cetera())->will($mockMethodBehavior);
- $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn(
+ $resolveIpLocation = $this->visitToLocation->resolveVisitLocation(Argument::any())->willReturn(
Location::emptyInstance(),
);
@@ -117,36 +117,29 @@ class LocateVisitsCommandTest extends TestCase
* @test
* @dataProvider provideIgnoredAddresses
*/
- public function localhostAndEmptyAddressesAreIgnored(?string $address, string $message): void
+ public function localhostAndEmptyAddressesAreIgnored(IpCannotBeLocatedException $e, string $message): void
{
- $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $address, ''));
+ $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance());
$location = VisitLocation::fromGeolocation(Location::emptyInstance());
$locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(
$this->invokeHelperMethods($visit, $location),
);
- $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn(
- Location::emptyInstance(),
- );
+ $resolveIpLocation = $this->visitToLocation->resolveVisitLocation(Argument::any())->willThrow($e);
$this->commandTester->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]);
$output = $this->commandTester->getDisplay();
+ self::assertStringContainsString('Processing IP', $output);
self::assertStringContainsString($message, $output);
- if (empty($address)) {
- self::assertStringNotContainsString('Processing IP', $output);
- } else {
- self::assertStringContainsString('Processing IP', $output);
- }
$locateVisits->shouldHaveBeenCalledOnce();
- $resolveIpLocation->shouldNotHaveBeenCalled();
+ $resolveIpLocation->shouldHaveBeenCalledOnce();
}
public function provideIgnoredAddresses(): iterable
{
- yield 'with empty address' => ['', 'Ignored visit with no IP address'];
- yield 'with null address' => [null, 'Ignored visit with no IP address'];
- yield 'with localhost address' => [IpAddress::LOCALHOST, 'Ignored localhost address'];
+ yield 'empty address' => [IpCannotBeLocatedException::forEmptyAddress(), 'Ignored visit with no IP address'];
+ yield 'localhost address' => [IpCannotBeLocatedException::forLocalhost(), 'Ignored localhost address'];
}
/** @test */
@@ -158,7 +151,9 @@ class LocateVisitsCommandTest extends TestCase
$locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(
$this->invokeHelperMethods($visit, $location),
);
- $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willThrow(WrongIpException::class);
+ $resolveIpLocation = $this->visitToLocation->resolveVisitLocation(Argument::any())->willThrow(
+ IpCannotBeLocatedException::forError(WrongIpException::fromIpAddress('1.2.3.4')),
+ );
$this->commandTester->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]);
@@ -187,7 +182,7 @@ class LocateVisitsCommandTest extends TestCase
$locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(function (): void {
});
- $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]);
+ $resolveIpLocation = $this->visitToLocation->resolveVisitLocation(Argument::any());
$this->commandTester->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]);
$output = $this->commandTester->getDisplay();