Updated ImportedLinksProcessor to support importing visits if provided

composer.json

@@ -49,7 +49,7 @@
         "shlinkio/shlink-common": "dev-main#554e370 as 3.7",
         "shlinkio/shlink-config": "^1.0",
         "shlinkio/shlink-event-dispatcher": "^2.1",
-        "shlinkio/shlink-importer": "dev-main#d7e2762 as 2.3",
+        "shlinkio/shlink-importer": "dev-main#174e352 as 2.3",
         "shlinkio/shlink-installer": "dev-develop#aa50ea9 as 5.5",
         "shlinkio/shlink-ip-geolocation": "^1.5",
         "symfony/console": "^5.1",

config/test/test_config.global.php

@@ -11,7 +11,6 @@ use Laminas\ServiceManager\Factory\InvokableFactory;
 use Laminas\Stdlib\Glob;
 use Monolog\Handler\StreamHandler;
 use Monolog\Logger;
-use PDO;
 use PHPUnit\Runner\Version;
 use SebastianBergmann\CodeCoverage\CodeCoverage;
 use SebastianBergmann\CodeCoverage\Driver\Selector;

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

@@ -103,7 +103,7 @@ class GetVisitsCommandTest extends TestCase
         $this->visitsHelper->visitsForShortUrl(new ShortUrlIdentifier($shortCode), Argument::any())->willReturn(
             new Paginator(new ArrayAdapter([
                 Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('bar', 'foo', '', ''))->locate(
-                    new VisitLocation(new Location('', 'Spain', '', '', 0, 0, '')),
+                    VisitLocation::fromGeolocation(new Location('', 'Spain', '', '', 0, 0, '')),
                 ),
             ])),
         )->shouldBeCalledOnce();

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

@@ -76,7 +76,7 @@ class LocateVisitsCommandTest extends TestCase
         array $args
     ): void {
         $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', ''));
-        $location = new VisitLocation(Location::emptyInstance());
+        $location = VisitLocation::fromGeolocation(Location::emptyInstance());
         $mockMethodBehavior = $this->invokeHelperMethods($visit, $location);
 
         $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will($mockMethodBehavior);
@@ -120,7 +120,7 @@ class LocateVisitsCommandTest extends TestCase
     public function localhostAndEmptyAddressesAreIgnored(?string $address, string $message): void
     {
         $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $address, ''));
-        $location = new VisitLocation(Location::emptyInstance());
+        $location = VisitLocation::fromGeolocation(Location::emptyInstance());
 
         $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(
             $this->invokeHelperMethods($visit, $location),
@@ -153,7 +153,7 @@ class LocateVisitsCommandTest extends TestCase
     public function errorWhileLocatingIpIsDisplayed(): void
     {
         $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', ''));
-        $location = new VisitLocation(Location::emptyInstance());
+        $location = VisitLocation::fromGeolocation(Location::emptyInstance());
 
         $locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(
             $this->invokeHelperMethods($visit, $location),

module/Core/src/Entity/Visit.php

@@ -11,6 +11,7 @@ use Shlinkio\Shlink\Common\Exception\InvalidArgumentException;
 use Shlinkio\Shlink\Common\Util\IpAddress;
 use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Visit\Model\VisitLocationInterface;
+use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisit;
 
 class Visit extends AbstractEntity implements JsonSerializable
 {
@@ -21,22 +22,71 @@ class Visit extends AbstractEntity implements JsonSerializable
 
     private string $referer;
     private Chronos $date;
-    private ?string $remoteAddr;
+    private ?string $remoteAddr = null;
-    private ?string $visitedUrl;
+    private ?string $visitedUrl = null;
     private string $userAgent;
     private string $type;
     private ?ShortUrl $shortUrl;
     private ?VisitLocation $visitLocation = null;
 
-    private function __construct(?ShortUrl $shortUrl, Visitor $visitor, string $type, bool $anonymize = true)
+    private function __construct(?ShortUrl $shortUrl, string $type)
     {
         $this->shortUrl = $shortUrl;
         $this->date = Chronos::now();
+        $this->type = $type;
+    }
+
+    public static function forValidShortUrl(ShortUrl $shortUrl, Visitor $visitor, bool $anonymize = true): self
+    {
+        $instance = new self($shortUrl, self::TYPE_VALID_SHORT_URL);
+        $instance->hydrateFromVisitor($visitor, $anonymize);
+
+        return $instance;
+    }
+
+    public static function fromImport(ImportedShlinkVisit $importedVisit, ShortUrl $shortUrl): self
+    {
+        $instance = new self($shortUrl, self::TYPE_VALID_SHORT_URL);
+        $instance->userAgent = $importedVisit->userAgent();
+        $instance->referer = $importedVisit->referer();
+        $instance->date = Chronos::instance($importedVisit->date());
+
+        $importedLocation = $importedVisit->location();
+        $instance->visitLocation = $importedLocation !== null ? VisitLocation::fromImport($importedLocation) : null;
+
+        return $instance;
+    }
+
+    public static function forBasePath(Visitor $visitor, bool $anonymize = true): self
+    {
+        $instance = new self(null, self::TYPE_BASE_URL);
+        $instance->hydrateFromVisitor($visitor, $anonymize);
+
+        return $instance;
+    }
+
+    public static function forInvalidShortUrl(Visitor $visitor, bool $anonymize = true): self
+    {
+        $instance = new self(null, self::TYPE_INVALID_SHORT_URL);
+        $instance->hydrateFromVisitor($visitor, $anonymize);
+
+        return $instance;
+    }
+
+    public static function forRegularNotFound(Visitor $visitor, bool $anonymize = true): self
+    {
+        $instance = new self(null, self::TYPE_REGULAR_404);
+        $instance->hydrateFromVisitor($visitor, $anonymize);
+
+        return $instance;
+    }
+
+    private function hydrateFromVisitor(Visitor $visitor, bool $anonymize = true): void
+    {
         $this->userAgent = $visitor->getUserAgent();
         $this->referer = $visitor->getReferer();
         $this->remoteAddr = $this->processAddress($anonymize, $visitor->getRemoteAddress());
         $this->visitedUrl = $visitor->getVisitedUrl();
-        $this->type = $type;
     }
 
     private function processAddress(bool $anonymize, ?string $address): ?string
@@ -53,26 +103,6 @@ class Visit extends AbstractEntity implements JsonSerializable
         }
     }
 
-    public static function forValidShortUrl(ShortUrl $shortUrl, Visitor $visitor, bool $anonymize = true): self
-    {
-        return new self($shortUrl, $visitor, self::TYPE_VALID_SHORT_URL, $anonymize);
-    }
-
-    public static function forBasePath(Visitor $visitor, bool $anonymize = true): self
-    {
-        return new self(null, $visitor, self::TYPE_BASE_URL, $anonymize);
-    }
-
-    public static function forInvalidShortUrl(Visitor $visitor, bool $anonymize = true): self
-    {
-        return new self(null, $visitor, self::TYPE_INVALID_SHORT_URL, $anonymize);
-    }
-
-    public static function forRegularNotFound(Visitor $visitor, bool $anonymize = true): self
-    {
-        return new self(null, $visitor, self::TYPE_REGULAR_404, $anonymize);
-    }
-
     public function getRemoteAddr(): ?string
     {
         return $this->remoteAddr;

module/Core/src/Entity/VisitLocation.php

@@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Entity;
 
 use Shlinkio\Shlink\Common\Entity\AbstractEntity;
 use Shlinkio\Shlink\Core\Visit\Model\VisitLocationInterface;
+use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisitLocation;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
 
 class VisitLocation extends AbstractEntity implements VisitLocationInterface
@@ -19,9 +20,53 @@ class VisitLocation extends AbstractEntity implements VisitLocationInterface
     private string $timezone;
     private bool $isEmpty;
 
-    public function __construct(Location $location)
+    private function __construct()
     {
-        $this->exchangeLocationInfo($location);
+    }
+
+    public static function fromGeolocation(Location $location): self
+    {
+        $instance = new self();
+
+        $instance->countryCode = $location->countryCode();
+        $instance->countryName = $location->countryName();
+        $instance->regionName = $location->regionName();
+        $instance->cityName = $location->city();
+        $instance->latitude = $location->latitude();
+        $instance->longitude = $location->longitude();
+        $instance->timezone = $location->timeZone();
+        $instance->computeIsEmpty();
+
+        return $instance;
+    }
+
+    public static function fromImport(ImportedShlinkVisitLocation $location): self
+    {
+        $instance = new self();
+
+        $instance->countryCode = $location->countryCode();
+        $instance->countryName = $location->countryName();
+        $instance->regionName = $location->regionName();
+        $instance->cityName = $location->cityName();
+        $instance->latitude = $location->latitude();
+        $instance->longitude = $location->longitude();
+        $instance->timezone = $location->timeZone();
+        $instance->computeIsEmpty();
+
+        return $instance;
+    }
+
+    private function computeIsEmpty(): void
+    {
+        $this->isEmpty = (
+            $this->countryCode === '' &&
+            $this->countryName === '' &&
+            $this->regionName === '' &&
+            $this->cityName === '' &&
+            $this->latitude === 0.0 &&
+            $this->longitude === 0.0 &&
+            $this->timezone === ''
+        );
     }
 
     public function getCountryName(): string
@@ -49,26 +94,6 @@ class VisitLocation extends AbstractEntity implements VisitLocationInterface
         return $this->isEmpty;
     }
 
-    private function exchangeLocationInfo(Location $info): void
-    {
-        $this->countryCode = $info->countryCode();
-        $this->countryName = $info->countryName();
-        $this->regionName = $info->regionName();
-        $this->cityName = $info->city();
-        $this->latitude = $info->latitude();
-        $this->longitude = $info->longitude();
-        $this->timezone = $info->timeZone();
-        $this->isEmpty = (
-            $this->countryCode === '' &&
-            $this->countryName === '' &&
-            $this->regionName === '' &&
-            $this->cityName === '' &&
-            $this->latitude === 0.0 &&
-            $this->longitude === 0.0 &&
-            $this->timezone === ''
-        );
-    }
-
     public function jsonSerialize(): array
     {
         return [

module/Core/src/EventDispatcher/LocateVisit.php

@@ -71,7 +71,7 @@ class LocateVisit
         try {
             $location = $isLocatable ? $this->ipLocationResolver->resolveIpLocation($addr) : Location::emptyInstance();
 
-            $visit->locate(new VisitLocation($location));
+            $visit->locate(VisitLocation::fromGeolocation($location));
             $this->em->flush();
         } catch (WrongIpException $e) {
             $this->logger->warning(

module/Core/src/Importer/ImportedLinksProcessor.php

@@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Importer;
 
 use Doctrine\ORM\EntityManagerInterface;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface;
 use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelperInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface;
@@ -45,30 +46,38 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface
         $importShortCodes = $params['import_short_codes'];
         $iterable = $this->batchHelper->wrapIterable($shlinkUrls, 100);
 
-        /** @var ImportedShlinkUrl $url */
+        /** @var ImportedShlinkUrl $importedUrl */
-        foreach ($iterable as $url) {
+        foreach ($iterable as $importedUrl) {
-            $longUrl = $url->longUrl();
+            $longUrl = $importedUrl->longUrl();
 
             // Skip already imported URLs
-            if ($shortUrlRepo->importedUrlExists($url)) {
+            if ($shortUrlRepo->importedUrlExists($importedUrl)) {
                 // TODO If the URL exists, allow to merge visits instead of just skipping completely
                 $io->text(sprintf('%s: <comment>Skipped</comment>', $longUrl));
                 continue;
             }
 
-            $shortUrl = ShortUrl::fromImport($url, $importShortCodes, $this->relationResolver);
+            $shortUrl = ShortUrl::fromImport($importedUrl, $importShortCodes, $this->relationResolver);
-            if (! $this->handleShortCodeUniqueness($url, $shortUrl, $io, $importShortCodes)) {
+            if (! $this->handleShortCodeUniqueness($importedUrl, $shortUrl, $io, $importShortCodes)) {
                 $io->text(sprintf('%s: <comment>Skipped</comment>', $longUrl));
                 continue;
             }
 
             $this->em->persist($shortUrl);
-            $io->text(sprintf('%s: <info>Imported</info>', $longUrl));
-
-            // Process only missing visits when possible
-            if ($url->visitsCount() !== null) {
 
+            // TODO Process only missing visits when possible: $importedUrl->visitsCount();
+            // TODO Make importing visits optional based on params
+            $importedVisits = 0;
+            foreach ($importedUrl->visits() as $importedVisit) {
+                $this->em->persist(Visit::fromImport($importedVisit, $shortUrl));
+                $importedVisits++;
             }
+
+            $io->text(
+                $importedVisits === 0
+                    ? sprintf('%s: <info>Imported</info>', $longUrl)
+                    : sprintf('%s: <info>Imported</info> with <info>%s</info> visits', $longUrl, $importedVisits),
+            );
         }
     }
 

module/Core/src/Visit/VisitLocator.php

@@ -63,8 +63,7 @@ class VisitLocator implements VisitLocatorInterface
                 $location = Location::emptyInstance();
             }
 
-            $location = new VisitLocation($location);
+            $this->locateVisit($visit, VisitLocation::fromGeolocation($location), $helper);
-            $this->locateVisit($visit, $location, $helper);
 
             // Flush and clear after X iterations
             if ($count % $persistBlock === 0) {

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

@@ -57,7 +57,7 @@ class VisitRepositoryTest extends DatabaseTestCase
             $visit = Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance());
 
             if ($i >= 2) {
-                $location = new VisitLocation(Location::emptyInstance());
+                $location = VisitLocation::fromGeolocation(Location::emptyInstance());
                 $this->getEntityManager()->persist($location);
                 $visit->locate($location);
             }

module/Core/test/Entity/VisitLocationTest.php

@@ -17,7 +17,7 @@ class VisitLocationTest extends TestCase
     public function isEmptyReturnsTrueWhenAllValuesAreEmpty(array $args, bool $isEmpty): void
     {
         $payload = new Location(...$args);
-        $location = new VisitLocation($payload);
+        $location = VisitLocation::fromGeolocation($payload);
 
         self::assertEquals($isEmpty, $location->isEmpty());
     }

module/Core/test/EventDispatcher/LocateVisitTest.php

@@ -168,7 +168,7 @@ class LocateVisitTest extends TestCase
 
         ($this->locateVisit)($event);
 
-        self::assertEquals($visit->getVisitLocation(), new VisitLocation(Location::emptyInstance()));
+        self::assertEquals($visit->getVisitLocation(), VisitLocation::fromGeolocation(Location::emptyInstance()));
         $findVisit->shouldHaveBeenCalledOnce();
         $flush->shouldHaveBeenCalledOnce();
         $resolveIp->shouldNotHaveBeenCalled();
@@ -204,7 +204,7 @@ class LocateVisitTest extends TestCase
 
         ($this->locateVisit)($event);
 
-        self::assertEquals($visit->getVisitLocation(), new VisitLocation($location));
+        self::assertEquals($visit->getVisitLocation(), VisitLocation::fromGeolocation($location));
         $findVisit->shouldHaveBeenCalledOnce();
         $flush->shouldHaveBeenCalledOnce();
         $resolveIp->shouldHaveBeenCalledOnce();

module/Core/test/Visit/Transformer/OrphanVisitDataTransformerTest.php

@@ -68,7 +68,7 @@ class OrphanVisitDataTransformerTest extends TestCase
                                                        ->withHeader('Referer', 'referer')
                                                        ->withUri(new Uri('https://doma.in/foo/bar')),
                 ),
-            )->locate($location = new VisitLocation(Location::emptyInstance())),
+            )->locate($location = VisitLocation::fromGeolocation(Location::emptyInstance())),
             [
                 'referer' => 'referer',
                 'date' => $visit->getDate()->toAtomString(),