Updated tests

module/CLI/src/Util/GeolocationDbUpdater.php

@@ -35,9 +35,7 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
     public function checkDbUpdate(callable $mustBeUpdated = null, callable $handleProgress = null): void
     {
         $lock = $this->locker->createLock(self::LOCK_NAME);
-        if (! $lock->acquire()) {
+        $lock->acquire(true); // Block until lock is released
-            return;
-        }
 
         try {
             $meta = $this->geoLiteDbReader->metadata();

module/CLI/test/Util/GeolocationDbUpdaterTest.php

@@ -8,11 +8,13 @@ use GeoIp2\Database\Reader;
 use InvalidArgumentException;
 use MaxMind\Db\Reader\Metadata;
 use PHPUnit\Framework\TestCase;
+use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
 use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater;
 use Shlinkio\Shlink\Common\Exception\RuntimeException;
 use Shlinkio\Shlink\Common\IpGeolocation\GeoLite2\DbUpdaterInterface;
+use Symfony\Component\Lock;
 use Throwable;
 
 use function Functional\map;
@@ -26,15 +28,27 @@ class GeolocationDbUpdaterTest extends TestCase
     private $dbUpdater;
     /** @var ObjectProphecy */
     private $geoLiteDbReader;
+    /** @var ObjectProphecy */
+    private $locker;
+    /** @var ObjectProphecy */
+    private $lock;
 
     public function setUp(): void
     {
         $this->dbUpdater = $this->prophesize(DbUpdaterInterface::class);
         $this->geoLiteDbReader = $this->prophesize(Reader::class);
 
+        $this->locker = $this->prophesize(Lock\Factory::class);
+        $this->lock = $this->prophesize(Lock\LockInterface::class);
+        $this->lock->acquire(true)->willReturn(true);
+        $this->lock->release()->will(function () {
+        });
+        $this->locker->createLock(Argument::type('string'))->willReturn($this->lock->reveal());
+
         $this->geolocationDbUpdater = new GeolocationDbUpdater(
             $this->dbUpdater->reveal(),
-            $this->geoLiteDbReader->reveal()
+            $this->geoLiteDbReader->reveal(),
+            $this->locker->reveal()
         );
     }
 

module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php

@@ -8,6 +8,8 @@ use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Psr\Log\LoggerInterface;
+use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
+use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface;
 use Shlinkio\Shlink\Common\Exception\WrongIpException;
 use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface;
 use Shlinkio\Shlink\Common\IpGeolocation\Model\Location;
@@ -18,6 +20,7 @@ use Shlinkio\Shlink\Core\Entity\VisitLocation;
 use Shlinkio\Shlink\Core\EventDispatcher\LocateShortUrlVisit;
 use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited;
 use Shlinkio\Shlink\Core\Model\Visitor;
+use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation;
 
 class LocateShortUrlVisitTest extends TestCase
 {
@@ -29,17 +32,21 @@ class LocateShortUrlVisitTest extends TestCase
     private $em;
     /** @var ObjectProphecy */
     private $logger;
+    /** @var ObjectProphecy */
+    private $dbUpdater;
 
     public function setUp(): void
     {
         $this->ipLocationResolver = $this->prophesize(IpLocationResolverInterface::class);
         $this->em = $this->prophesize(EntityManagerInterface::class);
         $this->logger = $this->prophesize(LoggerInterface::class);
+        $this->dbUpdater = $this->prophesize(GeolocationDbUpdaterInterface::class);
 
         $this->locateVisit = new LocateShortUrlVisit(
             $this->ipLocationResolver->reveal(),
             $this->em->reveal(),
-            $this->logger->reveal()
+            $this->logger->reveal(),
+            $this->dbUpdater->reveal()
         );
     }
 
@@ -132,4 +139,61 @@ class LocateShortUrlVisitTest extends TestCase
         $resolveIp->shouldHaveBeenCalledOnce();
         $this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled();
     }
+
+    /** @test */
+    public function errorWhenUpdatingGeoliteWithExistingCopyLogsWarning(): void
+    {
+        $e = GeolocationDbUpdateFailedException::create(true);
+        $ipAddr = '1.2.3.0';
+        $visit = new Visit(new ShortUrl(''), new Visitor('', '', $ipAddr));
+        $location = new Location('', '', '', '', 0.0, 0.0, '');
+        $event = new ShortUrlVisited('123');
+
+        $findVisit = $this->em->find(Visit::class, '123')->willReturn($visit);
+        $flush = $this->em->flush($visit)->will(function () {
+        });
+        $resolveIp = $this->ipLocationResolver->resolveIpLocation($ipAddr)->willReturn($location);
+        $checkUpdateDb = $this->dbUpdater->checkDbUpdate(Argument::cetera())->willThrow($e);
+
+        ($this->locateVisit)($event);
+
+        $this->assertEquals($visit->getVisitLocation(), new VisitLocation($location));
+        $findVisit->shouldHaveBeenCalledOnce();
+        $flush->shouldHaveBeenCalledOnce();
+        $resolveIp->shouldHaveBeenCalledOnce();
+        $checkUpdateDb->shouldHaveBeenCalledOnce();
+        $this->logger->warning(
+            'GeoLite2 database update failed. Proceeding with old version. {e}',
+            ['e' => $e]
+        )->shouldHaveBeenCalledOnce();
+    }
+
+    /** @test */
+    public function errorWhenDownloadingGeoliteCancelsLocation(): void
+    {
+        $e = GeolocationDbUpdateFailedException::create(false);
+        $ipAddr = '1.2.3.0';
+        $visit = new Visit(new ShortUrl(''), new Visitor('', '', $ipAddr));
+        $location = new Location('', '', '', '', 0.0, 0.0, '');
+        $event = new ShortUrlVisited('123');
+
+        $findVisit = $this->em->find(Visit::class, '123')->willReturn($visit);
+        $flush = $this->em->flush($visit)->will(function () {
+        });
+        $resolveIp = $this->ipLocationResolver->resolveIpLocation($ipAddr)->willReturn($location);
+        $checkUpdateDb = $this->dbUpdater->checkDbUpdate(Argument::cetera())->willThrow($e);
+        $logError = $this->logger->error(
+            'GeoLite2 database download failed. It is not possible to locate visit with id 123. {e}',
+            ['e' => $e]
+        );
+
+        ($this->locateVisit)($event);
+
+        $this->assertEquals($visit->getVisitLocation(), new UnknownVisitLocation());
+        $findVisit->shouldHaveBeenCalledOnce();
+        $flush->shouldNotHaveBeenCalled();
+        $resolveIp->shouldNotHaveBeenCalled();
+        $checkUpdateDb->shouldHaveBeenCalledOnce();
+        $logError->shouldHaveBeenCalledOnce();
+    }
 }