mirrors/shlink
1
0
Fork 0
mirror of https://github.com/shlinkio/shlink.git synced 2025-03-14 04:00:57 +03:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #694 from acelaya-forks/feature/process-retry

Browse source 
Feature/process retry
This commit is contained in:
Alejandro Celaya 2020-03-28 12:06:09 +01:00 • committed by GitHub
commit 1e2d115768
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
24 changed files with 630 additions and 307 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
CHANGELOG.md
View file

@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org).
## [Unreleased]
## 2.1.0 - 2020-03-28
#### Added
@ -17,6 +17,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
#### Changed
* [#656](https://github.com/shlinkio/shlink/issues/656) Updated to PHPUnit 9.
* [#641](https://github.com/shlinkio/shlink/issues/641) Added two new flags to the `visit:locate` command, `--retry` and `--all`.
* When `--retry` is provided, it will try to re-locate visits which IP address was originally considered not found, in case it was a temporal issue.
* When `--all` is provided together with `--retry`, it will try to re-locate all existing visits. A warning and confirmation are displayed, as this can have side effects.
#### Deprecated

2
composer.json
View file

@ -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",

5
config/autoload/request_id.global.php
View file

@ -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],
],

44
data/migrations/Version20200323190014.php Normal file
View file

@ -0,0 +1,44 @@
<?php
declare(strict_types=1);
namespace ShlinkMigrations;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\DBAL\Types\Types;
use Doctrine\Migrations\AbstractMigration;
final class Version20200323190014 extends AbstractMigration
{
public function up(Schema $schema): void
{
$visitLocations = $schema->getTable('visit_locations');
$this->skipIf($visitLocations->hasColumn('is_empty'));
$visitLocations->addColumn('is_empty', Types::BOOLEAN, ['default' => false]);
}
public function postUp(Schema $schema): void
{
$qb = $this->connection->createQueryBuilder();
$qb->update('visit_locations')
->set('is_empty', true)
->where($qb->expr()->eq('country_code', ':empty'))
->andWhere($qb->expr()->eq('country_name', ':empty'))
->andWhere($qb->expr()->eq('region_name', ':empty'))
->andWhere($qb->expr()->eq('city_name', ':empty'))
->andWhere($qb->expr()->eq('timezone', ':empty'))
->andWhere($qb->expr()->eq('lat', 0))
->andWhere($qb->expr()->eq('lon', 0))
->setParameter('empty', '')
->execute();
}
public function down(Schema $schema): void
{
$visitLocations = $schema->getTable('visit_locations');
$this->skipIf(!$visitLocations->hasColumn('is_empty'));
$visitLocations->dropColumn('is_empty');
}
}

4
docker/README.md
View file

@ -37,10 +37,10 @@ Or you can list all tags with:
docker exec -it shlink_container shlink tag:list
```
Or process remaining visits with:
Or locate remaining visits with:
```bash
docker exec -it shlink_container shlink visit:process
docker exec -it shlink_container shlink visit:locate
```
All shlink commands will work the same way.

3
module/CLI/config/dependencies.config.php
View file

@ -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,

3
module/CLI/src/Command/ShortUrl/GetVisitsCommand.php
View file

@ -12,6 +12,7 @@ use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\Model\VisitsParams;
use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface;
use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
@ -76,7 +77,7 @@ class GetVisitsCommand extends AbstractWithDateRangeCommand
$rows = map($paginator->getCurrentItems(), function (Visit $visit) {
$rowData = $visit->jsonSerialize();
$rowData['country'] = $visit->getVisitLocation()->getCountryName();
$rowData['country'] = ($visit->getVisitLocation() ?? new UnknownVisitLocation())->getCountryName();
return select_keys($rowData, ['referer', 'date', 'userAgent', 'country']);
});
ShlinkTable::fromOutput($output)->render(['Referer', 'Date', 'User agent', 'Country'], $rows);

102
module/CLI/src/Command/Visit/LocateVisitsCommand.php
View file

@ -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,15 @@ 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\VisitGeolocationHelperInterface;
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;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;
use Symfony\Component\Lock\LockFactory;
@ -27,11 +29,11 @@ use Throwable;
use function sprintf;
class LocateVisitsCommand extends AbstractLockedCommand
class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocationHelperInterface
{
public const NAME = 'visit:locate';
private VisitServiceInterface $visitService;
private VisitLocatorInterface $visitLocator;
private IpLocationResolverInterface $ipLocationResolver;
private GeolocationDbUpdaterInterface $dbUpdater;
@ -39,13 +41,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 +56,79 @@ 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 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,
'When provided together with --retry, will locate all existing visits, regardless the fact that they '
. 'have already been located.',
);
}
protected function initialize(InputInterface $input, OutputInterface $output): void
{
$this->io = new SymfonyStyle($input, $output);
}
protected function interact(InputInterface $input, OutputInterface $output): void
{
$retry = $input->getOption('retry');
$all = $input->getOption('all');
if ($all && !$retry) {
$this->io->writeln(
'<comment>The <fg=yellow;options=bold>--all</> flag has no effect on its own. You have to provide it '
. 'together with <fg=yellow;options=bold>--retry</>.</comment>',
);
}
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->visitService->locateUnlocatedVisits(
[$this, 'getGeolocationDataForVisit'],
static function (VisitLocation $location) use ($output): void {
if (!$location->isEmpty()) {
$output->writeln(
sprintf(' [<info>Address located at "%s"</info>]', $location->getCountryName()),
);
}
},
);
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());
if ($e instanceof Exception && $this->io->isVerbose()) {
if ($e instanceof Throwable && $this->io->isVerbose()) {
$this->getApplication()->renderThrowable($e, $this->io);
}
@ -87,7 +136,10 @@ class LocateVisitsCommand extends AbstractLockedCommand
}
}
public function getGeolocationDataForVisit(Visit $visit): Location
/**
* @throws IpCannotBeLocatedException
*/
public function geolocateVisit(Visit $visit): Location
{
if (! $visit->hasRemoteAddr()) {
$this->io->writeln(
@ -116,6 +168,14 @@ class LocateVisitsCommand extends AbstractLockedCommand
}
}
public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void
{
$message = ! $visitLocation->isEmpty()
? sprintf(' [<info>Address located in "%s"</info>]', $visitLocation->getCountryName())
: ' [<comment>Address not found</comment>]';
$this->io->writeln($message);
}
private function checkDbUpdate(): void
{
try {

115
module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
View file

@ -15,18 +15,21 @@ 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\VisitGeolocationHelperInterface;
use Shlinkio\Shlink\Core\Visit\VisitLocator;
use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
use Shlinkio\Shlink\IpGeolocation\Model\Location;
use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
use Symfony\Component\Console\Application;
use Symfony\Component\Console\Exception\RuntimeException;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Tester\CommandTester;
use Symfony\Component\Lock;
use function array_shift;
use function sprintf;
use const PHP_EOL;
class LocateVisitsCommandTest extends TestCase
{
private CommandTester $commandTester;
@ -38,7 +41,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);
@ -61,31 +64,53 @@ class LocateVisitsCommandTest extends TestCase
$this->commandTester = new CommandTester($command);
}
/** @test */
public function allPendingVisitsAreProcessed(): void
{
/**
* @test
* @dataProvider provideArgs
*/
public function expectedSetOfVisitsIsProcessedBasedOnArgs(
int $expectedUnlocatedCalls,
int $expectedEmptyCalls,
int $expectedAllCalls,
bool $expectWarningPrint,
array $args
): void {
$visit = new Visit(new ShortUrl(''), new Visitor('', '', '1.2.3.4'));
$location = new VisitLocation(Location::emptyInstance());
$mockMethodBehavior = $this->invokeHelperMethods($visit, $location);
$locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(
function (array $args) use ($visit, $location): void {
$firstCallback = array_shift($args);
$firstCallback($visit);
$secondCallback = array_shift($args);
$secondCallback($location, $visit);
},
$locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will($mockMethodBehavior);
$locateEmptyVisits = $this->visitService->locateVisitsWithEmptyLocation(Argument::cetera())->will(
$mockMethodBehavior,
);
$locateAllVisits = $this->visitService->locateAllVisits(Argument::cetera())->will($mockMethodBehavior);
$resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn(
Location::emptyInstance(),
);
$this->commandTester->execute([]);
$this->commandTester->setInputs(['y']);
$this->commandTester->execute($args);
$output = $this->commandTester->getDisplay();
$this->assertStringContainsString('Processing IP 1.2.3.0', $output);
$locateVisits->shouldHaveBeenCalledOnce();
$resolveIpLocation->shouldHaveBeenCalledOnce();
if ($expectWarningPrint) {
$this->assertStringContainsString('Continue at your own risk', $output);
} else {
$this->assertStringNotContainsString('Continue at your own risk', $output);
}
$locateVisits->shouldHaveBeenCalledTimes($expectedUnlocatedCalls);
$locateEmptyVisits->shouldHaveBeenCalledTimes($expectedEmptyCalls);
$locateAllVisits->shouldHaveBeenCalledTimes($expectedAllCalls);
$resolveIpLocation->shouldHaveBeenCalledTimes(
$expectedUnlocatedCalls + $expectedEmptyCalls + $expectedAllCalls,
);
}
public function provideArgs(): iterable
{
yield 'no args' => [1, 0, 0, false, []];
yield 'retry' => [1, 1, 0, false, ['--retry' => true]];
yield 'all' => [0, 0, 1, true, ['--retry' => true, '--all' => true]];
}
/**
@ -98,13 +123,7 @@ class LocateVisitsCommandTest extends TestCase
$location = new VisitLocation(Location::emptyInstance());
$locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(
function (array $args) use ($visit, $location): void {
$firstCallback = array_shift($args);
$firstCallback($visit);
$secondCallback = array_shift($args);
$secondCallback($location, $visit);
},
$this->invokeHelperMethods($visit, $location),
);
$resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn(
Location::emptyInstance(),
@ -137,13 +156,7 @@ class LocateVisitsCommandTest extends TestCase
$location = new VisitLocation(Location::emptyInstance());
$locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(
function (array $args) use ($visit, $location): void {
$firstCallback = array_shift($args);
$firstCallback($visit);
$secondCallback = array_shift($args);
$secondCallback($location, $visit);
},
$this->invokeHelperMethods($visit, $location),
);
$resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willThrow(WrongIpException::class);
@ -156,6 +169,17 @@ class LocateVisitsCommandTest extends TestCase
$resolveIpLocation->shouldHaveBeenCalledOnce();
}
private function invokeHelperMethods(Visit $visit, VisitLocation $location): callable
{
return function (array $args) use ($visit, $location): void {
/** @var VisitGeolocationHelperInterface $helper */
[$helper] = $args;
$helper->geolocateVisit($visit);
$helper->onVisitLocated($location, $visit);
};
}
/** @test */
public function noActionIsPerformedIfLockIsAcquired(): void
{
@ -212,4 +236,33 @@ class LocateVisitsCommandTest extends TestCase
yield [true, '[Warning] GeoLite2 database update failed. Proceeding with old version.'];
yield [false, 'GeoLite2 database download failed. It is not possible to locate visits.'];
}
/** @test */
public function providingAllFlagOnItsOwnDisplaysNotice(): void
{
$this->commandTester->execute(['--all' => true]);
$output = $this->commandTester->getDisplay();
$this->assertStringContainsString('The --all flag has no effect on its own', $output);
}
/**
* @test
* @dataProvider provideAbortInputs
*/
public function processingAllCancelsCommandIfUserDoesNotActivelyAgreeToConfirmation(array $inputs): void
{
$this->expectException(RuntimeException::class);
$this->expectExceptionMessage('Execution aborted');
$this->commandTester->setInputs($inputs);
$this->commandTester->execute(['--all' => true, '--retry' => true]);
}
public function provideAbortInputs(): iterable
{
yield 'n' => [['n']];
yield 'no' => [['no']];
yield 'default' => [[PHP_EOL]];
}
}

4
module/Core/config/dependencies.config.php
View file

@ -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',

6
module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.VisitLocation.php
View file

@ -44,4 +44,10 @@ return static function (ClassMetadata $metadata, array $emConfig): void {
->columnName('lon')
->nullable(false)
->build();
$builder->createField('isEmpty', Types::BOOLEAN)
->columnName('is_empty')
->option('default', false)
->nullable(false)
->build();
};

5
module/Core/src/Entity/Visit.php
View file

@ -10,7 +10,6 @@ use Shlinkio\Shlink\Common\Entity\AbstractEntity;
use Shlinkio\Shlink\Common\Exception\InvalidArgumentException;
use Shlinkio\Shlink\Common\Util\IpAddress;
use Shlinkio\Shlink\Core\Model\Visitor;
use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation;
use Shlinkio\Shlink\Core\Visit\Model\VisitLocationInterface;
class Visit extends AbstractEntity implements JsonSerializable
@ -60,9 +59,9 @@ class Visit extends AbstractEntity implements JsonSerializable
return $this->shortUrl;
}
public function getVisitLocation(): VisitLocationInterface
public function getVisitLocation(): ?VisitLocationInterface
{
return $this->visitLocation ?? new UnknownVisitLocation();
return $this->visitLocation;
}
public function isLocatable(): bool

28
module/Core/src/Entity/VisitLocation.php
View file

@ -17,6 +17,7 @@ class VisitLocation extends AbstractEntity implements VisitLocationInterface
private float $latitude;
private float $longitude;
private string $timezone;
private bool $isEmpty;
public function __construct(Location $location)
{
@ -43,6 +44,11 @@ class VisitLocation extends AbstractEntity implements VisitLocationInterface
return $this->cityName;
}
public function isEmpty(): bool
{
return $this->isEmpty;
}
private function exchangeLocationInfo(Location $info): void
{
$this->countryCode = $info->countryCode();
@ -52,6 +58,15 @@ class VisitLocation extends AbstractEntity implements VisitLocationInterface
$this->latitude = $info->latitude();
$this->longitude = $info->longitude();
$this->timezone = $info->timeZone();
$this->isEmpty = (
$this->countryCode === '' &&
$this->countryName === '' &&
$this->regionName === '' &&
$this->cityName === '' &&
$this->latitude === 0.0 &&
$this->longitude === 0.0 &&
$this->timezone === ''
);
}
public function jsonSerialize(): array
@ -64,18 +79,7 @@ class VisitLocation extends AbstractEntity implements VisitLocationInterface
'latitude' => $this->latitude,
'longitude' => $this->longitude,
'timezone' => $this->timezone,
'isEmpty' => $this->isEmpty,
];
}
public function isEmpty(): bool
{
return
$this->countryCode === '' &&
$this->countryName === '' &&
$this->regionName === '' &&
$this->cityName === '' &&
$this->latitude === 0.0 &&
$this->longitude === 0.0 &&
$this->timezone === '';
}
}

72
module/Core/src/Repository/VisitRepository.php
View file

@ -12,33 +12,63 @@ use Shlinkio\Shlink\Core\Entity\Visit;
class VisitRepository extends EntityRepository implements VisitRepositoryInterface
{
/**
* 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.
* If you plan to do so, pass the first argument as false in order to disable applying offsets while slicing the
* dataset
*
* @return iterable|Visit[]
*/
public function findUnlocatedVisits(bool $applyOffset = true, int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable
public function findUnlocatedVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable
{
$dql = <<<DQL
SELECT v FROM Shlinkio\Shlink\Core\Entity\Visit AS v WHERE v.visitLocation IS NULL
DQL;
$query = $this->getEntityManager()->createQuery($dql)
->setMaxResults($blockSize);
$remainingVisitsToProcess = $this->count(['visitLocation' => null]);
$offset = 0;
$qb = $this->getEntityManager()->createQueryBuilder();
$qb->select('v')
->from(Visit::class, 'v')
->where($qb->expr()->isNull('v.visitLocation'));
while ($remainingVisitsToProcess > 0) {
$iterator = $query->setFirstResult($applyOffset ? $offset : null)->iterate();
foreach ($iterator as $key => [$value]) {
yield $key => $value;
return $this->findVisitsForQuery($qb, $blockSize);
}
/**
* @return iterable|Visit[]
*/
public function findVisitsWithEmptyLocation(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable
{
$qb = $this->getEntityManager()->createQueryBuilder();
$qb->select('v')
->from(Visit::class, 'v')
->join('v.visitLocation', 'vl')
->where($qb->expr()->isNotNull('v.visitLocation'))
->andWhere($qb->expr()->eq('vl.isEmpty', ':isEmpty'))
->setParameter('isEmpty', true);
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)
->orderBy('v.id', 'ASC');
$lastId = '0';
do {
$qb = (clone $originalQueryBuilder)->andWhere($qb->expr()->gt('v.id', $lastId));
$iterator = $qb->getQuery()->iterate();
$resultsFound = false;
/** @var Visit $visit */
foreach ($iterator as $key => [$visit]) {
$resultsFound = true;
yield $key => $visit;
}
$remainingVisitsToProcess -= $blockSize;
$offset += $blockSize;
}
// As the query is ordered by ID, we can take the last one every time in order to exclude the whole list
$lastId = isset($visit) ? $visit->getId() : $lastId;
} while ($resultsFound);
}
/**

18
module/Core/src/Repository/VisitRepositoryInterface.php
View file

@ -13,15 +13,19 @@ interface VisitRepositoryInterface extends ObjectRepository
public const DEFAULT_BLOCK_SIZE = 10000;
/**
* 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.
* If you plan to do so, pass the first argument as false in order to disable applying offsets while slicing the
* dataset
*
* @return iterable|Visit[]
*/
public function findUnlocatedVisits(bool $applyOffset = true, int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable;
public function findUnlocatedVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable;
/**
* @return iterable|Visit[]
*/
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[]

70
module/Core/src/Service/VisitService.php
View file

@ -1,70 +0,0 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Service;
use Doctrine\ORM\EntityManagerInterface;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
use Shlinkio\Shlink\Core\Repository\VisitRepository;
use Shlinkio\Shlink\IpGeolocation\Model\Location;
class VisitService implements VisitServiceInterface
{
private EntityManagerInterface $em;
public function __construct(EntityManagerInterface $em)
{
$this->em = $em;
}
public function locateUnlocatedVisits(callable $geolocateVisit, ?callable $notifyVisitWithLocation = null): void
{
/** @var VisitRepository $repo */
$repo = $this->em->getRepository(Visit::class);
$results = $repo->findUnlocatedVisits(false);
$count = 0;
$persistBlock = 200;
foreach ($results as $visit) {
$count++;
try {
/** @var Location $location */
$location = $geolocateVisit($visit);
} catch (IpCannotBeLocatedException $e) {
if (! $e->isNonLocatableAddress()) {
// Skip if the visit's IP could not be located because of an error
continue;
}
// If the IP address is non-locatable, locate it as empty to prevent next processes to pick it again
$location = Location::emptyInstance();
}
$location = new VisitLocation($location);
$this->locateVisit($visit, $location, $notifyVisitWithLocation);
// Flush and clear after X iterations
if ($count % $persistBlock === 0) {
$this->em->flush();
$this->em->clear();
}
}
$this->em->flush();
$this->em->clear();
}
private function locateVisit(Visit $visit, VisitLocation $location, ?callable $notifyVisitWithLocation): void
{
$visit->locate($location);
$this->em->persist($visit);
if ($notifyVisitWithLocation !== null) {
$notifyVisitWithLocation($location, $visit);
}
}
}

10
module/Core/src/Service/VisitServiceInterface.php
View file

@ -1,10 +0,0 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Service;
interface VisitServiceInterface
{
public function locateUnlocatedVisits(callable $geolocateVisit, ?callable $notifyVisitWithLocation = null): void;
}

20
module/Core/src/Visit/VisitGeolocationHelperInterface.php Normal file
View file

@ -0,0 +1,20 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Visit;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
use Shlinkio\Shlink\IpGeolocation\Model\Location;
interface VisitGeolocationHelperInterface
{
/**
* @throws IpCannotBeLocatedException
*/
public function geolocateVisit(Visit $visit): Location;
public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void;
}

94
module/Core/src/Visit/VisitLocator.php Normal file
View file

@ -0,0 +1,94 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Visit;
use Doctrine\ORM\EntityManagerInterface;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface;
use Shlinkio\Shlink\IpGeolocation\Model\Location;
class VisitLocator implements VisitLocatorInterface
{
private EntityManagerInterface $em;
private VisitRepositoryInterface $repo;
public function __construct(EntityManagerInterface $em)
{
$this->em = $em;
/** @var VisitRepositoryInterface $repo */
$repo = $em->getRepository(Visit::class);
$this->repo = $repo;
}
public function locateUnlocatedVisits(VisitGeolocationHelperInterface $helper): void
{
$this->locateVisits($this->repo->findUnlocatedVisits(), $helper);
}
public function locateVisitsWithEmptyLocation(VisitGeolocationHelperInterface $helper): void
{
$this->locateVisits($this->repo->findVisitsWithEmptyLocation(), $helper);
}
public function locateAllVisits(VisitGeolocationHelperInterface $helper): void
{
$this->locateVisits($this->repo->findAllVisits(), $helper);
}
/**
* @param iterable|Visit[] $results
*/
private function locateVisits(iterable $results, VisitGeolocationHelperInterface $helper): void
{
$count = 0;
$persistBlock = 200;
foreach ($results as $visit) {
$count++;
try {
$location = $helper->geolocateVisit($visit);
} catch (IpCannotBeLocatedException $e) {
if (! $e->isNonLocatableAddress()) {
// Skip if the visit's IP could not be located because of an error
continue;
}
// If the IP address is non-locatable, locate it as empty to prevent next processes to pick it again
$location = Location::emptyInstance();
}
$location = new VisitLocation($location);
$this->locateVisit($visit, $location, $helper);
// Flush and clear after X iterations
if ($count % $persistBlock === 0) {
$this->em->flush();
$this->em->clear();
}
}
$this->em->flush();
$this->em->clear();
}
private function locateVisit(Visit $visit, VisitLocation $location, VisitGeolocationHelperInterface $helper): void
{
$prevLocation = $visit->getVisitLocation();
$visit->locate($location);
$this->em->persist($visit);
// In order to avoid leaving orphan locations, remove the previous one
if ($prevLocation !== null) {
$this->em->remove($prevLocation);
}
$helper->onVisitLocated($location, $visit);
}
}

14
module/Core/src/Visit/VisitLocatorInterface.php Normal file
View file

@ -0,0 +1,14 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Visit;
interface VisitLocatorInterface
{
public function locateUnlocatedVisits(VisitGeolocationHelperInterface $helper): void;
public function locateVisitsWithEmptyLocation(VisitGeolocationHelperInterface $helper): void;
public function locateAllVisits(VisitGeolocationHelperInterface $helper): void;
}

28
module/Core/test-db/Repository/VisitRepositoryTest.php
View file

@ -40,15 +40,23 @@ class VisitRepositoryTest extends DatabaseTestCase
* @test
* @dataProvider provideBlockSize
*/
public function findUnlocatedVisitsReturnsProperVisits(int $blockSize): void
public function findVisitsReturnsProperVisits(int $blockSize): void
{
$shortUrl = new ShortUrl('');
$this->getEntityManager()->persist($shortUrl);
$countIterable = function (iterable $results): int {
$resultsCount = 0;
foreach ($results as $value) {
$resultsCount++;
}
return $resultsCount;
};
for ($i = 0; $i < 6; $i++) {
$visit = new Visit($shortUrl, Visitor::emptyInstance());
if ($i % 2 === 0) {
if ($i >= 2) {
$location = new VisitLocation(Location::emptyInstance());
$this->getEntityManager()->persist($location);
$visit->locate($location);
@ -58,18 +66,20 @@ class VisitRepositoryTest extends DatabaseTestCase
}
$this->getEntityManager()->flush();
$resultsCount = 0;
$results = $this->repo->findUnlocatedVisits(true, $blockSize);
foreach ($results as $value) {
$resultsCount++;
}
$withEmptyLocation = $this->repo->findVisitsWithEmptyLocation($blockSize);
$unlocated = $this->repo->findUnlocatedVisits($blockSize);
$all = $this->repo->findAllVisits($blockSize);
$this->assertEquals(3, $resultsCount);
// 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
{
return map(range(1, 5), fn (int $value) => [$value]);
return map(range(1, 10), fn (int $value) => [$value]);
}
/** @test */

3
module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php
View file

@ -20,7 +20,6 @@ use Shlinkio\Shlink\Core\EventDispatcher\LocateShortUrlVisit;
use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited;
use Shlinkio\Shlink\Core\EventDispatcher\VisitLocated;
use Shlinkio\Shlink\Core\Model\Visitor;
use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation;
use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
use Shlinkio\Shlink\IpGeolocation\Model\Location;
use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
@ -218,7 +217,7 @@ class LocateShortUrlVisitTest extends TestCase
($this->locateVisit)($event);
$this->assertEquals($visit->getVisitLocation(), new UnknownVisitLocation());
$this->assertNull($visit->getVisitLocation());
$findVisit->shouldHaveBeenCalledOnce();
$flush->shouldNotHaveBeenCalled();
$resolveIp->shouldNotHaveBeenCalled();

112
module/Core/test/Service/VisitServiceTest.php
View file

@ -1,112 +0,0 @@
<?php
declare(strict_types=1);
namespace ShlinkioTest\Shlink\Core\Service;
use Doctrine\ORM\EntityManager;
use Exception;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\Prophecy\ObjectProphecy;
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\Repository\VisitRepository;
use Shlinkio\Shlink\Core\Service\VisitService;
use Shlinkio\Shlink\IpGeolocation\Model\Location;
use function array_shift;
use function count;
use function floor;
use function func_get_args;
use function Functional\map;
use function range;
use function sprintf;
class VisitServiceTest extends TestCase
{
private VisitService $visitService;
private ObjectProphecy $em;
public function setUp(): void
{
$this->em = $this->prophesize(EntityManager::class);
$this->visitService = new VisitService($this->em->reveal());
}
/** @test */
public function locateVisitsIteratesAndLocatesUnlocatedVisits(): void
{
$unlocatedVisits = map(
range(1, 200),
fn (int $i) => new Visit(new ShortUrl(sprintf('short_code_%s', $i)), Visitor::emptyInstance()),
);
$repo = $this->prophesize(VisitRepository::class);
$findUnlocatedVisits = $repo->findUnlocatedVisits(false)->willReturn($unlocatedVisits);
$getRepo = $this->em->getRepository(Visit::class)->willReturn($repo->reveal());
$persist = $this->em->persist(Argument::type(Visit::class))->will(function (): void {
});
$flush = $this->em->flush()->will(function (): void {
});
$clear = $this->em->clear()->will(function (): void {
});
$this->visitService->locateUnlocatedVisits(fn () => Location::emptyInstance(), function (): void {
$args = func_get_args();
$this->assertInstanceOf(VisitLocation::class, array_shift($args));
$this->assertInstanceOf(Visit::class, array_shift($args));
});
$findUnlocatedVisits->shouldHaveBeenCalledOnce();
$getRepo->shouldHaveBeenCalledOnce();
$persist->shouldHaveBeenCalledTimes(count($unlocatedVisits));
$flush->shouldHaveBeenCalledTimes(floor(count($unlocatedVisits) / 200) + 1);
$clear->shouldHaveBeenCalledTimes(floor(count($unlocatedVisits) / 200) + 1);
}
/**
* @test
* @dataProvider provideIsNonLocatableAddress
*/
public function visitsWhichCannotBeLocatedAreIgnoredOrLocatedAsEmpty(bool $isNonLocatableAddress): void
{
$unlocatedVisits = [
new Visit(new ShortUrl('foo'), Visitor::emptyInstance()),
];
$repo = $this->prophesize(VisitRepository::class);
$findUnlocatedVisits = $repo->findUnlocatedVisits(false)->willReturn($unlocatedVisits);
$getRepo = $this->em->getRepository(Visit::class)->willReturn($repo->reveal());
$persist = $this->em->persist(Argument::type(Visit::class))->will(function (): void {
});
$flush = $this->em->flush()->will(function (): void {
});
$clear = $this->em->clear()->will(function (): void {
});
$this->visitService->locateUnlocatedVisits(function () use ($isNonLocatableAddress): void {
throw $isNonLocatableAddress
? new IpCannotBeLocatedException('Cannot be located')
: IpCannotBeLocatedException::forError(new Exception(''));
});
$findUnlocatedVisits->shouldHaveBeenCalledOnce();
$getRepo->shouldHaveBeenCalledOnce();
$persist->shouldHaveBeenCalledTimes($isNonLocatableAddress ? 1 : 0);
$flush->shouldHaveBeenCalledOnce();
$clear->shouldHaveBeenCalledOnce();
}
public function provideIsNonLocatableAddress(): iterable
{
yield 'The address is locatable' => [false];
yield 'The address is non-locatable' => [true];
}
}

169
module/Core/test/Visit/VisitLocatorTest.php Normal file
View file

@ -0,0 +1,169 @@
<?php
declare(strict_types=1);
namespace ShlinkioTest\Shlink\Core\Visit;
use Doctrine\ORM\EntityManager;
use Exception;
use PHPUnit\Framework\Assert;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\Prophecy\MethodProphecy;
use Prophecy\Prophecy\ObjectProphecy;
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\Repository\VisitRepositoryInterface;
use Shlinkio\Shlink\Core\Visit\VisitGeolocationHelperInterface;
use Shlinkio\Shlink\Core\Visit\VisitLocator;
use Shlinkio\Shlink\IpGeolocation\Model\Location;
use function array_shift;
use function count;
use function floor;
use function func_get_args;
use function Functional\map;
use function range;
use function sprintf;
class VisitLocatorTest extends TestCase
{
private VisitLocator $visitService;
private ObjectProphecy $em;
private ObjectProphecy $repo;
public function setUp(): void
{
$this->em = $this->prophesize(EntityManager::class);
$this->repo = $this->prophesize(VisitRepositoryInterface::class);
$this->em->getRepository(Visit::class)->willReturn($this->repo->reveal());
$this->visitService = new VisitLocator($this->em->reveal());
}
/**
* @test
* @dataProvider provideMethodNames
*/
public function locateVisitsIteratesAndLocatesExpectedVisits(
string $serviceMethodName,
string $expectedRepoMethodName
): void {
$unlocatedVisits = map(
range(1, 200),
fn (int $i) => new Visit(new ShortUrl(sprintf('short_code_%s', $i)), Visitor::emptyInstance()),
);
$findVisits = $this->mockRepoMethod($expectedRepoMethodName)->willReturn($unlocatedVisits);
$persist = $this->em->persist(Argument::type(Visit::class))->will(function (): void {
});
$flush = $this->em->flush()->will(function (): void {
});
$clear = $this->em->clear()->will(function (): void {
});
$this->visitService->{$serviceMethodName}(new class implements VisitGeolocationHelperInterface {
public function geolocateVisit(Visit $visit): Location
{
return Location::emptyInstance();
}
public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void
{
$args = func_get_args();
Assert::assertInstanceOf(VisitLocation::class, array_shift($args));
Assert::assertInstanceOf(Visit::class, array_shift($args));
}
});
$findVisits->shouldHaveBeenCalledOnce();
$persist->shouldHaveBeenCalledTimes(count($unlocatedVisits));
$flush->shouldHaveBeenCalledTimes(floor(count($unlocatedVisits) / 200) + 1);
$clear->shouldHaveBeenCalledTimes(floor(count($unlocatedVisits) / 200) + 1);
}
public function provideMethodNames(): iterable
{
yield 'locateUnlocatedVisits' => ['locateUnlocatedVisits', 'findUnlocatedVisits'];
yield 'locateVisitsWithEmptyLocation' => ['locateVisitsWithEmptyLocation', 'findVisitsWithEmptyLocation'];
yield 'locateAllVisits' => ['locateAllVisits', 'findAllVisits'];
}
/**
* @test
* @dataProvider provideIsNonLocatableAddress
*/
public function visitsWhichCannotBeLocatedAreIgnoredOrLocatedAsEmpty(
string $serviceMethodName,
string $expectedRepoMethodName,
bool $isNonLocatableAddress
): void {
$unlocatedVisits = [
new Visit(new ShortUrl('foo'), Visitor::emptyInstance()),
];
$findVisits = $this->mockRepoMethod($expectedRepoMethodName)->willReturn($unlocatedVisits);
$persist = $this->em->persist(Argument::type(Visit::class))->will(function (): void {
});
$flush = $this->em->flush()->will(function (): void {
});
$clear = $this->em->clear()->will(function (): void {
});
$this->visitService->{$serviceMethodName}(
new class ($isNonLocatableAddress) implements VisitGeolocationHelperInterface {
private bool $isNonLocatableAddress;
public function __construct(bool $isNonLocatableAddress)
{
$this->isNonLocatableAddress = $isNonLocatableAddress;
}
public function geolocateVisit(Visit $visit): Location
{
throw $this->isNonLocatableAddress
? new IpCannotBeLocatedException('Cannot be located')
: IpCannotBeLocatedException::forError(new Exception(''));
}
public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void
{
}
},
);
$findVisits->shouldHaveBeenCalledOnce();
$persist->shouldHaveBeenCalledTimes($isNonLocatableAddress ? 1 : 0);
$flush->shouldHaveBeenCalledOnce();
$clear->shouldHaveBeenCalledOnce();
}
public function provideIsNonLocatableAddress(): iterable
{
yield 'locateUnlocatedVisits - locatable address' => ['locateUnlocatedVisits', 'findUnlocatedVisits', false];
yield 'locateUnlocatedVisits - non-locatable address' => ['locateUnlocatedVisits', 'findUnlocatedVisits', true];
yield 'locateVisitsWithEmptyLocation - locatable address' => [
'locateVisitsWithEmptyLocation',
'findVisitsWithEmptyLocation',
false,
];
yield 'locateVisitsWithEmptyLocation - non-locatable address' => [
'locateVisitsWithEmptyLocation',
'findVisitsWithEmptyLocation',
true,
];
yield 'locateAllVisits - locatable address' => ['locateAllVisits', 'findAllVisits', false];
yield 'locateAllVisits - non-locatable address' => ['locateAllVisits', 'findAllVisits', true];
}
private function mockRepoMethod(string $methodName): MethodProphecy
{
return (new MethodProphecy($this->repo, $methodName, new Argument\ArgumentsWildcard([])));
}
}