diff --git a/composer.json b/composer.json index aadb06ba..9782b137 100644 --- a/composer.json +++ b/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", diff --git a/config/test/test_config.global.php b/config/test/test_config.global.php index c8e9510b..c2375cba 100644 --- a/config/test/test_config.global.php +++ b/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; diff --git a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php index 044866ed..b9262217 100644 --- a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php +++ b/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(); diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php index 6e4213f6..74148f9c 100644 --- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php +++ b/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), diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index 61739dec..e2d5f09e 100644 --- a/module/Core/src/Entity/Visit.php +++ b/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 $visitedUrl; + private ?string $remoteAddr = null; + 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; diff --git a/module/Core/src/Entity/VisitLocation.php b/module/Core/src/Entity/VisitLocation.php index ef545bba..594126a7 100644 --- a/module/Core/src/Entity/VisitLocation.php +++ b/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 [ diff --git a/module/Core/src/EventDispatcher/LocateVisit.php b/module/Core/src/EventDispatcher/LocateVisit.php index 80dc18eb..0150c529 100644 --- a/module/Core/src/EventDispatcher/LocateVisit.php +++ b/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( diff --git a/module/Core/src/Importer/ImportedLinksProcessor.php b/module/Core/src/Importer/ImportedLinksProcessor.php index e88020c7..4a5562bc 100644 --- a/module/Core/src/Importer/ImportedLinksProcessor.php +++ b/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 */ - foreach ($iterable as $url) { - $longUrl = $url->longUrl(); + /** @var ImportedShlinkUrl $importedUrl */ + foreach ($iterable as $importedUrl) { + $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: Skipped', $longUrl)); continue; } - $shortUrl = ShortUrl::fromImport($url, $importShortCodes, $this->relationResolver); - if (! $this->handleShortCodeUniqueness($url, $shortUrl, $io, $importShortCodes)) { + $shortUrl = ShortUrl::fromImport($importedUrl, $importShortCodes, $this->relationResolver); + if (! $this->handleShortCodeUniqueness($importedUrl, $shortUrl, $io, $importShortCodes)) { $io->text(sprintf('%s: Skipped', $longUrl)); continue; } $this->em->persist($shortUrl); - $io->text(sprintf('%s: Imported', $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: Imported', $longUrl) + : sprintf('%s: Imported with %s visits', $longUrl, $importedVisits), + ); } } diff --git a/module/Core/src/Visit/VisitLocator.php b/module/Core/src/Visit/VisitLocator.php index 46a30559..d7f0e426 100644 --- a/module/Core/src/Visit/VisitLocator.php +++ b/module/Core/src/Visit/VisitLocator.php @@ -63,8 +63,7 @@ class VisitLocator implements VisitLocatorInterface $location = Location::emptyInstance(); } - $location = new VisitLocation($location); - $this->locateVisit($visit, $location, $helper); + $this->locateVisit($visit, VisitLocation::fromGeolocation($location), $helper); // Flush and clear after X iterations if ($count % $persistBlock === 0) { diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index 4e634493..27ab3252 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/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); } diff --git a/module/Core/test/Entity/VisitLocationTest.php b/module/Core/test/Entity/VisitLocationTest.php index 057a1920..6021b124 100644 --- a/module/Core/test/Entity/VisitLocationTest.php +++ b/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()); } diff --git a/module/Core/test/EventDispatcher/LocateVisitTest.php b/module/Core/test/EventDispatcher/LocateVisitTest.php index fda45c58..406e8146 100644 --- a/module/Core/test/EventDispatcher/LocateVisitTest.php +++ b/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(); diff --git a/module/Core/test/Visit/Transformer/OrphanVisitDataTransformerTest.php b/module/Core/test/Visit/Transformer/OrphanVisitDataTransformerTest.php index cf36c052..61193c86 100644 --- a/module/Core/test/Visit/Transformer/OrphanVisitDataTransformerTest.php +++ b/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(),