diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 5f3d8fae..efdd7d33 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -10,6 +10,7 @@ use Psr\EventDispatcher\EventDispatcherInterface; use Shlinkio\Shlink\Common\Doctrine\EntityRepositoryFactory; use Shlinkio\Shlink\Config\Factory\ValinorConfigFactory; use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; +use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; use Symfony\Component\Lock; @@ -101,6 +102,7 @@ return [ Matomo\MatomoOptions::class => [ValinorConfigFactory::class, 'config.matomo'], Matomo\MatomoTrackerBuilder::class => ConfigAbstractFactory::class, + Matomo\MatomoVisitSender::class => ConfigAbstractFactory::class, ], 'aliases' => [ @@ -110,6 +112,7 @@ return [ ConfigAbstractFactory::class => [ Matomo\MatomoTrackerBuilder::class => [Matomo\MatomoOptions::class], + Matomo\MatomoVisitSender::class => [Matomo\MatomoTrackerBuilder::class, ShortUrlStringifier::class], ErrorHandler\NotFoundTypeResolverMiddleware::class => ['config.router.base_path'], ErrorHandler\NotFoundTrackerMiddleware::class => [Visit\RequestTracker::class], diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 8fc534d4..f401f255 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -12,7 +12,6 @@ use Shlinkio\Shlink\Common\Mercure\MercureHubPublishingHelper; use Shlinkio\Shlink\Common\Mercure\MercureOptions; use Shlinkio\Shlink\Common\RabbitMq\RabbitMqPublishingHelper; use Shlinkio\Shlink\Core\Matomo\MatomoOptions; -use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; use Shlinkio\Shlink\Core\Visit\Geolocation\VisitLocator; use Shlinkio\Shlink\Core\Visit\Geolocation\VisitToLocationHelper; use Shlinkio\Shlink\EventDispatcher\Listener\EnabledListenerCheckerInterface; @@ -157,9 +156,8 @@ return (static function (): array { EventDispatcher\Matomo\SendVisitToMatomo::class => [ 'em', 'Logger_Shlink', - ShortUrlStringifier::class, Matomo\MatomoOptions::class, - Matomo\MatomoTrackerBuilder::class, + Matomo\MatomoVisitSender::class, ], EventDispatcher\UpdateGeoLiteDb::class => [ diff --git a/module/Core/src/Config/PostProcessor/ShortUrlMethodsProcessor.php b/module/Core/src/Config/PostProcessor/ShortUrlMethodsProcessor.php index 42f00889..23ffe326 100644 --- a/module/Core/src/Config/PostProcessor/ShortUrlMethodsProcessor.php +++ b/module/Core/src/Config/PostProcessor/ShortUrlMethodsProcessor.php @@ -40,7 +40,7 @@ class ShortUrlMethodsProcessor $redirectStatus = RedirectStatus::tryFrom( $config['redirects']['redirect_status_code'] ?? 0, ) ?? DEFAULT_REDIRECT_STATUS_CODE; - $redirectRoute['allowed_methods'] = $redirectStatus->isLegacyStatus() + $redirectRoute['allowed_methods'] = $redirectStatus->isGetOnlyStatus() ? [RequestMethodInterface::METHOD_GET] : Route::HTTP_METHOD_ANY; diff --git a/module/Core/src/EventDispatcher/Matomo/SendVisitToMatomo.php b/module/Core/src/EventDispatcher/Matomo/SendVisitToMatomo.php index d0fa1035..c47b9cba 100644 --- a/module/Core/src/EventDispatcher/Matomo/SendVisitToMatomo.php +++ b/module/Core/src/EventDispatcher/Matomo/SendVisitToMatomo.php @@ -8,8 +8,7 @@ use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; use Shlinkio\Shlink\Core\Matomo\MatomoOptions; -use Shlinkio\Shlink\Core\Matomo\MatomoTrackerBuilderInterface; -use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; +use Shlinkio\Shlink\Core\Matomo\MatomoVisitSenderInterface; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Throwable; @@ -18,9 +17,8 @@ readonly class SendVisitToMatomo public function __construct( private EntityManagerInterface $em, private LoggerInterface $logger, - private ShortUrlStringifier $shortUrlStringifier, private MatomoOptions $matomoOptions, - private MatomoTrackerBuilderInterface $trackerBuilder, + private MatomoVisitSenderInterface $visitSender, ) { } @@ -42,48 +40,10 @@ readonly class SendVisitToMatomo } try { - $tracker = $this->trackerBuilder->buildMatomoTracker(); - - $tracker - ->setUrl($this->resolveUrlToTrack($visit)) - ->setCustomTrackingParameter('type', $visit->type->value) - ->setUserAgent($visit->userAgent) - ->setUrlReferrer($visit->referer); - - $location = $visit->getVisitLocation(); - if ($location !== null) { - $tracker - ->setCity($location->cityName) - ->setCountry($location->countryName) - ->setLatitude($location->latitude) - ->setLongitude($location->longitude); - } - - // Set not obfuscated IP if possible, as matomo handles obfuscation itself - $ip = $visitLocated->originalIpAddress ?? $visit->remoteAddr; - if ($ip !== null) { - $tracker->setIp($ip); - } - - if ($visit->isOrphan()) { - $tracker->setCustomTrackingParameter('orphan', 'true'); - } - - // Send the short URL title or an empty document title to avoid different actions to be created by matomo - $tracker->doTrackPageView($visit->shortUrl?->title() ?? ''); + $this->visitSender->sendVisitToMatomo($visit, $visitLocated->originalIpAddress); } catch (Throwable $e) { // Capture all exceptions to make sure this does not interfere with the regular execution $this->logger->error('An error occurred while trying to send visit to Matomo. {e}', ['e' => $e]); } } - - public function resolveUrlToTrack(Visit $visit): string - { - $shortUrl = $visit->shortUrl; - if ($shortUrl === null) { - return $visit->visitedUrl ?? ''; - } - - return $this->shortUrlStringifier->stringify($shortUrl); - } } diff --git a/module/Core/src/Matomo/MatomoTrackerBuilder.php b/module/Core/src/Matomo/MatomoTrackerBuilder.php index 4bad6799..e006271b 100644 --- a/module/Core/src/Matomo/MatomoTrackerBuilder.php +++ b/module/Core/src/Matomo/MatomoTrackerBuilder.php @@ -7,11 +7,11 @@ namespace Shlinkio\Shlink\Core\Matomo; use MatomoTracker; use Shlinkio\Shlink\Core\Exception\RuntimeException; -class MatomoTrackerBuilder implements MatomoTrackerBuilderInterface +readonly class MatomoTrackerBuilder implements MatomoTrackerBuilderInterface { public const MATOMO_DEFAULT_TIMEOUT = 10; // Time in seconds - public function __construct(private readonly MatomoOptions $options) + public function __construct(private MatomoOptions $options) { } diff --git a/module/Core/src/Matomo/MatomoVisitSender.php b/module/Core/src/Matomo/MatomoVisitSender.php new file mode 100644 index 00000000..8caed3cc --- /dev/null +++ b/module/Core/src/Matomo/MatomoVisitSender.php @@ -0,0 +1,60 @@ +trackerBuilder->buildMatomoTracker(); + + $tracker + ->setUrl($this->resolveUrlToTrack($visit)) + ->setCustomTrackingParameter('type', $visit->type->value) + ->setUserAgent($visit->userAgent) + ->setUrlReferrer($visit->referer); + + $location = $visit->getVisitLocation(); + if ($location !== null) { + $tracker + ->setCity($location->cityName) + ->setCountry($location->countryName) + ->setLatitude($location->latitude) + ->setLongitude($location->longitude); + } + + // Set not obfuscated IP if possible, as matomo handles obfuscation itself + $ip = $originalIpAddress ?? $visit->remoteAddr; + if ($ip !== null) { + $tracker->setIp($ip); + } + + if ($visit->isOrphan()) { + $tracker->setCustomTrackingParameter('orphan', 'true'); + } + + // Send the short URL title or an empty document title to avoid different actions to be created by matomo + $tracker->doTrackPageView($visit->shortUrl?->title() ?? ''); + } + + private function resolveUrlToTrack(Visit $visit): string + { + $shortUrl = $visit->shortUrl; + if ($shortUrl === null) { + return $visit->visitedUrl ?? ''; + } + + return $this->shortUrlStringifier->stringify($shortUrl); + } +} diff --git a/module/Core/src/Matomo/MatomoVisitSenderInterface.php b/module/Core/src/Matomo/MatomoVisitSenderInterface.php new file mode 100644 index 00000000..fef16367 --- /dev/null +++ b/module/Core/src/Matomo/MatomoVisitSenderInterface.php @@ -0,0 +1,12 @@ +em = $this->createMock(EntityManagerInterface::class); $this->logger = $this->createMock(LoggerInterface::class); - $this->trackerBuilder = $this->createMock(MatomoTrackerBuilderInterface::class); + $this->visitSender = $this->createMock(MatomoVisitSenderInterface::class); } #[Test] public function visitIsNotSentWhenMatomoIsDisabled(): void { $this->em->expects($this->never())->method('find'); - $this->trackerBuilder->expects($this->never())->method('buildMatomoTracker'); + $this->visitSender->expects($this->never())->method('sendVisitToMatomo'); $this->logger->expects($this->never())->method('error'); $this->logger->expects($this->never())->method('warning'); @@ -53,7 +46,7 @@ class SendVisitToMatomoTest extends TestCase public function visitIsNotSentWhenItDoesNotExist(): void { $this->em->expects($this->once())->method('find')->willReturn(null); - $this->trackerBuilder->expects($this->never())->method('buildMatomoTracker'); + $this->visitSender->expects($this->never())->method('sendVisitToMatomo'); $this->logger->expects($this->never())->method('error'); $this->logger->expects($this->once())->method('warning')->with( 'Tried to send visit with id "{visitId}" to matomo, but it does not exist.', @@ -63,97 +56,24 @@ class SendVisitToMatomoTest extends TestCase ($this->listener())(new VisitLocated('123')); } - #[Test, DataProvider('provideTrackerMethods')] - public function visitIsSentWhenItExists(Visit $visit, ?string $originalIpAddress, array $invokedMethods): void + #[Test, DataProvider('provideOriginalIpAddress')] + public function visitIsSentWhenItExists(?string $originalIpAddress): void { $visitId = '123'; - - $tracker = $this->createMock(MatomoTracker::class); - $tracker->expects($this->once())->method('setUrl')->willReturn($tracker); - $tracker->expects($this->once())->method('setUserAgent')->willReturn($tracker); - $tracker->expects($this->once())->method('setUrlReferrer')->willReturn($tracker); - $tracker->expects($this->once())->method('doTrackPageView')->with($visit->shortUrl?->title() ?? ''); - - if ($visit->isOrphan()) { - $tracker->expects($this->exactly(2))->method('setCustomTrackingParameter')->willReturnMap([ - ['type', $visit->type->value, $tracker], - ['orphan', 'true', $tracker], - ]); - } else { - $tracker->expects($this->once())->method('setCustomTrackingParameter')->with( - 'type', - $visit->type->value, - )->willReturn($tracker); - } - - foreach ($invokedMethods as $invokedMethod) { - $tracker->expects($this->once())->method($invokedMethod)->willReturn($tracker); - } + $visit = Visit::forBasePath(Visitor::emptyInstance()); $this->em->expects($this->once())->method('find')->with(Visit::class, $visitId)->willReturn($visit); - $this->trackerBuilder->expects($this->once())->method('buildMatomoTracker')->willReturn($tracker); + $this->visitSender->expects($this->once())->method('sendVisitToMatomo')->with($visit, $originalIpAddress); $this->logger->expects($this->never())->method('error'); $this->logger->expects($this->never())->method('warning'); ($this->listener())(new VisitLocated($visitId, $originalIpAddress)); } - public static function provideTrackerMethods(): iterable + public static function provideOriginalIpAddress(): iterable { - yield 'unlocated orphan visit' => [Visit::forBasePath(Visitor::emptyInstance()), null, []]; - yield 'located regular visit' => [ - Visit::forValidShortUrl(ShortUrl::withLongUrl('https://shlink.io'), Visitor::emptyInstance()) - ->locate(VisitLocation::fromGeolocation(new Location( - countryCode: 'countryCode', - countryName: 'countryName', - regionName: 'regionName', - city: 'city', - latitude: 123, - longitude: 123, - timeZone: 'timeZone', - ))), - '1.2.3.4', - ['setCity', 'setCountry', 'setLatitude', 'setLongitude', 'setIp'], - ]; - yield 'fallback IP' => [Visit::forBasePath(new Visitor('', '', '1.2.3.4', '')), null, ['setIp']]; - } - - #[Test, DataProvider('provideUrlsToTrack')] - public function properUrlIsTracked(Visit $visit, string $expectedTrackedUrl): void - { - $visitId = '123'; - - $tracker = $this->createMock(MatomoTracker::class); - $tracker->expects($this->once())->method('setUrl')->with($expectedTrackedUrl)->willReturn($tracker); - $tracker->expects($this->once())->method('setUserAgent')->willReturn($tracker); - $tracker->expects($this->once())->method('setUrlReferrer')->willReturn($tracker); - $tracker->expects($this->any())->method('setCustomTrackingParameter')->willReturn($tracker); - $tracker->expects($this->once())->method('doTrackPageView'); - - $this->em->expects($this->once())->method('find')->with(Visit::class, $visitId)->willReturn($visit); - $this->trackerBuilder->expects($this->once())->method('buildMatomoTracker')->willReturn($tracker); - $this->logger->expects($this->never())->method('error'); - $this->logger->expects($this->never())->method('warning'); - - ($this->listener())(new VisitLocated($visitId)); - } - - public static function provideUrlsToTrack(): iterable - { - yield 'orphan visit without visited URL' => [Visit::forBasePath(Visitor::emptyInstance()), '']; - yield 'orphan visit with visited URL' => [ - Visit::forBasePath(new Visitor('', '', null, 'https://s.test/foo')), - 'https://s.test/foo', - ]; - yield 'non-orphan visit' => [ - Visit::forValidShortUrl(ShortUrl::create( - ShortUrlCreation::fromRawData([ - ShortUrlInputFilter::LONG_URL => 'https://shlink.io', - ShortUrlInputFilter::CUSTOM_SLUG => 'bar', - ]), - ), Visitor::emptyInstance()), - 'http://s2.test/bar', - ]; + yield 'no original IP address' => [null]; + yield 'original IP address' => ['1.2.3.4']; } #[Test] @@ -165,7 +85,7 @@ class SendVisitToMatomoTest extends TestCase $this->em->expects($this->once())->method('find')->with(Visit::class, $visitId)->willReturn( $this->createMock(Visit::class), ); - $this->trackerBuilder->expects($this->once())->method('buildMatomoTracker')->willThrowException($e); + $this->visitSender->expects($this->once())->method('sendVisitToMatomo')->willThrowException($e); $this->logger->expects($this->never())->method('warning'); $this->logger->expects($this->once())->method('error')->with( 'An error occurred while trying to send visit to Matomo. {e}', @@ -180,9 +100,8 @@ class SendVisitToMatomoTest extends TestCase return new SendVisitToMatomo( $this->em, $this->logger, - new ShortUrlStringifier(['hostname' => 's2.test']), new MatomoOptions(enabled: $enabled), - $this->trackerBuilder, + $this->visitSender, ); } } diff --git a/module/Core/test/Matomo/MatomoVisitSenderTest.php b/module/Core/test/Matomo/MatomoVisitSenderTest.php new file mode 100644 index 00000000..3e08d6aa --- /dev/null +++ b/module/Core/test/Matomo/MatomoVisitSenderTest.php @@ -0,0 +1,119 @@ +trackerBuilder = $this->createMock(MatomoTrackerBuilderInterface::class); + $this->visitSender = new MatomoVisitSender( + $this->trackerBuilder, + new ShortUrlStringifier(['hostname' => 's2.test']), + ); + } + + #[Test, DataProvider('provideTrackerMethods')] + public function visitIsSentToMatomo(Visit $visit, ?string $originalIpAddress, array $invokedMethods): void + { + $tracker = $this->createMock(MatomoTracker::class); + $tracker->expects($this->once())->method('setUrl')->willReturn($tracker); + $tracker->expects($this->once())->method('setUserAgent')->willReturn($tracker); + $tracker->expects($this->once())->method('setUrlReferrer')->willReturn($tracker); + $tracker->expects($this->once())->method('doTrackPageView')->with($visit->shortUrl?->title() ?? ''); + + if ($visit->isOrphan()) { + $tracker->expects($this->exactly(2))->method('setCustomTrackingParameter')->willReturnMap([ + ['type', $visit->type->value, $tracker], + ['orphan', 'true', $tracker], + ]); + } else { + $tracker->expects($this->once())->method('setCustomTrackingParameter')->with( + 'type', + $visit->type->value, + )->willReturn($tracker); + } + + foreach ($invokedMethods as $invokedMethod) { + $tracker->expects($this->once())->method($invokedMethod)->willReturn($tracker); + } + + $this->trackerBuilder->expects($this->once())->method('buildMatomoTracker')->willReturn($tracker); + + $this->visitSender->sendVisitToMatomo($visit, $originalIpAddress); + } + + public static function provideTrackerMethods(): iterable + { + yield 'unlocated orphan visit' => [Visit::forBasePath(Visitor::emptyInstance()), null, []]; + yield 'located regular visit' => [ + Visit::forValidShortUrl(ShortUrl::withLongUrl('https://shlink.io'), Visitor::emptyInstance()) + ->locate(VisitLocation::fromGeolocation(new Location( + countryCode: 'countryCode', + countryName: 'countryName', + regionName: 'regionName', + city: 'city', + latitude: 123, + longitude: 123, + timeZone: 'timeZone', + ))), + '1.2.3.4', + ['setCity', 'setCountry', 'setLatitude', 'setLongitude', 'setIp'], + ]; + yield 'fallback IP' => [Visit::forBasePath(new Visitor('', '', '1.2.3.4', '')), null, ['setIp']]; + } + + #[Test, DataProvider('provideUrlsToTrack')] + public function properUrlIsTracked(Visit $visit, string $expectedTrackedUrl): void + { + $tracker = $this->createMock(MatomoTracker::class); + $tracker->expects($this->once())->method('setUrl')->with($expectedTrackedUrl)->willReturn($tracker); + $tracker->expects($this->once())->method('setUserAgent')->willReturn($tracker); + $tracker->expects($this->once())->method('setUrlReferrer')->willReturn($tracker); + $tracker->expects($this->any())->method('setCustomTrackingParameter')->willReturn($tracker); + $tracker->expects($this->once())->method('doTrackPageView'); + + $this->trackerBuilder->expects($this->once())->method('buildMatomoTracker')->willReturn($tracker); + + $this->visitSender->sendVisitToMatomo($visit); + } + + public static function provideUrlsToTrack(): iterable + { + yield 'orphan visit without visited URL' => [Visit::forBasePath(Visitor::emptyInstance()), '']; + yield 'orphan visit with visited URL' => [ + Visit::forBasePath(new Visitor('', '', null, 'https://s.test/foo')), + 'https://s.test/foo', + ]; + yield 'non-orphan visit' => [ + Visit::forValidShortUrl(ShortUrl::create( + ShortUrlCreation::fromRawData([ + ShortUrlInputFilter::LONG_URL => 'https://shlink.io', + ShortUrlInputFilter::CUSTOM_SLUG => 'bar', + ]), + ), Visitor::emptyInstance()), + 'http://s2.test/bar', + ]; + } +}