Ensured old visit locations are deleted when relocating a visit that has already been located

This commit is contained in:
Alejandro Celaya 2020-03-28 09:27:45 +01:00
parent fb8ab0b5fe
commit 55778eb810
4 changed files with 12 additions and 6 deletions

View File

@ -12,6 +12,7 @@ use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Model\VisitsParams;
use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface;
use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation;
use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Input\InputOption;
@ -76,7 +77,7 @@ class GetVisitsCommand extends AbstractWithDateRangeCommand
$rows = map($paginator->getCurrentItems(), function (Visit $visit) { $rows = map($paginator->getCurrentItems(), function (Visit $visit) {
$rowData = $visit->jsonSerialize(); $rowData = $visit->jsonSerialize();
$rowData['country'] = $visit->getVisitLocation()->getCountryName(); $rowData['country'] = ($visit->getVisitLocation() ?? new UnknownVisitLocation())->getCountryName();
return select_keys($rowData, ['referer', 'date', 'userAgent', 'country']); return select_keys($rowData, ['referer', 'date', 'userAgent', 'country']);
}); });
ShlinkTable::fromOutput($output)->render(['Referer', 'Date', 'User agent', 'Country'], $rows); ShlinkTable::fromOutput($output)->render(['Referer', 'Date', 'User agent', 'Country'], $rows);

View File

@ -10,7 +10,6 @@ use Shlinkio\Shlink\Common\Entity\AbstractEntity;
use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException;
use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Common\Util\IpAddress;
use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\Visitor;
use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation;
use Shlinkio\Shlink\Core\Visit\Model\VisitLocationInterface; use Shlinkio\Shlink\Core\Visit\Model\VisitLocationInterface;
class Visit extends AbstractEntity implements JsonSerializable class Visit extends AbstractEntity implements JsonSerializable
@ -60,9 +59,9 @@ class Visit extends AbstractEntity implements JsonSerializable
return $this->shortUrl; return $this->shortUrl;
} }
public function getVisitLocation(): VisitLocationInterface public function getVisitLocation(): ?VisitLocationInterface
{ {
return $this->visitLocation ?? new UnknownVisitLocation(); return $this->visitLocation;
} }
public function isLocatable(): bool public function isLocatable(): bool

View File

@ -79,9 +79,16 @@ class VisitLocator implements VisitLocatorInterface
private function locateVisit(Visit $visit, VisitLocation $location, VisitGeolocationHelperInterface $helper): void private function locateVisit(Visit $visit, VisitLocation $location, VisitGeolocationHelperInterface $helper): void
{ {
$prevLocation = $visit->getVisitLocation();
$visit->locate($location); $visit->locate($location);
$this->em->persist($visit); $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); $helper->onVisitLocated($location, $visit);
} }
} }

View File

@ -20,7 +20,6 @@ use Shlinkio\Shlink\Core\EventDispatcher\LocateShortUrlVisit;
use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited; use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited;
use Shlinkio\Shlink\Core\EventDispatcher\VisitLocated; use Shlinkio\Shlink\Core\EventDispatcher\VisitLocated;
use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\Visitor;
use Shlinkio\Shlink\Core\Visit\Model\UnknownVisitLocation;
use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException; use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
use Shlinkio\Shlink\IpGeolocation\Model\Location; use Shlinkio\Shlink\IpGeolocation\Model\Location;
use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
@ -218,7 +217,7 @@ class LocateShortUrlVisitTest extends TestCase
($this->locateVisit)($event); ($this->locateVisit)($event);
$this->assertEquals($visit->getVisitLocation(), new UnknownVisitLocation()); $this->assertNull($visit->getVisitLocation());
$findVisit->shouldHaveBeenCalledOnce(); $findVisit->shouldHaveBeenCalledOnce();
$flush->shouldNotHaveBeenCalled(); $flush->shouldNotHaveBeenCalled();
$resolveIp->shouldNotHaveBeenCalled(); $resolveIp->shouldNotHaveBeenCalled();