diff --git a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php index 43949993..a0c2c91a 100644 --- a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php @@ -12,6 +12,7 @@ use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; +use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -76,7 +77,7 @@ class GetVisitsCommand extends AbstractWithDateRangeCommand $rows = map($paginator->getCurrentItems(), function (Visit $visit) { $rowData = $visit->jsonSerialize(); - $rowData['country'] = $visit->getVisitLocation()->getCountryName(); + $rowData['country'] = ($visit->getVisitLocation() ?? new UnknownVisitLocation())->getCountryName(); return select_keys($rowData, ['referer', 'date', 'userAgent', 'country']); }); ShlinkTable::fromOutput($output)->render(['Referer', 'Date', 'User agent', 'Country'], $rows); diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index d278ed6a..e8cbb119 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -10,7 +10,6 @@ use Shlinkio\Shlink\Common\Entity\AbstractEntity; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Model\Visitor; -use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation; use Shlinkio\Shlink\Core\Visit\Model\VisitLocationInterface; class Visit extends AbstractEntity implements JsonSerializable @@ -60,9 +59,9 @@ class Visit extends AbstractEntity implements JsonSerializable return $this->shortUrl; } - public function getVisitLocation(): VisitLocationInterface + public function getVisitLocation(): ?VisitLocationInterface { - return $this->visitLocation ?? new UnknownVisitLocation(); + return $this->visitLocation; } public function isLocatable(): bool diff --git a/module/Core/src/Visit/VisitLocator.php b/module/Core/src/Visit/VisitLocator.php index f955ef29..46a30559 100644 --- a/module/Core/src/Visit/VisitLocator.php +++ b/module/Core/src/Visit/VisitLocator.php @@ -79,9 +79,16 @@ class VisitLocator implements VisitLocatorInterface private function locateVisit(Visit $visit, VisitLocation $location, VisitGeolocationHelperInterface $helper): void { + $prevLocation = $visit->getVisitLocation(); + $visit->locate($location); $this->em->persist($visit); + // In order to avoid leaving orphan locations, remove the previous one + if ($prevLocation !== null) { + $this->em->remove($prevLocation); + } + $helper->onVisitLocated($location, $visit); } } diff --git a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php index c8f6f388..087c0e0b 100644 --- a/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php +++ b/module/Core/test/EventDispatcher/LocateShortUrlVisitTest.php @@ -20,7 +20,6 @@ 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; use Shlinkio\Shlink\IpGeolocation\Model\Location; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; @@ -218,7 +217,7 @@ class LocateShortUrlVisitTest extends TestCase ($this->locateVisit)($event); - $this->assertEquals($visit->getVisitLocation(), new UnknownVisitLocation()); + $this->assertNull($visit->getVisitLocation()); $findVisit->shouldHaveBeenCalledOnce(); $flush->shouldNotHaveBeenCalled(); $resolveIp->shouldNotHaveBeenCalled();