diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index 99f71bce..63d19d4d 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -11,6 +11,7 @@ use RKA\Middleware\IpAddress; use Shlinkio\Shlink\Common\Middleware\AccessLogMiddleware; use Shlinkio\Shlink\Common\Middleware\ContentLengthMiddleware; use Shlinkio\Shlink\Common\Middleware\RequestIdMiddleware; +use Shlinkio\Shlink\Core\Geolocation\Middleware\IpGeolocationMiddleware; return [ @@ -67,8 +68,11 @@ return [ ], 'not-found' => [ 'middleware' => [ - // This middleware is in front of tracking actions explicitly. Putting here for orphan visits tracking + // These two middlewares are in front of other tracking actions. + // Putting them here for orphan visits tracking IpAddress::class, + IpGeolocationMiddleware::class, + Core\ErrorHandler\NotFoundTypeResolverMiddleware::class, Core\ShortUrl\Middleware\ExtraPathRedirectMiddleware::class, Core\ErrorHandler\NotFoundTrackerMiddleware::class, diff --git a/config/autoload/routes.config.php b/config/autoload/routes.config.php index da3d1778..1f5425b5 100644 --- a/config/autoload/routes.config.php +++ b/config/autoload/routes.config.php @@ -8,6 +8,7 @@ use Fig\Http\Message\RequestMethodInterface; use RKA\Middleware\IpAddress; use Shlinkio\Shlink\Core\Action as CoreAction; use Shlinkio\Shlink\Core\Config\EnvVars; +use Shlinkio\Shlink\Core\Geolocation\Middleware\IpGeolocationMiddleware; use Shlinkio\Shlink\Core\ShortUrl\Middleware\TrimTrailingSlashMiddleware; use Shlinkio\Shlink\Rest\Action; use Shlinkio\Shlink\Rest\ConfigProvider; @@ -88,6 +89,7 @@ return (static function (): array { 'path' => '/{shortCode}/track', 'middleware' => [ IpAddress::class, + IpGeolocationMiddleware::class, CoreAction\PixelAction::class, ], 'allowed_methods' => [RequestMethodInterface::METHOD_GET], @@ -105,6 +107,7 @@ return (static function (): array { 'path' => sprintf('/{shortCode}%s', $shortUrlRouteSuffix), 'middleware' => [ IpAddress::class, + IpGeolocationMiddleware::class, TrimTrailingSlashMiddleware::class, CoreAction\RedirectAction::class, ], diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index ad3452e4..4844e6d5 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -11,6 +11,7 @@ use Shlinkio\Shlink\Common\Doctrine\EntityRepositoryFactory; use Shlinkio\Shlink\Core\Config\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface; +use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Symfony\Component\Lock; @@ -102,6 +103,8 @@ return [ EventDispatcher\PublishingUpdatesGenerator::class => ConfigAbstractFactory::class, + Geolocation\Middleware\IpGeolocationMiddleware::class => ConfigAbstractFactory::class, + Importer\ImportedLinksProcessor::class => ConfigAbstractFactory::class, Crawling\CrawlingHelper::class => ConfigAbstractFactory::class, @@ -237,6 +240,13 @@ return [ EventDispatcher\PublishingUpdatesGenerator::class => [ShortUrl\Transformer\ShortUrlDataTransformer::class], + Geolocation\Middleware\IpGeolocationMiddleware::class => [ + IpLocationResolverInterface::class, + DbUpdater::class, + 'Logger_Shlink', + Config\Options\TrackingOptions::class, + ], + Importer\ImportedLinksProcessor::class => [ 'em', ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class, diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 2491d606..4e130fcf 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -15,23 +15,18 @@ use Shlinkio\Shlink\Core\Matomo\MatomoOptions; use Shlinkio\Shlink\Core\Visit\Geolocation\VisitLocator; use Shlinkio\Shlink\Core\Visit\Geolocation\VisitToLocationHelper; use Shlinkio\Shlink\EventDispatcher\Listener\EnabledListenerCheckerInterface; -use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater; use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2Options; -use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use function Shlinkio\Shlink\Config\runningInRoadRunner; return (static function (): array { $regularEvents = [ - EventDispatcher\Event\UrlVisited::class => [ - EventDispatcher\LocateVisit::class, - ], EventDispatcher\Event\GeoLiteDbCreated::class => [ EventDispatcher\LocateUnlocatedVisits::class, ], ]; $asyncEvents = [ - EventDispatcher\Event\VisitLocated::class => [ + EventDispatcher\Event\UrlVisited::class => [ EventDispatcher\Mercure\NotifyVisitToMercure::class, EventDispatcher\RabbitMq\NotifyVisitToRabbitMq::class, EventDispatcher\RedisPubSub\NotifyVisitToRedis::class, @@ -46,9 +41,9 @@ return (static function (): array { // Send visits to matomo asynchronously if the runtime allows it if (runningInRoadRunner()) { - $asyncEvents[EventDispatcher\Event\VisitLocated::class][] = EventDispatcher\Matomo\SendVisitToMatomo::class; + $asyncEvents[EventDispatcher\Event\UrlVisited::class][] = EventDispatcher\Matomo\SendVisitToMatomo::class; } else { - $regularEvents[EventDispatcher\Event\VisitLocated::class] = [EventDispatcher\Matomo\SendVisitToMatomo::class]; + $regularEvents[EventDispatcher\Event\UrlVisited::class] = [EventDispatcher\Matomo\SendVisitToMatomo::class]; } return [ @@ -60,7 +55,6 @@ return (static function (): array { 'dependencies' => [ 'factories' => [ - EventDispatcher\LocateVisit::class => ConfigAbstractFactory::class, EventDispatcher\Matomo\SendVisitToMatomo::class => ConfigAbstractFactory::class, EventDispatcher\LocateUnlocatedVisits::class => ConfigAbstractFactory::class, EventDispatcher\Mercure\NotifyVisitToMercure::class => ConfigAbstractFactory::class, @@ -104,13 +98,6 @@ return (static function (): array { ], ConfigAbstractFactory::class => [ - EventDispatcher\LocateVisit::class => [ - IpLocationResolverInterface::class, - 'em', - 'Logger_Shlink', - DbUpdater::class, - EventDispatcherInterface::class, - ], EventDispatcher\LocateUnlocatedVisits::class => [VisitLocator::class, VisitToLocationHelper::class], EventDispatcher\Mercure\NotifyVisitToMercure::class => [ MercureHubPublishingHelper::class, diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index b7ddb69a..78eebc05 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -15,7 +15,6 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; -use Shlinkio\Shlink\IpGeolocation\Model\Location; abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMethodInterface { @@ -31,12 +30,9 @@ abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMet try { $shortUrl = $this->urlResolver->resolveEnabledShortUrl($identifier); - $visit = $this->requestTracker->trackIfApplicable($shortUrl, $request); + $this->requestTracker->trackIfApplicable($shortUrl, $request); - return $this->createSuccessResp( - $shortUrl, - $request->withAttribute(Location::class, $visit?->getVisitLocation()), - ); + return $this->createSuccessResp($shortUrl, $request); } catch (ShortUrlNotFoundException) { return $this->createErrorResp($request, $handler); } diff --git a/module/Core/src/EventDispatcher/Async/AbstractNotifyVisitListener.php b/module/Core/src/EventDispatcher/Async/AbstractNotifyVisitListener.php index 3ec9417c..e871588f 100644 --- a/module/Core/src/EventDispatcher/Async/AbstractNotifyVisitListener.php +++ b/module/Core/src/EventDispatcher/Async/AbstractNotifyVisitListener.php @@ -8,7 +8,7 @@ use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; use Shlinkio\Shlink\Common\UpdatePublishing\Update; -use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; +use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Throwable; @@ -25,7 +25,7 @@ abstract class AbstractNotifyVisitListener extends AbstractAsyncListener ) { } - public function __invoke(VisitLocated $visitLocated): void + public function __invoke(UrlVisited $visitLocated): void { if (! $this->isEnabled()) { return; diff --git a/module/Core/src/EventDispatcher/Event/AbstractVisitEvent.php b/module/Core/src/EventDispatcher/Event/AbstractVisitEvent.php deleted file mode 100644 index c1fa440a..00000000 --- a/module/Core/src/EventDispatcher/Event/AbstractVisitEvent.php +++ /dev/null @@ -1,27 +0,0 @@ - $this->visitId, 'originalIpAddress' => $this->originalIpAddress]; - } - - public static function fromPayload(array $payload): self - { - return new static($payload['visitId'] ?? '', $payload['originalIpAddress'] ?? null); - } -} diff --git a/module/Core/src/EventDispatcher/Event/ShortUrlCreated.php b/module/Core/src/EventDispatcher/Event/ShortUrlCreated.php index b6ab1a0c..4055935f 100644 --- a/module/Core/src/EventDispatcher/Event/ShortUrlCreated.php +++ b/module/Core/src/EventDispatcher/Event/ShortUrlCreated.php @@ -7,9 +7,9 @@ namespace Shlinkio\Shlink\Core\EventDispatcher\Event; use JsonSerializable; use Shlinkio\Shlink\EventDispatcher\Util\JsonUnserializable; -final class ShortUrlCreated implements JsonSerializable, JsonUnserializable +final readonly class ShortUrlCreated implements JsonSerializable, JsonUnserializable { - public function __construct(public readonly string $shortUrlId) + public function __construct(public string $shortUrlId) { } diff --git a/module/Core/src/EventDispatcher/Event/UrlVisited.php b/module/Core/src/EventDispatcher/Event/UrlVisited.php index d1158a4e..0d25b1a1 100644 --- a/module/Core/src/EventDispatcher/Event/UrlVisited.php +++ b/module/Core/src/EventDispatcher/Event/UrlVisited.php @@ -4,6 +4,24 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\EventDispatcher\Event; -final class UrlVisited extends AbstractVisitEvent +use JsonSerializable; +use Shlinkio\Shlink\EventDispatcher\Util\JsonUnserializable; + +final readonly class UrlVisited implements JsonSerializable, JsonUnserializable { + final public function __construct( + public string $visitId, + public string|null $originalIpAddress = null, + ) { + } + + public function jsonSerialize(): array + { + return ['visitId' => $this->visitId, 'originalIpAddress' => $this->originalIpAddress]; + } + + public static function fromPayload(array $payload): self + { + return new self($payload['visitId'] ?? '', $payload['originalIpAddress'] ?? null); + } } diff --git a/module/Core/src/EventDispatcher/Event/VisitLocated.php b/module/Core/src/EventDispatcher/Event/VisitLocated.php deleted file mode 100644 index 99b7a05e..00000000 --- a/module/Core/src/EventDispatcher/Event/VisitLocated.php +++ /dev/null @@ -1,9 +0,0 @@ -visitId; - - /** @var Visit|null $visit */ - $visit = $this->em->find(Visit::class, $visitId); - if ($visit === null) { - $this->logger->warning('Tried to locate visit with id "{visitId}", but it does not exist.', [ - 'visitId' => $visitId, - ]); - return; - } - - $this->locateVisit($visitId, $shortUrlVisited->originalIpAddress, $visit); - $this->eventDispatcher->dispatch(new VisitLocated($visitId, $shortUrlVisited->originalIpAddress)); - } - - private function locateVisit(string $visitId, string|null $originalIpAddress, Visit $visit): void - { - if (! $this->dbUpdater->databaseFileExists()) { - $this->logger->warning('Tried to locate visit with id "{visitId}", but a GeoLite2 db was not found.', [ - 'visitId' => $visitId, - ]); - return; - } - - $isLocatable = $originalIpAddress !== null || $visit->isLocatable(); - $addr = $originalIpAddress ?? $visit->remoteAddr ?? ''; - - try { - $location = $isLocatable ? $this->ipLocationResolver->resolveIpLocation($addr) : Location::emptyInstance(); - - $visit->locate(VisitLocation::fromGeolocation($location)); - $this->em->flush(); - } catch (WrongIpException $e) { - $this->logger->warning( - 'Tried to locate visit with id "{visitId}", but its address seems to be wrong. {e}', - ['e' => $e, 'visitId' => $visitId], - ); - } catch (Throwable $e) { - $this->logger->error( - 'An unexpected error occurred while trying to locate visit with id "{visitId}". {e}', - ['e' => $e, 'visitId' => $visitId], - ); - } - } -} diff --git a/module/Core/src/EventDispatcher/Matomo/SendVisitToMatomo.php b/module/Core/src/EventDispatcher/Matomo/SendVisitToMatomo.php index 5a85aed4..c7b5bd3c 100644 --- a/module/Core/src/EventDispatcher/Matomo/SendVisitToMatomo.php +++ b/module/Core/src/EventDispatcher/Matomo/SendVisitToMatomo.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core\EventDispatcher\Matomo; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; +use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited; use Shlinkio\Shlink\Core\Matomo\MatomoOptions; use Shlinkio\Shlink\Core\Matomo\MatomoVisitSenderInterface; use Shlinkio\Shlink\Core\Visit\Entity\Visit; @@ -22,7 +22,7 @@ readonly class SendVisitToMatomo ) { } - public function __invoke(VisitLocated $visitLocated): void + public function __invoke(UrlVisited $visitLocated): void { if (! $this->matomoOptions->enabled) { return; diff --git a/module/Core/src/Geolocation/Middleware/IpGeolocationMiddleware.php b/module/Core/src/Geolocation/Middleware/IpGeolocationMiddleware.php new file mode 100644 index 00000000..4e2e533b --- /dev/null +++ b/module/Core/src/Geolocation/Middleware/IpGeolocationMiddleware.php @@ -0,0 +1,58 @@ +trackingOptions->isGeolocationRelevant()) { + return $handler->handle($request); + } + + if (! $this->dbUpdater->databaseFileExists()) { + $this->logger->warning('Tried to geolocate IP address, but a GeoLite2 db was not found.'); + return $handler->handle($request); + } + + $location = $this->geolocateIpAddress(ipAddressFromRequest($request)); + return $handler->handle($request->withAttribute(Location::class, $location)); + } + + private function geolocateIpAddress(string|null $ipAddress): Location + { + try { + return $ipAddress === null ? Location::empty() : $this->ipLocationResolver->resolveIpLocation($ipAddress); + } catch (WrongIpException $e) { + $this->logger->warning('Tried to locate IP address, but it seems to be wrong. {e}', ['e' => $e]); + return Location::empty(); + } catch (Throwable $e) { + $this->logger->error('An unexpected error occurred while trying to locate IP address. {e}', ['e' => $e]); + return Location::empty(); + } + } +} diff --git a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php index 2b4ac7cc..4a02f6e9 100644 --- a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php @@ -17,7 +17,6 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; -use Shlinkio\Shlink\IpGeolocation\Model\Location; use function array_slice; use function count; @@ -74,13 +73,9 @@ readonly class ExtraPathRedirectMiddleware implements MiddlewareInterface try { $shortUrl = $this->resolver->resolveEnabledShortUrl($identifier); - $visit = $this->requestTracker->trackIfApplicable($shortUrl, $request); + $this->requestTracker->trackIfApplicable($shortUrl, $request); - $longUrl = $this->redirectionBuilder->buildShortUrlRedirect( - $shortUrl, - $request->withAttribute(Location::class, $visit?->getVisitLocation()), - $extraPath, - ); + $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request, $extraPath); return $this->redirectResponseHelper->buildRedirectResponse($longUrl); } catch (ShortUrlNotFoundException) { if ($extraPath === null || ! $this->urlShortenerOptions->multiSegmentSlugsEnabled) { diff --git a/module/Core/src/Visit/Entity/Visit.php b/module/Core/src/Visit/Entity/Visit.php index 4e3c48a8..e26d5f80 100644 --- a/module/Core/src/Visit/Entity/Visit.php +++ b/module/Core/src/Visit/Entity/Visit.php @@ -59,6 +59,7 @@ class Visit extends AbstractEntity implements JsonSerializable Visitor $visitor, bool $anonymize, ): self { + $geolocation = $visitor->geolocation; return new self( shortUrl: $shortUrl, type: $type, @@ -67,6 +68,7 @@ class Visit extends AbstractEntity implements JsonSerializable potentialBot: $visitor->potentialBot, remoteAddr: self::processAddress($visitor->remoteAddress, $anonymize), visitedUrl: $visitor->visitedUrl, + visitLocation: $geolocation !== null ? VisitLocation::fromGeolocation($geolocation) : null, ); } diff --git a/module/Core/test/EventDispatcher/LocateVisitTest.php b/module/Core/test/EventDispatcher/LocateVisitTest.php deleted file mode 100644 index b88bf470..00000000 --- a/module/Core/test/EventDispatcher/LocateVisitTest.php +++ /dev/null @@ -1,203 +0,0 @@ -ipLocationResolver = $this->createMock(IpLocationResolverInterface::class); - $this->em = $this->createMock(EntityManagerInterface::class); - $this->logger = $this->createMock(LoggerInterface::class); - $this->eventDispatcher = $this->createMock(EventDispatcherInterface::class); - $this->dbUpdater = $this->createMock(DbUpdaterInterface::class); - - $this->locateVisit = new LocateVisit( - $this->ipLocationResolver, - $this->em, - $this->logger, - $this->dbUpdater, - $this->eventDispatcher, - ); - } - - #[Test] - public function invalidVisitLogsWarning(): void - { - $event = new UrlVisited('123'); - $this->em->expects($this->once())->method('find')->with(Visit::class, '123')->willReturn(null); - $this->em->expects($this->never())->method('flush'); - $this->logger->expects($this->once())->method('warning')->with( - 'Tried to locate visit with id "{visitId}", but it does not exist.', - ['visitId' => 123], - ); - $this->eventDispatcher->expects($this->never())->method('dispatch')->with(new VisitLocated('123')); - $this->ipLocationResolver->expects($this->never())->method('resolveIpLocation'); - - ($this->locateVisit)($event); - } - - #[Test] - public function nonExistingGeoLiteDbLogsWarning(): void - { - $event = new UrlVisited('123'); - $this->em->expects($this->once())->method('find')->with(Visit::class, '123')->willReturn( - Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::fromParams(remoteAddress: '1.2.3.4')), - ); - $this->em->expects($this->never())->method('flush'); - $this->dbUpdater->expects($this->once())->method('databaseFileExists')->withAnyParameters()->willReturn(false); - $this->logger->expects($this->once())->method('warning')->with( - 'Tried to locate visit with id "{visitId}", but a GeoLite2 db was not found.', - ['visitId' => 123], - ); - $this->eventDispatcher->expects($this->once())->method('dispatch')->with(new VisitLocated('123')); - $this->ipLocationResolver->expects($this->never())->method('resolveIpLocation'); - - ($this->locateVisit)($event); - } - - #[Test] - public function invalidAddressLogsWarning(): void - { - $event = new UrlVisited('123'); - $this->em->expects($this->once())->method('find')->with(Visit::class, '123')->willReturn( - Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::fromParams(remoteAddress: '1.2.3.4')), - ); - $this->em->expects($this->never())->method('flush'); - $this->dbUpdater->expects($this->once())->method('databaseFileExists')->withAnyParameters()->willReturn(true); - $this->ipLocationResolver->expects( - $this->once(), - )->method('resolveIpLocation')->withAnyParameters()->willThrowException(WrongIpException::fromIpAddress('')); - $this->logger->expects($this->once())->method('warning')->with( - 'Tried to locate visit with id "{visitId}", but its address seems to be wrong. {e}', - $this->isType('array'), - ); - $this->eventDispatcher->expects($this->once())->method('dispatch')->with(new VisitLocated('123')); - - ($this->locateVisit)($event); - } - - #[Test] - public function unhandledExceptionLogsError(): void - { - $event = new UrlVisited('123'); - $this->em->expects($this->once())->method('find')->with(Visit::class, '123')->willReturn( - Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::fromParams(remoteAddress: '1.2.3.4')), - ); - $this->em->expects($this->never())->method('flush'); - $this->dbUpdater->expects($this->once())->method('databaseFileExists')->withAnyParameters()->willReturn(true); - $this->ipLocationResolver->expects( - $this->once(), - )->method('resolveIpLocation')->withAnyParameters()->willThrowException(new OutOfRangeException()); - $this->logger->expects($this->once())->method('error')->with( - 'An unexpected error occurred while trying to locate visit with id "{visitId}". {e}', - $this->isType('array'), - ); - $this->eventDispatcher->expects($this->once())->method('dispatch')->with(new VisitLocated('123')); - - ($this->locateVisit)($event); - } - - #[Test, DataProvider('provideNonLocatableVisits')] - public function nonLocatableVisitsResolveToEmptyLocations(Visit $visit): void - { - $event = new UrlVisited('123'); - $this->em->expects($this->once())->method('find')->with(Visit::class, '123')->willReturn($visit); - $this->em->expects($this->once())->method('flush'); - $this->dbUpdater->expects($this->once())->method('databaseFileExists')->withAnyParameters()->willReturn(true); - $this->ipLocationResolver->expects($this->never())->method('resolveIpLocation'); - - $this->eventDispatcher->expects($this->once())->method('dispatch')->with(new VisitLocated('123')); - $this->logger->expects($this->never())->method('warning'); - - ($this->locateVisit)($event); - - self::assertEquals($visit->getVisitLocation(), VisitLocation::fromGeolocation(Location::emptyInstance())); - } - - public static function provideNonLocatableVisits(): iterable - { - $shortUrl = ShortUrl::createFake(); - - yield 'null IP' => [Visit::forValidShortUrl($shortUrl, Visitor::empty())]; - yield 'empty IP' => [Visit::forValidShortUrl($shortUrl, Visitor::fromParams(remoteAddress: ''))]; - yield 'localhost' => [ - Visit::forValidShortUrl($shortUrl, Visitor::fromParams(remoteAddress: IpAddress::LOCALHOST)), - ]; - } - - #[Test, DataProvider('provideIpAddresses')] - public function locatableVisitsResolveToLocation(Visit $visit, string|null $originalIpAddress): void - { - $ipAddr = $originalIpAddress ?? $visit->remoteAddr; - $location = new Location('', '', '', '', 0.0, 0.0, ''); - $event = new UrlVisited('123', $originalIpAddress); - - $this->em->expects($this->once())->method('find')->with(Visit::class, '123')->willReturn($visit); - $this->em->expects($this->once())->method('flush'); - $this->dbUpdater->expects($this->once())->method('databaseFileExists')->withAnyParameters()->willReturn(true); - $this->ipLocationResolver->expects($this->once())->method('resolveIpLocation')->with($ipAddr)->willReturn( - $location, - ); - - $this->eventDispatcher->expects($this->once())->method('dispatch')->with( - new VisitLocated('123', $originalIpAddress), - ); - $this->logger->expects($this->never())->method('warning'); - - ($this->locateVisit)($event); - - self::assertEquals($visit->getVisitLocation(), VisitLocation::fromGeolocation($location)); - } - - public static function provideIpAddresses(): iterable - { - yield 'no original IP address' => [ - Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::fromParams(remoteAddress: '1.2.3.4')), - null, - ]; - yield 'original IP address' => [ - Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::fromParams(remoteAddress: '1.2.3.4')), - '1.2.3.4', - ]; - yield 'base url' => [Visit::forBasePath(Visitor::fromParams(remoteAddress: '1.2.3.4')), '1.2.3.4']; - yield 'invalid short url' => [ - Visit::forInvalidShortUrl(Visitor::fromParams(remoteAddress: '1.2.3.4')), - '1.2.3.4', - ]; - yield 'regular not found' => [ - Visit::forRegularNotFound(Visitor::fromParams(remoteAddress: '1.2.3.4')), - '1.2.3.4', - ]; - } -} diff --git a/module/Core/test/EventDispatcher/Matomo/SendVisitToMatomoTest.php b/module/Core/test/EventDispatcher/Matomo/SendVisitToMatomoTest.php index 725980a1..8a4c1b7d 100644 --- a/module/Core/test/EventDispatcher/Matomo/SendVisitToMatomoTest.php +++ b/module/Core/test/EventDispatcher/Matomo/SendVisitToMatomoTest.php @@ -11,7 +11,7 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; +use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited; use Shlinkio\Shlink\Core\EventDispatcher\Matomo\SendVisitToMatomo; use Shlinkio\Shlink\Core\Matomo\MatomoOptions; use Shlinkio\Shlink\Core\Matomo\MatomoVisitSenderInterface; @@ -39,7 +39,7 @@ class SendVisitToMatomoTest extends TestCase $this->logger->expects($this->never())->method('error'); $this->logger->expects($this->never())->method('warning'); - ($this->listener(enabled: false))(new VisitLocated('123')); + ($this->listener(enabled: false))(new UrlVisited('123')); } #[Test] @@ -53,7 +53,7 @@ class SendVisitToMatomoTest extends TestCase ['visitId' => '123'], ); - ($this->listener())(new VisitLocated('123')); + ($this->listener())(new UrlVisited('123')); } #[Test, DataProvider('provideOriginalIpAddress')] @@ -67,7 +67,7 @@ class SendVisitToMatomoTest extends TestCase $this->logger->expects($this->never())->method('error'); $this->logger->expects($this->never())->method('warning'); - ($this->listener())(new VisitLocated($visitId, $originalIpAddress)); + ($this->listener())(new UrlVisited($visitId, $originalIpAddress)); } public static function provideOriginalIpAddress(): iterable @@ -92,7 +92,7 @@ class SendVisitToMatomoTest extends TestCase ['e' => $e], ); - ($this->listener())(new VisitLocated($visitId)); + ($this->listener())(new UrlVisited($visitId)); } private function listener(bool $enabled = true): SendVisitToMatomo diff --git a/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php b/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php index 1e3dfb96..91569c9b 100644 --- a/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php +++ b/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php @@ -13,7 +13,7 @@ use Psr\Log\LoggerInterface; use RuntimeException; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; use Shlinkio\Shlink\Common\UpdatePublishing\Update; -use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; +use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited; use Shlinkio\Shlink\Core\EventDispatcher\Mercure\NotifyVisitToMercure; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; @@ -54,7 +54,7 @@ class NotifyVisitToMercureTest extends TestCase $this->updatesGenerator->expects($this->never())->method('newVisitUpdate'); $this->helper->expects($this->never())->method('publishUpdate'); - ($this->listener)(new VisitLocated($visitId)); + ($this->listener)(new UrlVisited($visitId)); } #[Test] @@ -74,7 +74,7 @@ class NotifyVisitToMercureTest extends TestCase $this->updatesGenerator->expects($this->once())->method('newVisitUpdate')->with($visit)->willReturn($update); $this->helper->expects($this->exactly(2))->method('publishUpdate')->with($update); - ($this->listener)(new VisitLocated($visitId)); + ($this->listener)(new UrlVisited($visitId)); } #[Test] @@ -98,7 +98,7 @@ class NotifyVisitToMercureTest extends TestCase $this->updatesGenerator->expects($this->once())->method('newVisitUpdate')->with($visit)->willReturn($update); $this->helper->expects($this->once())->method('publishUpdate')->with($update)->willThrowException($e); - ($this->listener)(new VisitLocated($visitId)); + ($this->listener)(new UrlVisited($visitId)); } #[Test, DataProvider('provideOrphanVisits')] @@ -117,7 +117,7 @@ class NotifyVisitToMercureTest extends TestCase $this->updatesGenerator->expects($this->never())->method('newVisitUpdate'); $this->helper->expects($this->once())->method('publishUpdate')->with($update); - ($this->listener)(new VisitLocated($visitId)); + ($this->listener)(new UrlVisited($visitId)); } public static function provideOrphanVisits(): iterable diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php index 26785897..a3cebaa7 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php @@ -17,7 +17,7 @@ use RuntimeException; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; use Shlinkio\Shlink\Common\UpdatePublishing\Update; use Shlinkio\Shlink\Core\Config\Options\RabbitMqOptions; -use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; +use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; use Shlinkio\Shlink\Core\EventDispatcher\RabbitMq\NotifyVisitToRabbitMq; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; @@ -52,7 +52,7 @@ class NotifyVisitToRabbitMqTest extends TestCase $this->logger->expects($this->never())->method('warning'); $this->logger->expects($this->never())->method('debug'); - ($this->listener(new RabbitMqOptions(enabled: false)))(new VisitLocated('123')); + ($this->listener(new RabbitMqOptions(enabled: false)))(new UrlVisited('123')); } #[Test] @@ -67,7 +67,7 @@ class NotifyVisitToRabbitMqTest extends TestCase $this->logger->expects($this->never())->method('debug'); $this->helper->expects($this->never())->method('publishUpdate'); - ($this->listener())(new VisitLocated($visitId)); + ($this->listener())(new UrlVisited($visitId)); } #[Test, DataProvider('provideVisits')] @@ -85,7 +85,7 @@ class NotifyVisitToRabbitMqTest extends TestCase ); $this->logger->expects($this->never())->method('debug'); - ($this->listener())(new VisitLocated($visitId)); + ($this->listener())(new UrlVisited($visitId)); } public static function provideVisits(): iterable @@ -121,7 +121,7 @@ class NotifyVisitToRabbitMqTest extends TestCase ['e' => $e, 'name' => 'RabbitMQ'], ); - ($this->listener())(new VisitLocated($visitId)); + ($this->listener())(new UrlVisited($visitId)); } public static function provideExceptions(): iterable @@ -142,7 +142,7 @@ class NotifyVisitToRabbitMqTest extends TestCase $setup($this->updatesGenerator); $expect($this->helper, $this->updatesGenerator); - ($this->listener())(new VisitLocated($visitId)); + ($this->listener())(new UrlVisited($visitId)); } public static function providePayloads(): iterable diff --git a/module/Core/test/EventDispatcher/RedisPubSub/NotifyVisitToRedisTest.php b/module/Core/test/EventDispatcher/RedisPubSub/NotifyVisitToRedisTest.php index 20cd786a..7cbf68b6 100644 --- a/module/Core/test/EventDispatcher/RedisPubSub/NotifyVisitToRedisTest.php +++ b/module/Core/test/EventDispatcher/RedisPubSub/NotifyVisitToRedisTest.php @@ -15,7 +15,7 @@ use Psr\Log\LoggerInterface; use RuntimeException; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; use Shlinkio\Shlink\Common\UpdatePublishing\Update; -use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; +use Shlinkio\Shlink\Core\EventDispatcher\Event\UrlVisited; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; use Shlinkio\Shlink\Core\EventDispatcher\RedisPubSub\NotifyVisitToRedis; use Shlinkio\Shlink\Core\Visit\Entity\Visit; @@ -45,7 +45,7 @@ class NotifyVisitToRedisTest extends TestCase $this->logger->expects($this->never())->method('warning'); $this->logger->expects($this->never())->method('debug'); - $this->createListener(false)(new VisitLocated('123')); + $this->createListener(false)(new UrlVisited('123')); } #[Test, DataProvider('provideExceptions')] @@ -64,7 +64,7 @@ class NotifyVisitToRedisTest extends TestCase ['e' => $e, 'name' => 'Redis pub/sub'], ); - $this->createListener()(new VisitLocated($visitId)); + $this->createListener()(new UrlVisited($visitId)); } public static function provideExceptions(): iterable diff --git a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php index 2e35c1a4..85168020 100644 --- a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php +++ b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php @@ -9,7 +9,6 @@ use Laminas\Diactoros\ServerRequestFactory; use Laminas\Diactoros\Uri; use Mezzio\Router\Route; use Mezzio\Router\RouteResult; -use PHPUnit\Framework\Assert; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; @@ -27,7 +26,6 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; -use Shlinkio\Shlink\IpGeolocation\Model\Location; use function Laminas\Stratigility\middleware; use function str_starts_with; @@ -155,10 +153,7 @@ class ExtraPathRedirectMiddlewareTest extends TestCase ); $this->redirectionBuilder->expects($this->once())->method('buildShortUrlRedirect')->with( $shortUrl, - $this->callback(function (ServerRequestInterface $req) { - Assert::assertArrayHasKey(Location::class, $req->getAttributes()); - return true; - }), + $this->isInstanceOf(ServerRequestInterface::class), $expectedExtraPath, )->willReturn('the_built_long_url'); $this->redirectResponseHelper->expects($this->once())->method('buildRedirectResponse')->with(