From fb8ab0b5fe1b7f730b46ff576abcd7665d43bde7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 28 Mar 2020 09:12:15 +0100 Subject: [PATCH] Implemented how to reprocess the locations of all existing visits --- .../src/Command/Visit/LocateVisitsCommand.php | 51 ++++++++++++++++--- .../Core/src/Repository/VisitRepository.php | 9 ++++ .../Repository/VisitRepositoryInterface.php | 9 +++- module/Core/src/Visit/VisitLocator.php | 5 ++ .../Core/src/Visit/VisitLocatorInterface.php | 2 + .../Repository/VisitRepositoryTest.php | 6 ++- 6 files changed, 71 insertions(+), 11 deletions(-) diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php index 89a8aa71..1a7d0bd0 100644 --- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -18,6 +18,7 @@ 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\Exception\RuntimeException; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -60,30 +61,66 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat 'retry', 'r', InputOption::VALUE_NONE, - 'Will retry visits that were located with an empty location, in case it was a temporal issue.', + 'Will retry the location of visits that were located with a not-found location, in case it was due to ' + . 'a temporal issue.', ) ->addOption( 'all', 'a', InputOption::VALUE_NONE, - 'Will locate all visits, ignoring if they have already been located.', + 'When provided together with --retry, will locate all existing visits, regardless the fact that they ' + . 'have already been located.', ); } + protected function interact(InputInterface $input, OutputInterface $output): void + { + $this->io = new SymfonyStyle($input, $output); + $retry = $input->getOption('retry'); + $all = $input->getOption('all'); + + if ($all && !$retry) { + $this->io->writeln( + 'The --all flag has no effect on its own. You have to provide it ' + . 'together with --retry.', + ); + } + + if ($all && $retry && ! $this->warnAndVerifyContinue()) { + throw new RuntimeException('Execution aborted'); + } + } + + private function warnAndVerifyContinue(): bool + { + $this->io->warning([ + 'You are about to process the location of all existing visits your short URLs received.', + 'Since shlink saves visitors IP addresses anonymized, you could end up losing precision on some of ' + . 'your visits.', + 'Also, if you have a large amount of visits, this can be a very time consuming process. ' + . 'Continue at your own risk.', + ]); + return $this->io->confirm('Do you want to proceed?', false); + } + protected function lockedExecute(InputInterface $input, OutputInterface $output): int { - $this->io = new SymfonyStyle($input, $output); $retry = $input->getOption('retry'); + $all = $retry && $input->getOption('all'); try { $this->checkDbUpdate(); - $this->visitLocator->locateUnlocatedVisits($this); - if ($retry) { - $this->visitLocator->locateVisitsWithEmptyLocation($this); + if ($all) { + $this->visitLocator->locateAllVisits($this); + } else { + $this->visitLocator->locateUnlocatedVisits($this); + if ($retry) { + $this->visitLocator->locateVisitsWithEmptyLocation($this); + } } - $this->io->success('Finished processing all IPs'); + $this->io->success('Finished locating visits'); return ExitCodes::EXIT_SUCCESS; } catch (Throwable $e) { $this->io->error($e->getMessage()); diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index ed18df10..61b2afb8 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -40,6 +40,15 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa return $this->findVisitsForQuery($qb, $blockSize); } + public function findAllVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable + { + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->select('v') + ->from(Visit::class, 'v'); + + return $this->findVisitsForQuery($qb, $blockSize); + } + private function findVisitsForQuery(QueryBuilder $qb, int $blockSize): iterable { $originalQueryBuilder = $qb->setMaxResults($blockSize) diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php index b076cff3..f9cbc8d9 100644 --- a/module/Core/src/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Repository/VisitRepositoryInterface.php @@ -15,12 +15,17 @@ interface VisitRepositoryInterface extends ObjectRepository /** * @return iterable|Visit[] */ - public function findUnlocatedVisits(int $defaultBlockSize = self::DEFAULT_BLOCK_SIZE): iterable; + public function findUnlocatedVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable; /** * @return iterable|Visit[] */ - public function findVisitsWithEmptyLocation(int $defaultBlockSize = self::DEFAULT_BLOCK_SIZE): iterable; + public function findVisitsWithEmptyLocation(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable; + + /** + * @return iterable|Visit[] + */ + public function findAllVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable; /** * @return Visit[] diff --git a/module/Core/src/Visit/VisitLocator.php b/module/Core/src/Visit/VisitLocator.php index 5a66208b..f955ef29 100644 --- a/module/Core/src/Visit/VisitLocator.php +++ b/module/Core/src/Visit/VisitLocator.php @@ -35,6 +35,11 @@ class VisitLocator implements VisitLocatorInterface $this->locateVisits($this->repo->findVisitsWithEmptyLocation(), $helper); } + public function locateAllVisits(VisitGeolocationHelperInterface $helper): void + { + $this->locateVisits($this->repo->findAllVisits(), $helper); + } + /** * @param iterable|Visit[] $results */ diff --git a/module/Core/src/Visit/VisitLocatorInterface.php b/module/Core/src/Visit/VisitLocatorInterface.php index 6e06d296..1c99de36 100644 --- a/module/Core/src/Visit/VisitLocatorInterface.php +++ b/module/Core/src/Visit/VisitLocatorInterface.php @@ -9,4 +9,6 @@ interface VisitLocatorInterface public function locateUnlocatedVisits(VisitGeolocationHelperInterface $helper): void; public function locateVisitsWithEmptyLocation(VisitGeolocationHelperInterface $helper): void; + + public function locateAllVisits(VisitGeolocationHelperInterface $helper): void; } diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index b72453d6..034b15f9 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -68,11 +68,13 @@ class VisitRepositoryTest extends DatabaseTestCase $withEmptyLocation = $this->repo->findVisitsWithEmptyLocation($blockSize); $unlocated = $this->repo->findUnlocatedVisits($blockSize); + $all = $this->repo->findAllVisits($blockSize); - // Important! assertCount will not work here, as this iterable object loads data dynamically and counts to - // 0 if not iterated + // Important! assertCount will not work here, as this iterable object loads data dynamically and the count + // is 0 if not iterated $this->assertEquals(2, $countIterable($unlocated)); $this->assertEquals(4, $countIterable($withEmptyLocation)); + $this->assertEquals(6, $countIterable($all)); } public function provideBlockSize(): iterable