Updated LocateShortUrlVisit so that it dispatches a VisitLocated event

module/Core/config/event_dispatcher.config.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core;
 
+use Psr\EventDispatcher\EventDispatcherInterface;
 use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater;
 use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
 use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory;
@@ -16,7 +17,7 @@ return [
             EventDispatcher\ShortUrlVisited::class => [
                 EventDispatcher\LocateShortUrlVisit::class,
             ],
-            EventDispatcher\ShortUrlLocated::class => [
+            EventDispatcher\VisitLocated::class => [
                 EventDispatcher\NotifyVisitToWebHooks::class,
             ],
         ],
@@ -34,6 +35,7 @@ return [
             'em',
             'Logger_Shlink',
             GeolocationDbUpdater::class,
+            EventDispatcherInterface::class,
         ],
         EventDispatcher\NotifyVisitToWebHooks::class => [
             'httpClient',

module/Core/src/EventDispatcher/LocateShortUrlVisit.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core\EventDispatcher;
 
 use Doctrine\ORM\EntityManagerInterface;
+use Psr\EventDispatcher\EventDispatcherInterface;
 use Psr\Log\LoggerInterface;
 use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
 use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface;
@@ -26,17 +27,21 @@ class LocateShortUrlVisit
     private $logger;
     /** @var GeolocationDbUpdaterInterface */
     private $dbUpdater;
+    /** @var EventDispatcherInterface */
+    private $eventDispatcher;
 
     public function __construct(
         IpLocationResolverInterface $ipLocationResolver,
         EntityManagerInterface $em,
         LoggerInterface $logger,
-        GeolocationDbUpdaterInterface $dbUpdater
+        GeolocationDbUpdaterInterface $dbUpdater,
+        EventDispatcherInterface $eventDispatcher
     ) {
         $this->ipLocationResolver = $ipLocationResolver;
         $this->em = $em;
         $this->logger = $logger;
         $this->dbUpdater = $dbUpdater;
+        $this->eventDispatcher = $eventDispatcher;
     }
 
     public function __invoke(ShortUrlVisited $shortUrlVisited): void
@@ -52,6 +57,15 @@ class LocateShortUrlVisit
             return;
         }
 
+        if ($this->downloadOrUpdateGeoLiteDb($visitId)) {
+            $this->locateVisit($visitId, $visit);
+        }
+
+        $this->eventDispatcher->dispatch(new VisitLocated($visitId));
+    }
+
+    private function downloadOrUpdateGeoLiteDb(string $visitId): bool
+    {
         try {
             $this->dbUpdater->checkDbUpdate(function (bool $olderDbExists) {
                 $this->logger->notice(sprintf('%s GeoLite2 database...', $olderDbExists ? 'Updating' : 'Downloading'));
@@ -62,25 +76,29 @@ class LocateShortUrlVisit
                     'GeoLite2 database download failed. It is not possible to locate visit with id {visitId}. {e}',
                     ['e' => $e, 'visitId' => $visitId]
                 );
-                return;
+                return false;
             }
 
             $this->logger->warning('GeoLite2 database update failed. Proceeding with old version. {e}', ['e' => $e]);
         }
 
+        return true;
+    }
+
+    private function locateVisit(string $visitId, Visit $visit): void
+    {
         try {
             $location = $visit->isLocatable()
                 ? $this->ipLocationResolver->resolveIpLocation($visit->getRemoteAddr())
                 : Location::emptyInstance();
+
+            $visit->locate(new VisitLocation($location));
+            $this->em->flush();
         } catch (WrongIpException $e) {
             $this->logger->warning(
                 'Tried to locate visit with id "{visitId}", but its address seems to be wrong. {e}',
                 ['e' => $e, 'visitId' => $visitId]
             );
-            return;
         }
-
-        $visit->locate(new VisitLocation($location));
-        $this->em->flush();
     }
 }

module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php

@@ -51,7 +51,7 @@ class NotifyVisitToWebHooks
         $this->appOptions = $appOptions;
     }
 
-    public function __invoke(ShortUrlLocated $shortUrlLocated): void
+    public function __invoke(VisitLocated $shortUrlLocated): void
     {
         if (empty($this->webhooks)) {
             return;

module/Core/src/EventDispatcher/VisitLocated.php

@@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core\EventDispatcher;
 
 use JsonSerializable;
 
-final class ShortUrlLocated implements JsonSerializable
+final class VisitLocated implements JsonSerializable
 {
     /** @var string */
     private $visitId;

module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php

@@ -8,6 +8,7 @@ use Doctrine\ORM\EntityManagerInterface;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
+use Psr\EventDispatcher\EventDispatcherInterface;
 use Psr\Log\LoggerInterface;
 use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
 use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface;
@@ -17,6 +18,7 @@ use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Entity\VisitLocation;
 use Shlinkio\Shlink\Core\EventDispatcher\LocateShortUrlVisit;
 use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited;
+use Shlinkio\Shlink\Core\EventDispatcher\VisitLocated;
 use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation;
 use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
@@ -35,6 +37,8 @@ class LocateShortUrlVisitTest extends TestCase
     private $logger;
     /** @var ObjectProphecy */
     private $dbUpdater;
+    /** @var ObjectProphecy */
+    private $eventDispatcher;
 
     public function setUp(): void
     {
@@ -42,12 +46,14 @@ class LocateShortUrlVisitTest extends TestCase
         $this->em = $this->prophesize(EntityManagerInterface::class);
         $this->logger = $this->prophesize(LoggerInterface::class);
         $this->dbUpdater = $this->prophesize(GeolocationDbUpdaterInterface::class);
+        $this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class);
 
         $this->locateVisit = new LocateShortUrlVisit(
             $this->ipLocationResolver->reveal(),
             $this->em->reveal(),
             $this->logger->reveal(),
-            $this->dbUpdater->reveal()
+            $this->dbUpdater->reveal(),
+            $this->eventDispatcher->reveal()
         );
     }
 
@@ -59,6 +65,8 @@ class LocateShortUrlVisitTest extends TestCase
         $logWarning = $this->logger->warning('Tried to locate visit with id "{visitId}", but it does not exist.', [
             'visitId' => 123,
         ]);
+        $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function () {
+        });
 
         ($this->locateVisit)($event);
 
@@ -66,6 +74,7 @@ class LocateShortUrlVisitTest extends TestCase
         $this->em->flush()->shouldNotHaveBeenCalled();
         $this->ipLocationResolver->resolveIpLocation(Argument::cetera())->shouldNotHaveBeenCalled();
         $logWarning->shouldHaveBeenCalled();
+        $dispatch->shouldNotHaveBeenCalled();
     }
 
     /** @test */
@@ -82,6 +91,8 @@ class LocateShortUrlVisitTest extends TestCase
             Argument::containingString('Tried to locate visit with id "{visitId}", but its address seems to be wrong.'),
             Argument::type('array')
         );
+        $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function () {
+        });
 
         ($this->locateVisit)($event);
 
@@ -89,6 +100,7 @@ class LocateShortUrlVisitTest extends TestCase
         $resolveLocation->shouldHaveBeenCalledOnce();
         $logWarning->shouldHaveBeenCalled();
         $this->em->flush()->shouldNotHaveBeenCalled();
+        $dispatch->shouldHaveBeenCalledOnce();
     }
 
     /**
@@ -102,6 +114,8 @@ class LocateShortUrlVisitTest extends TestCase
         $flush = $this->em->flush()->will(function () {
         });
         $resolveIp = $this->ipLocationResolver->resolveIpLocation(Argument::any());
+        $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function () {
+        });
 
         ($this->locateVisit)($event);
 
@@ -110,6 +124,7 @@ class LocateShortUrlVisitTest extends TestCase
         $flush->shouldHaveBeenCalledOnce();
         $resolveIp->shouldNotHaveBeenCalled();
         $this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled();
+        $dispatch->shouldHaveBeenCalledOnce();
     }
 
     public function provideNonLocatableVisits(): iterable
@@ -133,6 +148,8 @@ class LocateShortUrlVisitTest extends TestCase
         $flush = $this->em->flush()->will(function () {
         });
         $resolveIp = $this->ipLocationResolver->resolveIpLocation($ipAddr)->willReturn($location);
+        $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function () {
+        });
 
         ($this->locateVisit)($event);
 
@@ -141,6 +158,7 @@ class LocateShortUrlVisitTest extends TestCase
         $flush->shouldHaveBeenCalledOnce();
         $resolveIp->shouldHaveBeenCalledOnce();
         $this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled();
+        $dispatch->shouldHaveBeenCalledOnce();
     }
 
     /** @test */
@@ -157,6 +175,8 @@ class LocateShortUrlVisitTest extends TestCase
         });
         $resolveIp = $this->ipLocationResolver->resolveIpLocation($ipAddr)->willReturn($location);
         $checkUpdateDb = $this->dbUpdater->checkDbUpdate(Argument::cetera())->willThrow($e);
+        $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function () {
+        });
 
         ($this->locateVisit)($event);
 
@@ -169,6 +189,7 @@ class LocateShortUrlVisitTest extends TestCase
             'GeoLite2 database update failed. Proceeding with old version. {e}',
             ['e' => $e]
         )->shouldHaveBeenCalledOnce();
+        $dispatch->shouldHaveBeenCalledOnce();
     }
 
     /** @test */
@@ -189,6 +210,8 @@ class LocateShortUrlVisitTest extends TestCase
             'GeoLite2 database download failed. It is not possible to locate visit with id {visitId}. {e}',
             ['e' => $e, 'visitId' => 123]
         );
+        $dispatch = $this->eventDispatcher->dispatch(new VisitLocated('123'))->will(function () {
+        });
 
         ($this->locateVisit)($event);
 
@@ -198,5 +221,6 @@ class LocateShortUrlVisitTest extends TestCase
         $resolveIp->shouldNotHaveBeenCalled();
         $checkUpdateDb->shouldHaveBeenCalledOnce();
         $logError->shouldHaveBeenCalledOnce();
+        $dispatch->shouldHaveBeenCalledOnce();
     }
 }

module/Rest/test/EventDispatcher/NotifyVisitToWebHooksTest.php

@@ -19,7 +19,7 @@ use Psr\Log\LoggerInterface;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\EventDispatcher\NotifyVisitToWebHooks;
-use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlLocated;
+use Shlinkio\Shlink\Core\EventDispatcher\VisitLocated;
 use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Options\AppOptions;
 
@@ -47,7 +47,7 @@ class NotifyVisitToWebHooksTest extends TestCase
     {
         $find = $this->em->find(Visit::class, '1')->willReturn(null);
 
-        $this->createListener([])(new ShortUrlLocated('1'));
+        $this->createListener([])(new VisitLocated('1'));
 
         $find->shouldNotHaveBeenCalled();
     }
@@ -66,7 +66,7 @@ class NotifyVisitToWebHooksTest extends TestCase
             ['visitId' => '1']
         );
 
-        $this->createListener(['foo', 'bar'])(new ShortUrlLocated('1'));
+        $this->createListener(['foo', 'bar'])(new VisitLocated('1'));
 
         $find->shouldHaveBeenCalledOnce();
         $logWarning->shouldHaveBeenCalledOnce();
@@ -111,7 +111,7 @@ class NotifyVisitToWebHooksTest extends TestCase
             })
         );
 
-        $this->createListener($webhooks)(new ShortUrlLocated('1'));
+        $this->createListener($webhooks)(new VisitLocated('1'));
 
         $find->shouldHaveBeenCalledOnce();
         $requestAsync->shouldHaveBeenCalledTimes(count($webhooks));