Merge pull request #277 from acelaya/feature/increase-msi

Feature/increase msi

CHANGELOG.md

@@ -4,6 +4,30 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org).
 
+## 1.14.1 - 2018-11-17
+
+#### Added
+
+* *Nothing*
+
+#### Changed
+
+* [#260](https://github.com/shlinkio/shlink/issues/260) Increased mutation score to 65%.
+
+#### Deprecated
+
+* *Nothing*
+
+#### Removed
+
+* *Nothing*
+
+#### Fixed
+
+* [#271](https://github.com/shlinkio/shlink/issues/271) Fixed memory leak produced when processing high amounts of visits at the same time.
+* [#272](https://github.com/shlinkio/shlink/issues/272) Fixed errors produced when trying to process visits multiple times in parallel, by using a lock which ensures only one instance is run at a time.
+
+
 ## 1.14.0 - 2018-11-16
 
 #### Added

README.md

@@ -114,6 +114,8 @@ Those tasks can be performed using shlink's CLI, so it should be easy to schedul
 
     Running this will improve the performance of the `doma.in/abc123/preview` URLs, which return a preview of the site.
 
+*Any of those commands accept the `-q` flag, which makes it not display any output. This is recommended when configuring the commands as cron jobs.*
+
 ## Update to new version
 
 When a new Shlink version is available, you don't need to repeat the entire process yourself. Instead, follow these steps:
@@ -191,6 +193,7 @@ Available commands:
   tag:rename                  Renames one existing tag.
  visit
   visit:process               Processes visits where location is not set yet
+  visit:update-db             Updates the GeoLite2 database file used to geolocate IP addresses
 ```
 
 > This product includes GeoLite2 data created by MaxMind, available from [https://www.maxmind.com](https://www.maxmind.com)

composer.json

@@ -32,6 +32,7 @@
         "roave/security-advisories": "dev-master",
         "symfony/console": "^4.1",
         "symfony/filesystem": "^4.1",
+        "symfony/lock": "^4.1",
         "symfony/process": "^4.1",
         "theorchard/monolog-cascade": "^0.4",
         "zendframework/zend-config": "^3.0",
@@ -122,9 +123,13 @@
         ],
         "test:unit:pretty": "phpdbg -qrr vendor/bin/phpunit --coverage-html build/coverage --order-by=random",
 
-        "infect": "infection --threads=4 --min-msi=60 --log-verbosity=2 --only-covered",
-        "infect:ci": "infection --threads=4 --min-msi=60 --log-verbosity=2 --only-covered --coverage=build",
-        "infect:show": "infection --threads=4 --min-msi=60 --log-verbosity=2 --only-covered --show-mutations"
+        "infect": "infection --threads=4 --min-msi=65 --log-verbosity=2 --only-covered",
+        "infect:ci": "infection --threads=4 --min-msi=65 --log-verbosity=2 --only-covered --coverage=build",
+        "infect:show": "infection --threads=4 --min-msi=65 --log-verbosity=2 --only-covered --show-mutations",
+        "infect:test": [
+            "@test:unit:ci",
+            "@infect:ci"
+        ]
     },
     "scripts-descriptions": {
         "check": "<fg=blue;options=bold>Alias for \"cs\", \"stan\", \"test\" and \"infect\"</>",

config/autoload/locks.global.php

@@ -0,0 +1,25 @@
+<?php
+declare(strict_types=1);
+
+use Symfony\Component\Lock;
+use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory;
+
+return [
+
+    'locks' => [
+        'locks_dir' => __DIR__ . '/../../data/locks',
+    ],
+
+    'dependencies' => [
+        'factories' => [
+            Lock\Store\FlockStore::class => ConfigAbstractFactory::class,
+            Lock\Factory::class => ConfigAbstractFactory::class,
+        ],
+    ],
+
+    ConfigAbstractFactory::class => [
+        Lock\Store\FlockStore::class => ['config.locks.locks_dir'],
+        Lock\Factory::class => [Lock\Store\FlockStore::class],
+    ],
+
+];

data/locks/.gitignore

@@ -0,0 +1,2 @@
+*
+!.gitignore

infection.json

@@ -4,7 +4,7 @@
             "module/*/src"
         ]
     },
-    "timeout": 10,
+    "timeout": 5,
     "logs": {
         "text": "build/infection/infection-log.txt",
         "summary": "build/infection/summary-log.txt",

module/CLI/config/dependencies.config.php

@@ -9,6 +9,7 @@ use Shlinkio\Shlink\Common\Service\PreviewGenerator;
 use Shlinkio\Shlink\Core\Service;
 use Shlinkio\Shlink\Rest\Service\ApiKeyService;
 use Symfony\Component\Console\Application;
+use Symfony\Component\Lock;
 use Zend\I18n\Translator\Translator;
 use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory;
 
@@ -68,6 +69,7 @@ return [
         Command\Visit\ProcessVisitsCommand::class => [
             Service\VisitService::class,
             IpLocationResolverInterface::class,
+            Lock\Factory::class,
             'translator',
         ],
         Command\Visit\UpdateDbCommand::class => [DbUpdater::class, 'translator'],

module/CLI/lang/es.po

@@ -1,8 +1,8 @@
 msgid ""
 msgstr ""
 "Project-Id-Version: Shlink 1.0\n"
-"POT-Creation-Date: 2018-11-12 21:01+0100\n"
-"PO-Revision-Date: 2018-11-12 21:03+0100\n"
+"POT-Creation-Date: 2018-11-17 14:29+0100\n"
+"PO-Revision-Date: 2018-11-17 14:29+0100\n"
 "Last-Translator: Alejandro Celaya <alejandro@alejandrocelaya.com>\n"
 "Language-Team: \n"
 "Language: es_ES\n"
@@ -340,6 +340,17 @@ 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"
 
+#, php-format
+msgid "There is already an instance of the \"%s\" command in execution"
+msgstr "Ya existe una instancia del comando \"%s\" en ejecución"
+
+#, php-format
+msgid "Address located at \"%s\""
+msgstr "Dirección localizada en \"%s\""
+
+msgid "Finished processing all IPs"
+msgstr "Finalizado el procesado de todas las IPs"
+
 msgid "Ignored visit with no IP address"
 msgstr "Ignorada visita sin dirección IP"
 
@@ -349,16 +360,9 @@ msgstr "Procesando IP"
 msgid "Ignored localhost address"
 msgstr "Ignorada IP de localhost"
 
-#, php-format
-msgid "Address located at \"%s\""
-msgstr "Dirección localizada en \"%s\""
-
 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 "Updates the GeoLite2 database file used to geolocate IP addresses"
 msgstr ""
 "Actualiza el fichero de base de datos de GeoLite2 usado para geolocalizar "

module/CLI/src/Command/Api/ListKeysCommand.php

@@ -57,12 +57,11 @@ class ListKeysCommand extends Command
         $enabledOnly = $input->getOption('enabledOnly');
 
         $rows = array_map(function (ApiKey $apiKey) use ($enabledOnly) {
-            $key = (string) $apiKey;
             $expiration = $apiKey->getExpirationDate();
             $messagePattern = $this->determineMessagePattern($apiKey);
 
             // Set columns for this row
-            $rowData = [sprintf($messagePattern, $key)];
+            $rowData = [sprintf($messagePattern, $apiKey)];
             if (! $enabledOnly) {
                 $rowData[] = sprintf($messagePattern, $this->getEnabledSymbol($apiKey));
             }

module/CLI/src/Command/Visit/ProcessVisitsCommand.php

@@ -6,12 +6,15 @@ namespace Shlinkio\Shlink\CLI\Command\Visit;
 use Shlinkio\Shlink\Common\Exception\WrongIpException;
 use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface;
 use Shlinkio\Shlink\Common\Util\IpAddress;
+use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Entity\VisitLocation;
+use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
 use Shlinkio\Shlink\Core\Service\VisitServiceInterface;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
+use Symfony\Component\Lock\Factory as Locker;
 use Zend\I18n\Translator\TranslatorInterface;
 use function sprintf;
 
@@ -31,73 +34,99 @@ class ProcessVisitsCommand extends Command
      * @var TranslatorInterface
      */
     private $translator;
+    /**
+     * @var Locker
+     */
+    private $locker;
+    /**
+     * @var OutputInterface
+     */
+    private $output;
 
     public function __construct(
         VisitServiceInterface $visitService,
         IpLocationResolverInterface $ipLocationResolver,
+        Locker $locker,
         TranslatorInterface $translator
     ) {
         $this->visitService = $visitService;
         $this->ipLocationResolver = $ipLocationResolver;
         $this->translator = $translator;
+        $this->locker = $locker;
         parent::__construct();
     }
 
     protected function configure(): void
     {
-        $this->setName(self::NAME)
-             ->setDescription(
-                 $this->translator->translate('Processes visits where location is not set yet')
-             );
+        $this
+            ->setName(self::NAME)
+            ->setDescription($this->translator->translate('Processes visits where location is not set yet'));
     }
 
     protected function execute(InputInterface $input, OutputInterface $output): void
     {
+        $this->output = $output;
         $io = new SymfonyStyle($input, $output);
-        $visits = $this->visitService->getUnlocatedVisits();
 
-        foreach ($visits as $visit) {
-            if (! $visit->hasRemoteAddr()) {
-                $io->writeln(
-                    sprintf('<comment>%s</comment>', $this->translator->translate('Ignored visit with no IP address')),
-                    OutputInterface::VERBOSITY_VERBOSE
-                );
-                continue;
-            }
-
-            $ipAddr = $visit->getRemoteAddr();
-            $io->write(sprintf('%s <fg=blue>%s</>', $this->translator->translate('Processing IP'), $ipAddr));
-            if ($ipAddr === IpAddress::LOCALHOST) {
-                $io->writeln(
-                    sprintf(' [<comment>%s</comment>]', $this->translator->translate('Ignored localhost address'))
-                );
-                continue;
-            }
-
-            try {
-                $result = $this->ipLocationResolver->resolveIpLocation($ipAddr);
-
-                $location = new VisitLocation($result);
-                $visit->setVisitLocation($location);
-                $this->visitService->saveVisit($visit);
-
-                $io->writeln(sprintf(
-                    ' [<info>' . $this->translator->translate('Address located at "%s"') . '</info>]',
-                    $location->getCountryName()
-                ));
-            } catch (WrongIpException $e) {
-                $io->writeln(
-                    sprintf(
-                        ' [<fg=red>%s</>]',
-                        $this->translator->translate('An error occurred while locating IP. Skipped')
-                    )
-                );
-                if ($io->isVerbose()) {
-                    $this->getApplication()->renderException($e, $output);
-                }
-            }
+        $lock = $this->locker->createLock(self::NAME);
+        if (! $lock->acquire()) {
+            $io->warning(sprintf(
+                $this->translator->translate('There is already an instance of the "%s" command in execution'),
+                self::NAME
+            ));
+            return;
         }
 
-        $io->success($this->translator->translate('Finished processing all IPs'));
+        try {
+            $this->visitService->locateVisits(
+                [$this, 'getGeolocationDataForVisit'],
+                function (VisitLocation $location) use ($output) {
+                    $output->writeln(sprintf(
+                        ' [<info>' . $this->translator->translate('Address located at "%s"') . '</info>]',
+                        $location->getCountryName()
+                    ));
+                }
+            );
+
+            $io->success($this->translator->translate('Finished processing all IPs'));
+        } finally {
+            $lock->release();
+        }
+    }
+
+    public function getGeolocationDataForVisit(Visit $visit): array
+    {
+        if (! $visit->hasRemoteAddr()) {
+            $this->output->writeln(sprintf(
+                '<comment>%s</comment>',
+                $this->translator->translate('Ignored visit with no IP address')
+            ), OutputInterface::VERBOSITY_VERBOSE);
+            throw new IpCannotBeLocatedException('Ignored visit with no IP address');
+        }
+
+        $ipAddr = $visit->getRemoteAddr();
+        $this->output->write(sprintf('%s <fg=blue>%s</>', $this->translator->translate('Processing IP'), $ipAddr));
+        if ($ipAddr === IpAddress::LOCALHOST) {
+            $this->output->writeln(
+                sprintf(' [<comment>%s</comment>]', $this->translator->translate('Ignored localhost address'))
+            );
+            throw new IpCannotBeLocatedException('Ignored localhost address');
+        }
+
+        try {
+            return $this->ipLocationResolver->resolveIpLocation($ipAddr);
+        } catch (WrongIpException $e) {
+            $this->output->writeln(
+                sprintf(
+                    ' [<fg=red>%s</>]',
+                    $this->translator->translate('An error occurred while locating IP. Skipped')
+                )
+            );
+            if ($this->output->isVerbose()) {
+                $this->getApplication()->renderException($e, $this->output);
+            }
+
+            throw new IpCannotBeLocatedException('An error occurred while locating IP', $e->getCode(), $e);
+        }
     }
 }

module/CLI/test/Command/Api/DisableKeyCommandTest.php

@@ -39,10 +39,14 @@ class DisableKeyCommandTest extends TestCase
     {
         $apiKey = 'abcd1234';
         $this->apiKeyService->disable($apiKey)->shouldBeCalledOnce();
+
         $this->commandTester->execute([
             'command' => 'api-key:disable',
             'apiKey' => $apiKey,
         ]);
+        $output = $this->commandTester->getDisplay();
+
+        $this->assertContains('API key "abcd1234" properly disabled', $output);
     }
 
     /**
@@ -51,14 +55,15 @@ class DisableKeyCommandTest extends TestCase
     public function errorIsReturnedIfServiceThrowsException()
     {
         $apiKey = 'abcd1234';
-        $this->apiKeyService->disable($apiKey)->willThrow(InvalidArgumentException::class)
-                                              ->shouldBeCalledOnce();
+        $disable = $this->apiKeyService->disable($apiKey)->willThrow(InvalidArgumentException::class);
 
         $this->commandTester->execute([
             'command' => 'api-key:disable',
             'apiKey' => $apiKey,
         ]);
         $output = $this->commandTester->getDisplay();
+
         $this->assertContains('API key "abcd1234" does not exist.', $output);
+        $disable->shouldHaveBeenCalledOnce();
     }
 }

module/CLI/test/Command/Api/GenerateKeyCommandTest.php

@@ -39,11 +39,15 @@ class GenerateKeyCommandTest extends TestCase
      */
     public function noExpirationDateIsDefinedIfNotProvided()
     {
-        $this->apiKeyService->create(null)->shouldBeCalledOnce()
-                                          ->willReturn(new ApiKey());
+        $create = $this->apiKeyService->create(null)->willReturn(new ApiKey());
+
         $this->commandTester->execute([
             'command' => 'api-key:generate',
         ]);
+        $output = $this->commandTester->getDisplay();
+
+        $this->assertContains('Generated API key: ', $output);
+        $create->shouldHaveBeenCalledOnce();
     }
 
     /**

module/CLI/test/Command/Api/ListKeysCommandTest.php

@@ -35,30 +35,46 @@ class ListKeysCommandTest extends TestCase
     /**
      * @test
      */
-    public function ifEnabledOnlyIsNotProvidedEverythingIsListed()
+    public function everythingIsListedIfEnabledOnlyIsNotProvided()
     {
         $this->apiKeyService->listKeys(false)->willReturn([
             new ApiKey(),
             new ApiKey(),
             new ApiKey(),
         ])->shouldBeCalledOnce();
+
         $this->commandTester->execute([
-            'command' => 'api-key:list',
+            'command' => ListKeysCommand::NAME,
         ]);
+        $output = $this->commandTester->getDisplay();
+
+        $this->assertContains('Key', $output);
+        $this->assertContains('Is enabled', $output);
+        $this->assertContains(' +++ ', $output);
+        $this->assertNotContains(' --- ', $output);
+        $this->assertContains('Expiration date', $output);
     }
 
     /**
      * @test
      */
-    public function ifEnabledOnlyIsProvidedOnlyThoseKeysAreListed()
+    public function onlyEnabledKeysAreListedIfEnabledOnlyIsProvided()
     {
         $this->apiKeyService->listKeys(true)->willReturn([
-            new ApiKey(),
+            (new ApiKey())->disable(),
             new ApiKey(),
         ])->shouldBeCalledOnce();
+
         $this->commandTester->execute([
-            'command' => 'api-key:list',
+            'command' => ListKeysCommand::NAME,
             '--enabledOnly' => true,
         ]);
+        $output = $this->commandTester->getDisplay();
+
+        $this->assertContains('Key', $output);
+        $this->assertNotContains('Is enabled', $output);
+        $this->assertNotContains(' +++ ', $output);
+        $this->assertNotContains(' --- ', $output);
+        $this->assertContains('Expiration date', $output);
     }
 }

module/CLI/test/Command/ShortUrl/GeneratePreviewCommandTest.php

@@ -62,13 +62,22 @@ class GeneratePreviewCommandTest extends TestCase
         ]);
         $this->shortUrlService->listShortUrls(1)->willReturn($paginator)->shouldBeCalledOnce();
 
-        $this->previewGenerator->generatePreview('http://foo.com')->shouldBeCalledOnce();
-        $this->previewGenerator->generatePreview('https://bar.com')->shouldBeCalledOnce();
-        $this->previewGenerator->generatePreview('http://baz.com/something')->shouldBeCalledOnce();
+        $generatePreview1 = $this->previewGenerator->generatePreview('http://foo.com')->willReturn('');
+        $generatePreview2 = $this->previewGenerator->generatePreview('https://bar.com')->willReturn('');
+        $generatePreview3 = $this->previewGenerator->generatePreview('http://baz.com/something')->willReturn('');
 
         $this->commandTester->execute([
             'command' => 'shortcode:process-previews',
         ]);
+        $output = $this->commandTester->getDisplay();
+
+        $this->assertContains('Processing URL http://foo.com', $output);
+        $this->assertContains('Processing URL https://bar.com', $output);
+        $this->assertContains('Processing URL http://baz.com/something', $output);
+        $this->assertContains('Finished processing all URLs', $output);
+        $generatePreview1->shouldHaveBeenCalledOnce();
+        $generatePreview2->shouldHaveBeenCalledOnce();
+        $generatePreview3->shouldHaveBeenCalledOnce();
     }
 
     /**

module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php

@@ -3,9 +3,11 @@ declare(strict_types=1);
 
 namespace ShlinkioTest\Shlink\CLI\Command\ShortUrl;
 
+use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
+use Psr\Http\Message\UriInterface;
 use Shlinkio\Shlink\CLI\Command\ShortUrl\GenerateShortUrlCommand;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
@@ -13,9 +15,8 @@ use Shlinkio\Shlink\Core\Service\UrlShortener;
 use Symfony\Component\Console\Application;
 use Symfony\Component\Console\Tester\CommandTester;
 use Zend\I18n\Translator\Translator;
-use function strpos;
 
-class GenerateShortcodeCommandTest extends TestCase
+class GenerateShortUrlCommandTest extends TestCase
 {
     /**
      * @var CommandTester
@@ -43,18 +44,19 @@ class GenerateShortcodeCommandTest extends TestCase
      */
     public function properShortCodeIsCreatedIfLongUrlIsCorrect()
     {
-        $this->urlShortener->urlToShortCode(Argument::cetera())
-            ->willReturn(
-                (new ShortUrl(''))->setShortCode('abc123')
-            )
-            ->shouldBeCalledOnce();
+        $urlToShortCode = $this->urlShortener->urlToShortCode(Argument::cetera())->willReturn(
+            (new ShortUrl(''))->setShortCode('abc123')
+        );
 
         $this->commandTester->execute([
             'command' => 'shortcode:generate',
             'longUrl' => 'http://domain.com/foo/bar',
+            '--maxVisits' => '3',
         ]);
         $output = $this->commandTester->getDisplay();
-        $this->assertTrue(strpos($output, 'http://foo.com/abc123') > 0);
+
+        $this->assertContains('http://foo.com/abc123', $output);
+        $urlToShortCode->shouldHaveBeenCalledOnce();
     }
 
     /**
@@ -75,4 +77,29 @@ class GenerateShortcodeCommandTest extends TestCase
             $output
         );
     }
+
+    /**
+     * @test
+     */
+    public function properlyProcessesProvidedTags()
+    {
+        $urlToShortCode = $this->urlShortener->urlToShortCode(
+            Argument::type(UriInterface::class),
+            Argument::that(function (array $tags) {
+                Assert::assertEquals(['foo', 'bar', 'baz', 'boo', 'zar'], $tags);
+                return $tags;
+            }),
+            Argument::cetera()
+        )->willReturn((new ShortUrl(''))->setShortCode('abc123'));
+
+        $this->commandTester->execute([
+            'command' => 'shortcode:generate',
+            'longUrl' => 'http://domain.com/foo/bar',
+            '--tags' => ['foo,bar', 'baz', 'boo,zar'],
+        ]);
+        $output = $this->commandTester->getDisplay();
+
+        $this->assertContains('http://foo.com/abc123', $output);
+        $urlToShortCode->shouldHaveBeenCalledOnce();
+    }
 }

module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php

@@ -81,7 +81,7 @@ class GetVisitsCommandTest extends TestCase
     {
         $shortCode = 'abc123';
         $this->visitsTracker->info($shortCode, Argument::any())->willReturn([
-            (new Visit(new ShortUrl(''), new Visitor('bar', 'foo', '')))->setVisitLocation(
+            (new Visit(new ShortUrl(''), new Visitor('bar', 'foo', '')))->locate(
                 new VisitLocation(['country_name' => 'Spain'])
             ),
         ])->shouldBeCalledOnce();

module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php

@@ -15,7 +15,7 @@ use Zend\I18n\Translator\Translator;
 use Zend\Paginator\Adapter\ArrayAdapter;
 use Zend\Paginator\Paginator;
 
-class ListShortcodesCommandTest extends TestCase
+class ListShortUrlsCommandTest extends TestCase
 {
     /**
      * @var CommandTester
@@ -52,7 +52,7 @@ class ListShortcodesCommandTest extends TestCase
      */
     public function loadingMorePagesCallsListMoreTimes()
     {
-        // The paginator will return more than one page for the first 3 times
+        // The paginator will return more than one page
         $data = [];
         for ($i = 0; $i < 50; $i++) {
             $data[] = new ShortUrl('url_' . $i);
@@ -64,6 +64,11 @@ class ListShortcodesCommandTest extends TestCase
 
         $this->commandTester->setInputs(['y', 'y', 'n']);
         $this->commandTester->execute(['command' => 'shortcode:list']);
+        $output = $this->commandTester->getDisplay();
+
+        $this->assertContains('Continue with page 2?', $output);
+        $this->assertContains('Continue with page 3?', $output);
+        $this->assertContains('Continue with page 4?', $output);
     }
 
     /**
@@ -77,11 +82,20 @@ class ListShortcodesCommandTest extends TestCase
             $data[] = new ShortUrl('url_' . $i);
         }
 
-        $this->shortUrlService->listShortUrls(Argument::cetera())->willReturn(new Paginator(new ArrayAdapter($data)))
-                                                                 ->shouldBeCalledOnce();
+        $this->shortUrlService->listShortUrls(1, null, [], null)->willReturn(new Paginator(new ArrayAdapter($data)))
+                                                                ->shouldBeCalledOnce();
 
         $this->commandTester->setInputs(['n']);
         $this->commandTester->execute(['command' => 'shortcode:list']);
+        $output = $this->commandTester->getDisplay();
+
+        $this->assertContains('url_1', $output);
+        $this->assertContains('url_9', $output);
+        $this->assertNotContains('url_10', $output);
+        $this->assertNotContains('url_20', $output);
+        $this->assertNotContains('url_30', $output);
+        $this->assertContains('Continue with page 2?', $output);
+        $this->assertNotContains('Continue with page 3?', $output);
     }
 
     /**

module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php

@@ -7,16 +7,23 @@ use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\CLI\Command\Visit\ProcessVisitsCommand;
+use Shlinkio\Shlink\Common\Exception\WrongIpException;
 use Shlinkio\Shlink\Common\IpGeolocation\IpApiLocationResolver;
+use Shlinkio\Shlink\Common\Util\IpAddress;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
+use Shlinkio\Shlink\Core\Entity\VisitLocation;
+use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
 use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Service\VisitService;
 use Symfony\Component\Console\Application;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Tester\CommandTester;
+use Symfony\Component\Lock;
+use Throwable;
 use Zend\I18n\Translator\Translator;
-use function count;
+use function array_shift;
+use function sprintf;
 
 class ProcessVisitsCommandTest extends TestCase
 {
@@ -32,15 +39,31 @@ class ProcessVisitsCommandTest extends TestCase
      * @var ObjectProphecy
      */
     private $ipResolver;
+    /**
+     * @var ObjectProphecy
+     */
+    private $locker;
+    /**
+     * @var ObjectProphecy
+     */
+    private $lock;
 
     public function setUp()
     {
         $this->visitService = $this->prophesize(VisitService::class);
         $this->ipResolver = $this->prophesize(IpApiLocationResolver::class);
 
+        $this->locker = $this->prophesize(Lock\Factory::class);
+        $this->lock = $this->prophesize(Lock\LockInterface::class);
+        $this->lock->acquire()->willReturn(true);
+        $this->lock->release()->will(function () {
+        });
+        $this->locker->createLock(Argument::type('string'))->willReturn($this->lock->reveal());
+
         $command = new ProcessVisitsCommand(
             $this->visitService->reveal(),
             $this->ipResolver->reveal(),
+            $this->locker->reveal(),
             Translator::factory([])
         );
         $app = new Application();
@@ -52,59 +75,131 @@ class ProcessVisitsCommandTest extends TestCase
     /**
      * @test
      */
-    public function allReturnedVisitsIpsAreProcessed()
+    public function allPendingVisitsAreProcessed()
     {
-        $shortUrl = new ShortUrl('');
+        $visit = new Visit(new ShortUrl(''), new Visitor('', '', '1.2.3.4'));
+        $location = new VisitLocation([]);
 
-        $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')),
-        ];
-        $this->visitService->getUnlocatedVisits()->willReturn($visits)
-                                                 ->shouldBeCalledOnce();
+        $locateVisits = $this->visitService->locateVisits(Argument::cetera())->will(
+            function (array $args) use ($visit, $location) {
+                $firstCallback = array_shift($args);
+                $firstCallback($visit);
 
-        $this->visitService->saveVisit(Argument::any())->shouldBeCalledTimes(count($visits));
-        $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([])
-                                                             ->shouldBeCalledTimes(count($visits));
+                $secondCallback = array_shift($args);
+                $secondCallback($location, $visit);
+            }
+        );
+        $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]);
 
         $this->commandTester->execute([
             'command' => 'visit:process',
         ]);
         $output = $this->commandTester->getDisplay();
+
         $this->assertContains('Processing IP 1.2.3.0', $output);
-        $this->assertContains('Processing IP 4.3.2.0', $output);
-        $this->assertContains('Processing IP 12.34.56.0', $output);
+        $locateVisits->shouldHaveBeenCalledOnce();
+        $resolveIpLocation->shouldHaveBeenCalledOnce();
+    }
+
+    /**
+     * @test
+     * @dataProvider provideIgnoredAddresses
+     */
+    public function localhostAndEmptyAddressesAreIgnored(?string $address, string $message)
+    {
+        $visit = new Visit(new ShortUrl(''), new Visitor('', '', $address));
+        $location = new VisitLocation([]);
+
+        $locateVisits = $this->visitService->locateVisits(Argument::cetera())->will(
+            function (array $args) use ($visit, $location) {
+                $firstCallback = array_shift($args);
+                $firstCallback($visit);
+
+                $secondCallback = array_shift($args);
+                $secondCallback($location, $visit);
+            }
+        );
+        $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]);
+
+        try {
+            $this->commandTester->execute([
+                'command' => 'visit:process',
+            ], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]);
+        } catch (Throwable $e) {
+            $output = $this->commandTester->getDisplay();
+
+            $this->assertInstanceOf(IpCannotBeLocatedException::class, $e);
+
+            $this->assertContains($message, $output);
+            $locateVisits->shouldHaveBeenCalledOnce();
+            $resolveIpLocation->shouldNotHaveBeenCalled();
+        }
+    }
+
+    public function provideIgnoredAddresses(): array
+    {
+        return [
+            ['', 'Ignored visit with no IP address'],
+            [null, 'Ignored visit with no IP address'],
+            [IpAddress::LOCALHOST, 'Ignored localhost address'],
+        ];
     }
 
     /**
      * @test
      */
-    public function localhostAndEmptyAddressIsIgnored()
+    public function errorWhileLocatingIpIsDisplayed()
     {
-        $shortUrl = new ShortUrl('');
+        $visit = new Visit(new ShortUrl(''), new Visitor('', '', '1.2.3.4'));
+        $location = new VisitLocation([]);
 
-        $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('', '', '127.0.0.1')),
-            new Visit($shortUrl, new Visitor('', '', '127.0.0.1')),
-            new Visit($shortUrl, new Visitor('', '', '')),
-            new Visit($shortUrl, new Visitor('', '', null)),
-        ];
-        $this->visitService->getUnlocatedVisits()->willReturn($visits)
-            ->shouldBeCalledOnce();
+        $locateVisits = $this->visitService->locateVisits(Argument::cetera())->will(
+            function (array $args) use ($visit, $location) {
+                $firstCallback = array_shift($args);
+                $firstCallback($visit);
 
-        $this->visitService->saveVisit(Argument::any())->shouldBeCalledTimes(count($visits) - 4);
-        $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([])
-                                                             ->shouldBeCalledTimes(count($visits) - 4);
+                $secondCallback = array_shift($args);
+                $secondCallback($location, $visit);
+            }
+        );
+        $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willThrow(WrongIpException::class);
+
+        try {
+            $this->commandTester->execute([
+                'command' => 'visit:process',
+            ], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]);
+        } catch (Throwable $e) {
+            $output = $this->commandTester->getDisplay();
+
+            $this->assertInstanceOf(IpCannotBeLocatedException::class, $e);
+
+            $this->assertContains('An error occurred while locating IP. Skipped', $output);
+            $locateVisits->shouldHaveBeenCalledOnce();
+            $resolveIpLocation->shouldHaveBeenCalledOnce();
+        }
+    }
+
+    /**
+     * @test
+     */
+    public function noActionIsPerformedIfLockIsAcquired()
+    {
+        $this->lock->acquire()->willReturn(false);
+
+        $locateVisits = $this->visitService->locateVisits(Argument::cetera())->will(function () {
+        });
+        $resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]);
 
         $this->commandTester->execute([
             'command' => 'visit:process',
         ], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]);
         $output = $this->commandTester->getDisplay();
-        $this->assertContains('Ignored localhost address', $output);
-        $this->assertContains('Ignored visit with no IP address', $output);
+
+        $this->assertContains(
+            sprintf('There is already an instance of the "%s" command', ProcessVisitsCommand::NAME),
+            $output
+        );
+        $locateVisits->shouldNotHaveBeenCalled();
+        $resolveIpLocation->shouldNotHaveBeenCalled();
     }
 }

module/CLI/test/Factory/ApplicationFactoryTest.php

@@ -4,6 +4,8 @@ declare(strict_types=1);
 namespace ShlinkioTest\Shlink\CLI\Factory;
 
 use PHPUnit\Framework\TestCase;
+use Prophecy\Argument;
+use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\CLI\Factory\ApplicationFactory;
 use Shlinkio\Shlink\Core\Options\AppOptions;
 use Symfony\Component\Console\Application;
@@ -29,7 +31,7 @@ class ApplicationFactoryTest extends TestCase
      */
     public function serviceIsCreated()
     {
-        $instance = $this->factory->__invoke($this->createServiceManager(), '');
+        $instance = ($this->factory)($this->createServiceManager(), '');
         $this->assertInstanceOf(Application::class, $instance);
     }
 
@@ -40,21 +42,24 @@ class ApplicationFactoryTest extends TestCase
     {
         $sm = $this->createServiceManager([
             'commands' => [
-                'foo',
-                'bar',
-                'baz',
+                'foo' => 'foo',
+                'bar' => 'bar',
+                'baz' => 'baz',
             ],
         ]);
-        $sm->setService('foo', $this->prophesize(Command::class)->reveal());
-        $sm->setService('baz', $this->prophesize(Command::class)->reveal());
+        $sm->setService('foo', $this->createCommandMock('foo')->reveal());
+        $sm->setService('bar', $this->createCommandMock('bar')->reveal());
 
         /** @var Application $instance */
-        $instance = $this->factory->__invoke($sm, '');
+        $instance = ($this->factory)($sm, '');
         $this->assertInstanceOf(Application::class, $instance);
-        $this->assertCount(2, $instance->all());
+
+        $this->assertTrue($instance->has('foo'));
+        $this->assertTrue($instance->has('bar'));
+        $this->assertFalse($instance->has('baz'));
     }
 
-    protected function createServiceManager($config = [])
+    private function createServiceManager(array $config = []): ServiceManager
     {
         return new ServiceManager(['services' => [
             'config' => [
@@ -64,4 +69,17 @@ class ApplicationFactoryTest extends TestCase
             Translator::class => Translator::factory([]),
         ]]);
     }
+
+    private function createCommandMock(string $name): ObjectProphecy
+    {
+        $command = $this->prophesize(Command::class);
+        $command->getName()->willReturn($name);
+        $command->getDefinition()->willReturn($name);
+        $command->isEnabled()->willReturn(true);
+        $command->getAliases()->willReturn([]);
+        $command->setApplication(Argument::type(Application::class))->willReturn(function () {
+        });
+
+        return $command;
+    }
 }

module/Common/src/Factory/CacheFactory.php

@@ -45,11 +45,7 @@ class CacheFactory implements FactoryInterface
         return $adapter;
     }
 
-    /**
-     * @param ContainerInterface $container
-     * @return Cache\CacheProvider
-     */
-    protected function getAdapter(ContainerInterface $container)
+    private function getAdapter(ContainerInterface $container): Cache\CacheProvider
     {
         // Try to get the adapter from config
         $config = $container->get('config');
@@ -61,11 +57,7 @@ class CacheFactory implements FactoryInterface
         return env('APP_ENV', 'pro') === 'pro' ? new Cache\ApcuCache() : new Cache\ArrayCache();
     }
 
-    /**
-     * @param array $cacheConfig
-     * @return Cache\CacheProvider
-     */
-    protected function resolveCacheAdapter(array $cacheConfig)
+    private function resolveCacheAdapter(array $cacheConfig): Cache\CacheProvider
     {
         switch ($cacheConfig['adapter']) {
             case Cache\ArrayCache::class:

module/Common/src/Factory/LoggerFactory.php

@@ -29,10 +29,10 @@ class LoggerFactory implements FactoryInterface
     public function __invoke(ContainerInterface $container, $requestedName, array $options = null)
     {
         $config = $container->has('config') ? $container->get('config') : [];
-        Cascade::fileConfig(isset($config['logger']) ? $config['logger'] : ['loggers' => []]);
+        Cascade::fileConfig($config['logger'] ?? ['loggers' => []]);
 
         // Compose requested logger name
-        $loggerName = isset($options) & isset($options['logger_name']) ? $options['logger_name'] : 'Logger';
+        $loggerName = $options['logger_name'] ?? 'Logger';
         $nameParts = explode('_', $requestedName);
         if (count($nameParts) > 1) {
             $loggerName = $nameParts[1];

module/Common/src/Image/ImageFactory.php

@@ -27,9 +27,9 @@ class ImageFactory implements FactoryInterface
     public function __invoke(ContainerInterface $container, $requestedName, array $options = null)
     {
         $config = $container->get('config')['phpwkhtmltopdf'];
-        $image = new Image(isset($config['images']) ? $config['images'] : null);
+        $image = new Image($config['images'] ?? null);
 
-        if (isset($options) && isset($options['url'])) {
+        if ($options['url'] ?? null) {
             $image->setPage($options['url']);
         }
 

module/Common/src/Middleware/LocaleMiddleware.php

@@ -45,11 +45,7 @@ class LocaleMiddleware implements MiddlewareInterface
         return $delegate->handle($request);
     }
 
-    /**
-     * @param string $locale
-     * @return string
-     */
-    protected function normalizeLocale($locale)
+    private function normalizeLocale(string $locale): string
     {
         $parts = explode('_', $locale);
         if (count($parts) > 1) {

module/Common/src/Response/PixelResponse.php

@@ -3,6 +3,7 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Common\Response;
 
+use Fig\Http\Message\StatusCodeInterface as StatusCode;
 use Psr\Http\Message\StreamInterface;
 use Zend\Diactoros\Response;
 use Zend\Diactoros\Stream;
@@ -13,7 +14,7 @@ class PixelResponse extends Response
     private const BASE_64_IMAGE = 'R0lGODlhAQABAJAAAP8AAAAAACH5BAUQAAAALAAAAAABAAEAAAICBAEAOw==';
     private const CONTENT_TYPE = 'image/gif';
 
-    public function __construct(int $status = 200, array $headers = [])
+    public function __construct(int $status = StatusCode::STATUS_OK, array $headers = [])
     {
         $headers['content-type'] = self::CONTENT_TYPE;
         parent::__construct($this->createBody(), $status, $headers);
@@ -27,7 +28,7 @@ class PixelResponse extends Response
     private function createBody(): StreamInterface
     {
         $body = new Stream('php://temp', 'wb+');
-        $body->write((string) base64_decode(self::BASE_64_IMAGE));
+        $body->write(base64_decode(self::BASE_64_IMAGE));
         $body->rewind();
         return $body;
     }

module/Common/src/Response/QrCodeResponse.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Common\Response;
 
 use Endroid\QrCode\QrCode;
+use Fig\Http\Message\StatusCodeInterface as StatusCode;
 use Psr\Http\Message\StreamInterface;
 use Zend\Diactoros\Response;
 use Zend\Diactoros\Stream;
@@ -12,7 +13,7 @@ class QrCodeResponse extends Response
 {
     use Response\InjectContentTypeTrait;
 
-    public function __construct(QrCode $qrCode, $status = 200, array $headers = [])
+    public function __construct(QrCode $qrCode, int $status = StatusCode::STATUS_OK, array $headers = [])
     {
         parent::__construct(
             $this->createBody($qrCode),
@@ -21,13 +22,7 @@ class QrCodeResponse extends Response
         );
     }
 
-    /**
-     * Create the message body.
-     *
-     * @param QrCode $qrCode
-     * @return StreamInterface
-     */
-    private function createBody(QrCode $qrCode)
+    private function createBody(QrCode $qrCode): StreamInterface
     {
         $body = new Stream('php://temp', 'wb+');
         $body->write($qrCode->get());

module/Common/src/Service/PreviewGenerator.php

@@ -35,11 +35,9 @@ class PreviewGenerator implements PreviewGeneratorInterface
     /**
      * Generates and stores preview for provided website and returns the path to the image file
      *
-     * @param string $url
-     * @return string
      * @throws PreviewGenerationException
      */
-    public function generatePreview($url)
+    public function generatePreview(string $url): string
     {
         /** @var Image $image */
         $image = $this->imageBuilder->build(Image::class, ['url' => $url]);

module/Common/src/Service/PreviewGeneratorInterface.php

@@ -10,9 +10,7 @@ interface PreviewGeneratorInterface
     /**
      * Generates and stores preview for provided website and returns the path to the image file
      *
-     * @param string $url
-     * @return string
      * @throws PreviewGenerationException
      */
-    public function generatePreview($url);
+    public function generatePreview(string $url): string;
 }

module/Common/src/Util/ResponseUtilsTrait.php

@@ -3,37 +3,25 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Common\Util;
 
+use Fig\Http\Message\StatusCodeInterface as StatusCode;
 use finfo;
 use Psr\Http\Message\ResponseInterface;
 use Zend\Diactoros\Response;
 use Zend\Diactoros\Stream;
 use Zend\Stdlib\ArrayUtils;
 use const FILEINFO_MIME;
-use function basename;
 
 trait ResponseUtilsTrait
 {
-    protected function generateDownloadFileResponse(string $filePath): ResponseInterface
-    {
-        return $this->generateBinaryResponse($filePath, [
-            'Content-Disposition' => 'attachment; filename=' . basename($filePath),
-            'Content-Transfer-Encoding' => 'Binary',
-            'Content-Description' => 'File Transfer',
-            'Pragma' => 'public',
-            'Expires' => '0',
-            'Cache-Control' => 'must-revalidate',
-        ]);
-    }
-
-    protected function generateImageResponse(string $imagePath): ResponseInterface
+    private function generateImageResponse(string $imagePath): ResponseInterface
     {
         return $this->generateBinaryResponse($imagePath);
     }
 
-    protected function generateBinaryResponse(string $path, array $extraHeaders = []): ResponseInterface
+    private function generateBinaryResponse(string $path, array $extraHeaders = []): ResponseInterface
     {
         $body = new Stream($path);
-        return new Response($body, 200, ArrayUtils::merge([
+        return new Response($body, StatusCode::STATUS_OK, ArrayUtils::merge([
             'Content-Type' => (new finfo(FILEINFO_MIME))->file($path),
             'Content-Length' => (string) $body->getSize(),
         ], $extraHeaders));

module/Common/src/Util/StringUtilsTrait.php

@@ -9,7 +9,7 @@ use function strlen;
 
 trait StringUtilsTrait
 {
-    private function generateRandomString($length = 10): string
+    private function generateRandomString(int $length = 10): string
     {
         $characters = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
         $charactersLength = strlen($characters);

module/Common/test/Exception/WrongIpExceptionTest.php

@@ -0,0 +1,35 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Common\Exception;
+
+use Exception;
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\Common\Exception\WrongIpException;
+
+class WrongIpExceptionTest extends TestCase
+{
+    /**
+     * @test
+     */
+    public function fromIpAddressProperlyCreatesExceptionWithoutPrev()
+    {
+        $e = WrongIpException::fromIpAddress('1.2.3.4');
+
+        $this->assertEquals('Provided IP "1.2.3.4" is invalid', $e->getMessage());
+        $this->assertEquals(0, $e->getCode());
+        $this->assertNull($e->getPrevious());
+    }
+    /**
+     * @test
+     */
+    public function fromIpAddressProperlyCreatesExceptionWithPrev()
+    {
+        $prev = new Exception('Previous error');
+        $e = WrongIpException::fromIpAddress('1.2.3.4', $prev);
+
+        $this->assertEquals('Provided IP "1.2.3.4" is invalid', $e->getMessage());
+        $this->assertEquals(0, $e->getCode());
+        $this->assertSame($prev, $e->getPrevious());
+    }
+}

module/Common/test/IpGeolocation/GeoLite2/DbUpdaterTest.php

@@ -55,6 +55,7 @@ class DbUpdaterTest extends TestCase
         $request = $this->httpClient->request(Argument::cetera())->willThrow(ClientException::class);
 
         $this->expectException(RuntimeException::class);
+        $this->expectExceptionCode(0);
         $this->expectExceptionMessage(
             'An error occurred while trying to download a fresh copy of the GeoLite2 database'
         );
@@ -73,6 +74,7 @@ class DbUpdaterTest extends TestCase
         $request = $this->httpClient->request(Argument::cetera())->willReturn(new Response());
 
         $this->expectException(RuntimeException::class);
+        $this->expectExceptionCode(0);
         $this->expectExceptionMessage(
             'An error occurred while trying to extract the GeoLite2 database from __invalid__/GeoLite2-City.tar.gz'
         );
@@ -91,6 +93,7 @@ class DbUpdaterTest extends TestCase
         $copy = $this->filesystem->copy(Argument::cetera())->willThrow($e);
 
         $this->expectException(RuntimeException::class);
+        $this->expectExceptionCode(0);
         $this->expectExceptionMessage('An error occurred while trying to copy GeoLite2 db file to destination');
         $request->shouldBeCalledOnce();
         $copy->shouldBeCalledOnce();

module/Common/test/IpGeolocation/GeoLite2LocationResolverTest.php

@@ -41,6 +41,7 @@ class GeoLite2LocationResolverTest extends TestCase
 
         $this->expectException(WrongIpException::class);
         $this->expectExceptionMessage($message);
+        $this->expectExceptionCode(0);
         $cityMethod->shouldBeCalledOnce();
 
         $this->resolver->resolveIpLocation($ipAddress);

module/Common/test/Response/PixelResponseTest.php

@@ -0,0 +1,29 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Common\Response;
+
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\Common\Response\PixelResponse;
+
+class PixelResponseTest extends TestCase
+{
+    /**
+     * @var PixelResponse
+     */
+    private $resp;
+
+    public function setUp()
+    {
+        $this->resp = new PixelResponse();
+    }
+
+    /**
+     * @test
+     */
+    public function responseHasGifTypeAndIsNotEmpty()
+    {
+        $this->assertEquals('image/gif', $this->resp->getHeaderLine('Content-Type'));
+        $this->assertNotEmpty((string) $this->resp->getBody());
+    }
+}

module/Common/test/Response/QrCodeResponseTest.php

@@ -0,0 +1,23 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Common\Response;
+
+use Endroid\QrCode\QrCode;
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\Common\Response\QrCodeResponse;
+
+class QrCodeResponseTest extends TestCase
+{
+    /**
+     * @test
+     */
+    public function providedQrCoideIsSetAsBody()
+    {
+        $qrCode = new QrCode('Hello');
+        $resp = new QrCodeResponse($qrCode);
+
+        $this->assertEquals($qrCode->getContentType(), $resp->getHeaderLine('Content-Type'));
+        $this->assertEquals($qrCode->get(), (string) $resp->getBody());
+    }
+}

module/Common/test/Util/DateRangeTest.php

@@ -32,4 +32,24 @@ class DateRangeTest extends TestCase
         $this->assertSame($endDate, $range->getEndDate());
         $this->assertFalse($range->isEmpty());
     }
+
+    /**
+     * @test
+     * @dataProvider provideDates
+     */
+    public function isConsideredEmptyOnlyIfNoneOfTheDatesIsSet(?Chronos $startDate, ?Chronos $endDate, bool $isEmpty)
+    {
+        $range = new DateRange($startDate, $endDate);
+        $this->assertEquals($isEmpty, $range->isEmpty());
+    }
+
+    public function provideDates(): array
+    {
+        return [
+            [null, null, true],
+            [null, Chronos::now(), false],
+            [Chronos::now(), null, false],
+            [Chronos::now(), Chronos::now(), false],
+        ];
+    }
 }

module/Common/test/Util/StringUtilsTraitTest.php

@@ -0,0 +1,45 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Common\Util;
+
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\Common\Util\StringUtilsTrait;
+use function strlen;
+
+class StringUtilsTraitTest extends TestCase
+{
+    use StringUtilsTrait;
+
+    /**
+     * @test
+     * @dataProvider provideLengths
+     */
+    public function generateRandomStringGeneratesStringOfProvidedLength(int $length)
+    {
+        $this->assertEquals($length, strlen($this->generateRandomString($length)));
+    }
+
+    public function provideLengths(): array
+    {
+        return [
+            [1],
+            [10],
+            [15],
+        ];
+    }
+
+    /**
+     * @test
+     */
+    public function generatesUuidV4()
+    {
+        $uuidPattern = '/[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}/';
+
+        $this->assertRegExp($uuidPattern, $this->generateV4Uuid());
+        $this->assertRegExp($uuidPattern, $this->generateV4Uuid());
+        $this->assertRegExp($uuidPattern, $this->generateV4Uuid());
+        $this->assertRegExp($uuidPattern, $this->generateV4Uuid());
+        $this->assertRegExp($uuidPattern, $this->generateV4Uuid());
+    }
+}

module/Core/src/Entity/Visit.php

@@ -93,7 +93,7 @@ class Visit extends AbstractEntity implements JsonSerializable
         return $this->visitLocation;
     }
 
-    public function setVisitLocation(VisitLocation $visitLocation): self
+    public function locate(VisitLocation $visitLocation): self
     {
         $this->visitLocation = $visitLocation;
         return $this;
@@ -118,4 +118,12 @@ class Visit extends AbstractEntity implements JsonSerializable
             'remoteAddr' => null,
         ];
     }
+
+    /**
+     * @internal
+     */
+    public function getDate(): Chronos
+    {
+        return $this->date;
+    }
 }

module/Core/src/Exception/InvalidShortCodeException.php

@@ -8,7 +8,7 @@ use function sprintf;
 
 class InvalidShortCodeException extends RuntimeException
 {
-    public static function fromCharset($shortCode, $charSet, Exception $previous = null)
+    public static function fromCharset(string $shortCode, string $charSet, Exception $previous = null): self
     {
         $code = $previous !== null ? $previous->getCode() : -1;
         return new static(
@@ -18,7 +18,7 @@ class InvalidShortCodeException extends RuntimeException
         );
     }
 
-    public static function fromNotFoundShortCode($shortCode)
+    public static function fromNotFoundShortCode(string $shortCode): self
     {
         return new static(sprintf('Provided short code "%s" does not belong to a short URL', $shortCode));
     }

module/Core/src/Exception/InvalidUrlException.php

@@ -10,7 +10,7 @@ class InvalidUrlException extends RuntimeException
 {
     public static function fromUrl($url, Throwable $previous = null)
     {
-        $code = isset($previous) ? $previous->getCode() : -1;
+        $code = $previous !== null ? $previous->getCode() : -1;
         return new static(sprintf('Provided URL "%s" is not an existing and valid URL', $url), $code, $previous);
     }
 }

module/Core/src/Exception/IpCannotBeLocatedException.php

@@ -0,0 +1,8 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Exception;
+
+class IpCannotBeLocatedException extends RuntimeException
+{
+}

module/Core/src/Exception/ValidationException.php

@@ -42,7 +42,7 @@ class ValidationException extends RuntimeException
      * @param \Throwable|null $prev
      * @return ValidationException
      */
-    public static function fromArray(array $invalidData, Throwable $prev = null): self
+    private static function fromArray(array $invalidData, Throwable $prev = null): self
     {
         return new self(
             sprintf(

module/Core/src/Repository/VisitRepository.php

@@ -10,15 +10,12 @@ use Shlinkio\Shlink\Core\Entity\Visit;
 
 class VisitRepository extends EntityRepository implements VisitRepositoryInterface
 {
-    /**
-     * @return Visit[]
-     */
-    public function findUnlocatedVisits(): array
+    public function findUnlocatedVisits(): iterable
     {
         $qb = $this->createQueryBuilder('v');
         $qb->where($qb->expr()->isNull('v.visitLocation'));
 
-        return $qb->getQuery()->getResult();
+        return $qb->getQuery()->iterate();
     }
 
     /**

module/Core/src/Repository/VisitRepositoryInterface.php

@@ -10,10 +10,7 @@ use Shlinkio\Shlink\Core\Entity\Visit;
 
 interface VisitRepositoryInterface extends ObjectRepository
 {
-    /**
-     * @return Visit[]
-     */
-    public function findUnlocatedVisits(): array;
+    public function findUnlocatedVisits(): iterable;
 
     /**
      * @param ShortUrl|int $shortUrl

module/Core/src/Service/VisitService.php

@@ -5,6 +5,8 @@ namespace Shlinkio\Shlink\Core\Service;
 
 use Doctrine\ORM\EntityManagerInterface;
 use Shlinkio\Shlink\Core\Entity\Visit;
+use Shlinkio\Shlink\Core\Entity\VisitLocation;
+use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
 use Shlinkio\Shlink\Core\Repository\VisitRepository;
 
 class VisitService implements VisitServiceInterface
@@ -19,22 +21,36 @@ class VisitService implements VisitServiceInterface
         $this->em = $em;
     }
 
-    /**
-     * @return Visit[]
-     */
-    public function getUnlocatedVisits()
+    public function locateVisits(callable $getGeolocationData, ?callable $locatedVisit = null): void
     {
         /** @var VisitRepository $repo */
         $repo = $this->em->getRepository(Visit::class);
-        return $repo->findUnlocatedVisits();
+        $results = $repo->findUnlocatedVisits();
+
+        foreach ($results as [$visit]) {
+            try {
+                $locationData = $getGeolocationData($visit);
+            } catch (IpCannotBeLocatedException $e) {
+                // Skip if the visit's IP could not be located
+                continue;
+            }
+
+            $location = new VisitLocation($locationData);
+            $this->locateVisit($visit, $location, $locatedVisit);
+        }
     }
 
-    /**
-     * @param Visit $visit
-     */
-    public function saveVisit(Visit $visit)
+    private function locateVisit(Visit $visit, VisitLocation $location, ?callable $locatedVisit): void
     {
+        $visit->locate($location);
+
         $this->em->persist($visit);
         $this->em->flush();
+
+        if ($locatedVisit !== null) {
+            $locatedVisit($location, $visit);
+        }
+
+        $this->em->clear();
     }
 }

module/Core/src/Service/VisitServiceInterface.php

@@ -3,17 +3,7 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core\Service;
 
-use Shlinkio\Shlink\Core\Entity\Visit;
-
 interface VisitServiceInterface
 {
-    /**
-     * @return Visit[]
-     */
-    public function getUnlocatedVisits();
-
-    /**
-     * @param Visit $visit
-     */
-    public function saveVisit(Visit $visit);
+    public function locateVisits(callable $getGeolocationData, ?callable $locatedVisit = null): void;
 }

module/Core/test-func/Repository/VisitRepositoryTest.php

@@ -45,14 +45,20 @@ class VisitRepositoryTest extends DatabaseTestCase
             if ($i % 2 === 0) {
                 $location = new VisitLocation([]);
                 $this->getEntityManager()->persist($location);
-                $visit->setVisitLocation($location);
+                $visit->locate($location);
             }
 
             $this->getEntityManager()->persist($visit);
         }
         $this->getEntityManager()->flush();
 
-        $this->assertCount(3, $this->repo->findUnlocatedVisits());
+        $resultsCount = 0;
+        $results = $this->repo->findUnlocatedVisits();
+        foreach ($results as $value) {
+            $resultsCount++;
+        }
+
+        $this->assertEquals(3, $resultsCount);
     }
 
     /**

module/Core/test/Entity/VisitTest.php

@@ -0,0 +1,40 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Core\Entity;
+
+use Cake\Chronos\Chronos;
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\Core\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\Entity\Visit;
+use Shlinkio\Shlink\Core\Model\Visitor;
+
+class VisitTest extends TestCase
+{
+    /**
+     * @test
+     * @dataProvider provideDates
+     */
+    public function isProperlyJsonSerialized(?Chronos $date)
+    {
+        $visit = new Visit(new ShortUrl(''), new Visitor('Chrome', 'some site', '1.2.3.4'), $date);
+
+        $this->assertEquals([
+            'referer' => 'some site',
+            'date' => ($date ?? $visit->getDate())->toAtomString(),
+            'userAgent' => 'Chrome',
+            'visitLocation' => null,
+
+            // Deprecated
+            'remoteAddr' => null,
+        ], $visit->jsonSerialize());
+    }
+
+    public function provideDates(): array
+    {
+        return [
+            [null],
+            [Chronos::now()->subDays(10)],
+        ];
+    }
+}

module/Core/test/Exception/DeleteShortUrlExceptionTest.php

@@ -21,6 +21,7 @@ class DeleteShortUrlExceptionTest extends TestCase
     ) {
         $e = DeleteShortUrlException::fromVisitsThreshold($threshold, $shortCode);
         $this->assertEquals($expectedMessage, $e->getMessage());
+        $this->assertEquals(0, $e->getCode());
     }
 
     public function provideMessages(): array

module/Core/test/Exception/InvalidShortCodeExceptionTest.php

@@ -0,0 +1,43 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Core\Exception;
+
+use Exception;
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
+use Throwable;
+
+class InvalidShortCodeExceptionTest extends TestCase
+{
+    /**
+     * @test
+     * @dataProvider providePrevious
+     */
+    public function properlyCreatesExceptionFromCharset(?Throwable $prev)
+    {
+        $e = InvalidShortCodeException::fromCharset('abc123', 'def456', $prev);
+
+        $this->assertEquals('Provided short code "abc123" does not match the char set "def456"', $e->getMessage());
+        $this->assertEquals($prev !== null ? $prev->getCode() : -1, $e->getCode());
+        $this->assertEquals($prev, $e->getPrevious());
+    }
+
+    public function providePrevious(): array
+    {
+        return [
+            [null],
+            [new Exception('Previos error', 10)],
+        ];
+    }
+
+    /**
+     * @test
+     */
+    public function properlyCreatesExceptionFromNotFoundShortCode()
+    {
+        $e = InvalidShortCodeException::fromNotFoundShortCode('abc123');
+
+        $this->assertEquals('Provided short code "abc123" does not belong to a short URL', $e->getMessage());
+    }
+}

module/Core/test/Exception/InvalidUrlExceptionTest.php

@@ -0,0 +1,33 @@
+<?php
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Core\Exception;
+
+use Exception;
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
+use Throwable;
+
+class InvalidUrlExceptionTest extends TestCase
+{
+    /**
+     * @test
+     * @dataProvider providePrevious
+     */
+    public function properlyCreatesExceptionFromUrl(?Throwable $prev)
+    {
+        $e = InvalidUrlException::fromUrl('http://the_url.com', $prev);
+
+        $this->assertEquals('Provided URL "http://the_url.com" is not an existing and valid URL', $e->getMessage());
+        $this->assertEquals($prev !== null ? $prev->getCode() : -1, $e->getCode());
+        $this->assertEquals($prev, $e->getPrevious());
+    }
+
+    public function providePrevious(): array
+    {
+        return [
+            [null],
+            [new Exception('Previos error', 10)],
+        ];
+    }
+}

module/Core/test/Service/VisitServiceTest.php

@@ -5,12 +5,18 @@ namespace ShlinkioTest\Shlink\Core\Service;
 
 use Doctrine\ORM\EntityManager;
 use PHPUnit\Framework\TestCase;
+use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
+use Shlinkio\Shlink\Core\Entity\VisitLocation;
+use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
 use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Repository\VisitRepository;
 use Shlinkio\Shlink\Core\Service\VisitService;
+use function array_shift;
+use function count;
+use function func_get_args;
 
 class VisitServiceTest extends TestCase
 {
@@ -32,22 +38,68 @@ class VisitServiceTest extends TestCase
     /**
      * @test
      */
-    public function saveVisitsPersistsProvidedVisit()
+    public function locateVisitsIteratesAndLocatesUnlocatedVisits()
     {
-        $visit = new Visit(new ShortUrl(''), Visitor::emptyInstance());
-        $this->em->persist($visit)->shouldBeCalledOnce();
-        $this->em->flush()->shouldBeCalledOnce();
-        $this->visitService->saveVisit($visit);
+        $unlocatedVisits = [
+            [new Visit(new ShortUrl('foo'), Visitor::emptyInstance())],
+            [new Visit(new ShortUrl('bar'), Visitor::emptyInstance())],
+        ];
+
+        $repo = $this->prophesize(VisitRepository::class);
+        $findUnlocatedVisits = $repo->findUnlocatedVisits()->willReturn($unlocatedVisits);
+        $getRepo = $this->em->getRepository(Visit::class)->willReturn($repo->reveal());
+
+        $persist = $this->em->persist(Argument::type(Visit::class))->will(function () {
+        });
+        $flush = $this->em->flush()->will(function () {
+        });
+        $clear = $this->em->clear()->will(function () {
+        });
+
+        $this->visitService->locateVisits(function () {
+            return [];
+        }, function () {
+            $args = func_get_args();
+
+            $this->assertInstanceOf(VisitLocation::class, array_shift($args));
+            $this->assertInstanceOf(Visit::class, array_shift($args));
+        });
+
+        $findUnlocatedVisits->shouldHaveBeenCalledOnce();
+        $getRepo->shouldHaveBeenCalledOnce();
+        $persist->shouldHaveBeenCalledTimes(count($unlocatedVisits));
+        $flush->shouldHaveBeenCalledTimes(count($unlocatedVisits));
+        $clear->shouldHaveBeenCalledTimes(count($unlocatedVisits));
     }
 
     /**
      * @test
      */
-    public function getUnlocatedVisitsFallbacksToRepository()
+    public function visitsWhichCannotBeLocatedAreIgnored()
     {
+        $unlocatedVisits = [
+            [new Visit(new ShortUrl('foo'), Visitor::emptyInstance())],
+        ];
+
         $repo = $this->prophesize(VisitRepository::class);
-        $repo->findUnlocatedVisits()->shouldBeCalledOnce();
-        $this->em->getRepository(Visit::class)->willReturn($repo->reveal())->shouldBeCalledOnce();
-        $this->visitService->getUnlocatedVisits();
+        $findUnlocatedVisits = $repo->findUnlocatedVisits()->willReturn($unlocatedVisits);
+        $getRepo = $this->em->getRepository(Visit::class)->willReturn($repo->reveal());
+
+        $persist = $this->em->persist(Argument::type(Visit::class))->will(function () {
+        });
+        $flush = $this->em->flush()->will(function () {
+        });
+        $clear = $this->em->clear()->will(function () {
+        });
+
+        $this->visitService->locateVisits(function () {
+            throw new IpCannotBeLocatedException('Cannot be located');
+        });
+
+        $findUnlocatedVisits->shouldHaveBeenCalledOnce();
+        $getRepo->shouldHaveBeenCalledOnce();
+        $persist->shouldNotHaveBeenCalled();
+        $flush->shouldNotHaveBeenCalled();
+        $clear->shouldNotHaveBeenCalled();
     }
 }