From 2d1d7357a316534b904f44b87fac846efb47e8e9 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Tue, 26 Feb 2019 21:39:45 +0100
Subject: [PATCH] Given more semantic cases in which a visit cannot be located

---
 .../Command/Visit/ProcessVisitsCommand.php    |  6 +--
 .../Exception/IpCannotBeLocatedException.php  | 37 +++++++++++++++++++
 module/Core/src/Service/VisitService.php      | 10 ++++-
 module/Core/test/Service/VisitServiceTest.php |  2 +-
 4 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php
index 25006cc9..215977d8 100644
--- a/module/CLI/src/Command/Visit/ProcessVisitsCommand.php
+++ b/module/CLI/src/Command/Visit/ProcessVisitsCommand.php
@@ -83,14 +83,14 @@ class ProcessVisitsCommand extends Command
                 '<comment>Ignored visit with no IP address</comment>',
                 OutputInterface::VERBOSITY_VERBOSE
             );
-            throw new IpCannotBeLocatedException('Ignored visit with no IP address');
+            throw IpCannotBeLocatedException::forEmptyAddress();
         }
 
         $ipAddr = $visit->getRemoteAddr();
         $this->output->write(sprintf('Processing IP <fg=blue>%s</>', $ipAddr));
         if ($ipAddr === IpAddress::LOCALHOST) {
             $this->output->writeln(' [<comment>Ignored localhost address</comment>]');
-            throw new IpCannotBeLocatedException('Ignored localhost address');
+            throw IpCannotBeLocatedException::forLocalhost();
         }
 
         try {
@@ -101,7 +101,7 @@ class ProcessVisitsCommand extends Command
                 $this->getApplication()->renderException($e, $this->output);
             }
 
-            throw new IpCannotBeLocatedException('An error occurred while locating IP', $e->getCode(), $e);
+            throw IpCannotBeLocatedException::forError($e);
         }
     }
 }
diff --git a/module/Core/src/Exception/IpCannotBeLocatedException.php b/module/Core/src/Exception/IpCannotBeLocatedException.php
index e172e9c9..3908385e 100644
--- a/module/Core/src/Exception/IpCannotBeLocatedException.php
+++ b/module/Core/src/Exception/IpCannotBeLocatedException.php
@@ -3,6 +3,43 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core\Exception;
 
+use Throwable;
+
 class IpCannotBeLocatedException extends RuntimeException
 {
+    /** @var bool */
+    private $isNonLocatableAddress;
+
+    public function __construct(
+        bool $isNonLocatableAddress,
+        string $message,
+        int $code = 0,
+        ?Throwable $previous = null
+    ) {
+        $this->isNonLocatableAddress = $isNonLocatableAddress;
+        parent::__construct($message, $code, $previous);
+    }
+
+    public static function forEmptyAddress(): self
+    {
+        return new self(true, 'Ignored visit with no IP address');
+    }
+
+    public static function forLocalhost(): self
+    {
+        return new self(true, 'Ignored localhost address');
+    }
+
+    public static function forError(Throwable $e): self
+    {
+        return new self(false, 'An error occurred while locating IP', $e->getCode(), $e);
+    }
+
+    /**
+     * Tells if this error belongs to an address that will never be possible locate
+     */
+    public function isNonLocatableAddress(): bool
+    {
+        return $this->isNonLocatableAddress;
+    }
 }
diff --git a/module/Core/src/Service/VisitService.php b/module/Core/src/Service/VisitService.php
index 4abbf4a2..62e48010 100644
--- a/module/Core/src/Service/VisitService.php
+++ b/module/Core/src/Service/VisitService.php
@@ -30,12 +30,18 @@ class VisitService implements VisitServiceInterface
 
         foreach ($results as $visit) {
             $count++;
+
             try {
                 /** @var Location $location */
                 $location = $geolocateVisit($visit);
             } catch (IpCannotBeLocatedException $e) {
-                // Skip if the visit's IP could not be located
-                continue;
+                if (!$e->isNonLocatableAddress()) {
+                    // Skip if the visit's IP could not be located because of an error
+                    continue;
+                }
+
+                // If the IP address is non-locatable, locate it as empty to prevent next processes to pick it again
+                $location = Location::emptyInstance();
             }
 
             $location = new VisitLocation($location);
diff --git a/module/Core/test/Service/VisitServiceTest.php b/module/Core/test/Service/VisitServiceTest.php
index be2bf8ba..46f1da37 100644
--- a/module/Core/test/Service/VisitServiceTest.php
+++ b/module/Core/test/Service/VisitServiceTest.php
@@ -89,7 +89,7 @@ class VisitServiceTest extends TestCase
         });
 
         $this->visitService->locateUnlocatedVisits(function () {
-            throw new IpCannotBeLocatedException('Cannot be located');
+            throw new IpCannotBeLocatedException(false, 'Cannot be located');
         });
 
         $findUnlocatedVisits->shouldHaveBeenCalledOnce();