diff --git a/composer.json b/composer.json index 5ee67a4d..9cadd738 100644 --- a/composer.json +++ b/composer.json @@ -52,7 +52,7 @@ "shlinkio/shlink-common": "^3.0", "shlinkio/shlink-config": "^1.0", "shlinkio/shlink-event-dispatcher": "^1.4", - "shlinkio/shlink-installer": "^4.3", + "shlinkio/shlink-installer": "^4.3.1", "shlinkio/shlink-ip-geolocation": "^1.4", "symfony/console": "^5.0", "symfony/filesystem": "^5.0", diff --git a/config/autoload/request_id.global.php b/config/autoload/request_id.global.php index 0a9ed6ce..f057bb09 100644 --- a/config/autoload/request_id.global.php +++ b/config/autoload/request_id.global.php @@ -28,7 +28,10 @@ return [ 'config.request_id.allow_override', 'config.request_id.header_name', ], - RequestId\RequestIdMiddleware::class => [RequestId\RequestIdProviderFactory::class], + RequestId\RequestIdMiddleware::class => [ + RequestId\RequestIdProviderFactory::class, + 'config.request_id.header_name', + ], RequestId\MonologProcessor::class => [RequestId\RequestIdMiddleware::class], ], diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 5edb6499..0f2e70a5 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -11,6 +11,7 @@ use Laminas\ServiceManager\Factory\InvokableFactory; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater; use Shlinkio\Shlink\Common\Doctrine\NoDbNameConnectionFactory; use Shlinkio\Shlink\Core\Service; +use Shlinkio\Shlink\Core\Visit; use Shlinkio\Shlink\Installer\Factory\ProcessHelperFactory; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; @@ -67,7 +68,7 @@ return [ Command\ShortUrl\DeleteShortUrlCommand::class => [Service\ShortUrl\DeleteShortUrlService::class], Command\Visit\LocateVisitsCommand::class => [ - Service\VisitService::class, + Visit\VisitLocator::class, IpLocationResolverInterface::class, LockFactory::class, GeolocationDbUpdater::class, diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php index b19e8b19..935b9eee 100644 --- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; -use Exception; use Shlinkio\Shlink\CLI\Command\Util\AbstractLockedCommand; use Shlinkio\Shlink\CLI\Command\Util\LockedCommandConfig; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; @@ -14,12 +13,13 @@ 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\Service\VisitServiceInterface; +use Shlinkio\Shlink\Core\Visit\VisitLocatorInterface; use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\Model\Location; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; use Symfony\Component\Lock\LockFactory; @@ -31,7 +31,7 @@ class LocateVisitsCommand extends AbstractLockedCommand { public const NAME = 'visit:locate'; - private VisitServiceInterface $visitService; + private VisitLocatorInterface $visitLocator; private IpLocationResolverInterface $ipLocationResolver; private GeolocationDbUpdaterInterface $dbUpdater; @@ -39,13 +39,13 @@ class LocateVisitsCommand extends AbstractLockedCommand private ?ProgressBar $progressBar = null; public function __construct( - VisitServiceInterface $visitService, + VisitLocatorInterface $visitLocator, IpLocationResolverInterface $ipLocationResolver, LockFactory $locker, GeolocationDbUpdaterInterface $dbUpdater ) { parent::__construct($locker); - $this->visitService = $visitService; + $this->visitLocator = $visitLocator; $this->ipLocationResolver = $ipLocationResolver; $this->dbUpdater = $dbUpdater; } @@ -54,32 +54,46 @@ class LocateVisitsCommand extends AbstractLockedCommand { $this ->setName(self::NAME) - ->setDescription('Resolves visits origin locations.'); + ->setDescription('Resolves visits origin locations.') + ->addOption( + 'retry', + 'r', + InputOption::VALUE_NONE, + 'Will retry visits that were located with an empty location, in case it was a temporal issue.', + ) + ->addOption( + 'all', + 'a', + InputOption::VALUE_NONE, + 'Will locate all visits, ignoring if they have already been located.', + ); } protected function lockedExecute(InputInterface $input, OutputInterface $output): int { $this->io = new SymfonyStyle($input, $output); + $retry = $input->getOption('retry'); + $geolocateVisit = [$this, 'getGeolocationDataForVisit']; + $notifyVisitWithLocation = static function (VisitLocation $location) use ($output): void { + $message = ! $location->isEmpty() + ? sprintf(' [Address located in "%s"]', $location->getCountryName()) + : ' [Address not found]'; + $output->writeln($message); + }; try { $this->checkDbUpdate(); - $this->visitService->locateUnlocatedVisits( - [$this, 'getGeolocationDataForVisit'], - static function (VisitLocation $location) use ($output): void { - if (!$location->isEmpty()) { - $output->writeln( - sprintf(' [Address located at "%s"]', $location->getCountryName()), - ); - } - }, - ); + $this->visitLocator->locateUnlocatedVisits($geolocateVisit, $notifyVisitWithLocation); + if ($retry) { + $this->visitLocator->locateVisitsWithEmptyLocation($geolocateVisit, $notifyVisitWithLocation); + } $this->io->success('Finished processing all IPs'); return ExitCodes::EXIT_SUCCESS; } catch (Throwable $e) { $this->io->error($e->getMessage()); - if ($e instanceof Exception && $this->io->isVerbose()) { + if ($e instanceof Throwable && $this->io->isVerbose()) { $this->getApplication()->renderThrowable($e, $this->io); } diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php index 90073f10..94cd0bd1 100644 --- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php @@ -15,7 +15,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; use Shlinkio\Shlink\Core\Model\Visitor; -use Shlinkio\Shlink\Core\Service\VisitService; +use Shlinkio\Shlink\Core\Visit\VisitLocator; use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\Model\Location; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; @@ -38,7 +38,7 @@ class LocateVisitsCommandTest extends TestCase public function setUp(): void { - $this->visitService = $this->prophesize(VisitService::class); + $this->visitService = $this->prophesize(VisitLocator::class); $this->ipResolver = $this->prophesize(IpLocationResolverInterface::class); $this->dbUpdater = $this->prophesize(GeolocationDbUpdaterInterface::class); diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 2800cdd6..13a74c36 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -28,7 +28,7 @@ return [ Service\UrlShortener::class => ConfigAbstractFactory::class, Service\VisitsTracker::class => ConfigAbstractFactory::class, Service\ShortUrlService::class => ConfigAbstractFactory::class, - Service\VisitService::class => ConfigAbstractFactory::class, + Visit\VisitLocator::class => ConfigAbstractFactory::class, Service\Tag\TagService::class => ConfigAbstractFactory::class, Service\ShortUrl\DeleteShortUrlService::class => ConfigAbstractFactory::class, Service\ShortUrl\ShortUrlResolver::class => ConfigAbstractFactory::class, @@ -57,7 +57,7 @@ return [ Service\UrlShortener::class => [Util\UrlValidator::class, 'em', Resolver\PersistenceDomainResolver::class], Service\VisitsTracker::class => ['em', EventDispatcherInterface::class], Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class, Util\UrlValidator::class], - Service\VisitService::class => ['em'], + Visit\VisitLocator::class => ['em'], Service\Tag\TagService::class => ['em'], Service\ShortUrl\DeleteShortUrlService::class => [ 'em', diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index 454323ef..28eb2466 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -14,7 +14,8 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa /** * This method will allow you to iterate the whole list of unlocated visits, but loading them into memory in * smaller blocks of a specific size. - * This will have side effects if you update those rows while you iterate them. + * This will have side effects if you update those rows while you iterate them, in a way that they are no longer + * unlocated. * If you plan to do so, pass the first argument as false in order to disable applying offsets while slicing the * dataset * @@ -23,8 +24,8 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa public function findUnlocatedVisits(bool $applyOffset = true, int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable { $dql = <<getEntityManager()->createQuery($dql) ->setMaxResults($blockSize); $remainingVisitsToProcess = $this->count(['visitLocation' => null]); diff --git a/module/Core/src/Service/VisitServiceInterface.php b/module/Core/src/Service/VisitServiceInterface.php deleted file mode 100644 index 78543549..00000000 --- a/module/Core/src/Service/VisitServiceInterface.php +++ /dev/null @@ -1,10 +0,0 @@ -em = $em; } - public function locateUnlocatedVisits(callable $geolocateVisit, ?callable $notifyVisitWithLocation = null): void + public function locateUnlocatedVisits(callable $geolocateVisit, callable $notifyVisitWithLocation): void { /** @var VisitRepository $repo */ $repo = $this->em->getRepository(Visit::class); - $results = $repo->findUnlocatedVisits(false); + $this->locateVisits($repo->findUnlocatedVisits(false), $geolocateVisit, $notifyVisitWithLocation); + } + + public function locateVisitsWithEmptyLocation(callable $geolocateVisit, callable $notifyVisitWithLocation): void + { + $this->locateVisits([], $geolocateVisit, $notifyVisitWithLocation); + } + + /** + * @param iterable|Visit[] $results + */ + private function locateVisits(iterable $results, callable $geolocateVisit, callable $notifyVisitWithLocation): void + { $count = 0; $persistBlock = 200; @@ -58,13 +70,11 @@ class VisitService implements VisitServiceInterface $this->em->clear(); } - private function locateVisit(Visit $visit, VisitLocation $location, ?callable $notifyVisitWithLocation): void + private function locateVisit(Visit $visit, VisitLocation $location, callable $notifyVisitWithLocation): void { $visit->locate($location); $this->em->persist($visit); - if ($notifyVisitWithLocation !== null) { - $notifyVisitWithLocation($location, $visit); - } + $notifyVisitWithLocation($location, $visit); } } diff --git a/module/Core/src/Visit/VisitLocatorInterface.php b/module/Core/src/Visit/VisitLocatorInterface.php new file mode 100644 index 00000000..ea06c1b1 --- /dev/null +++ b/module/Core/src/Visit/VisitLocatorInterface.php @@ -0,0 +1,12 @@ +em = $this->prophesize(EntityManager::class); - $this->visitService = new VisitService($this->em->reveal()); + $this->visitService = new VisitLocator($this->em->reveal()); } /** @test */ @@ -95,6 +95,7 @@ class VisitServiceTest extends TestCase throw $isNonLocatableAddress ? new IpCannotBeLocatedException('Cannot be located') : IpCannotBeLocatedException::forError(new Exception('')); + }, static function (): void { }); $findUnlocatedVisits->shouldHaveBeenCalledOnce();