Ensured orphan visits are located ASAP when using swoole

This commit is contained in:
Alejandro Celaya 2021-02-09 20:25:28 +01:00
parent b01487ac91
commit ab9042db24
8 changed files with 57 additions and 39 deletions

View File

@ -20,28 +20,28 @@ return [
],
],
'async' => [
EventDispatcher\Event\ShortUrlVisited::class => [
EventDispatcher\LocateShortUrlVisit::class,
EventDispatcher\Event\UrlVisited::class => [
EventDispatcher\LocateVisit::class,
],
],
],
'dependencies' => [
'factories' => [
EventDispatcher\LocateShortUrlVisit::class => ConfigAbstractFactory::class,
EventDispatcher\LocateVisit::class => ConfigAbstractFactory::class,
EventDispatcher\NotifyVisitToWebHooks::class => ConfigAbstractFactory::class,
EventDispatcher\NotifyVisitToMercure::class => ConfigAbstractFactory::class,
],
'delegators' => [
EventDispatcher\LocateShortUrlVisit::class => [
EventDispatcher\LocateVisit::class => [
EventDispatcher\CloseDbConnectionEventListenerDelegator::class,
],
],
],
ConfigAbstractFactory::class => [
EventDispatcher\LocateShortUrlVisit::class => [
EventDispatcher\LocateVisit::class => [
IpLocationResolverInterface::class,
'em',
'Logger_Shlink',

View File

@ -109,6 +109,11 @@ class Visit extends AbstractEntity implements JsonSerializable
return $this;
}
public function isOrphan(): bool
{
return $this->shortUrl === null;
}
public function jsonSerialize(): array
{
return [

View File

@ -4,7 +4,7 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\EventDispatcher\Event;
final class ShortUrlVisited extends AbstractVisitEvent
final class UrlVisited extends AbstractVisitEvent
{
private ?string $originalIpAddress;

View File

@ -11,7 +11,7 @@ use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
use Shlinkio\Shlink\Core\EventDispatcher\Event\ShortUrlVisited;
use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited;
use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated;
use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
use Shlinkio\Shlink\IpGeolocation\Model\Location;
@ -19,7 +19,7 @@ use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
use function sprintf;
class LocateShortUrlVisit
class LocateVisit
{
private IpLocationResolverInterface $ipLocationResolver;
private EntityManagerInterface $em;
@ -41,7 +41,7 @@ class LocateShortUrlVisit
$this->eventDispatcher = $eventDispatcher;
}
public function __invoke(ShortUrlVisited $shortUrlVisited): void
public function __invoke(UrlVisited $shortUrlVisited): void
{
$visitId = $shortUrlVisited->visitId();
@ -58,7 +58,9 @@ class LocateShortUrlVisit
$this->locateVisit($visitId, $shortUrlVisited->originalIpAddress(), $visit);
}
$this->eventDispatcher->dispatch(new VisitLocated($visitId));
if (! $visit->isOrphan()) {
$this->eventDispatcher->dispatch(new VisitLocated($visitId));
}
}
private function downloadOrUpdateGeoLiteDb(string $visitId): bool

View File

@ -8,7 +8,7 @@ use Doctrine\ORM;
use Psr\EventDispatcher\EventDispatcherInterface;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\EventDispatcher\Event\ShortUrlVisited;
use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited;
use Shlinkio\Shlink\Core\Model\Visitor;
class VisitsTracker implements VisitsTrackerInterface
@ -29,30 +29,29 @@ class VisitsTracker implements VisitsTrackerInterface
public function track(ShortUrl $shortUrl, Visitor $visitor): void
{
$visit = $this->trackVisit(Visit::forValidShortUrl($shortUrl, $visitor, $this->anonymizeRemoteAddr));
$this->eventDispatcher->dispatch(new ShortUrlVisited($visit->getId(), $visitor->getRemoteAddress()));
$this->trackVisit(Visit::forValidShortUrl($shortUrl, $visitor, $this->anonymizeRemoteAddr), $visitor);
}
public function trackInvalidShortUrlVisit(Visitor $visitor): void
{
$this->trackVisit(Visit::forInvalidShortUrl($visitor));
$this->trackVisit(Visit::forInvalidShortUrl($visitor, $this->anonymizeRemoteAddr), $visitor);
}
public function trackBaseUrlVisit(Visitor $visitor): void
{
$this->trackVisit(Visit::forBasePath($visitor));
$this->trackVisit(Visit::forBasePath($visitor, $this->anonymizeRemoteAddr), $visitor);
}
public function trackRegularNotFoundVisit(Visitor $visitor): void
{
$this->trackVisit(Visit::forRegularNotFound($visitor));
$this->trackVisit(Visit::forRegularNotFound($visitor, $this->anonymizeRemoteAddr), $visitor);
}
private function trackVisit(Visit $visit): Visit
private function trackVisit(Visit $visit, Visitor $visitor): void
{
$this->em->persist($visit);
$this->em->flush();
return $visit;
$this->eventDispatcher->dispatch(new UrlVisited($visit->getId(), $visitor->getRemoteAddress()));
}
}

View File

@ -4,7 +4,6 @@ declare(strict_types=1);
namespace ShlinkioTest\Shlink\Core\ErrorHandler;
use Closure;
use Laminas\Diactoros\Response;
use Laminas\Diactoros\ServerRequestFactory;
use Laminas\Diactoros\Uri;

View File

@ -17,19 +17,19 @@ use Shlinkio\Shlink\Common\Util\IpAddress;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
use Shlinkio\Shlink\Core\EventDispatcher\Event\ShortUrlVisited;
use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited;
use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated;
use Shlinkio\Shlink\Core\EventDispatcher\LocateShortUrlVisit;
use Shlinkio\Shlink\Core\EventDispatcher\LocateVisit;
use Shlinkio\Shlink\Core\Model\Visitor;
use Shlinkio\Shlink\IpGeolocation\Exception\WrongIpException;
use Shlinkio\Shlink\IpGeolocation\Model\Location;
use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
class LocateShortUrlVisitTest extends TestCase
class LocateVisitTest extends TestCase
{
use ProphecyTrait;
private LocateShortUrlVisit $locateVisit;
private LocateVisit $locateVisit;
private ObjectProphecy $ipLocationResolver;
private ObjectProphecy $em;
private ObjectProphecy $logger;
@ -44,7 +44,7 @@ class LocateShortUrlVisitTest extends TestCase
$this->dbUpdater = $this->prophesize(GeolocationDbUpdaterInterface::class);
$this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class);
$this->locateVisit = new LocateShortUrlVisit(
$this->locateVisit = new LocateVisit(
$this->ipLocationResolver->reveal(),
$this->em->reveal(),
$this->logger->reveal(),
@ -56,7 +56,7 @@ class LocateShortUrlVisitTest extends TestCase
/** @test */
public function invalidVisitLogsWarning(): void
{
$event = new ShortUrlVisited('123');
$event = new UrlVisited('123');
$findVisit = $this->em->find(Visit::class, '123')->willReturn(null);
$logWarning = $this->logger->warning('Tried to locate visit with id "{visitId}", but it does not exist.', [
'visitId' => 123,
@ -76,7 +76,7 @@ class LocateShortUrlVisitTest extends TestCase
/** @test */
public function invalidAddressLogsWarning(): void
{
$event = new ShortUrlVisited('123');
$event = new UrlVisited('123');
$findVisit = $this->em->find(Visit::class, '123')->willReturn(
Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')),
);
@ -105,7 +105,7 @@ class LocateShortUrlVisitTest extends TestCase
*/
public function nonLocatableVisitsResolveToEmptyLocations(Visit $visit): void
{
$event = new ShortUrlVisited('123');
$event = new UrlVisited('123');
$findVisit = $this->em->find(Visit::class, '123')->willReturn($visit);
$flush = $this->em->flush()->will(function (): void {
});
@ -136,12 +136,14 @@ class LocateShortUrlVisitTest extends TestCase
* @test
* @dataProvider provideIpAddresses
*/
public function locatableVisitsResolveToLocation(string $anonymizedIpAddress, ?string $originalIpAddress): void
{
$ipAddr = $originalIpAddress ?? $anonymizedIpAddress;
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr, ''));
public function locatableVisitsResolveToLocation(
Visit $visit,
?string $originalIpAddress,
int $expectedDispatchCalls
): void {
$ipAddr = $originalIpAddress ?? $visit->getRemoteAddr();
$location = new Location('', '', '', '', 0.0, 0.0, '');
$event = new ShortUrlVisited('123', $originalIpAddress);
$event = new UrlVisited('123', $originalIpAddress);
$findVisit = $this->em->find(Visit::class, '123')->willReturn($visit);
$flush = $this->em->flush()->will(function (): void {
@ -157,13 +159,24 @@ class LocateShortUrlVisitTest extends TestCase
$flush->shouldHaveBeenCalledOnce();
$resolveIp->shouldHaveBeenCalledOnce();
$this->logger->warning(Argument::cetera())->shouldNotHaveBeenCalled();
$dispatch->shouldHaveBeenCalledOnce();
$dispatch->shouldHaveBeenCalledTimes($expectedDispatchCalls);
}
public function provideIpAddresses(): iterable
{
yield 'no original IP address' => ['1.2.3.0', null];
yield 'original IP address' => ['1.2.3.0', '1.2.3.4'];
yield 'no original IP address' => [
Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')),
null,
1,
];
yield 'original IP address' => [
Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')),
'1.2.3.4',
1,
];
yield 'base url' => [Visit::forBasePath(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4', 0];
yield 'invalid short url' => [Visit::forInvalidShortUrl(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4', 0];
yield 'regular not found' => [Visit::forRegularNotFound(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4', 0];
}
/** @test */
@ -173,7 +186,7 @@ class LocateShortUrlVisitTest extends TestCase
$ipAddr = '1.2.3.0';
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr, ''));
$location = new Location('', '', '', '', 0.0, 0.0, '');
$event = new ShortUrlVisited('123');
$event = new UrlVisited('123');
$findVisit = $this->em->find(Visit::class, '123')->willReturn($visit);
$flush = $this->em->flush()->will(function (): void {
@ -204,7 +217,7 @@ class LocateShortUrlVisitTest extends TestCase
$ipAddr = '1.2.3.0';
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr, ''));
$location = new Location('', '', '', '', 0.0, 0.0, '');
$event = new ShortUrlVisited('123');
$event = new UrlVisited('123');
$findVisit = $this->em->find(Visit::class, '123')->willReturn($visit);
$flush = $this->em->flush()->will(function (): void {

View File

@ -12,7 +12,7 @@ use Prophecy\Prophecy\ObjectProphecy;
use Psr\EventDispatcher\EventDispatcherInterface;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\EventDispatcher\Event\ShortUrlVisited;
use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited;
use Shlinkio\Shlink\Core\Model\Visitor;
use Shlinkio\Shlink\Core\Visit\VisitsTracker;
@ -42,6 +42,6 @@ class VisitsTrackerTest extends TestCase
$this->visitsTracker->track(ShortUrl::withLongUrl($shortCode), Visitor::emptyInstance());
$this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled();
$this->eventDispatcher->dispatch(Argument::type(UrlVisited::class))->shouldHaveBeenCalled();
}
}