Merge pull request #1138 from acelaya-forks/feature/fix-import-with-no-visits

Feature/fix import with no visits

.github/workflows/publish-release.yml

@@ -43,7 +43,6 @@ jobs:
         uses: docker://antonyurchenko/git-release:latest
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-          ALLOW_TAG_PREFIX: "true"
           ALLOW_EMPTY_CHANGELOG: "true"
         with:
           args: |

CHANGELOG.md

@@ -4,6 +4,58 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org).
 
+## [2.7.3] - 2021-08-02
+### Added
+* *Nothing*
+
+### Changed
+* *Nothing*
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* [#1135](https://github.com/shlinkio/shlink/issues/1135) Fixed error when importing short URLs with no visits from another Shlink instance.
+* [#1136](https://github.com/shlinkio/shlink/issues/1136) Fixed error when fetching tag/short-url/orphan visits for a page lower than 1.
+
+
+## [2.7.2] - 2021-07-30
+### Added
+* *Nothing*
+
+### Changed
+* *Nothing*
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* [#1128](https://github.com/shlinkio/shlink/issues/1128) Increased memory limit reserved for the docker image, preventing it from crashing on GeoLite db download.
+
+
+## [2.7.1] - 2021-05-30
+### Added
+* *Nothing*
+
+### Changed
+* *Nothing*
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* [#1100](https://github.com/shlinkio/shlink/issues/1100) Fixed Shlink trying to download GeoLite2 db files even when tracking has been disabled.
+
+
 ## [2.7.0] - 2021-05-23
 ### Added
 * [#1044](https://github.com/shlinkio/shlink/issues/1044) Added ability to set names on API keys, which helps to identify them when the list grows.

composer.json

@@ -24,6 +24,7 @@
         "endroid/qr-code": "^4.0",
         "geoip2/geoip2": "^2.9",
         "guzzlehttp/guzzle": "^7.0",
+        "guzzlehttp/psr7": "^1.7",
         "happyr/doctrine-specification": "^2.0",
         "jaybizzle/crawler-detect": "^1.2",
         "laminas/laminas-config": "^3.3",
@@ -50,9 +51,9 @@
         "shlinkio/shlink-common": "^3.7",
         "shlinkio/shlink-config": "^1.0",
         "shlinkio/shlink-event-dispatcher": "^2.1",
-        "shlinkio/shlink-importer": "^2.3",
+        "shlinkio/shlink-importer": "^2.3.1",
         "shlinkio/shlink-installer": "^6.0",
-        "shlinkio/shlink-ip-geolocation": "^1.5",
+        "shlinkio/shlink-ip-geolocation": "^2.0",
         "symfony/console": "^5.1",
         "symfony/filesystem": "^5.1",
         "symfony/lock": "^5.1",

docker/config/php.ini

@@ -1,3 +1,4 @@
 log_errors_max_len=0
 zend.assertions=1
 assert.exception=1
+memory_limit=256M

docker/config/shlink_in_docker.local.php

@@ -167,7 +167,7 @@ return [
     ],
 
     'geolite2' => [
-        'license_key' => env('GEOLITE_LICENSE_KEY', 'G4Lm0C60yJsnkdPi'), // Deprecated. Remove the default value
+        'license_key' => env('GEOLITE_LICENSE_KEY', 'G4Lm0C60yJsnkdPi'), // Deprecated. Remove hardcoded license on v3
     ],
 
     'mercure' => $helper->getMercureConfig(),

module/CLI/config/dependencies.config.php

@@ -10,6 +10,7 @@ use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory;
 use Laminas\ServiceManager\Factory\InvokableFactory;
 use Shlinkio\Shlink\Common\Doctrine\NoDbNameConnectionFactory;
 use Shlinkio\Shlink\Core\Domain\DomainService;
+use Shlinkio\Shlink\Core\Options\TrackingOptions;
 use Shlinkio\Shlink\Core\Service;
 use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier;
 use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer;
@@ -64,7 +65,12 @@ return [
     ],
 
     ConfigAbstractFactory::class => [
-        Util\GeolocationDbUpdater::class => [DbUpdater::class, Reader::class, LOCAL_LOCK_FACTORY],
+        Util\GeolocationDbUpdater::class => [
+            DbUpdater::class,
+            Reader::class,
+            LOCAL_LOCK_FACTORY,
+            TrackingOptions::class,
+        ],
         Util\ProcessRunner::class => [SymfonyCli\Helper\ProcessHelper::class],
         ApiKey\RoleResolver::class => [DomainService::class],
 

module/CLI/src/Util/GeolocationDbUpdater.php

@@ -8,6 +8,7 @@ use Cake\Chronos\Chronos;
 use GeoIp2\Database\Reader;
 use MaxMind\Db\Reader\Metadata;
 use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
+use Shlinkio\Shlink\Core\Options\TrackingOptions;
 use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException;
 use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface;
 use Symfony\Component\Lock\LockFactory;
@@ -21,12 +22,18 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
     private DbUpdaterInterface $dbUpdater;
     private Reader $geoLiteDbReader;
     private LockFactory $locker;
+    private TrackingOptions $trackingOptions;
 
-    public function __construct(DbUpdaterInterface $dbUpdater, Reader $geoLiteDbReader, LockFactory $locker)
-    {
+    public function __construct(
+        DbUpdaterInterface $dbUpdater,
+        Reader $geoLiteDbReader,
+        LockFactory $locker,
+        TrackingOptions $trackingOptions
+    ) {
         $this->dbUpdater = $dbUpdater;
         $this->geoLiteDbReader = $geoLiteDbReader;
         $this->locker = $locker;
+        $this->trackingOptions = $trackingOptions;
     }
 
     /**
@@ -34,6 +41,10 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
      */
     public function checkDbUpdate(?callable $beforeDownload = null, ?callable $handleProgress = null): void
     {
+        if ($this->trackingOptions->disableTracking() || $this->trackingOptions->disableIpTracking()) {
+            return;
+        }
+
         $lock = $this->locker->createLock(self::LOCK_NAME);
         $lock->acquire(true); // Block until lock is released
 

module/CLI/test/Util/GeolocationDbUpdaterTest.php

@@ -13,6 +13,7 @@ use Prophecy\PhpUnit\ProphecyTrait;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
 use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater;
+use Shlinkio\Shlink\Core\Options\TrackingOptions;
 use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException;
 use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface;
 use Symfony\Component\Lock;
@@ -28,11 +29,13 @@ class GeolocationDbUpdaterTest extends TestCase
     private GeolocationDbUpdater $geolocationDbUpdater;
     private ObjectProphecy $dbUpdater;
     private ObjectProphecy $geoLiteDbReader;
+    private TrackingOptions $trackingOptions;
 
     public function setUp(): void
     {
         $this->dbUpdater = $this->prophesize(DbUpdaterInterface::class);
         $this->geoLiteDbReader = $this->prophesize(Reader::class);
+        $this->trackingOptions = new TrackingOptions();
 
         $locker = $this->prophesize(Lock\LockFactory::class);
         $lock = $this->prophesize(Lock\LockInterface::class);
@@ -45,6 +48,7 @@ class GeolocationDbUpdaterTest extends TestCase
             $this->dbUpdater->reveal(),
             $this->geoLiteDbReader->reveal(),
             $locker->reveal(),
+            $this->trackingOptions,
         );
     }
 
@@ -174,4 +178,27 @@ class GeolocationDbUpdaterTest extends TestCase
             'record_size' => 4,
         ]);
     }
+
+    /**
+     * @test
+     * @dataProvider provideTrackingOptions
+     */
+    public function downloadDbIsSkippedIfTrackingIsDisabled(array $props): void
+    {
+        foreach ($props as $prop) {
+            $this->trackingOptions->{$prop} = true;
+        }
+
+        $this->geolocationDbUpdater->checkDbUpdate();
+
+        $this->dbUpdater->databaseFileExists(Argument::cetera())->shouldNotHaveBeenCalled();
+        $this->geoLiteDbReader->metadata(Argument::cetera())->shouldNotHaveBeenCalled();
+    }
+
+    public function provideTrackingOptions(): iterable
+    {
+        yield 'disableTracking' => [['disableTracking']];
+        yield 'disableIpTracking' => [['disableIpTracking']];
+        yield 'both' => [['disableTracking', 'disableIpTracking']];
+    }
 }

module/Core/src/Model/ShortUrlsParams.php

@@ -15,11 +15,11 @@ final class ShortUrlsParams
     public const DEFAULT_ITEMS_PER_PAGE = 10;
 
     private int $page;
+    private ?int $itemsPerPage = null;
     private ?string $searchTerm;
     private array $tags;
     private ShortUrlsOrdering $orderBy;
     private ?DateRange $dateRange;
-    private ?int $itemsPerPage = null;
 
     private function __construct()
     {

module/Core/src/Model/VisitsParams.php

@@ -25,11 +25,16 @@ final class VisitsParams
         bool $excludeBots = false
     ) {
         $this->dateRange = $dateRange ?? new DateRange();
-        $this->page = $page;
+        $this->page = $this->determinePage($page);
         $this->itemsPerPage = $this->determineItemsPerPage($itemsPerPage);
         $this->excludeBots = $excludeBots;
     }
 
+    private function determinePage(int $page): int
+    {
+        return $page > 0 ? $page : self::FIRST_PAGE;
+    }
+
     private function determineItemsPerPage(?int $itemsPerPage): int
     {
         if ($itemsPerPage !== null && $itemsPerPage < 0) {
@@ -43,7 +48,7 @@ final class VisitsParams
     {
         return new self(
             parseDateRangeFromQuery($query, 'startDate', 'endDate'),
-            (int) ($query['page'] ?? 1),
+            (int) ($query['page'] ?? self::FIRST_PAGE),
             isset($query['itemsPerPage']) ? (int) $query['itemsPerPage'] : null,
             isset($query['excludeBots']),
         );

module/Rest/src/Middleware/EmptyResponseImplicitOptionsMiddlewareFactory.php

@@ -6,11 +6,18 @@ namespace Shlinkio\Shlink\Rest\Middleware;
 
 use Laminas\Diactoros\Response\EmptyResponse;
 use Mezzio\Router\Middleware\ImplicitOptionsMiddleware;
+use Psr\Http\Message\ResponseFactoryInterface;
+use Psr\Http\Message\ResponseInterface;
 
 class EmptyResponseImplicitOptionsMiddlewareFactory
 {
     public function __invoke(): ImplicitOptionsMiddleware
     {
-        return new ImplicitOptionsMiddleware(fn () => new EmptyResponse());
+        return new ImplicitOptionsMiddleware(new class implements ResponseFactoryInterface {
+            public function createResponse(int $code = 200, string $reasonPhrase = ''): ResponseInterface
+            {
+                return new EmptyResponse();
+            }
+        });
     }
 }

module/Rest/test/Middleware/EmptyResponseImplicitOptionsMiddlewareFactoryTest.php

@@ -7,6 +7,7 @@ namespace ShlinkioTest\Shlink\Rest\Middleware;
 use Laminas\Diactoros\Response\EmptyResponse;
 use Mezzio\Router\Middleware\ImplicitOptionsMiddleware;
 use PHPUnit\Framework\TestCase;
+use Psr\Http\Message\ResponseFactoryInterface;
 use ReflectionObject;
 use Shlinkio\Shlink\Rest\Middleware\EmptyResponseImplicitOptionsMiddlewareFactory;
 
@@ -34,6 +35,10 @@ class EmptyResponseImplicitOptionsMiddlewareFactoryTest extends TestCase
         $ref = new ReflectionObject($instance);
         $prop = $ref->getProperty('responseFactory');
         $prop->setAccessible(true);
-        self::assertInstanceOf(EmptyResponse::class, $prop->getValue($instance)());
+
+        /** @var ResponseFactoryInterface $value */
+        $value = $prop->getValue($instance);
+
+        self::assertInstanceOf(EmptyResponse::class, $value->createResponse());
     }
 }