From 2c3cbe7146b8e9f3ddad55c245dbae9f3983c327 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Nov 2018 12:04:42 +0100 Subject: [PATCH 01/19] Installed geoip2 and added to docs --- README.md | 8 ++++++++ composer.json | 1 + 2 files changed, 9 insertions(+) diff --git a/README.md b/README.md index dbb18ce7..7b4d7bee 100644 --- a/README.md +++ b/README.md @@ -104,6 +104,12 @@ Those tasks can be performed using shlink's CLI, so it should be easy to schedul If you don't run this command regularly, the stats will say all visits come from *unknown* locations. +* Update IP geolocation database: `/path/to/shlink/bin/cli visit:update-db` + + When shlink is installed it downloads a fresh [GeoLite2](https://dev.maxmind.com/geoip/geoip2/geolite2/) db file. Running this command will update this file. + + The file does not change very frequently, so it shouldn't be needed to run this command more than once per month. + * Generate website previews: `/path/to/shlink/bin/cli short-url:process-previews` Running this will improve the performance of the `doma.in/abc123/preview` URLs, which return a preview of the site. @@ -186,3 +192,5 @@ Available commands: visit visit:process Processes visits where location is not set yet ``` + +> This product includes GeoLite2 data created by MaxMind, available from [https://www.maxmind.com](https://www.maxmind.com) diff --git a/composer.json b/composer.json index 9064973d..86b52828 100644 --- a/composer.json +++ b/composer.json @@ -24,6 +24,7 @@ "doctrine/orm": "^2.5", "endroid/qr-code": "^1.7", "firebase/php-jwt": "^4.0", + "geoip2/geoip2": "^2.9", "guzzlehttp/guzzle": "^6.2", "lstrojny/functional-php": "^1.8", "mikehaertl/phpwkhtmltopdf": "^2.2", From bbe85cde316fe15c1144f9a64b52723afd50ecd5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Nov 2018 12:40:40 +0100 Subject: [PATCH 02/19] Migrated to GeoLite2 for IP location resolution --- .gitignore | 1 + config/autoload/geolite2.global.php | 10 +++ module/CLI/config/dependencies.config.php | 4 +- .../Command/Visit/ProcessVisitsCommand.php | 11 ++- module/Common/config/dependencies.config.php | 7 ++ .../src/Service/GeoLite2LocationResolver.php | 73 +++++++++++++++++++ 6 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 config/autoload/geolite2.global.php create mode 100644 module/Common/src/Service/GeoLite2LocationResolver.php diff --git a/.gitignore b/.gitignore index 9426e5f3..bf3d5ae0 100644 --- a/.gitignore +++ b/.gitignore @@ -5,5 +5,6 @@ composer.phar vendor/ .env data/database.sqlite +data/GeoLite2-City.mmdb docs/swagger-ui docker-compose.override.yml diff --git a/config/autoload/geolite2.global.php b/config/autoload/geolite2.global.php new file mode 100644 index 00000000..0536a0be --- /dev/null +++ b/config/autoload/geolite2.global.php @@ -0,0 +1,10 @@ + [ + 'db_location' => __DIR__ . '/../../data/GeoLite2-City.mmdb', + ], + +]; diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 371c9175..2976e5b0 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -3,7 +3,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI; -use Shlinkio\Shlink\Common\Service\IpApiLocationResolver; +use Shlinkio\Shlink\Common\Service\IpLocationResolverInterface; use Shlinkio\Shlink\Common\Service\PreviewGenerator; use Shlinkio\Shlink\Core\Service; use Shlinkio\Shlink\Rest\Service\ApiKeyService; @@ -65,7 +65,7 @@ return [ Command\Visit\ProcessVisitsCommand::class => [ Service\VisitService::class, - IpApiLocationResolver::class, + IpLocationResolverInterface::class, 'translator', ], diff --git a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php index f1f70e35..3d36695c 100644 --- a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php +++ b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php @@ -68,10 +68,10 @@ class ProcessVisitsCommand extends Command } $ipAddr = $visit->getRemoteAddr(); - $io->write(sprintf('%s %s', $this->translator->translate('Processing IP'), $ipAddr)); + $io->write(sprintf('%s %s', $this->translator->translate('Processing IP'), $ipAddr)); if ($ipAddr === IpAddress::LOCALHOST) { $io->writeln( - sprintf(' (%s)', $this->translator->translate('Ignored localhost address')) + sprintf(' [%s]', $this->translator->translate('Ignored localhost address')) ); continue; } @@ -85,12 +85,15 @@ class ProcessVisitsCommand extends Command $this->visitService->saveVisit($visit); $io->writeln(sprintf( - ' (' . $this->translator->translate('Address located at "%s"') . ')', + ' [' . $this->translator->translate('Address located at "%s"') . ']', $location->getCityName() )); } catch (WrongIpException $e) { $io->writeln( - sprintf(' %s', $this->translator->translate('An error occurred while locating IP')) + sprintf( + ' [%s]', + $this->translator->translate('An error occurred while locating IP. Skipped') + ) ); if ($io->isVerbose()) { $this->getApplication()->renderException($e, $output); diff --git a/module/Common/config/dependencies.config.php b/module/Common/config/dependencies.config.php index 6278d63b..d8df79aa 100644 --- a/module/Common/config/dependencies.config.php +++ b/module/Common/config/dependencies.config.php @@ -5,6 +5,7 @@ namespace Shlinkio\Shlink\Common; use Doctrine\Common\Cache\Cache; use Doctrine\ORM\EntityManager; +use GeoIp2\Database\Reader; use GuzzleHttp\Client as GuzzleClient; use Monolog\Logger; use Psr\Log\LoggerInterface; @@ -23,6 +24,7 @@ return [ Cache::class => Factory\CacheFactory::class, 'Logger_Shlink' => Factory\LoggerFactory::class, Filesystem::class => InvokableFactory::class, + Reader::class => ConfigAbstractFactory::class, Translator::class => Factory\TranslatorFactory::class, Template\Extension\TranslatorExtension::class => ConfigAbstractFactory::class, @@ -33,6 +35,7 @@ return [ Image\ImageBuilder::class => Image\ImageBuilderFactory::class, Service\IpApiLocationResolver::class => ConfigAbstractFactory::class, + Service\GeoLite2LocationResolver::class => ConfigAbstractFactory::class, Service\PreviewGenerator::class => ConfigAbstractFactory::class, ], 'aliases' => [ @@ -42,6 +45,7 @@ return [ 'logger' => LoggerInterface::class, Logger::class => 'Logger_Shlink', LoggerInterface::class => 'Logger_Shlink', + Service\IpLocationResolverInterface::class => Service\GeoLite2LocationResolver::class, ], 'abstract_factories' => [ Factory\DottedAccessConfigAbstractFactory::class, @@ -49,9 +53,12 @@ return [ ], ConfigAbstractFactory::class => [ + Reader::class => ['config.geolite2.db_location'], + Template\Extension\TranslatorExtension::class => ['translator'], Middleware\LocaleMiddleware::class => ['translator'], Service\IpApiLocationResolver::class => ['httpClient'], + Service\GeoLite2LocationResolver::class => [Reader::class], Service\PreviewGenerator::class => [ Image\ImageBuilder::class, Filesystem::class, diff --git a/module/Common/src/Service/GeoLite2LocationResolver.php b/module/Common/src/Service/GeoLite2LocationResolver.php new file mode 100644 index 00000000..104a1ab3 --- /dev/null +++ b/module/Common/src/Service/GeoLite2LocationResolver.php @@ -0,0 +1,73 @@ +geoLiteDbReader = $geoLiteDbReader; + } + + /** + * @param string $ipAddress + * @return array + * @throws WrongIpException + */ + public function resolveIpLocation(string $ipAddress): array + { + try { + $city = $this->geoLiteDbReader->city($ipAddress); + return $this->mapFields($city); + } catch (AddressNotFoundException $e) { + throw WrongIpException::fromIpAddress($ipAddress, $e); + } catch (InvalidDatabaseException $e) { + throw new WrongIpException('Provided GeoLite2 db file is invalid', 0, $e); + } + } + + private function mapFields(City $city): array + { + return [ + 'country_code' => $city->country->isoCode ?? '', + 'country_name' => $city->country->name ?? '', + 'region_name' => $city->mostSpecificSubdivision->name ?? '', + 'city' => $city->city->name ?? '', + 'latitude' => (string) $city->location->latitude, // FIXME Cast to string for BC compatibility + 'longitude' => (string) $city->location->longitude, // FIXME Cast to string for BC compatibility + 'time_zone' => $city->location->timeZone ?? '', + ]; + } + + /** + * Returns the interval in seconds that needs to be waited when the API limit is reached + * + * @return int + */ + public function getApiInterval(): int + { + return 0; + } + + /** + * Returns the limit of requests that can be performed to the API in a specific interval, or null if no limit exists + * + * @return int|null + */ + public function getApiLimit(): ?int + { + return null; + } +} From b530cf446185bbdb68dc79bd1070a9c81a1555b2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Nov 2018 12:44:57 +0100 Subject: [PATCH 03/19] Created new namespace for IP geolocation elements --- module/CLI/config/dependencies.config.php | 2 +- module/CLI/src/Command/Visit/ProcessVisitsCommand.php | 2 +- .../test/Command/Visit/ProcessVisitsCommandTest.php | 2 +- module/Common/config/dependencies.config.php | 10 +++++----- .../GeoLite2LocationResolver.php | 2 +- .../IpApiLocationResolver.php | 2 +- .../IpLocationResolverInterface.php | 2 +- .../IpApiLocationResolverTest.php | 4 ++-- 8 files changed, 13 insertions(+), 13 deletions(-) rename module/Common/src/{Service => IpGeolocation}/GeoLite2LocationResolver.php (97%) rename module/Common/src/{Service => IpGeolocation}/IpApiLocationResolver.php (97%) rename module/Common/src/{Service => IpGeolocation}/IpLocationResolverInterface.php (93%) rename module/Common/test/{Service => IpGeolocation}/IpApiLocationResolverTest.php (95%) diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 2976e5b0..b226933c 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -3,7 +3,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI; -use Shlinkio\Shlink\Common\Service\IpLocationResolverInterface; +use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface; use Shlinkio\Shlink\Common\Service\PreviewGenerator; use Shlinkio\Shlink\Core\Service; use Shlinkio\Shlink\Rest\Service\ApiKeyService; diff --git a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php index 3d36695c..799b51e9 100644 --- a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php +++ b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; use Shlinkio\Shlink\Common\Exception\WrongIpException; -use Shlinkio\Shlink\Common\Service\IpLocationResolverInterface; +use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface; use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Entity\VisitLocation; use Shlinkio\Shlink\Core\Service\VisitServiceInterface; diff --git a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php index 361e23d0..e2a36ba6 100644 --- a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php @@ -7,7 +7,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Visit\ProcessVisitsCommand; -use Shlinkio\Shlink\Common\Service\IpApiLocationResolver; +use Shlinkio\Shlink\Common\IpGeolocation\IpApiLocationResolver; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\Visitor; diff --git a/module/Common/config/dependencies.config.php b/module/Common/config/dependencies.config.php index d8df79aa..4b63f602 100644 --- a/module/Common/config/dependencies.config.php +++ b/module/Common/config/dependencies.config.php @@ -34,8 +34,8 @@ return [ Image\ImageBuilder::class => Image\ImageBuilderFactory::class, - Service\IpApiLocationResolver::class => ConfigAbstractFactory::class, - Service\GeoLite2LocationResolver::class => ConfigAbstractFactory::class, + IpGeolocation\IpApiLocationResolver::class => ConfigAbstractFactory::class, + IpGeolocation\GeoLite2LocationResolver::class => ConfigAbstractFactory::class, Service\PreviewGenerator::class => ConfigAbstractFactory::class, ], 'aliases' => [ @@ -45,7 +45,7 @@ return [ 'logger' => LoggerInterface::class, Logger::class => 'Logger_Shlink', LoggerInterface::class => 'Logger_Shlink', - Service\IpLocationResolverInterface::class => Service\GeoLite2LocationResolver::class, + IpGeolocation\IpLocationResolverInterface::class => IpGeolocation\GeoLite2LocationResolver::class, ], 'abstract_factories' => [ Factory\DottedAccessConfigAbstractFactory::class, @@ -57,8 +57,8 @@ return [ Template\Extension\TranslatorExtension::class => ['translator'], Middleware\LocaleMiddleware::class => ['translator'], - Service\IpApiLocationResolver::class => ['httpClient'], - Service\GeoLite2LocationResolver::class => [Reader::class], + IpGeolocation\IpApiLocationResolver::class => ['httpClient'], + IpGeolocation\GeoLite2LocationResolver::class => [Reader::class], Service\PreviewGenerator::class => [ Image\ImageBuilder::class, Filesystem::class, diff --git a/module/Common/src/Service/GeoLite2LocationResolver.php b/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php similarity index 97% rename from module/Common/src/Service/GeoLite2LocationResolver.php rename to module/Common/src/IpGeolocation/GeoLite2LocationResolver.php index 104a1ab3..68becdb6 100644 --- a/module/Common/src/Service/GeoLite2LocationResolver.php +++ b/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php @@ -1,7 +1,7 @@ Date: Sun, 11 Nov 2018 13:04:41 +0100 Subject: [PATCH 04/19] Removed the concept of API limits in IP location resolvers --- .../Command/Visit/ProcessVisitsCommand.php | 13 ------ .../Visit/ProcessVisitsCommandTest.php | 41 ------------------- .../GeoLite2LocationResolver.php | 20 --------- .../IpGeolocation/IpApiLocationResolver.php | 20 --------- .../IpLocationResolverInterface.php | 14 ------- 5 files changed, 108 deletions(-) diff --git a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php index 799b51e9..305042e7 100644 --- a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php +++ b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php @@ -13,7 +13,6 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; use Zend\I18n\Translator\TranslatorInterface; -use function sleep; use function sprintf; class ProcessVisitsCommand extends Command @@ -57,7 +56,6 @@ class ProcessVisitsCommand extends Command $io = new SymfonyStyle($input, $output); $visits = $this->visitService->getUnlocatedVisits(); - $count = 0; foreach ($visits as $visit) { if (! $visit->hasRemoteAddr()) { $io->writeln( @@ -76,7 +74,6 @@ class ProcessVisitsCommand extends Command continue; } - $count++; try { $result = $this->ipLocationResolver->resolveIpLocation($ipAddr); @@ -99,16 +96,6 @@ class ProcessVisitsCommand extends Command $this->getApplication()->renderException($e, $output); } } - - if ($count === $this->ipLocationResolver->getApiLimit()) { - $count = 0; - $seconds = $this->ipLocationResolver->getApiInterval(); - $io->note(sprintf( - $this->translator->translate('IP location resolver limit reached. Waiting %s seconds...'), - $seconds - )); - sleep($seconds); - } } $io->success($this->translator->translate('Finished processing all IPs')); diff --git a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php index e2a36ba6..a92241f0 100644 --- a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php @@ -17,7 +17,6 @@ use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Tester\CommandTester; use Zend\I18n\Translator\Translator; use function count; -use function round; class ProcessVisitsCommandTest extends TestCase { @@ -38,7 +37,6 @@ class ProcessVisitsCommandTest extends TestCase { $this->visitService = $this->prophesize(VisitService::class); $this->ipResolver = $this->prophesize(IpApiLocationResolver::class); - $this->ipResolver->getApiLimit()->willReturn(10000000000); $command = new ProcessVisitsCommand( $this->visitService->reveal(), @@ -109,43 +107,4 @@ class ProcessVisitsCommandTest extends TestCase $this->assertContains('Ignored localhost address', $output); $this->assertContains('Ignored visit with no IP address', $output); } - - /** - * @test - */ - public function sleepsEveryTimeTheApiLimitIsReached() - { - $shortUrl = new ShortUrl(''); - - $visits = [ - new Visit($shortUrl, new Visitor('', '', '1.2.3.4')), - new Visit($shortUrl, new Visitor('', '', '4.3.2.1')), - new Visit($shortUrl, new Visitor('', '', '12.34.56.78')), - new Visit($shortUrl, new Visitor('', '', '1.2.3.4')), - new Visit($shortUrl, new Visitor('', '', '4.3.2.1')), - new Visit($shortUrl, new Visitor('', '', '12.34.56.78')), - new Visit($shortUrl, new Visitor('', '', '1.2.3.4')), - new Visit($shortUrl, new Visitor('', '', '4.3.2.1')), - new Visit($shortUrl, new Visitor('', '', '12.34.56.78')), - new Visit($shortUrl, new Visitor('', '', '4.3.2.1')), - ]; - $apiLimit = 3; - - $this->visitService->getUnlocatedVisits()->willReturn($visits); - $this->visitService->saveVisit(Argument::any())->will(function () { - }); - - $getApiLimit = $this->ipResolver->getApiLimit()->willReturn($apiLimit); - $getApiInterval = $this->ipResolver->getApiInterval()->willReturn(0); - $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]) - ->shouldBeCalledTimes(count($visits)); - - $this->commandTester->execute([ - 'command' => 'visit:process', - ]); - - $getApiLimit->shouldHaveBeenCalledTimes(count($visits)); - $getApiInterval->shouldHaveBeenCalledTimes(round(count($visits) / $apiLimit)); - $resolveIpLocation->shouldHaveBeenCalledTimes(count($visits)); - } } diff --git a/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php b/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php index 68becdb6..60023ee3 100644 --- a/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php +++ b/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php @@ -50,24 +50,4 @@ class GeoLite2LocationResolver implements IpLocationResolverInterface 'time_zone' => $city->location->timeZone ?? '', ]; } - - /** - * Returns the interval in seconds that needs to be waited when the API limit is reached - * - * @return int - */ - public function getApiInterval(): int - { - return 0; - } - - /** - * Returns the limit of requests that can be performed to the API in a specific interval, or null if no limit exists - * - * @return int|null - */ - public function getApiLimit(): ?int - { - return null; - } } diff --git a/module/Common/src/IpGeolocation/IpApiLocationResolver.php b/module/Common/src/IpGeolocation/IpApiLocationResolver.php index 6bf426bf..fe6680a8 100644 --- a/module/Common/src/IpGeolocation/IpApiLocationResolver.php +++ b/module/Common/src/IpGeolocation/IpApiLocationResolver.php @@ -53,24 +53,4 @@ class IpApiLocationResolver implements IpLocationResolverInterface 'time_zone' => $entry['timezone'] ?? '', ]; } - - /** - * Returns the interval in seconds that needs to be waited when the API limit is reached - * - * @return int - */ - public function getApiInterval(): int - { - return 65; // ip-api interval is 1 minute. Return 5 extra seconds just in case - } - - /** - * Returns the limit of requests that can be performed to the API in a specific interval, or null if no limit exists - * - * @return int|null - */ - public function getApiLimit(): ?int - { - return 145; // ip-api limit is 150 requests per minute. Leave 5 less requests just in case - } } diff --git a/module/Common/src/IpGeolocation/IpLocationResolverInterface.php b/module/Common/src/IpGeolocation/IpLocationResolverInterface.php index 47375e45..f9e41572 100644 --- a/module/Common/src/IpGeolocation/IpLocationResolverInterface.php +++ b/module/Common/src/IpGeolocation/IpLocationResolverInterface.php @@ -13,18 +13,4 @@ interface IpLocationResolverInterface * @throws WrongIpException */ public function resolveIpLocation(string $ipAddress): array; - - /** - * Returns the interval in seconds that needs to be waited when the API limit is reached - * - * @return int - */ - public function getApiInterval(): int; - - /** - * Returns the limit of requests that can be performed to the API in a specific interval, or null if no limit exists - * - * @return int|null - */ - public function getApiLimit(): ?int; } From fd6d180eba39a81cc7e88e6935d0da682768d42c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Nov 2018 13:18:21 +0100 Subject: [PATCH 05/19] Created chainIpLocationResolver --- .../Command/Api/DisableKeyCommandTest.php | 4 +- .../Command/Api/GenerateKeyCommandTest.php | 4 +- .../test/Command/Api/ListKeysCommandTest.php | 4 +- .../ShortUrl/DeleteShortCodeCommandTest.php | 6 +- .../ShortUrl/GeneratePreviewCommandTest.php | 10 +-- .../ShortUrl/GenerateShortcodeCommandTest.php | 4 +- .../Command/ShortUrl/GetVisitsCommandTest.php | 6 +- .../ShortUrl/ListShortcodesCommandTest.php | 8 +- .../ShortUrl/ResolveUrlCommandTest.php | 6 +- .../Visit/ProcessVisitsCommandTest.php | 4 +- module/Common/config/dependencies.config.php | 8 ++ .../IpGeolocation/ChainIpLocationResolver.php | 40 ++++++++++ .../GeoLite2LocationResolverTest.php | 80 +++++++++++++++++++ .../IpApiLocationResolverTest.php | 20 +---- .../PaginableRepositoryAdapterTest.php | 4 +- .../test/Service/PreviewGeneratorTest.php | 14 ++-- .../Extension/TranslatorExtensionTest.php | 4 +- module/Core/test/Action/PixelActionTest.php | 4 +- module/Core/test/Action/PreviewActionTest.php | 12 +-- module/Core/test/Action/QrCodeActionTest.php | 10 +-- .../Core/test/Action/RedirectActionTest.php | 12 +-- .../Middleware/QrCodeCacheMiddlewareTest.php | 2 +- .../ShortUrl/DeleteShortUrlServiceTest.php | 12 +-- .../Core/test/Service/ShortUrlServiceTest.php | 14 ++-- module/Core/test/Service/UrlShortenerTest.php | 10 +-- module/Core/test/Service/VisitServiceTest.php | 8 +- .../Core/test/Service/VisitsTrackerTest.php | 16 ++-- .../test/Command/InstallCommandTest.php | 10 +-- .../ApplicationConfigCustomizerTest.php | 6 +- .../Plugin/DatabaseConfigCustomizerTest.php | 4 +- .../Plugin/LanguageConfigCustomizerTest.php | 2 +- .../UrlShortenerConfigCustomizerTest.php | 8 +- .../test/Action/AuthenticateActionTest.php | 4 +- .../ShortUrl/CreateShortUrlActionTest.php | 8 +- .../ShortUrl/DeleteShortUrlActionTest.php | 4 +- .../ShortUrl/EditShortUrlTagsActionTest.php | 4 +- .../ShortUrl/ListShortUrlsActionTest.php | 4 +- .../ShortUrl/ResolveShortUrlActionTest.php | 8 +- .../test/Action/Visit/GetVisitsActionTest.php | 8 +- .../Plugin/ApiKeyHeaderPluginTest.php | 4 +- .../Plugin/AuthorizationHeaderPluginTest.php | 6 +- .../RequestToAuthPluginTest.php | 2 +- .../AuthenticationMiddlewareTest.php | 16 ++-- .../Middleware/BodyParserMiddlewareTest.php | 6 +- .../Middleware/CrossDomainMiddlewareTest.php | 6 +- .../ShortUrl/ShortCodePathMiddlewareTest.php | 2 +- .../Rest/test/Service/ApiKeyServiceTest.php | 26 +++--- 47 files changed, 288 insertions(+), 176 deletions(-) create mode 100644 module/Common/src/IpGeolocation/ChainIpLocationResolver.php create mode 100644 module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php diff --git a/module/CLI/test/Command/Api/DisableKeyCommandTest.php b/module/CLI/test/Command/Api/DisableKeyCommandTest.php index e51ee7a8..c910ea85 100644 --- a/module/CLI/test/Command/Api/DisableKeyCommandTest.php +++ b/module/CLI/test/Command/Api/DisableKeyCommandTest.php @@ -38,7 +38,7 @@ class DisableKeyCommandTest extends TestCase public function providedApiKeyIsDisabled() { $apiKey = 'abcd1234'; - $this->apiKeyService->disable($apiKey)->shouldBeCalledTimes(1); + $this->apiKeyService->disable($apiKey)->shouldBeCalledOnce(); $this->commandTester->execute([ 'command' => 'api-key:disable', 'apiKey' => $apiKey, @@ -52,7 +52,7 @@ class DisableKeyCommandTest extends TestCase { $apiKey = 'abcd1234'; $this->apiKeyService->disable($apiKey)->willThrow(InvalidArgumentException::class) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->commandTester->execute([ 'command' => 'api-key:disable', diff --git a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php index badf7979..78fcf4d5 100644 --- a/module/CLI/test/Command/Api/GenerateKeyCommandTest.php +++ b/module/CLI/test/Command/Api/GenerateKeyCommandTest.php @@ -39,7 +39,7 @@ class GenerateKeyCommandTest extends TestCase */ public function noExpirationDateIsDefinedIfNotProvided() { - $this->apiKeyService->create(null)->shouldBeCalledTimes(1) + $this->apiKeyService->create(null)->shouldBeCalledOnce() ->willReturn(new ApiKey()); $this->commandTester->execute([ 'command' => 'api-key:generate', @@ -51,7 +51,7 @@ class GenerateKeyCommandTest extends TestCase */ public function expirationDateIsDefinedIfProvided() { - $this->apiKeyService->create(Argument::type(Chronos::class))->shouldBeCalledTimes(1) + $this->apiKeyService->create(Argument::type(Chronos::class))->shouldBeCalledOnce() ->willReturn(new ApiKey()); $this->commandTester->execute([ 'command' => 'api-key:generate', diff --git a/module/CLI/test/Command/Api/ListKeysCommandTest.php b/module/CLI/test/Command/Api/ListKeysCommandTest.php index 1bceb5e5..72485ebe 100644 --- a/module/CLI/test/Command/Api/ListKeysCommandTest.php +++ b/module/CLI/test/Command/Api/ListKeysCommandTest.php @@ -41,7 +41,7 @@ class ListKeysCommandTest extends TestCase new ApiKey(), new ApiKey(), new ApiKey(), - ])->shouldBeCalledTimes(1); + ])->shouldBeCalledOnce(); $this->commandTester->execute([ 'command' => 'api-key:list', ]); @@ -55,7 +55,7 @@ class ListKeysCommandTest extends TestCase $this->apiKeyService->listKeys(true)->willReturn([ new ApiKey(), new ApiKey(), - ])->shouldBeCalledTimes(1); + ])->shouldBeCalledOnce(); $this->commandTester->execute([ 'command' => 'api-key:list', '--enabledOnly' => true, diff --git a/module/CLI/test/Command/ShortUrl/DeleteShortCodeCommandTest.php b/module/CLI/test/Command/ShortUrl/DeleteShortCodeCommandTest.php index f273ae14..61142a20 100644 --- a/module/CLI/test/Command/ShortUrl/DeleteShortCodeCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/DeleteShortCodeCommandTest.php @@ -50,7 +50,7 @@ class DeleteShortCodeCommandTest extends TestCase $output = $this->commandTester->getDisplay(); $this->assertContains(sprintf('Short URL with short code "%s" successfully deleted.', $shortCode), $output); - $deleteByShortCode->shouldHaveBeenCalledTimes(1); + $deleteByShortCode->shouldHaveBeenCalledOnce(); } /** @@ -67,7 +67,7 @@ class DeleteShortCodeCommandTest extends TestCase $output = $this->commandTester->getDisplay(); $this->assertContains(sprintf('Provided short code "%s" could not be found.', $shortCode), $output); - $deleteByShortCode->shouldHaveBeenCalledTimes(1); + $deleteByShortCode->shouldHaveBeenCalledOnce(); } /** @@ -117,6 +117,6 @@ class DeleteShortCodeCommandTest extends TestCase $shortCode ), $output); $this->assertContains('Short URL was not deleted.', $output); - $deleteByShortCode->shouldHaveBeenCalledTimes(1); + $deleteByShortCode->shouldHaveBeenCalledOnce(); } } diff --git a/module/CLI/test/Command/ShortUrl/GeneratePreviewCommandTest.php b/module/CLI/test/Command/ShortUrl/GeneratePreviewCommandTest.php index 966460af..8469d24d 100644 --- a/module/CLI/test/Command/ShortUrl/GeneratePreviewCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GeneratePreviewCommandTest.php @@ -60,11 +60,11 @@ class GeneratePreviewCommandTest extends TestCase new ShortUrl('https://bar.com'), new ShortUrl('http://baz.com/something'), ]); - $this->shortUrlService->listShortUrls(1)->willReturn($paginator)->shouldBeCalledTimes(1); + $this->shortUrlService->listShortUrls(1)->willReturn($paginator)->shouldBeCalledOnce(); - $this->previewGenerator->generatePreview('http://foo.com')->shouldBeCalledTimes(1); - $this->previewGenerator->generatePreview('https://bar.com')->shouldBeCalledTimes(1); - $this->previewGenerator->generatePreview('http://baz.com/something')->shouldBeCalledTimes(1); + $this->previewGenerator->generatePreview('http://foo.com')->shouldBeCalledOnce(); + $this->previewGenerator->generatePreview('https://bar.com')->shouldBeCalledOnce(); + $this->previewGenerator->generatePreview('http://baz.com/something')->shouldBeCalledOnce(); $this->commandTester->execute([ 'command' => 'shortcode:process-previews', @@ -82,7 +82,7 @@ class GeneratePreviewCommandTest extends TestCase new ShortUrl('http://baz.com/something'), ]; $paginator = $this->createPaginator($items); - $this->shortUrlService->listShortUrls(1)->willReturn($paginator)->shouldBeCalledTimes(1); + $this->shortUrlService->listShortUrls(1)->willReturn($paginator)->shouldBeCalledOnce(); $this->previewGenerator->generatePreview(Argument::any())->willThrow(PreviewGenerationException::class) ->shouldBeCalledTimes(count($items)); diff --git a/module/CLI/test/Command/ShortUrl/GenerateShortcodeCommandTest.php b/module/CLI/test/Command/ShortUrl/GenerateShortcodeCommandTest.php index 60ccba12..34d7e34a 100644 --- a/module/CLI/test/Command/ShortUrl/GenerateShortcodeCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GenerateShortcodeCommandTest.php @@ -47,7 +47,7 @@ class GenerateShortcodeCommandTest extends TestCase ->willReturn( (new ShortUrl(''))->setShortCode('abc123') ) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->commandTester->execute([ 'command' => 'shortcode:generate', @@ -63,7 +63,7 @@ class GenerateShortcodeCommandTest extends TestCase public function exceptionWhileParsingLongUrlOutputsError() { $this->urlShortener->urlToShortCode(Argument::cetera())->willThrow(new InvalidUrlException()) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->commandTester->execute([ 'command' => 'shortcode:generate', diff --git a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php index e27c7b59..f7523d78 100644 --- a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php @@ -46,7 +46,7 @@ class GetVisitsCommandTest extends TestCase { $shortCode = 'abc123'; $this->visitsTracker->info($shortCode, new DateRange(null, null))->willReturn([]) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->commandTester->execute([ 'command' => 'shortcode:visits', @@ -64,7 +64,7 @@ class GetVisitsCommandTest extends TestCase $endDate = '2016-02-01'; $this->visitsTracker->info($shortCode, new DateRange(Chronos::parse($startDate), Chronos::parse($endDate))) ->willReturn([]) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->commandTester->execute([ 'command' => 'shortcode:visits', @@ -84,7 +84,7 @@ class GetVisitsCommandTest extends TestCase (new Visit(new ShortUrl(''), new Visitor('bar', 'foo', '')))->setVisitLocation( new VisitLocation(['country_name' => 'Spain']) ), - ])->shouldBeCalledTimes(1); + ])->shouldBeCalledOnce(); $this->commandTester->execute([ 'command' => 'shortcode:visits', diff --git a/module/CLI/test/Command/ShortUrl/ListShortcodesCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortcodesCommandTest.php index 0a9cc38d..3480a289 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortcodesCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortcodesCommandTest.php @@ -41,7 +41,7 @@ class ListShortcodesCommandTest extends TestCase public function noInputCallsListJustOnce() { $this->shortUrlService->listShortUrls(1, null, [], null)->willReturn(new Paginator(new ArrayAdapter())) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->commandTester->setInputs(['n']); $this->commandTester->execute(['command' => 'shortcode:list']); @@ -78,7 +78,7 @@ class ListShortcodesCommandTest extends TestCase } $this->shortUrlService->listShortUrls(Argument::cetera())->willReturn(new Paginator(new ArrayAdapter($data))) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->commandTester->setInputs(['n']); $this->commandTester->execute(['command' => 'shortcode:list']); @@ -91,7 +91,7 @@ class ListShortcodesCommandTest extends TestCase { $page = 5; $this->shortUrlService->listShortUrls($page, null, [], null)->willReturn(new Paginator(new ArrayAdapter())) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->commandTester->setInputs(['y']); $this->commandTester->execute([ @@ -106,7 +106,7 @@ class ListShortcodesCommandTest extends TestCase public function ifTagsFlagIsProvidedTagsColumnIsIncluded() { $this->shortUrlService->listShortUrls(1, null, [], null)->willReturn(new Paginator(new ArrayAdapter())) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->commandTester->setInputs(['y']); $this->commandTester->execute([ diff --git a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php index 32edea44..0f17a7d7 100644 --- a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php @@ -45,7 +45,7 @@ class ResolveUrlCommandTest extends TestCase $expectedUrl = 'http://domain.com/foo/bar'; $shortUrl = new ShortUrl($expectedUrl); $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->commandTester->execute([ 'command' => 'shortcode:parse', @@ -62,7 +62,7 @@ class ResolveUrlCommandTest extends TestCase { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->commandTester->execute([ 'command' => 'shortcode:parse', @@ -79,7 +79,7 @@ class ResolveUrlCommandTest extends TestCase { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(new InvalidShortCodeException()) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->commandTester->execute([ 'command' => 'shortcode:parse', diff --git a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php index a92241f0..6988fb7c 100644 --- a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php @@ -62,7 +62,7 @@ class ProcessVisitsCommandTest extends TestCase new Visit($shortUrl, new Visitor('', '', '12.34.56.78')), ]; $this->visitService->getUnlocatedVisits()->willReturn($visits) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->visitService->saveVisit(Argument::any())->shouldBeCalledTimes(count($visits)); $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]) @@ -94,7 +94,7 @@ class ProcessVisitsCommandTest extends TestCase new Visit($shortUrl, new Visitor('', '', null)), ]; $this->visitService->getUnlocatedVisits()->willReturn($visits) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->visitService->saveVisit(Argument::any())->shouldBeCalledTimes(count($visits) - 4); $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]) diff --git a/module/Common/config/dependencies.config.php b/module/Common/config/dependencies.config.php index 4b63f602..deae26d8 100644 --- a/module/Common/config/dependencies.config.php +++ b/module/Common/config/dependencies.config.php @@ -36,6 +36,8 @@ return [ IpGeolocation\IpApiLocationResolver::class => ConfigAbstractFactory::class, IpGeolocation\GeoLite2LocationResolver::class => ConfigAbstractFactory::class, + IpGeolocation\ChainIpLocationResolver::class => ConfigAbstractFactory::class, + Service\PreviewGenerator::class => ConfigAbstractFactory::class, ], 'aliases' => [ @@ -57,8 +59,14 @@ return [ Template\Extension\TranslatorExtension::class => ['translator'], Middleware\LocaleMiddleware::class => ['translator'], + IpGeolocation\IpApiLocationResolver::class => ['httpClient'], IpGeolocation\GeoLite2LocationResolver::class => [Reader::class], + IpGeolocation\ChainIpLocationResolver::class => [ + IpGeolocation\GeoLite2LocationResolver::class, + IpGeolocation\IpApiLocationResolver::class, + ], + Service\PreviewGenerator::class => [ Image\ImageBuilder::class, Filesystem::class, diff --git a/module/Common/src/IpGeolocation/ChainIpLocationResolver.php b/module/Common/src/IpGeolocation/ChainIpLocationResolver.php new file mode 100644 index 00000000..8528c89e --- /dev/null +++ b/module/Common/src/IpGeolocation/ChainIpLocationResolver.php @@ -0,0 +1,40 @@ +resolvers = $resolvers; + } + + /** + * @param string $ipAddress + * @return array + * @throws WrongIpException + */ + public function resolveIpLocation(string $ipAddress): array + { + $error = null; + + foreach ($this->resolvers as $resolver) { + try { + return $resolver->resolveIpLocation($ipAddress); + } catch (WrongIpException $e) { + $error = $e; + } + } + + // If this instruction is reached, it means no resolver was capable of resolving the address + throw WrongIpException::fromIpAddress($ipAddress, $error); + } +} diff --git a/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php b/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php new file mode 100644 index 00000000..b643bf0b --- /dev/null +++ b/module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php @@ -0,0 +1,80 @@ +reader = $this->prophesize(Reader::class); + $this->resolver = new GeoLite2LocationResolver($this->reader->reveal()); + } + + /** + * @test + * @dataProvider provideReaderExceptions + */ + public function exceptionIsThrownIfReaderThrowsException(string $e, string $message) + { + $ipAddress = '1.2.3.4'; + + $cityMethod = $this->reader->city($ipAddress)->willThrow($e); + + $this->expectException(WrongIpException::class); + $this->expectExceptionMessage($message); + $cityMethod->shouldBeCalledOnce(); + + $this->resolver->resolveIpLocation($ipAddress); + } + + public function provideReaderExceptions(): array + { + return [ + [AddressNotFoundException::class, 'Provided IP "1.2.3.4" is invalid'], + [InvalidDatabaseException::class, 'Provided GeoLite2 db file is invalid'], + ]; + } + + /** + * @test + */ + public function resolvedCityIsProperlyMapped() + { + $ipAddress = '1.2.3.4'; + $city = new City([]); + + $cityMethod = $this->reader->city($ipAddress)->willReturn($city); + + $result = $this->resolver->resolveIpLocation($ipAddress); + + $this->assertEquals([ + 'country_code' => '', + 'country_name' => '', + 'region_name' => '', + 'city' => '', + 'latitude' => '', + 'longitude' => '', + 'time_zone' => '', + ], $result); + $cityMethod->shouldHaveBeenCalledOnce(); + } +} diff --git a/module/Common/test/IpGeolocation/IpApiLocationResolverTest.php b/module/Common/test/IpGeolocation/IpApiLocationResolverTest.php index c8c11499..999b8fc6 100644 --- a/module/Common/test/IpGeolocation/IpApiLocationResolverTest.php +++ b/module/Common/test/IpGeolocation/IpApiLocationResolverTest.php @@ -52,7 +52,7 @@ class IpApiLocationResolverTest extends TestCase $response->getBody()->rewind(); $this->client->get('http://ip-api.com/json/1.2.3.4')->willReturn($response) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->assertEquals($expected, $this->ipResolver->resolveIpLocation('1.2.3.4')); } @@ -63,23 +63,7 @@ class IpApiLocationResolverTest extends TestCase public function guzzleExceptionThrowsShlinkException() { $this->client->get('http://ip-api.com/json/1.2.3.4')->willThrow(new TransferException()) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->ipResolver->resolveIpLocation('1.2.3.4'); } - - /** - * @test - */ - public function getApiIntervalReturnsExpectedValue() - { - $this->assertEquals(65, $this->ipResolver->getApiInterval()); - } - - /** - * @test - */ - public function getApiLimitReturnsExpectedValue() - { - $this->assertEquals(145, $this->ipResolver->getApiLimit()); - } } diff --git a/module/Common/test/Paginator/PaginableRepositoryAdapterTest.php b/module/Common/test/Paginator/PaginableRepositoryAdapterTest.php index 70b85ef8..456f2cd8 100644 --- a/module/Common/test/Paginator/PaginableRepositoryAdapterTest.php +++ b/module/Common/test/Paginator/PaginableRepositoryAdapterTest.php @@ -30,7 +30,7 @@ class PaginableRepositoryAdapterTest extends TestCase */ public function getItemsFallbacksToFindList() { - $this->repo->findList(10, 5, 'search', ['foo', 'bar'], 'order')->shouldBeCalledTimes(1); + $this->repo->findList(10, 5, 'search', ['foo', 'bar'], 'order')->shouldBeCalledOnce(); $this->adapter->getItems(5, 10); } @@ -39,7 +39,7 @@ class PaginableRepositoryAdapterTest extends TestCase */ public function countFallbacksToCountList() { - $this->repo->countList('search', ['foo', 'bar'])->shouldBeCalledTimes(1); + $this->repo->countList('search', ['foo', 'bar'])->shouldBeCalledOnce(); $this->adapter->count(); } } diff --git a/module/Common/test/Service/PreviewGeneratorTest.php b/module/Common/test/Service/PreviewGeneratorTest.php index 650b0b03..882555e1 100644 --- a/module/Common/test/Service/PreviewGeneratorTest.php +++ b/module/Common/test/Service/PreviewGeneratorTest.php @@ -50,7 +50,7 @@ class PreviewGeneratorTest extends TestCase { $url = 'http://foo.com'; $this->filesystem->exists(sprintf('dir/preview_%s.png', urlencode($url)))->willReturn(true) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->image->saveAs(Argument::cetera())->shouldBeCalledTimes(0); $this->assertEquals(sprintf('dir/preview_%s.png', urlencode($url)), $this->generator->generatePreview($url)); } @@ -65,10 +65,10 @@ class PreviewGeneratorTest extends TestCase $expectedPath = 'dir/' . $cacheId; $this->filesystem->exists(sprintf('dir/preview_%s.png', urlencode($url)))->willReturn(false) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); - $this->image->saveAs($expectedPath)->shouldBeCalledTimes(1); - $this->image->getError()->willReturn('')->shouldBeCalledTimes(1); + $this->image->saveAs($expectedPath)->shouldBeCalledOnce(); + $this->image->getError()->willReturn('')->shouldBeCalledOnce(); $this->assertEquals($expectedPath, $this->generator->generatePreview($url)); } @@ -83,10 +83,10 @@ class PreviewGeneratorTest extends TestCase $expectedPath = 'dir/' . $cacheId; $this->filesystem->exists(sprintf('dir/preview_%s.png', urlencode($url)))->willReturn(false) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); - $this->image->saveAs($expectedPath)->shouldBeCalledTimes(1); - $this->image->getError()->willReturn('Error!!')->shouldBeCalledTimes(1); + $this->image->saveAs($expectedPath)->shouldBeCalledOnce(); + $this->image->getError()->willReturn('Error!!')->shouldBeCalledOnce(); $this->generator->generatePreview($url); } diff --git a/module/Common/test/Template/Extension/TranslatorExtensionTest.php b/module/Common/test/Template/Extension/TranslatorExtensionTest.php index 04d36188..6aa6a111 100644 --- a/module/Common/test/Template/Extension/TranslatorExtensionTest.php +++ b/module/Common/test/Template/Extension/TranslatorExtensionTest.php @@ -28,8 +28,8 @@ class TranslatorExtensionTest extends TestCase { $engine = $this->prophesize(Engine::class); - $engine->registerFunction('translate', Argument::type('callable'))->shouldBeCalledTimes(1); - $engine->registerFunction('translate_plural', Argument::type('callable'))->shouldBeCalledTimes(1); + $engine->registerFunction('translate', Argument::type('callable'))->shouldBeCalledOnce(); + $engine->registerFunction('translate_plural', Argument::type('callable'))->shouldBeCalledOnce(); $funcs = $this->extension->register($engine->reveal()); } diff --git a/module/Core/test/Action/PixelActionTest.php b/module/Core/test/Action/PixelActionTest.php index b8b580b8..daaf86fe 100644 --- a/module/Core/test/Action/PixelActionTest.php +++ b/module/Core/test/Action/PixelActionTest.php @@ -51,8 +51,8 @@ class PixelActionTest extends TestCase $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willReturn( new ShortUrl('http://domain.com/foo/bar') - )->shouldBeCalledTimes(1); - $this->visitTracker->track(Argument::cetera())->shouldBeCalledTimes(1); + )->shouldBeCalledOnce(); + $this->visitTracker->track(Argument::cetera())->shouldBeCalledOnce(); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); $response = $this->action->process($request, TestUtils::createReqHandlerMock()->reveal()); diff --git a/module/Core/test/Action/PreviewActionTest.php b/module/Core/test/Action/PreviewActionTest.php index 8aaadc36..d17fe383 100644 --- a/module/Core/test/Action/PreviewActionTest.php +++ b/module/Core/test/Action/PreviewActionTest.php @@ -50,9 +50,9 @@ class PreviewActionTest extends TestCase { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); - $delegate->handle(Argument::cetera())->shouldBeCalledTimes(1) + $delegate->handle(Argument::cetera())->shouldBeCalledOnce() ->willReturn(new Response()); $this->action->process( @@ -70,8 +70,8 @@ class PreviewActionTest extends TestCase $url = 'foobar.com'; $shortUrl = new ShortUrl($url); $path = __FILE__; - $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl)->shouldBeCalledTimes(1); - $this->previewGenerator->generatePreview($url)->willReturn($path)->shouldBeCalledTimes(1); + $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl)->shouldBeCalledOnce(); + $this->previewGenerator->generatePreview($url)->willReturn($path)->shouldBeCalledOnce(); $resp = $this->action->process( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode), @@ -89,7 +89,7 @@ class PreviewActionTest extends TestCase { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); /** @var MethodProphecy $process */ $process = $delegate->handle(Argument::any())->willReturn(new Response()); @@ -99,6 +99,6 @@ class PreviewActionTest extends TestCase $delegate->reveal() ); - $process->shouldHaveBeenCalledTimes(1); + $process->shouldHaveBeenCalledOnce(); } } diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 9009023c..fc15727a 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -46,7 +46,7 @@ class QrCodeActionTest extends TestCase { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); $process = $delegate->handle(Argument::any())->willReturn(new Response()); @@ -55,7 +55,7 @@ class QrCodeActionTest extends TestCase $delegate->reveal() ); - $process->shouldHaveBeenCalledTimes(1); + $process->shouldHaveBeenCalledOnce(); } /** @@ -65,7 +65,7 @@ class QrCodeActionTest extends TestCase { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); /** @var MethodProphecy $process */ $process = $delegate->handle(Argument::any())->willReturn(new Response()); @@ -75,7 +75,7 @@ class QrCodeActionTest extends TestCase $delegate->reveal() ); - $process->shouldHaveBeenCalledTimes(1); + $process->shouldHaveBeenCalledOnce(); } /** @@ -85,7 +85,7 @@ class QrCodeActionTest extends TestCase { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willReturn(new ShortUrl('')) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); $resp = $this->action->process( diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index 5de290f7..cbe87126 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -59,8 +59,8 @@ class RedirectActionTest extends TestCase $expectedUrl = 'http://domain.com/foo/bar'; $shortUrl = new ShortUrl($expectedUrl); $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl) - ->shouldBeCalledTimes(1); - $this->visitTracker->track(Argument::cetera())->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); + $this->visitTracker->track(Argument::cetera())->shouldBeCalledOnce(); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); $response = $this->action->process($request, TestUtils::createReqHandlerMock()->reveal()); @@ -78,7 +78,7 @@ class RedirectActionTest extends TestCase { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->visitTracker->track(Argument::cetera())->shouldNotBeCalled(); $handler = $this->prophesize(RequestHandlerInterface::class); @@ -87,7 +87,7 @@ class RedirectActionTest extends TestCase $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); $this->action->process($request, $handler->reveal()); - $handle->shouldHaveBeenCalledTimes(1); + $handle->shouldHaveBeenCalledOnce(); } /** @@ -111,7 +111,7 @@ class RedirectActionTest extends TestCase $this->assertEquals(302, $resp->getStatusCode()); $this->assertEquals('https://shlink.io', $resp->getHeaderLine('Location')); - $shortCodeToUrl->shouldHaveBeenCalledTimes(1); + $shortCodeToUrl->shouldHaveBeenCalledOnce(); $handle->shouldNotHaveBeenCalled(); } @@ -124,7 +124,7 @@ class RedirectActionTest extends TestCase $expectedUrl = 'http://domain.com/foo/bar'; $shortUrl = new ShortUrl($expectedUrl); $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->visitTracker->track(Argument::cetera())->shouldNotBeCalled(); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode) diff --git a/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php b/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php index f566a6bf..ae701aaf 100644 --- a/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php +++ b/module/Core/test/Middleware/QrCodeCacheMiddlewareTest.php @@ -36,7 +36,7 @@ class QrCodeCacheMiddlewareTest extends TestCase public function noCachedPathFallsBackToNextMiddleware() { $delegate = $this->prophesize(RequestHandlerInterface::class); - $delegate->handle(Argument::any())->willReturn(new Response())->shouldBeCalledTimes(1); + $delegate->handle(Argument::any())->willReturn(new Response())->shouldBeCalledOnce(); $this->middleware->process(ServerRequestFactory::fromGlobals()->withUri( new Uri('/foo/bar') diff --git a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php index db86a249..60ab0b6d 100644 --- a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php @@ -70,8 +70,8 @@ class DeleteShortUrlServiceTest extends TestCase $service->deleteByShortCode('abc123', true); - $remove->shouldHaveBeenCalledTimes(1); - $flush->shouldHaveBeenCalledTimes(1); + $remove->shouldHaveBeenCalledOnce(); + $flush->shouldHaveBeenCalledOnce(); } /** @@ -86,8 +86,8 @@ class DeleteShortUrlServiceTest extends TestCase $service->deleteByShortCode('abc123'); - $remove->shouldHaveBeenCalledTimes(1); - $flush->shouldHaveBeenCalledTimes(1); + $remove->shouldHaveBeenCalledOnce(); + $flush->shouldHaveBeenCalledOnce(); } /** @@ -102,8 +102,8 @@ class DeleteShortUrlServiceTest extends TestCase $service->deleteByShortCode('abc123'); - $remove->shouldHaveBeenCalledTimes(1); - $flush->shouldHaveBeenCalledTimes(1); + $remove->shouldHaveBeenCalledOnce(); + $flush->shouldHaveBeenCalledOnce(); } private function createService(bool $checkVisitsThreshold = true, int $visitsThreshold = 5): DeleteShortUrlService diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php index e2750453..4b8e5ede 100644 --- a/module/Core/test/Service/ShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrlServiceTest.php @@ -49,8 +49,8 @@ class ShortUrlServiceTest extends TestCase ]; $repo = $this->prophesize(ShortUrlRepository::class); - $repo->findList(Argument::cetera())->willReturn($list)->shouldBeCalledTimes(1); - $repo->countList(Argument::cetera())->willReturn(count($list))->shouldBeCalledTimes(1); + $repo->findList(Argument::cetera())->willReturn($list)->shouldBeCalledOnce(); + $repo->countList(Argument::cetera())->willReturn(count($list))->shouldBeCalledOnce(); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $list = $this->service->listShortUrls(); @@ -65,7 +65,7 @@ class ShortUrlServiceTest extends TestCase $shortCode = 'abc123'; $repo = $this->prophesize(ShortUrlRepository::class); $repo->findOneBy(['shortCode' => $shortCode])->willReturn(null) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $this->expectException(InvalidShortCodeException::class); @@ -78,16 +78,16 @@ class ShortUrlServiceTest extends TestCase public function providedTagsAreGetFromRepoAndSetToTheShortUrl() { $shortUrl = $this->prophesize(ShortUrl::class); - $shortUrl->setTags(Argument::any())->shouldBeCalledTimes(1); + $shortUrl->setTags(Argument::any())->shouldBeCalledOnce(); $shortCode = 'abc123'; $repo = $this->prophesize(ShortUrlRepository::class); $repo->findOneBy(['shortCode' => $shortCode])->willReturn($shortUrl->reveal()) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $tagRepo = $this->prophesize(EntityRepository::class); - $tagRepo->findOneBy(['name' => 'foo'])->willReturn(new Tag('foo'))->shouldbeCalledTimes(1); - $tagRepo->findOneBy(['name' => 'bar'])->willReturn(null)->shouldbeCalledTimes(1); + $tagRepo->findOneBy(['name' => 'foo'])->willReturn(new Tag('foo'))->shouldBeCalledOnce(); + $tagRepo->findOneBy(['name' => 'bar'])->willReturn(null)->shouldBeCalledOnce(); $this->em->getRepository(Tag::class)->willReturn($tagRepo->reveal()); $this->service->setTagsByShortCode($shortCode, ['foo', 'bar']); diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 5b6b473b..2269ae4a 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -98,8 +98,8 @@ class UrlShortenerTest extends TestCase $conn = $this->prophesize(Connection::class); $conn->isTransactionActive()->willReturn(true); $this->em->getConnection()->willReturn($conn->reveal()); - $this->em->rollback()->shouldBeCalledTimes(1); - $this->em->close()->shouldBeCalledTimes(1); + $this->em->rollback()->shouldBeCalledOnce(); + $this->em->close()->shouldBeCalledOnce(); $this->em->flush()->willThrow(new ORMException()); $this->urlShortener->urlToShortCode(new Uri('http://foobar.com/12345/hello?foo=bar')); @@ -135,7 +135,7 @@ class UrlShortenerTest extends TestCase 'custom-slug' ); - $slugify->shouldHaveBeenCalledTimes(1); + $slugify->shouldHaveBeenCalledOnce(); } /** @@ -153,8 +153,8 @@ class UrlShortenerTest extends TestCase /** @var MethodProphecy $getRepo */ $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $slugify->shouldBeCalledTimes(1); - $findBySlug->shouldBeCalledTimes(1); + $slugify->shouldBeCalledOnce(); + $findBySlug->shouldBeCalledOnce(); $getRepo->shouldBeCalled(); $this->expectException(NonUniqueSlugException::class); diff --git a/module/Core/test/Service/VisitServiceTest.php b/module/Core/test/Service/VisitServiceTest.php index c70642d7..728d18ee 100644 --- a/module/Core/test/Service/VisitServiceTest.php +++ b/module/Core/test/Service/VisitServiceTest.php @@ -35,8 +35,8 @@ class VisitServiceTest extends TestCase public function saveVisitsPersistsProvidedVisit() { $visit = new Visit(new ShortUrl(''), Visitor::emptyInstance()); - $this->em->persist($visit)->shouldBeCalledTimes(1); - $this->em->flush()->shouldBeCalledTimes(1); + $this->em->persist($visit)->shouldBeCalledOnce(); + $this->em->flush()->shouldBeCalledOnce(); $this->visitService->saveVisit($visit); } @@ -46,8 +46,8 @@ class VisitServiceTest extends TestCase public function getUnlocatedVisitsFallbacksToRepository() { $repo = $this->prophesize(VisitRepository::class); - $repo->findUnlocatedVisits()->shouldBeCalledTimes(1); - $this->em->getRepository(Visit::class)->willReturn($repo->reveal())->shouldBeCalledTimes(1); + $repo->findUnlocatedVisits()->shouldBeCalledOnce(); + $this->em->getRepository(Visit::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); $this->visitService->getUnlocatedVisits(); } } diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php index 4af82bdf..9d894c04 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/module/Core/test/Service/VisitsTrackerTest.php @@ -40,9 +40,9 @@ class VisitsTrackerTest extends TestCase $repo = $this->prophesize(EntityRepository::class); $repo->findOneBy(['shortCode' => $shortCode])->willReturn(new ShortUrl('')); - $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledTimes(1); - $this->em->persist(Argument::any())->shouldBeCalledTimes(1); - $this->em->flush(Argument::type(Visit::class))->shouldBeCalledTimes(1); + $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); + $this->em->persist(Argument::any())->shouldBeCalledOnce(); + $this->em->flush(Argument::type(Visit::class))->shouldBeCalledOnce(); $this->visitsTracker->track($shortCode, Visitor::emptyInstance()); } @@ -57,13 +57,13 @@ class VisitsTrackerTest extends TestCase $repo = $this->prophesize(EntityRepository::class); $repo->findOneBy(['shortCode' => $shortCode])->willReturn(new ShortUrl('')); - $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledTimes(1); + $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); $this->em->persist(Argument::any())->will(function ($args) use ($test) { /** @var Visit $visit */ $visit = $args[0]; $test->assertEquals('4.3.2.0', $visit->getRemoteAddr()); - })->shouldBeCalledTimes(1); - $this->em->flush(Argument::type(Visit::class))->shouldBeCalledTimes(1); + })->shouldBeCalledOnce(); + $this->em->flush(Argument::type(Visit::class))->shouldBeCalledOnce(); $this->visitsTracker->track($shortCode, new Visitor('', '', '4.3.2.1')); } @@ -77,7 +77,7 @@ class VisitsTrackerTest extends TestCase $shortUrl = new ShortUrl('http://domain.com/foo/bar'); $repo = $this->prophesize(EntityRepository::class); $repo->findOneBy(['shortCode' => $shortCode])->willReturn($shortUrl); - $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledTimes(1); + $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); $list = [ new Visit(new ShortUrl(''), Visitor::emptyInstance()), @@ -85,7 +85,7 @@ class VisitsTrackerTest extends TestCase ]; $repo2 = $this->prophesize(VisitRepository::class); $repo2->findVisitsByShortUrl($shortUrl, null)->willReturn($list); - $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledTimes(1); + $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce(); $this->assertEquals($list, $this->visitsTracker->info($shortCode)); } diff --git a/module/Installer/test/Command/InstallCommandTest.php b/module/Installer/test/Command/InstallCommandTest.php index 60115337..dd3c4220 100644 --- a/module/Installer/test/Command/InstallCommandTest.php +++ b/module/Installer/test/Command/InstallCommandTest.php @@ -81,7 +81,7 @@ class InstallCommandTest extends TestCase */ public function generatedConfigIsProperlyPersisted() { - $this->configWriter->toFile(Argument::any(), Argument::type('array'), false)->shouldBeCalledTimes(1); + $this->configWriter->toFile(Argument::any(), Argument::type('array'), false)->shouldBeCalledOnce(); $this->commandTester->execute([]); } @@ -97,8 +97,8 @@ class InstallCommandTest extends TestCase $this->commandTester->execute([]); - $appConfigExists->shouldHaveBeenCalledTimes(1); - $appConfigRemove->shouldHaveBeenCalledTimes(1); + $appConfigExists->shouldHaveBeenCalledOnce(); + $appConfigRemove->shouldHaveBeenCalledOnce(); } /** @@ -115,8 +115,8 @@ class InstallCommandTest extends TestCase $this->commandTester->execute([]); - $appConfigExists->shouldHaveBeenCalledTimes(1); - $appConfigRemove->shouldHaveBeenCalledTimes(1); + $appConfigExists->shouldHaveBeenCalledOnce(); + $appConfigRemove->shouldHaveBeenCalledOnce(); $configToFile->shouldNotHaveBeenCalled(); } diff --git a/module/Installer/test/Config/Plugin/ApplicationConfigCustomizerTest.php b/module/Installer/test/Config/Plugin/ApplicationConfigCustomizerTest.php index d5b162f2..ddd72814 100644 --- a/module/Installer/test/Config/Plugin/ApplicationConfigCustomizerTest.php +++ b/module/Installer/test/Config/Plugin/ApplicationConfigCustomizerTest.php @@ -51,7 +51,7 @@ class ApplicationConfigCustomizerTest extends TestCase 'CHECK_VISITS_THRESHOLD' => false, ], $config->getApp()); $ask->shouldHaveBeenCalledTimes(2); - $confirm->shouldHaveBeenCalledTimes(1); + $confirm->shouldHaveBeenCalledOnce(); } /** @@ -77,7 +77,7 @@ class ApplicationConfigCustomizerTest extends TestCase 'VISITS_THRESHOLD' => 20, ], $config->getApp()); $ask->shouldHaveBeenCalledTimes(3); - $confirm->shouldHaveBeenCalledTimes(1); + $confirm->shouldHaveBeenCalledOnce(); } /** @@ -101,7 +101,7 @@ class ApplicationConfigCustomizerTest extends TestCase 'CHECK_VISITS_THRESHOLD' => true, 'VISITS_THRESHOLD' => 20, ], $config->getApp()); - $ask->shouldHaveBeenCalledTimes(1); + $ask->shouldHaveBeenCalledOnce(); } /** diff --git a/module/Installer/test/Config/Plugin/DatabaseConfigCustomizerTest.php b/module/Installer/test/Config/Plugin/DatabaseConfigCustomizerTest.php index 9d4ab1d9..8758083b 100644 --- a/module/Installer/test/Config/Plugin/DatabaseConfigCustomizerTest.php +++ b/module/Installer/test/Config/Plugin/DatabaseConfigCustomizerTest.php @@ -55,7 +55,7 @@ class DatabaseConfigCustomizerTest extends TestCase 'HOST' => 'param', 'PORT' => 'param', ], $config->getDatabase()); - $choice->shouldHaveBeenCalledTimes(1); + $choice->shouldHaveBeenCalledOnce(); $ask->shouldHaveBeenCalledTimes(5); } @@ -137,6 +137,6 @@ class DatabaseConfigCustomizerTest extends TestCase $this->assertEquals([ 'DRIVER' => 'pdo_sqlite', ], $config->getDatabase()); - $copy->shouldHaveBeenCalledTimes(1); + $copy->shouldHaveBeenCalledOnce(); } } diff --git a/module/Installer/test/Config/Plugin/LanguageConfigCustomizerTest.php b/module/Installer/test/Config/Plugin/LanguageConfigCustomizerTest.php index 8c6b633c..1b54048c 100644 --- a/module/Installer/test/Config/Plugin/LanguageConfigCustomizerTest.php +++ b/module/Installer/test/Config/Plugin/LanguageConfigCustomizerTest.php @@ -63,7 +63,7 @@ class LanguageConfigCustomizerTest extends TestCase 'DEFAULT' => 'en', 'CLI' => 'es', ], $config->getLanguage()); - $choice->shouldHaveBeenCalledTimes(1); + $choice->shouldHaveBeenCalledOnce(); } /** diff --git a/module/Installer/test/Config/Plugin/UrlShortenerConfigCustomizerTest.php b/module/Installer/test/Config/Plugin/UrlShortenerConfigCustomizerTest.php index 2942248a..7ff579be 100644 --- a/module/Installer/test/Config/Plugin/UrlShortenerConfigCustomizerTest.php +++ b/module/Installer/test/Config/Plugin/UrlShortenerConfigCustomizerTest.php @@ -50,7 +50,7 @@ class UrlShortenerConfigCustomizerTest extends TestCase 'NOT_FOUND_REDIRECT_TO' => 'asked', ], $config->getUrlShortener()); $ask->shouldHaveBeenCalledTimes(3); - $choice->shouldHaveBeenCalledTimes(1); + $choice->shouldHaveBeenCalledOnce(); $confirm->shouldHaveBeenCalledTimes(2); } @@ -81,8 +81,8 @@ class UrlShortenerConfigCustomizerTest extends TestCase 'NOT_FOUND_REDIRECT_TO' => 'foo', ], $config->getUrlShortener()); $choice->shouldNotHaveBeenCalled(); - $ask->shouldHaveBeenCalledTimes(1); - $confirm->shouldHaveBeenCalledTimes(1); + $ask->shouldHaveBeenCalledOnce(); + $confirm->shouldHaveBeenCalledOnce(); } /** @@ -146,6 +146,6 @@ class UrlShortenerConfigCustomizerTest extends TestCase 'ENABLE_NOT_FOUND_REDIRECTION' => false, ], $config->getUrlShortener()); $ask->shouldNotHaveBeenCalled(); - $confirm->shouldHaveBeenCalledTimes(1); + $confirm->shouldHaveBeenCalledOnce(); } } diff --git a/module/Rest/test/Action/AuthenticateActionTest.php b/module/Rest/test/Action/AuthenticateActionTest.php index 123444cc..56db59a9 100644 --- a/module/Rest/test/Action/AuthenticateActionTest.php +++ b/module/Rest/test/Action/AuthenticateActionTest.php @@ -57,7 +57,7 @@ class AuthenticateActionTest extends TestCase public function properApiKeyReturnsTokenInResponse() { $this->apiKeyService->getByKey('foo')->willReturn((new ApiKey())->setId('5')) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $request = ServerRequestFactory::fromGlobals()->withParsedBody([ 'apiKey' => 'foo', @@ -75,7 +75,7 @@ class AuthenticateActionTest extends TestCase public function invalidApiKeyReturnsErrorResponse() { $this->apiKeyService->getByKey('foo')->willReturn((new ApiKey())->disable()) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $request = ServerRequestFactory::fromGlobals()->withParsedBody([ 'apiKey' => 'foo', diff --git a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php index 346e1f74..eb355c44 100644 --- a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php @@ -56,7 +56,7 @@ class CreateShortUrlActionTest extends TestCase ->willReturn( (new ShortUrl(''))->setShortCode('abc123') ) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $request = ServerRequestFactory::fromGlobals()->withParsedBody([ 'longUrl' => 'http://www.domain.com/foo/bar', @@ -73,7 +73,7 @@ class CreateShortUrlActionTest extends TestCase { $this->urlShortener->urlToShortCode(Argument::type(Uri::class), Argument::type('array'), Argument::cetera()) ->willThrow(InvalidUrlException::class) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $request = ServerRequestFactory::fromGlobals()->withParsedBody([ 'longUrl' => 'http://www.domain.com/foo/bar', @@ -95,7 +95,7 @@ class CreateShortUrlActionTest extends TestCase null, 'foo', Argument::cetera() - )->willThrow(NonUniqueSlugException::class)->shouldBeCalledTimes(1); + )->willThrow(NonUniqueSlugException::class)->shouldBeCalledOnce(); $request = ServerRequestFactory::fromGlobals()->withParsedBody([ 'longUrl' => 'http://www.domain.com/foo/bar', @@ -113,7 +113,7 @@ class CreateShortUrlActionTest extends TestCase { $this->urlShortener->urlToShortCode(Argument::type(Uri::class), Argument::type('array'), Argument::cetera()) ->willThrow(Exception::class) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $request = ServerRequestFactory::fromGlobals()->withParsedBody([ 'longUrl' => 'http://www.domain.com/foo/bar', diff --git a/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php index 7d0ef3b1..593c70fc 100644 --- a/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/DeleteShortUrlActionTest.php @@ -43,7 +43,7 @@ class DeleteShortUrlActionTest extends TestCase $resp = $this->action->handle(ServerRequestFactory::fromGlobals()); $this->assertEquals(204, $resp->getStatusCode()); - $deleteByShortCode->shouldHaveBeenCalledTimes(1); + $deleteByShortCode->shouldHaveBeenCalledOnce(); } /** @@ -60,7 +60,7 @@ class DeleteShortUrlActionTest extends TestCase $this->assertEquals($statusCode, $resp->getStatusCode()); $this->assertEquals($error, $payload['error']); - $deleteByShortCode->shouldHaveBeenCalledTimes(1); + $deleteByShortCode->shouldHaveBeenCalledOnce(); } public function provideExceptions(): array diff --git a/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php b/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php index 889cdbed..2b71903a 100644 --- a/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php @@ -45,7 +45,7 @@ class EditShortUrlTagsActionTest extends TestCase { $shortCode = 'abc123'; $this->shortUrlService->setTagsByShortCode($shortCode, [])->willThrow(InvalidShortCodeException::class) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $response = $this->action->handle( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', 'abc123') @@ -61,7 +61,7 @@ class EditShortUrlTagsActionTest extends TestCase { $shortCode = 'abc123'; $this->shortUrlService->setTagsByShortCode($shortCode, [])->willReturn(new ShortUrl('')) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $response = $this->action->handle( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', 'abc123') diff --git a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php index 1e6ca82d..a83a7dc0 100644 --- a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php @@ -40,7 +40,7 @@ class ListShortUrlsActionTest extends TestCase { $page = 3; $this->service->listShortUrls($page, null, [], null)->willReturn(new Paginator(new ArrayAdapter())) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $response = $this->action->handle(ServerRequestFactory::fromGlobals()->withQueryParams([ 'page' => $page, @@ -55,7 +55,7 @@ class ListShortUrlsActionTest extends TestCase { $page = 3; $this->service->listShortUrls($page, null, [], null)->willThrow(Exception::class) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $response = $this->action->handle(ServerRequestFactory::fromGlobals()->withQueryParams([ 'page' => $page, diff --git a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php index 81c397e3..58010acd 100644 --- a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php @@ -40,7 +40,7 @@ class ResolveShortUrlActionTest extends TestCase { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); $response = $this->action->handle($request); @@ -56,7 +56,7 @@ class ResolveShortUrlActionTest extends TestCase $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willReturn( new ShortUrl('http://domain.com/foo/bar') - )->shouldBeCalledTimes(1); + )->shouldBeCalledOnce(); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); $response = $this->action->handle($request); @@ -71,7 +71,7 @@ class ResolveShortUrlActionTest extends TestCase { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); $response = $this->action->handle($request); @@ -86,7 +86,7 @@ class ResolveShortUrlActionTest extends TestCase { $shortCode = 'abc123'; $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(Exception::class) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); $response = $this->action->handle($request); diff --git a/module/Rest/test/Action/Visit/GetVisitsActionTest.php b/module/Rest/test/Action/Visit/GetVisitsActionTest.php index d5cff5fd..a95868cb 100644 --- a/module/Rest/test/Action/Visit/GetVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/GetVisitsActionTest.php @@ -39,7 +39,7 @@ class GetVisitsActionTest extends TestCase { $shortCode = 'abc123'; $this->visitsTracker->info($shortCode, Argument::type(DateRange::class))->willReturn([]) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $response = $this->action->handle(ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode)); $this->assertEquals(200, $response->getStatusCode()); @@ -53,7 +53,7 @@ class GetVisitsActionTest extends TestCase $shortCode = 'abc123'; $this->visitsTracker->info($shortCode, Argument::type(DateRange::class))->willThrow( InvalidArgumentException::class - )->shouldBeCalledTimes(1); + )->shouldBeCalledOnce(); $response = $this->action->handle(ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode)); $this->assertEquals(404, $response->getStatusCode()); @@ -67,7 +67,7 @@ class GetVisitsActionTest extends TestCase $shortCode = 'abc123'; $this->visitsTracker->info($shortCode, Argument::type(DateRange::class))->willThrow( Exception::class - )->shouldBeCalledTimes(1); + )->shouldBeCalledOnce(); $response = $this->action->handle(ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode)); $this->assertEquals(500, $response->getStatusCode()); @@ -81,7 +81,7 @@ class GetVisitsActionTest extends TestCase $shortCode = 'abc123'; $this->visitsTracker->info($shortCode, new DateRange(null, Chronos::parse('2016-01-01 00:00:00'))) ->willReturn([]) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $response = $this->action->handle( ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode) diff --git a/module/Rest/test/Authentication/Plugin/ApiKeyHeaderPluginTest.php b/module/Rest/test/Authentication/Plugin/ApiKeyHeaderPluginTest.php index fef86e71..81256cfe 100644 --- a/module/Rest/test/Authentication/Plugin/ApiKeyHeaderPluginTest.php +++ b/module/Rest/test/Authentication/Plugin/ApiKeyHeaderPluginTest.php @@ -37,7 +37,7 @@ class ApiKeyHeaderPluginTest extends TestCase { $apiKey = 'abc-ABC'; $check = $this->apiKeyService->check($apiKey)->willReturn(false); - $check->shouldBeCalledTimes(1); + $check->shouldBeCalledOnce(); $this->expectException(VerifyAuthenticationException::class); $this->expectExceptionMessage('Provided API key does not exist or is invalid'); @@ -55,7 +55,7 @@ class ApiKeyHeaderPluginTest extends TestCase $this->plugin->verify($this->createRequest($apiKey)); - $check->shouldHaveBeenCalledTimes(1); + $check->shouldHaveBeenCalledOnce(); } /** diff --git a/module/Rest/test/Authentication/Plugin/AuthorizationHeaderPluginTest.php b/module/Rest/test/Authentication/Plugin/AuthorizationHeaderPluginTest.php index f5b97c2f..2a5c7723 100644 --- a/module/Rest/test/Authentication/Plugin/AuthorizationHeaderPluginTest.php +++ b/module/Rest/test/Authentication/Plugin/AuthorizationHeaderPluginTest.php @@ -90,7 +90,7 @@ class AuthorizationHeaderPluginTest extends TestCase $this->plugin->verify($request); - $jwtVerify->shouldHaveBeenCalledTimes(1); + $jwtVerify->shouldHaveBeenCalledOnce(); } /** @@ -107,7 +107,7 @@ class AuthorizationHeaderPluginTest extends TestCase $this->plugin->verify($request); - $jwtVerify->shouldHaveBeenCalledTimes(1); + $jwtVerify->shouldHaveBeenCalledOnce(); } /** @@ -126,6 +126,6 @@ class AuthorizationHeaderPluginTest extends TestCase $this->assertTrue($response->hasHeader(AuthorizationHeaderPlugin::HEADER_NAME)); $this->assertEquals('Bearer DEF-def', $response->getHeaderLine(AuthorizationHeaderPlugin::HEADER_NAME)); - $jwtRefresh->shouldHaveBeenCalledTimes(1); + $jwtRefresh->shouldHaveBeenCalledOnce(); } } diff --git a/module/Rest/test/Authentication/RequestToAuthPluginTest.php b/module/Rest/test/Authentication/RequestToAuthPluginTest.php index 454a93e4..59f1f5c1 100644 --- a/module/Rest/test/Authentication/RequestToAuthPluginTest.php +++ b/module/Rest/test/Authentication/RequestToAuthPluginTest.php @@ -64,7 +64,7 @@ class RequestToAuthPluginTest extends TestCase $this->requestToPlugin->fromRequest($request); - $getPlugin->shouldHaveBeenCalledTimes(1); + $getPlugin->shouldHaveBeenCalledOnce(); } public function provideHeaders(): array diff --git a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php index 7886133a..4e3c70f9 100644 --- a/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php +++ b/module/Rest/test/Middleware/AuthenticationMiddlewareTest.php @@ -68,7 +68,7 @@ class AuthenticationMiddlewareTest extends TestCase $this->middleware->process($request, $handler->reveal()); - $handle->shouldHaveBeenCalledTimes(1); + $handle->shouldHaveBeenCalledOnce(); $fromRequest->shouldNotHaveBeenCalled(); } @@ -116,7 +116,7 @@ class AuthenticationMiddlewareTest extends TestCase 'Expected one of the following authentication headers, but none were provided, ["%s"]', implode('", "', RequestToHttpAuthPlugin::SUPPORTED_AUTH_HEADERS) ), $payload['message']); - $fromRequest->shouldHaveBeenCalledTimes(1); + $fromRequest->shouldHaveBeenCalledOnce(); } public function provideExceptions(): array @@ -150,8 +150,8 @@ class AuthenticationMiddlewareTest extends TestCase $this->assertEquals('the_error', $payload['error']); $this->assertEquals('the_message', $payload['message']); - $verify->shouldHaveBeenCalledTimes(1); - $fromRequest->shouldHaveBeenCalledTimes(1); + $verify->shouldHaveBeenCalledOnce(); + $fromRequest->shouldHaveBeenCalledOnce(); } /** @@ -176,10 +176,10 @@ class AuthenticationMiddlewareTest extends TestCase $response = $this->middleware->process($request, $handler->reveal()); $this->assertSame($response, $newResponse); - $verify->shouldHaveBeenCalledTimes(1); - $update->shouldHaveBeenCalledTimes(1); - $handle->shouldHaveBeenCalledTimes(1); - $fromRequest->shouldHaveBeenCalledTimes(1); + $verify->shouldHaveBeenCalledOnce(); + $update->shouldHaveBeenCalledOnce(); + $handle->shouldHaveBeenCalledOnce(); + $fromRequest->shouldHaveBeenCalledOnce(); } private function getDummyMiddleware(): MiddlewareInterface diff --git a/module/Rest/test/Middleware/BodyParserMiddlewareTest.php b/module/Rest/test/Middleware/BodyParserMiddlewareTest.php index 2c09f4dc..2705e84b 100644 --- a/module/Rest/test/Middleware/BodyParserMiddlewareTest.php +++ b/module/Rest/test/Middleware/BodyParserMiddlewareTest.php @@ -38,7 +38,7 @@ class BodyParserMiddlewareTest extends TestCase $this->middleware->process($request, $delegate->reveal()); - $process->shouldHaveBeenCalledTimes(1); + $process->shouldHaveBeenCalledOnce(); } /** @@ -70,7 +70,7 @@ class BodyParserMiddlewareTest extends TestCase $this->middleware->process($request, $delegate->reveal()); - $process->shouldHaveBeenCalledTimes(1); + $process->shouldHaveBeenCalledOnce(); } /** @@ -101,6 +101,6 @@ class BodyParserMiddlewareTest extends TestCase $this->middleware->process($request, $delegate->reveal()); - $process->shouldHaveBeenCalledTimes(1); + $process->shouldHaveBeenCalledOnce(); } } diff --git a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php index 478113ff..ea5bf941 100644 --- a/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php +++ b/module/Rest/test/Middleware/CrossDomainMiddlewareTest.php @@ -34,7 +34,7 @@ class CrossDomainMiddlewareTest extends TestCase public function nonCrossDomainRequestsAreNotAffected() { $originalResponse = new Response(); - $this->delegate->handle(Argument::any())->willReturn($originalResponse)->shouldbeCalledTimes(1); + $this->delegate->handle(Argument::any())->willReturn($originalResponse)->shouldBeCalledOnce(); $response = $this->middleware->process(ServerRequestFactory::fromGlobals(), $this->delegate->reveal()); $this->assertSame($originalResponse, $response); @@ -50,7 +50,7 @@ class CrossDomainMiddlewareTest extends TestCase public function anyRequestIncludesTheAllowAccessHeader() { $originalResponse = new Response(); - $this->delegate->handle(Argument::any())->willReturn($originalResponse)->shouldbeCalledTimes(1); + $this->delegate->handle(Argument::any())->willReturn($originalResponse)->shouldBeCalledOnce(); $response = $this->middleware->process( ServerRequestFactory::fromGlobals()->withHeader('Origin', 'local'), @@ -70,7 +70,7 @@ class CrossDomainMiddlewareTest extends TestCase { $originalResponse = new Response(); $request = ServerRequestFactory::fromGlobals()->withMethod('OPTIONS')->withHeader('Origin', 'local'); - $this->delegate->handle(Argument::any())->willReturn($originalResponse)->shouldbeCalledTimes(1); + $this->delegate->handle(Argument::any())->willReturn($originalResponse)->shouldBeCalledOnce(); $response = $this->middleware->process($request, $this->delegate->reveal()); $this->assertNotSame($originalResponse, $response); diff --git a/module/Rest/test/Middleware/ShortUrl/ShortCodePathMiddlewareTest.php b/module/Rest/test/Middleware/ShortUrl/ShortCodePathMiddlewareTest.php index d28acf40..234f133a 100644 --- a/module/Rest/test/Middleware/ShortUrl/ShortCodePathMiddlewareTest.php +++ b/module/Rest/test/Middleware/ShortUrl/ShortCodePathMiddlewareTest.php @@ -45,6 +45,6 @@ class ShortCodePathMiddlewareTest extends TestCase $this->middleware->process($request->reveal(), $this->requestHandler->reveal()); - $withUri->shouldHaveBeenCalledTimes(1); + $withUri->shouldHaveBeenCalledOnce(); } } diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php index 1faa112f..74109eca 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -34,8 +34,8 @@ class ApiKeyServiceTest extends TestCase */ public function keyIsProperlyCreated() { - $this->em->flush()->shouldBeCalledTimes(1); - $this->em->persist(Argument::type(ApiKey::class))->shouldBeCalledTimes(1); + $this->em->flush()->shouldBeCalledOnce(); + $this->em->persist(Argument::type(ApiKey::class))->shouldBeCalledOnce(); $key = $this->service->create(); $this->assertNull($key->getExpirationDate()); @@ -46,8 +46,8 @@ class ApiKeyServiceTest extends TestCase */ public function keyIsProperlyCreatedWithExpirationDate() { - $this->em->flush()->shouldBeCalledTimes(1); - $this->em->persist(Argument::type(ApiKey::class))->shouldBeCalledTimes(1); + $this->em->flush()->shouldBeCalledOnce(); + $this->em->persist(Argument::type(ApiKey::class))->shouldBeCalledOnce(); $date = Chronos::parse('2030-01-01'); $key = $this->service->create($date); @@ -61,7 +61,7 @@ class ApiKeyServiceTest extends TestCase { $repo = $this->prophesize(EntityRepository::class); $repo->findOneBy(['key' => '12345'])->willReturn(null) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); $this->assertFalse($this->service->check('12345')); @@ -76,7 +76,7 @@ class ApiKeyServiceTest extends TestCase $key->disable(); $repo = $this->prophesize(EntityRepository::class); $repo->findOneBy(['key' => '12345'])->willReturn($key) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); $this->assertFalse($this->service->check('12345')); @@ -90,7 +90,7 @@ class ApiKeyServiceTest extends TestCase $key = new ApiKey(Chronos::now()->subDay()); $repo = $this->prophesize(EntityRepository::class); $repo->findOneBy(['key' => '12345'])->willReturn($key) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); $this->assertFalse($this->service->check('12345')); @@ -103,7 +103,7 @@ class ApiKeyServiceTest extends TestCase { $repo = $this->prophesize(EntityRepository::class); $repo->findOneBy(['key' => '12345'])->willReturn(new ApiKey()) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); $this->assertTrue($this->service->check('12345')); @@ -117,7 +117,7 @@ class ApiKeyServiceTest extends TestCase { $repo = $this->prophesize(EntityRepository::class); $repo->findOneBy(['key' => '12345'])->willReturn(null) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); $this->service->disable('12345'); @@ -131,10 +131,10 @@ class ApiKeyServiceTest extends TestCase $key = new ApiKey(); $repo = $this->prophesize(EntityRepository::class); $repo->findOneBy(['key' => '12345'])->willReturn($key) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); - $this->em->flush()->shouldBeCalledTimes(1); + $this->em->flush()->shouldBeCalledOnce(); $this->assertTrue($key->isEnabled()); $returnedKey = $this->service->disable('12345'); @@ -149,7 +149,7 @@ class ApiKeyServiceTest extends TestCase { $repo = $this->prophesize(EntityRepository::class); $repo->findBy([])->willReturn([]) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); $this->service->listKeys(); @@ -162,7 +162,7 @@ class ApiKeyServiceTest extends TestCase { $repo = $this->prophesize(EntityRepository::class); $repo->findBy(['enabled' => true])->willReturn([]) - ->shouldBeCalledTimes(1); + ->shouldBeCalledOnce(); $this->em->getRepository(ApiKey::class)->willReturn($repo->reveal()); $this->service->listKeys(true); From 0e3a0a1eecb744a92ebf0dcf6c23d1f0bdd219e9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Nov 2018 18:36:56 +0100 Subject: [PATCH 06/19] Created chain IP resolver which wrapps multiple resolver to fallback until one is capable of resolving an address --- .../ChainIpLocationResolverTest.php | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 module/Common/test/IpGeolocation/ChainIpLocationResolverTest.php diff --git a/module/Common/test/IpGeolocation/ChainIpLocationResolverTest.php b/module/Common/test/IpGeolocation/ChainIpLocationResolverTest.php new file mode 100644 index 00000000..b4933f21 --- /dev/null +++ b/module/Common/test/IpGeolocation/ChainIpLocationResolverTest.php @@ -0,0 +1,86 @@ +firstInnerResolver = $this->prophesize(IpLocationResolverInterface::class); + $this->secondInnerResolver = $this->prophesize(IpLocationResolverInterface::class); + + $this->resolver = new ChainIpLocationResolver( + $this->firstInnerResolver->reveal(), + $this->secondInnerResolver->reveal() + ); + } + + /** + * @test + */ + public function throwsExceptionWhenNoInnerResolverCanHandleTheResolution() + { + $ipAddress = '1.2.3.4'; + + $firstResolve = $this->firstInnerResolver->resolveIpLocation($ipAddress)->willThrow(WrongIpException::class); + $secondResolve = $this->secondInnerResolver->resolveIpLocation($ipAddress)->willThrow(WrongIpException::class); + + $this->expectException(WrongIpException::class); + $firstResolve->shouldBeCalledOnce(); + $secondResolve->shouldBeCalledOnce(); + + $this->resolver->resolveIpLocation($ipAddress); + } + + /** + * @test + */ + public function returnsResultOfFirstInnerResolver() + { + $ipAddress = '1.2.3.4'; + + $firstResolve = $this->firstInnerResolver->resolveIpLocation($ipAddress)->willReturn([]); + $secondResolve = $this->secondInnerResolver->resolveIpLocation($ipAddress)->willThrow(WrongIpException::class); + + $this->resolver->resolveIpLocation($ipAddress); + + $firstResolve->shouldHaveBeenCalledOnce(); + $secondResolve->shouldNotHaveBeenCalled(); + } + + /** + * @test + */ + public function returnsResultOfSecondInnerResolver() + { + $ipAddress = '1.2.3.4'; + + $firstResolve = $this->firstInnerResolver->resolveIpLocation($ipAddress)->willThrow(WrongIpException::class); + $secondResolve = $this->secondInnerResolver->resolveIpLocation($ipAddress)->willReturn([]); + + $this->resolver->resolveIpLocation($ipAddress); + + $firstResolve->shouldHaveBeenCalledOnce(); + $secondResolve->shouldHaveBeenCalledOnce(); + } +} From 9a0f9207bee82a78deb14f3db73389bb699286bd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Nov 2018 18:57:32 +0100 Subject: [PATCH 07/19] Fixed region resolved in GeoLite2 --- composer.json | 1 + .../src/IpGeolocation/GeoLite2LocationResolver.php | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index 86b52828..6c49c9e4 100644 --- a/composer.json +++ b/composer.json @@ -48,6 +48,7 @@ "zendframework/zend-stdlib": "^3.0" }, "require-dev": { + "devster/ubench": "^2.0", "filp/whoops": "^2.0", "infection/infection": "^0.11.0", "phpstan/phpstan": "^0.10.0", diff --git a/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php b/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php index 60023ee3..0bced338 100644 --- a/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php +++ b/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php @@ -6,8 +6,10 @@ namespace Shlinkio\Shlink\Common\IpGeolocation; use GeoIp2\Database\Reader; use GeoIp2\Exception\AddressNotFoundException; use GeoIp2\Model\City; +use GeoIp2\Record\Subdivision; use MaxMind\Db\Reader\InvalidDatabaseException; use Shlinkio\Shlink\Common\Exception\WrongIpException; +use function Functional\first; class GeoLite2LocationResolver implements IpLocationResolverInterface { @@ -40,13 +42,16 @@ class GeoLite2LocationResolver implements IpLocationResolverInterface private function mapFields(City $city): array { + /** @var Subdivision $region */ + $region = first($city->subdivisions); + return [ 'country_code' => $city->country->isoCode ?? '', 'country_name' => $city->country->name ?? '', - 'region_name' => $city->mostSpecificSubdivision->name ?? '', + 'region_name' => $region->name ?? '', 'city' => $city->city->name ?? '', - 'latitude' => (string) $city->location->latitude, // FIXME Cast to string for BC compatibility - 'longitude' => (string) $city->location->longitude, // FIXME Cast to string for BC compatibility + 'latitude' => $city->location->latitude ?? '', + 'longitude' => $city->location->longitude ?? '', 'time_zone' => $city->location->timeZone ?? '', ]; } From 4a383cecaf0137414bb60b1cacfeef336356cef3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Nov 2018 19:04:59 +0100 Subject: [PATCH 08/19] Set chain IP resolver as the default IP resolver --- module/CLI/src/Command/Visit/ProcessVisitsCommand.php | 2 +- module/Common/config/dependencies.config.php | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php index 305042e7..24e6e055 100644 --- a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php +++ b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php @@ -83,7 +83,7 @@ class ProcessVisitsCommand extends Command $io->writeln(sprintf( ' [' . $this->translator->translate('Address located at "%s"') . ']', - $location->getCityName() + $location->getCountryName() )); } catch (WrongIpException $e) { $io->writeln( diff --git a/module/Common/config/dependencies.config.php b/module/Common/config/dependencies.config.php index deae26d8..4ba6249d 100644 --- a/module/Common/config/dependencies.config.php +++ b/module/Common/config/dependencies.config.php @@ -44,10 +44,12 @@ return [ 'em' => EntityManager::class, 'httpClient' => GuzzleClient::class, 'translator' => Translator::class, + 'logger' => LoggerInterface::class, Logger::class => 'Logger_Shlink', LoggerInterface::class => 'Logger_Shlink', - IpGeolocation\IpLocationResolverInterface::class => IpGeolocation\GeoLite2LocationResolver::class, + + IpGeolocation\IpLocationResolverInterface::class => IpGeolocation\ChainIpLocationResolver::class, ], 'abstract_factories' => [ Factory\DottedAccessConfigAbstractFactory::class, From 06db082e3fa5f6f9e7fb07c8a50d8596b95bd8e1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Nov 2018 19:19:15 +0100 Subject: [PATCH 09/19] Updated translations --- module/CLI/lang/es.mo | Bin 9997 -> 9959 bytes module/CLI/lang/es.po | 18 ++++++++++-------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/module/CLI/lang/es.mo b/module/CLI/lang/es.mo index c2ea3578657240217e47a928f5681f8fbc3f02ca..c16c9d9f8f47a7b6f4ac85512be6f1a3a711e537 100644 GIT binary patch delta 1571 zcmXZcT}TvB6u|MLwYz?oW?7mwYF2(bS!o4niJ6w#W`QL=h}+hrsAwf&vP4$;poD|< z5EQ)y7Ey~r5cCiRR!~6+)tgZo5|l4R5%oXrEVJ{wb7$t>bMHC3gS9g?SNj6FBI1b> zS%#Z20r#Q@8*vpj<4Wws#dr;4@CNF8cTk^ujEOjf2F{@iW1>ZtU;?IKInWY*FaXa?l9vnxV@Glye6f4q+CCHpIias32MVJy73Un=|ab1Ku z-{H7`NCb^jbVOp8bE6lDL55LRcoUoOA?9Nu%VBIOM;*wE#puUeyom+)0&DRP>W)?~ z4F!G>OSx_h(AY@hE^^2_P8!G;OvdlH8KdJv*JY?HY{cd0M@`9j=lX)As-O0eP)A1hb=|++uo34_ z1E?fFydpKIsW^oi;0S6hjiILG3F>ptQBymE0n#FWX|!SjZ!HuBYIQ$Dz5f==@T+6C z5vtx6JVO5{y3s|nG$jVTn1?0?P%J6ZOidqeyVTIGC6!@@p^J>l+-7tD_83%6%u z{0Tnetkq?mHoC3TCylmtqv}AgG`2V*&NTB((=fMfH%oE~&0yd1p2bo5M%9V7_Rc0> od+^_?ui3i^2r9~-!Z66VFvqqTmC};7Yy=4B z0*pqt05*E#g>lKkl@L?8An1*WCR}*og&|~K2+6=jiAEy?qKV&MdzS8e-uFG{yzlcq z&)eSLJ+;#2=(4)sOL(!0)IdU?_v!u zs5e`Rb+{HAu^Bhkn`f(O>}NnPzJu?hhwE?=tGtFCxCL_<#V_z>ypLKq667AfimYij zFoECW0t|;Lf%f2f`rA?KB|{#I&^X0F9iH=VOd&DYG-`)e(8X^tf^BSvxoreBk&Byg z9G}B0*n_jU2kVzrGCGP1_-!1*Gaijj8ebzHo8zKD9%2((gW2=27S)fTc6bz5;y9`# zlYak_?`3?3@o%snf5BC_occ~+H!6ePv_CM9dl_)qZa033THt3?DgN?pV7tmdFRFBJ zp`M?>L7YYfIEz%r?x6zt8@q57lisT`+lpkuvkqoaO@?th#&Hxc<1Vb`#W>gTweMzK-2Q zrHIGzI9@=V5!K@`G@t@FhP+~VR4Fc^0=$7bOS7nw%%Pt96II$E4|$ZuT4|(l5S8i~ z)am{mb^i%&#idTg7!J|Dgop4)3}Bc#(zJfuha=dBGpG#QLIw02>P-Bzp8RX)Yv^g< zFc&R+*6&ZEUbu?HVAuWe+Zd+*FK)w5e#O|79mR9_0ltZ$HnaC|40q!nxCviuuN>Y& zJ12%YY?1*!c9n}J{t1<;Dhj0+)?pOiK)y+K0af$Qk&k`FMLV4>-mGb@YA-&h>8X09 z*b?Xnh8V5kzx-F+7w8G!J8i|OK!+wQAAM~>dFm>D7nrDfftC(*b7@EITy<$+ z@%ie(n3K-N-9jpxadOFgHa(uqIqB4~RDrG=KaxyDoP%zvkjjiX?Rh7kjAt{6d?XSn zUR!dc^ir^MQK&b%p>LDZH@IbBbGWy^R9Ja>Vd-(>eD&}yO_OvI$#i**bn29w$mZxf nJ7amrO~>7gMk3CR{PASY%_OpwCB{zBEU$5=IbF)OoUHpFiJi*_ diff --git a/module/CLI/lang/es.po b/module/CLI/lang/es.po index d662948f..8a52fed2 100644 --- a/module/CLI/lang/es.po +++ b/module/CLI/lang/es.po @@ -1,8 +1,8 @@ msgid "" msgstr "" "Project-Id-Version: Shlink 1.0\n" -"POT-Creation-Date: 2018-09-16 18:36+0200\n" -"PO-Revision-Date: 2018-09-16 18:37+0200\n" +"POT-Creation-Date: 2018-11-11 19:17+0100\n" +"PO-Revision-Date: 2018-11-11 19:18+0100\n" "Last-Translator: Alejandro Celaya \n" "Language-Team: \n" "Language: es_ES\n" @@ -340,6 +340,9 @@ msgstr "Una etiqueta con nombre \"%s\" no ha sido encontrada" msgid "Processes visits where location is not set yet" msgstr "Procesa las visitas donde la localización no ha sido establecida aún" +msgid "Ignored visit with no IP address" +msgstr "Ignorada visita sin dirección IP" + msgid "Processing IP" msgstr "Procesando IP" @@ -350,16 +353,15 @@ msgstr "Ignorada IP de localhost" msgid "Address located at \"%s\"" msgstr "Dirección localizada en \"%s\"" -msgid "An error occurred while locating IP" -msgstr "Se produjo un error al localizar la IP" - -#, php-format -msgid "IP location resolver limit reached. Waiting %s seconds..." -msgstr "Limite del localizador de IPs alcanzado. Esperando %s segundos..." +msgid "An error occurred while locating IP. Skipped" +msgstr "Se produjo un error al localizar la IP. Ignorado" msgid "Finished processing all IPs" msgstr "Finalizado el procesado de todas las IPs" +#~ msgid "IP location resolver limit reached. Waiting %s seconds..." +#~ msgstr "Limite del localizador de IPs alcanzado. Esperando %s segundos..." + #~ msgid "Remote Address" #~ msgstr "Dirección remota" From 3d7cf6992e3f07b1845dd8a814f72d11d4a10dda Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 11 Nov 2018 21:08:23 +0100 Subject: [PATCH 10/19] Created service to update geolite2 database file --- config/autoload/geolite2.global.php | 2 + module/Common/config/dependencies.config.php | 8 ++ .../src/IpGeolocation/GeoLite2/DbUpdater.php | 105 +++++++++++++++ .../GeoLite2/DbUpdaterInterface.php | 14 ++ .../GeoLite2/GeoLite2Options.php | 46 +++++++ module/Common/test-resources/.gitignore | 1 + .../test-resources/GeoLite2-City.tar.gz | Bin 0 -> 202 bytes .../IpGeolocation/GeoLite2/DbUpdaterTest.php | 126 ++++++++++++++++++ 8 files changed, 302 insertions(+) create mode 100644 module/Common/src/IpGeolocation/GeoLite2/DbUpdater.php create mode 100644 module/Common/src/IpGeolocation/GeoLite2/DbUpdaterInterface.php create mode 100644 module/Common/src/IpGeolocation/GeoLite2/GeoLite2Options.php create mode 100644 module/Common/test-resources/.gitignore create mode 100644 module/Common/test-resources/GeoLite2-City.tar.gz create mode 100644 module/Common/test/IpGeolocation/GeoLite2/DbUpdaterTest.php diff --git a/config/autoload/geolite2.global.php b/config/autoload/geolite2.global.php index 0536a0be..da52a62c 100644 --- a/config/autoload/geolite2.global.php +++ b/config/autoload/geolite2.global.php @@ -5,6 +5,8 @@ return [ 'geolite2' => [ 'db_location' => __DIR__ . '/../../data/GeoLite2-City.mmdb', + 'temp_dir' => sys_get_temp_dir(), + 'download_from' => 'http://geolite.maxmind.com/download/geoip/database/GeoLite2-City.tar.gz', ], ]; diff --git a/module/Common/config/dependencies.config.php b/module/Common/config/dependencies.config.php index 4ba6249d..8ac6311b 100644 --- a/module/Common/config/dependencies.config.php +++ b/module/Common/config/dependencies.config.php @@ -37,6 +37,8 @@ return [ IpGeolocation\IpApiLocationResolver::class => ConfigAbstractFactory::class, IpGeolocation\GeoLite2LocationResolver::class => ConfigAbstractFactory::class, IpGeolocation\ChainIpLocationResolver::class => ConfigAbstractFactory::class, + IpGeolocation\GeoLite2\GeoLite2Options::class => ConfigAbstractFactory::class, + IpGeolocation\GeoLite2\DbUpdater::class => ConfigAbstractFactory::class, Service\PreviewGenerator::class => ConfigAbstractFactory::class, ], @@ -68,6 +70,12 @@ return [ IpGeolocation\GeoLite2LocationResolver::class, IpGeolocation\IpApiLocationResolver::class, ], + IpGeolocation\GeoLite2\GeoLite2Options::class => ['config.geolite2'], + IpGeolocation\GeoLite2\DbUpdater::class => [ + GuzzleClient::class, + Filesystem::class, + IpGeolocation\GeoLite2\GeoLite2Options::class, + ], Service\PreviewGenerator::class => [ Image\ImageBuilder::class, diff --git a/module/Common/src/IpGeolocation/GeoLite2/DbUpdater.php b/module/Common/src/IpGeolocation/GeoLite2/DbUpdater.php new file mode 100644 index 00000000..e8628cf2 --- /dev/null +++ b/module/Common/src/IpGeolocation/GeoLite2/DbUpdater.php @@ -0,0 +1,105 @@ +httpClient = $httpClient; + $this->filesystem = $filesystem; + $this->options = $options; + } + + /** + * @throws RuntimeException + */ + public function downloadFreshCopy(): void + { + $tempDir = $this->options->getTempDir(); + $compressedFile = sprintf('%s/%s', $tempDir, self::DB_COMPRESSED_FILE); + + $this->downloadDbFile($compressedFile); + $tempFullPath = $this->extractDbFile($compressedFile, $tempDir); + $this->copyNewDbFile($tempFullPath); + $this->deleteTempFiles([$compressedFile, $tempFullPath]); + } + + private function downloadDbFile(string $dest): void + { + try { + $this->httpClient->request(RequestMethod::METHOD_GET, $this->options->getDownloadFrom(), [ + RequestOptions::SINK => $dest, + ]); + } catch (Throwable | GuzzleException $e) { + throw new RuntimeException( + 'An error occurred while trying to download a fresh copy of the GeoLite2 database', + 0, + $e + ); + } + } + + private function extractDbFile(string $compressedFile, string $tempDir): string + { + try { + $phar = new PharData($compressedFile); + $internalPathToDb = sprintf('%s/%s', $phar->getBasename(), self::DB_DECOMPRESSED_FILE); + $phar->extractTo($tempDir, $internalPathToDb, true); + + return sprintf('%s/%s', $tempDir, $internalPathToDb); + } catch (Throwable $e) { + throw new RuntimeException( + sprintf('An error occurred while trying to extract the GeoLite2 database from %s', $compressedFile), + 0, + $e + ); + } + } + + private function copyNewDbFile(string $from): void + { + try { + $this->filesystem->copy($from, $this->options->getDbLocation(), true); + } catch (FilesystemException\FileNotFoundException | FilesystemException\IOException $e) { + throw new RuntimeException('An error occurred while trying to copy GeoLite2 db file to destination', 0, $e); + } + } + + private function deleteTempFiles(array $files): void + { + try { + $this->filesystem->remove($files); + } catch (FilesystemException\IOException $e) { + // Ignore any error produced when trying to delete temp files + } + } +} diff --git a/module/Common/src/IpGeolocation/GeoLite2/DbUpdaterInterface.php b/module/Common/src/IpGeolocation/GeoLite2/DbUpdaterInterface.php new file mode 100644 index 00000000..1fad0825 --- /dev/null +++ b/module/Common/src/IpGeolocation/GeoLite2/DbUpdaterInterface.php @@ -0,0 +1,14 @@ +dbLocation; + } + + protected function setDbLocation(string $dbLocation): self + { + $this->dbLocation = $dbLocation; + return $this; + } + + public function getTempDir(): string + { + return $this->tempDir; + } + + protected function setTempDir(string $tempDir): self + { + $this->tempDir = $tempDir; + return $this; + } + + public function getDownloadFrom(): string + { + return $this->downloadFrom; + } + + protected function setDownloadFrom(string $downloadFrom): self + { + $this->downloadFrom = $downloadFrom; + return $this; + } +} diff --git a/module/Common/test-resources/.gitignore b/module/Common/test-resources/.gitignore new file mode 100644 index 00000000..3ffd27ba --- /dev/null +++ b/module/Common/test-resources/.gitignore @@ -0,0 +1 @@ +geolite2-testing-db diff --git a/module/Common/test-resources/GeoLite2-City.tar.gz b/module/Common/test-resources/GeoLite2-City.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..c945d2954ff2b596fd58f14cfe460588e2d100f8 GIT binary patch literal 202 zcmb2|=3vn2dJ)aQ{Pxm$u0sk84Ta}zTlE80&i=_DyQ}0ir`YDywY<}0tA20OcyYl# zL*U}~-5gpgYQ*;GIe$r$_-og;O5)k+jW1?qS*Gs}v|gQeTzlchttpClient = $this->prophesize(ClientInterface::class); + $this->filesystem = $this->prophesize(Filesystem::class); + $this->options = new GeoLite2Options([ + 'temp_dir' => __DIR__ . '/../../../test-resources', + 'db_location' => '', + 'download_from' => '', + ]); + + $this->dbUpdater = new DbUpdater($this->httpClient->reveal(), $this->filesystem->reveal(), $this->options); + } + + /** + * @test + */ + public function anExceptionIsThrownIfFreshDbCannotBeDownloaded() + { + $request = $this->httpClient->request(Argument::cetera())->willThrow(ClientException::class); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage( + 'An error occurred while trying to download a fresh copy of the GeoLite2 database' + ); + $request->shouldBeCalledOnce(); + + $this->dbUpdater->downloadFreshCopy(); + } + + /** + * @test + */ + public function anExceptionIsThrownIfFreshDbCannotBeExtracted() + { + $this->options->tempDir = '__invalid__'; + + $request = $this->httpClient->request(Argument::cetera())->willReturn(new Response()); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage( + 'An error occurred while trying to extract the GeoLite2 database from __invalid__/GeoLite2-City.tar.gz' + ); + $request->shouldBeCalledOnce(); + + $this->dbUpdater->downloadFreshCopy(); + } + + /** + * @test + * @dataProvider provideFilesystemExceptions + */ + public function anExceptionIsThrownIfFreshDbCannotBeCopiedToDestination(string $e) + { + $request = $this->httpClient->request(Argument::cetera())->willReturn(new Response()); + $copy = $this->filesystem->copy(Argument::cetera())->willThrow($e); + + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('An error occurred while trying to copy GeoLite2 db file to destination'); + $request->shouldBeCalledOnce(); + $copy->shouldBeCalledOnce(); + + $this->dbUpdater->downloadFreshCopy(); + } + + public function provideFilesystemExceptions(): array + { + return [ + [FilesystemException\FileNotFoundException::class], + [FilesystemException\IOException::class], + ]; + } + + /** + * @test + */ + public function noExceptionsAreThrownIfEverythingWorksFine() + { + $request = $this->httpClient->request(Argument::cetera())->willReturn(new Response()); + $copy = $this->filesystem->copy(Argument::cetera())->will(function () { + }); + $remove = $this->filesystem->remove(Argument::cetera())->will(function () { + }); + + $this->dbUpdater->downloadFreshCopy(); + + $request->shouldHaveBeenCalledOnce(); + $copy->shouldHaveBeenCalledOnce(); + $remove->shouldHaveBeenCalledOnce(); + } +} From de0470d200027e1e4c6f6143b08715dffef7403c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 12 Nov 2018 20:06:12 +0100 Subject: [PATCH 11/19] Created command to update GeoLite2 database --- module/CLI/config/cli.config.php | 1 + module/CLI/config/dependencies.config.php | 3 + .../Command/Visit/ProcessVisitsCommand.php | 2 +- .../CLI/src/Command/Visit/UpdateDbCommand.php | 61 +++++++++++++++++++ 4 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 module/CLI/src/Command/Visit/UpdateDbCommand.php diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index fb07cb47..42b8ab4c 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -18,6 +18,7 @@ return [ Command\ShortUrl\DeleteShortUrlCommand::NAME => Command\ShortUrl\DeleteShortUrlCommand::class, Command\Visit\ProcessVisitsCommand::NAME => Command\Visit\ProcessVisitsCommand::class, + Command\Visit\UpdateDbCommand::NAME => Command\Visit\UpdateDbCommand::class, Command\Config\GenerateCharsetCommand::NAME => Command\Config\GenerateCharsetCommand::class, Command\Config\GenerateSecretCommand::NAME => Command\Config\GenerateSecretCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index b226933c..8a31cb02 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -3,6 +3,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI; +use Shlinkio\Shlink\Common\IpGeolocation\GeoLite2\DbUpdater; use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface; use Shlinkio\Shlink\Common\Service\PreviewGenerator; use Shlinkio\Shlink\Core\Service; @@ -25,6 +26,7 @@ return [ Command\ShortUrl\DeleteShortUrlCommand::class => ConfigAbstractFactory::class, Command\Visit\ProcessVisitsCommand::class => ConfigAbstractFactory::class, + Command\Visit\UpdateDbCommand::class => ConfigAbstractFactory::class, Command\Config\GenerateCharsetCommand::class => ConfigAbstractFactory::class, Command\Config\GenerateSecretCommand::class => ConfigAbstractFactory::class, @@ -68,6 +70,7 @@ return [ IpLocationResolverInterface::class, 'translator', ], + Command\Visit\UpdateDbCommand::class => [DbUpdater::class, 'translator'], Command\Config\GenerateCharsetCommand::class => ['translator'], Command\Config\GenerateSecretCommand::class => ['translator'], diff --git a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php index 24e6e055..a5a33fcb 100644 --- a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php +++ b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php @@ -40,7 +40,7 @@ class ProcessVisitsCommand extends Command $this->visitService = $visitService; $this->ipLocationResolver = $ipLocationResolver; $this->translator = $translator; - parent::__construct(null); + parent::__construct(); } protected function configure(): void diff --git a/module/CLI/src/Command/Visit/UpdateDbCommand.php b/module/CLI/src/Command/Visit/UpdateDbCommand.php new file mode 100644 index 00000000..ca4963cf --- /dev/null +++ b/module/CLI/src/Command/Visit/UpdateDbCommand.php @@ -0,0 +1,61 @@ +geoLiteDbUpdater = $geoLiteDbUpdater; + $this->translator = $translator; + parent::__construct(); + } + + protected function configure(): void + { + $this + ->setName(self::NAME) + ->setDescription( + $this->translator->translate('Updates the GeoLite2 database file used to geolocate IP addresses') + ) + ->setHelp($this->translator->translate( + 'The GeoLite2 database is updated first Tuesday every month, so this command should be ideally run ' + . 'every first Wednesday' + )); + } + + protected function execute(InputInterface $input, OutputInterface $output): void + { + $io = new SymfonyStyle($input, $output); + + try { + $this->geoLiteDbUpdater->downloadFreshCopy(); + $io->success($this->translator->translate('GeoLite2 database properly updated')); + } catch (RuntimeException $e) { + $io->error($this->translator->translate('An error occurred while updating GeoLite2 database')); + if ($io->isVerbose()) { + $this->getApplication()->renderException($e, $output); + } + } + } +} From e915b7e4998bf88cd7734d835f5787826dbaa680 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 12 Nov 2018 20:22:42 +0100 Subject: [PATCH 12/19] Updated GeoLite2 db reader service so that it is lazily created --- config/autoload/dependencies.global.php | 6 ++++++ config/autoload/dependencies.local.php.dist | 12 ++++++++++++ module/Common/config/dependencies.config.php | 14 ++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 config/autoload/dependencies.local.php.dist diff --git a/config/autoload/dependencies.global.php b/config/autoload/dependencies.global.php index aa105e6d..cda3aeea 100644 --- a/config/autoload/dependencies.global.php +++ b/config/autoload/dependencies.global.php @@ -23,6 +23,12 @@ return [ Container\ApplicationConfigInjectionDelegator::class, ], ], + + 'lazy_services' => [ + 'proxies_target_dir' => 'data/proxies', + 'proxies_namespace' => 'ShlinkProxy', + 'write_proxy_files' => true, + ], ], ]; diff --git a/config/autoload/dependencies.local.php.dist b/config/autoload/dependencies.local.php.dist new file mode 100644 index 00000000..5ce9874b --- /dev/null +++ b/config/autoload/dependencies.local.php.dist @@ -0,0 +1,12 @@ + [ + 'lazy_services' => [ + 'write_proxy_files' => false, + ], + ], + +]; diff --git a/module/Common/config/dependencies.config.php b/module/Common/config/dependencies.config.php index 8ac6311b..a43d1e5c 100644 --- a/module/Common/config/dependencies.config.php +++ b/module/Common/config/dependencies.config.php @@ -14,6 +14,7 @@ use Symfony\Component\Filesystem\Filesystem; use Zend\I18n\Translator\Translator; use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory; use Zend\ServiceManager\Factory\InvokableFactory; +use Zend\ServiceManager\Proxy\LazyServiceFactory; return [ @@ -56,6 +57,19 @@ return [ 'abstract_factories' => [ Factory\DottedAccessConfigAbstractFactory::class, ], + 'delegators' => [ + // The GeoLite2 db reader has to be lazy so that it does not try to load the DB file at app bootstrapping. + // By doing so, it would fail the first time shlink tries to download it. + Reader::class => [ + LazyServiceFactory::class, + ], + ], + + 'lazy_services' => [ + 'class_map' => [ + Reader::class => Reader::class, + ], + ], ], ConfigAbstractFactory::class => [ From bf56e6adaf31856b31366a3995af4e6d633c4417 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 12 Nov 2018 20:37:30 +0100 Subject: [PATCH 13/19] Created UpdateDbCommandTest --- .../Visit/ProcessVisitsCommandTest.php | 6 +- .../Command/Visit/UpdateDbCommandTest.php | 65 +++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 module/CLI/test/Command/Visit/UpdateDbCommandTest.php diff --git a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php index 6988fb7c..5a28e5ab 100644 --- a/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php @@ -23,15 +23,15 @@ class ProcessVisitsCommandTest extends TestCase /** * @var CommandTester */ - protected $commandTester; + private $commandTester; /** * @var ObjectProphecy */ - protected $visitService; + private $visitService; /** * @var ObjectProphecy */ - protected $ipResolver; + private $ipResolver; public function setUp() { diff --git a/module/CLI/test/Command/Visit/UpdateDbCommandTest.php b/module/CLI/test/Command/Visit/UpdateDbCommandTest.php new file mode 100644 index 00000000..eeb4f1ff --- /dev/null +++ b/module/CLI/test/Command/Visit/UpdateDbCommandTest.php @@ -0,0 +1,65 @@ +dbUpdater = $this->prophesize(DbUpdaterInterface::class); + + $command = new UpdateDbCommand($this->dbUpdater->reveal(), Translator::factory([])); + $app = new Application(); + $app->add($command); + + $this->commandTester = new CommandTester($command); + } + + /** + * @test + */ + public function successMessageIsPrintedIfEverythingWorks() + { + $download = $this->dbUpdater->downloadFreshCopy()->will(function () { + }); + + $this->commandTester->execute([]); + $output = $this->commandTester->getDisplay(); + + $this->assertContains('GeoLite2 database properly updated', $output); + $download->shouldHaveBeenCalledOnce(); + } + + /** + * @test + */ + public function errorMessageIsPrintedIfAnExceptionIsThrown() + { + $download = $this->dbUpdater->downloadFreshCopy()->willThrow(RuntimeException::class); + + $this->commandTester->execute([]); + $output = $this->commandTester->getDisplay(); + + $this->assertContains('An error occurred while updating GeoLite2 database', $output); + $download->shouldHaveBeenCalledOnce(); + } +} From 1aa78f766a58207ff2ac5f596244a58aa1b0e0c2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 12 Nov 2018 20:51:53 +0100 Subject: [PATCH 14/19] Added step to download GeoLite2 db during installation --- .../Installer/src/Command/InstallCommand.php | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/module/Installer/src/Command/InstallCommand.php b/module/Installer/src/Command/InstallCommand.php index a3807a6b..f3d9a23c 100644 --- a/module/Installer/src/Command/InstallCommand.php +++ b/module/Installer/src/Command/InstallCommand.php @@ -10,7 +10,6 @@ use Shlinkio\Shlink\Installer\Config\Plugin; use Shlinkio\Shlink\Installer\Model\CustomizableAppConfig; use Shlinkio\Shlink\Installer\Util\AskUtilsTrait; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Exception\InvalidArgumentException; use Symfony\Component\Console\Exception\LogicException; use Symfony\Component\Console\Exception\RuntimeException; use Symfony\Component\Console\Helper\ProcessHelper; @@ -149,7 +148,7 @@ class InstallCommand extends Command // If current command is not update, generate database if (! $this->isUpdate) { $this->io->write('Initializing database...'); - if (! $this->runPhpCommand( + if (! $this->execPhp( 'vendor/doctrine/orm/bin/doctrine.php orm:schema-tool:create', 'Error generating database.', $output @@ -160,7 +159,7 @@ class InstallCommand extends Command // Run database migrations $this->io->write('Updating database...'); - if (! $this->runPhpCommand( + if (! $this->execPhp( 'vendor/doctrine/migrations/bin/doctrine-migrations.php migrations:migrate', 'Error updating database.', $output @@ -170,7 +169,7 @@ class InstallCommand extends Command // Generate proxies $this->io->write('Generating proxies...'); - if (! $this->runPhpCommand( + if (! $this->execPhp( 'vendor/doctrine/orm/bin/doctrine.php orm:generate-proxies', 'Error generating proxies.', $output @@ -178,6 +177,12 @@ class InstallCommand extends Command return; } + // Download GeoLite2 db filte + $this->io->write('Downloading GeoLite2 db...'); + if (! $this->execPhp('bin/cli visit:update-db', 'Error downloading GeoLite2 db.', $output)) { + return; + } + $this->io->success('Installation complete!'); } @@ -226,15 +231,7 @@ class InstallCommand extends Command return $config; } - /** - * @param string $command - * @param string $errorMessage - * @param OutputInterface $output - * @return bool - * @throws LogicException - * @throws InvalidArgumentException - */ - private function runPhpCommand($command, $errorMessage, OutputInterface $output): bool + private function execPhp(string $command, string $errorMessage, OutputInterface $output): bool { if ($this->processHelper === null) { $this->processHelper = $this->getHelper('process'); @@ -256,7 +253,7 @@ class InstallCommand extends Command } if (! $this->io->isVerbose()) { - $this->io->error($errorMessage . ' Run this command with -vvv to see specific error info.'); + $this->io->error($errorMessage . ' Run this command with -vvv to see specific error info.'); } return false; From c7339f6cfab77008203ebb68b15374a0b88d8a29 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 12 Nov 2018 20:58:14 +0100 Subject: [PATCH 15/19] Created an EmptyIpLocationResolver which always returns an empty resolution and can be used as a fallback while resolving IP addresses --- module/Common/config/dependencies.config.php | 2 + .../IpGeolocation/ChainIpLocationResolver.php | 2 - .../IpGeolocation/EmptyIpLocationResolver.php | 25 +++++++++ .../GeoLite2LocationResolver.php | 2 - .../IpGeolocation/IpApiLocationResolver.php | 2 - .../IpLocationResolverInterface.php | 2 - .../EmptyIpLocationResolverTest.php | 51 +++++++++++++++++++ 7 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 module/Common/src/IpGeolocation/EmptyIpLocationResolver.php create mode 100644 module/Common/test/IpGeolocation/EmptyIpLocationResolverTest.php diff --git a/module/Common/config/dependencies.config.php b/module/Common/config/dependencies.config.php index a43d1e5c..b03520a3 100644 --- a/module/Common/config/dependencies.config.php +++ b/module/Common/config/dependencies.config.php @@ -37,6 +37,7 @@ return [ IpGeolocation\IpApiLocationResolver::class => ConfigAbstractFactory::class, IpGeolocation\GeoLite2LocationResolver::class => ConfigAbstractFactory::class, + IpGeolocation\EmptyIpLocationResolver::class => InvokableFactory::class, IpGeolocation\ChainIpLocationResolver::class => ConfigAbstractFactory::class, IpGeolocation\GeoLite2\GeoLite2Options::class => ConfigAbstractFactory::class, IpGeolocation\GeoLite2\DbUpdater::class => ConfigAbstractFactory::class, @@ -83,6 +84,7 @@ return [ IpGeolocation\ChainIpLocationResolver::class => [ IpGeolocation\GeoLite2LocationResolver::class, IpGeolocation\IpApiLocationResolver::class, + IpGeolocation\EmptyIpLocationResolver::class, ], IpGeolocation\GeoLite2\GeoLite2Options::class => ['config.geolite2'], IpGeolocation\GeoLite2\DbUpdater::class => [ diff --git a/module/Common/src/IpGeolocation/ChainIpLocationResolver.php b/module/Common/src/IpGeolocation/ChainIpLocationResolver.php index 8528c89e..2b48a70f 100644 --- a/module/Common/src/IpGeolocation/ChainIpLocationResolver.php +++ b/module/Common/src/IpGeolocation/ChainIpLocationResolver.php @@ -18,8 +18,6 @@ class ChainIpLocationResolver implements IpLocationResolverInterface } /** - * @param string $ipAddress - * @return array * @throws WrongIpException */ public function resolveIpLocation(string $ipAddress): array diff --git a/module/Common/src/IpGeolocation/EmptyIpLocationResolver.php b/module/Common/src/IpGeolocation/EmptyIpLocationResolver.php new file mode 100644 index 00000000..c9e1f4a4 --- /dev/null +++ b/module/Common/src/IpGeolocation/EmptyIpLocationResolver.php @@ -0,0 +1,25 @@ + '', + 'country_name' => '', + 'region_name' => '', + 'city' => '', + 'latitude' => '', + 'longitude' => '', + 'time_zone' => '', + ]; + } +} diff --git a/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php b/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php index 0bced338..24607ff5 100644 --- a/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php +++ b/module/Common/src/IpGeolocation/GeoLite2LocationResolver.php @@ -24,8 +24,6 @@ class GeoLite2LocationResolver implements IpLocationResolverInterface } /** - * @param string $ipAddress - * @return array * @throws WrongIpException */ public function resolveIpLocation(string $ipAddress): array diff --git a/module/Common/src/IpGeolocation/IpApiLocationResolver.php b/module/Common/src/IpGeolocation/IpApiLocationResolver.php index fe6680a8..8ea8be4b 100644 --- a/module/Common/src/IpGeolocation/IpApiLocationResolver.php +++ b/module/Common/src/IpGeolocation/IpApiLocationResolver.php @@ -25,8 +25,6 @@ class IpApiLocationResolver implements IpLocationResolverInterface } /** - * @param string $ipAddress - * @return array * @throws WrongIpException */ public function resolveIpLocation(string $ipAddress): array diff --git a/module/Common/src/IpGeolocation/IpLocationResolverInterface.php b/module/Common/src/IpGeolocation/IpLocationResolverInterface.php index f9e41572..6017db61 100644 --- a/module/Common/src/IpGeolocation/IpLocationResolverInterface.php +++ b/module/Common/src/IpGeolocation/IpLocationResolverInterface.php @@ -8,8 +8,6 @@ use Shlinkio\Shlink\Common\Exception\WrongIpException; interface IpLocationResolverInterface { /** - * @param string $ipAddress - * @return array * @throws WrongIpException */ public function resolveIpLocation(string $ipAddress): array; diff --git a/module/Common/test/IpGeolocation/EmptyIpLocationResolverTest.php b/module/Common/test/IpGeolocation/EmptyIpLocationResolverTest.php new file mode 100644 index 00000000..c394f85e --- /dev/null +++ b/module/Common/test/IpGeolocation/EmptyIpLocationResolverTest.php @@ -0,0 +1,51 @@ + '', + 'country_name' => '', + 'region_name' => '', + 'city' => '', + 'latitude' => '', + 'longitude' => '', + 'time_zone' => '', + ]; + + /** + * @var EmptyIpLocationResolver + */ + private $resolver; + + public function setUp() + { + $this->resolver = new EmptyIpLocationResolver(); + } + + /** + * @test + * @dataProvider provideEmptyResponses + */ + public function alwaysReturnsAnEmptyResponse(array $expected, string $ipAddress) + { + $this->assertEquals($expected, $this->resolver->resolveIpLocation($ipAddress)); + } + + public function provideEmptyResponses(): array + { + return map(range(0, 5), function () { + return [self::EMPTY_RESP, $this->generateRandomString(10)]; + }); + } +} From 58e8c8e1823c380c46dfd851dd953144fd397818 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 12 Nov 2018 21:04:02 +0100 Subject: [PATCH 16/19] Updated spanish translations --- module/CLI/lang/es.mo | Bin 9959 -> 10654 bytes module/CLI/lang/es.po | 22 ++++++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/module/CLI/lang/es.mo b/module/CLI/lang/es.mo index c16c9d9f8f47a7b6f4ac85512be6f1a3a711e537..8bffeb7b9dfd512f45807ecdd8bd621aad638c5f 100644 GIT binary patch delta 2765 zcmZ|PS!`5Q9LMoPTe`7Y%2JDZWJgPZX_Z9=Uvf?$g%;AQS9GdMH1+?fF}CWE2~ zCd4Qq1``!S41wT+6Q1+|LE`d&Pm*Aw(HE+53A}0G!S8Q6Lrh3d@}JLL&-tJKId}Tk zyth98b3@e;L)%JBBpNG?$>Z>9erUC|#*D){9FG>~;C$@H1TMpKxD>1FjG2U;sQyxT zH@=Q~|0H(cm&o&ZQ(13J3l~;k1hY5>PhbOnjx~4*oA5GDz-w5Ab;C#&N218|+9AV6I>_QEE z7%hH;AzngqH60_3@v#djvN?leRO+)F+>76$2L1!7W-(2wWa>y)*T*2am>H;Vg(rD7TvR1mQA@cQ^?VNJ<1y^OE2xB<$5DTl(M%%?W!8o>u^X=rF=j8S zGTYg4TB4UwyZktA!hY<+C=V&&H&8RXjQ3z2<(h>v@hs@QF9*E(NCr8H;|&4A?#q5*)*c=PbnEiCGsq)pS`H(_oJ3Lf0P5w z_&m~=`4g$Osil`AI1#7eXSe~cU=&-admHY=)%YPQ;p?bM)iY`zwxIUVyZAJIj7p^5 z7Qec@8O?##XcltMY(#Cg=TU3)GLn-yge;pmg)Fx@he`YaRr)BSYWE*R-G3XG;OUZo z;bP8PS?8xPjn(@8zu~}g;d@+%*KjUICl6F2i%RGKa?8AhQ9N5Z{|7a2^OVy60qXfK z)P%ZA*LUIloF7HKe-ZCyd{aBsn0IhG?!)uA5tmc<*YJH@qx;j1c@T4`-F+IB&{xPS zrXMxH2o_cqTY!4L8yDaqRKH&%U#PLHdtRF%!hzO!8EWQ99D-VJoof(FP{Cs6P<&p? zq1|0g)Dgo7b-abpX530t66L0SrSbH|XaYTqU(9SaC;A*#66~+y27%%ZmED9km#VOd zP+LUkTYre?Ak-*8@&BY2Rf&a!T05c5T0=ZasOcYYwATL-4pcs^l{Svr1B5ooT4Fe{ zoKPuMzH-y1)fc1Ba+ihP_}$5>F7Ti2t`GoGc}@rt64j z2)?r7A4w;cTMGx;h?`3nDzJ*s|4X?o=3p7oM9e3~uHdKKp5!1xtS(%v2`VBTY3sr; z2yGCH<-*YUc6U6HbT-%Ld)Y*KyItXeu0+;FEa%=fFXIYr_4_I&-FDgbg+ZSSlY0hU zb$;RO@Wjyk?aw7L~`Lr$Z8&Zibm> z1H0V?$sne8SFN?4?}sjvahb#4H6E*p*vQ<4bJ_|!#~!R&wBBjHer`u#l}53GUebE8 zY|cw2UhqPj^sK67{bDXbMhE3$6#i}cy`n8T*li}wO2o2W%B9KVR;RvK_;up`>inSn zTE@Hev~@{xOr%_B*Lb1kPX;m1_iV~#T5KO{o($~HoKvbnUo3Hmj WvmL2TO6fmwc4Xa)vZ-k4r;1y%mJ4FFfK;EVR0OVzvD>kz*IbrS$Gol+&R>JH!&OgF@hgZ zeaXr+8-iiX$3iSnSLabt%i^d9cj6H|f@=9Nz7oRou4Z&6O7s9D^C>+u9G!;h$j z&SwUyCx7Btyn$u-6h~t^FH;AKumyKv1@ZL z;Zl5sYUxy3rIBgG?bwD)2YZHb{D5jeBl+PGTaHS_7E}X!P;==TDkZm2_dP(e;MiL# zq{Y5rJBE2mL+zku_dV40f3Ozc`<6sV2gh6SS3Hkt7^1aQEeo5m45R3vM&JsnL3goG z>;Ewo_57oE!fZM-MIT;=Y}U$74|F2mwr>Bti&Hr6#|0P|pO~Z#_y@-uaUH(H!&o(~77;4J1m0dB(}!b(*0JWzOhPN*7ec*LgF}Q0 z?eL0L0t?8SS8EfeY!331a=acXk1BM~TkrZC78BKk$}~bf)89)&t)j`Nl`)&BCzw87 ze!(h_U9!yeD_WYG6Aj4=xRR(Kmbjq-+XC*{ zjDrDp*`UoKw{-ZWKzBh-Z+bKmomCU9c5ml&rn;T^?*i_bF^AIJyG6lZ_qgJY@76K4 Az5oCK diff --git a/module/CLI/lang/es.po b/module/CLI/lang/es.po index 8a52fed2..a81a3611 100644 --- a/module/CLI/lang/es.po +++ b/module/CLI/lang/es.po @@ -1,8 +1,8 @@ msgid "" msgstr "" "Project-Id-Version: Shlink 1.0\n" -"POT-Creation-Date: 2018-11-11 19:17+0100\n" -"PO-Revision-Date: 2018-11-11 19:18+0100\n" +"POT-Creation-Date: 2018-11-12 21:01+0100\n" +"PO-Revision-Date: 2018-11-12 21:03+0100\n" "Last-Translator: Alejandro Celaya \n" "Language-Team: \n" "Language: es_ES\n" @@ -359,6 +359,24 @@ msgstr "Se produjo un error al localizar la IP. Ignorado" msgid "Finished processing all IPs" msgstr "Finalizado el procesado de todas las IPs" +msgid "Updates the GeoLite2 database file used to geolocate IP addresses" +msgstr "" +"Actualiza el fichero de base de datos de GeoLite2 usado para geolocalizar " +"direcciones IP" + +msgid "" +"The GeoLite2 database is updated first Tuesday every month, so this command " +"should be ideally run every first Wednesday" +msgstr "" +"La base de datos de GeoLite2 se actualiza el primer Martes de cada mes, por " +"lo que la opción ideal es ejecutar este comando cada primer miércoles de mes" + +msgid "GeoLite2 database properly updated" +msgstr "Base de datos de GeoLite2 correctamente actualizada" + +msgid "An error occurred while updating GeoLite2 database" +msgstr "Se produjo un error al actualizar la base de datos de GeoLite2" + #~ msgid "IP location resolver limit reached. Waiting %s seconds..." #~ msgstr "Limite del localizador de IPs alcanzado. Esperando %s segundos..." From 9964d3e24b1082b9645aaa0759fcfb60ce0644d7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 12 Nov 2018 21:30:30 +0100 Subject: [PATCH 17/19] Added progress bar to command downloading new GeoLite2 database file --- module/CLI/src/Command/Visit/UpdateDbCommand.php | 15 ++++++++++++++- .../test/Command/Visit/UpdateDbCommandTest.php | 5 +++-- .../src/IpGeolocation/GeoLite2/DbUpdater.php | 7 ++++--- .../IpGeolocation/GeoLite2/DbUpdaterInterface.php | 2 +- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/module/CLI/src/Command/Visit/UpdateDbCommand.php b/module/CLI/src/Command/Visit/UpdateDbCommand.php index ca4963cf..a17c36fd 100644 --- a/module/CLI/src/Command/Visit/UpdateDbCommand.php +++ b/module/CLI/src/Command/Visit/UpdateDbCommand.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\Visit; use Shlinkio\Shlink\Common\Exception\RuntimeException; use Shlinkio\Shlink\Common\IpGeolocation\GeoLite2\DbUpdaterInterface; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; @@ -47,11 +48,23 @@ class UpdateDbCommand extends Command protected function execute(InputInterface $input, OutputInterface $output): void { $io = new SymfonyStyle($input, $output); + $progressBar = new ProgressBar($output); + $progressBar->start(); try { - $this->geoLiteDbUpdater->downloadFreshCopy(); + $this->geoLiteDbUpdater->downloadFreshCopy(function (int $total, int $downloaded) use ($progressBar) { + $progressBar->setMaxSteps($total); + $progressBar->setProgress($downloaded); + }); + + $progressBar->finish(); + $io->writeln(''); + $io->success($this->translator->translate('GeoLite2 database properly updated')); } catch (RuntimeException $e) { + $progressBar->finish(); + $io->writeln(''); + $io->error($this->translator->translate('An error occurred while updating GeoLite2 database')); if ($io->isVerbose()) { $this->getApplication()->renderException($e, $output); diff --git a/module/CLI/test/Command/Visit/UpdateDbCommandTest.php b/module/CLI/test/Command/Visit/UpdateDbCommandTest.php index eeb4f1ff..9e7d0736 100644 --- a/module/CLI/test/Command/Visit/UpdateDbCommandTest.php +++ b/module/CLI/test/Command/Visit/UpdateDbCommandTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\CLI\Command\Visit; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Visit\UpdateDbCommand; use Shlinkio\Shlink\Common\Exception\RuntimeException; @@ -39,7 +40,7 @@ class UpdateDbCommandTest extends TestCase */ public function successMessageIsPrintedIfEverythingWorks() { - $download = $this->dbUpdater->downloadFreshCopy()->will(function () { + $download = $this->dbUpdater->downloadFreshCopy(Argument::type('callable'))->will(function () { }); $this->commandTester->execute([]); @@ -54,7 +55,7 @@ class UpdateDbCommandTest extends TestCase */ public function errorMessageIsPrintedIfAnExceptionIsThrown() { - $download = $this->dbUpdater->downloadFreshCopy()->willThrow(RuntimeException::class); + $download = $this->dbUpdater->downloadFreshCopy(Argument::type('callable'))->willThrow(RuntimeException::class); $this->commandTester->execute([]); $output = $this->commandTester->getDisplay(); diff --git a/module/Common/src/IpGeolocation/GeoLite2/DbUpdater.php b/module/Common/src/IpGeolocation/GeoLite2/DbUpdater.php index e8628cf2..c20ad4ca 100644 --- a/module/Common/src/IpGeolocation/GeoLite2/DbUpdater.php +++ b/module/Common/src/IpGeolocation/GeoLite2/DbUpdater.php @@ -42,22 +42,23 @@ class DbUpdater implements DbUpdaterInterface /** * @throws RuntimeException */ - public function downloadFreshCopy(): void + public function downloadFreshCopy(callable $handleProgress = null): void { $tempDir = $this->options->getTempDir(); $compressedFile = sprintf('%s/%s', $tempDir, self::DB_COMPRESSED_FILE); - $this->downloadDbFile($compressedFile); + $this->downloadDbFile($compressedFile, $handleProgress); $tempFullPath = $this->extractDbFile($compressedFile, $tempDir); $this->copyNewDbFile($tempFullPath); $this->deleteTempFiles([$compressedFile, $tempFullPath]); } - private function downloadDbFile(string $dest): void + private function downloadDbFile(string $dest, callable $handleProgress = null): void { try { $this->httpClient->request(RequestMethod::METHOD_GET, $this->options->getDownloadFrom(), [ RequestOptions::SINK => $dest, + RequestOptions::PROGRESS => $handleProgress, ]); } catch (Throwable | GuzzleException $e) { throw new RuntimeException( diff --git a/module/Common/src/IpGeolocation/GeoLite2/DbUpdaterInterface.php b/module/Common/src/IpGeolocation/GeoLite2/DbUpdaterInterface.php index 1fad0825..bf304c6f 100644 --- a/module/Common/src/IpGeolocation/GeoLite2/DbUpdaterInterface.php +++ b/module/Common/src/IpGeolocation/GeoLite2/DbUpdaterInterface.php @@ -10,5 +10,5 @@ interface DbUpdaterInterface /** * @throws RuntimeException */ - public function downloadFreshCopy(): void; + public function downloadFreshCopy(callable $handleProgress = null): void; } From b9dd975bc623dc2a07f74903ed297ce194c23417 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 12 Nov 2018 21:34:45 +0100 Subject: [PATCH 18/19] Updated changelog with new geolocation service --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index baffe8ee..09904f6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), This new option will be asked by the installer both for new shlink installations and for any previous shlink version which is updated. +* [#189](https://github.com/shlinkio/shlink/issues/189) and [#240](https://github.com/shlinkio/shlink/issues/240) Added new [GeoLite2](https://dev.maxmind.com/geoip/geoip2/geolite2/)-based geolocation service which is faster and more reliable than previous one. + + It does not have API limit problems, since it uses a local database file. + + Previous service is still used as a fallback in case GeoLite DB does not contain any IP address. + #### Changed * [#241](https://github.com/shlinkio/shlink/issues/241) Fixed columns in `visit_locations` table, to be snake_case instead of camelCase. From a07e4b17be653f7a4780a932bf9df8e3abf5ac75 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 12 Nov 2018 21:37:04 +0100 Subject: [PATCH 19/19] Updated docs --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 7b4d7bee..cf6d34c3 100644 --- a/README.md +++ b/README.md @@ -108,7 +108,7 @@ Those tasks can be performed using shlink's CLI, so it should be easy to schedul When shlink is installed it downloads a fresh [GeoLite2](https://dev.maxmind.com/geoip/geoip2/geolite2/) db file. Running this command will update this file. - The file does not change very frequently, so it shouldn't be needed to run this command more than once per month. + The file is updated the first Tuesday of every month, so it should be enough running this command the first Wednesday. * Generate website previews: `/path/to/shlink/bin/cli short-url:process-previews`