diff --git a/CHANGELOG.md b/CHANGELOG.md index b46c26ec..7afac46b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this This command can be run periodically by those who create many disposable URLs which are valid only for a period of time, and then can be deleted to save space. +* [#1925](https://github.com/shlinkio/shlink/issues/1925) Add new `integration:matomo:send-visits` console command that can be used to send existing visits to integrated Matomo instance. + ### Changed * [#2034](https://github.com/shlinkio/shlink/issues/2034) Modernize entities, using constructor property promotion and readonly wherever possible. * [#2036](https://github.com/shlinkio/shlink/issues/2036) Deep performance improvement in some endpoints which involve counting visits: diff --git a/composer.json b/composer.json index 99dc8a88..a8f52bd0 100644 --- a/composer.json +++ b/composer.json @@ -118,7 +118,7 @@ "@parallel test:unit test:db", "@parallel test:api test:cli" ], - "test:unit": "@php vendor/bin/phpunit --order-by=random --colors=always --testdox", + "test:unit": "COLUMNS=120 vendor/bin/phpunit --order-by=random --colors=always --testdox", "test:unit:ci": "@test:unit --coverage-php=build/coverage-unit.cov", "test:unit:pretty": "@test:unit --coverage-html build/coverage-unit/coverage-html", "test:db": "@parallel test:db:sqlite:ci test:db:mysql test:db:maria test:db:postgres test:db:ms", diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 63b2de6f..2ee33a1d 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -42,6 +42,8 @@ return [ Command\RedirectRule\ManageRedirectRulesCommand::NAME => Command\RedirectRule\ManageRedirectRulesCommand::class, + + Command\Integration\MatomoSendVisitsCommand::NAME => Command\Integration\MatomoSendVisitsCommand::class, ], ], diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 875c8226..f9b90dac 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -8,6 +8,7 @@ use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory; use Laminas\ServiceManager\Factory\InvokableFactory; use Shlinkio\Shlink\Common\Doctrine\NoDbNameConnectionFactory; use Shlinkio\Shlink\Core\Domain\DomainService; +use Shlinkio\Shlink\Core\Matomo; use Shlinkio\Shlink\Core\Options\TrackingOptions; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleService; @@ -71,6 +72,8 @@ return [ Command\Domain\GetDomainVisitsCommand::class => ConfigAbstractFactory::class, Command\RedirectRule\ManageRedirectRulesCommand::class => ConfigAbstractFactory::class, + + Command\Integration\MatomoSendVisitsCommand::class => ConfigAbstractFactory::class, ], ], @@ -129,6 +132,11 @@ return [ RedirectRule\RedirectRuleHandler::class, ], + Command\Integration\MatomoSendVisitsCommand::class => [ + Matomo\MatomoOptions::class, + Matomo\MatomoVisitSender::class, + ], + Command\Db\CreateDatabaseCommand::class => [ LockFactory::class, Util\ProcessRunner::class, diff --git a/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php b/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php new file mode 100644 index 00000000..ba9a794e --- /dev/null +++ b/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php @@ -0,0 +1,140 @@ +matomoEnabled = $matomoOptions->enabled; + parent::__construct(); + } + + protected function configure(): void + { + $help = <<%command.name% + + Send all visits created before 2024: + %command.name% --until 2023-12-31 + + Send all visits created after a specific day: + %command.name% --since 2022-03-27 + + Send all visits created during 2022: + %command.name% --since 2022-01-01 --until 2022-12-31 + HELP; + + $this + ->setName(self::NAME) + ->setDescription(sprintf( + '%sSend existing visits to the configured matomo instance', + $this->matomoEnabled ? '' : '[MATOMO INTEGRATION DISABLED] ', + )) + ->setHelp($help) + ->addOption( + 'since', + 's', + InputOption::VALUE_REQUIRED, + 'Only visits created since this date, inclusively, will be sent to Matomo', + ) + ->addOption( + 'until', + 'u', + InputOption::VALUE_REQUIRED, + 'Only visits created until this date, inclusively, will be sent to Matomo', + ); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $this->io = new SymfonyStyle($input, $output); + + if (! $this->matomoEnabled) { + $this->io->warning('Matomo integration is not enabled in this Shlink instance'); + return ExitCode::EXIT_WARNING; + } + + // TODO Validate provided date formats + $since = $input->getOption('since'); + $until = $input->getOption('until'); + $dateRange = buildDateRange( + startDate: $since !== null ? Chronos::parse($since) : null, + endDate: $until !== null ? Chronos::parse($until) : null, + ); + + if ($input->isInteractive()) { + $this->io->warning([ + 'You are about to send visits from this Shlink instance to Matomo', + 'Resolved date range -> ' . dateRangeToHumanFriendly($dateRange), + 'Shlink will not check for already sent visits, which could result in some duplications. Make sure ' + . 'you have verified only visits in the right date range are going to be sent.', + ]); + if (! $this->io->confirm('Continue?', default: false)) { + return ExitCode::EXIT_WARNING; + } + } + + $result = $this->visitSender->sendVisitsInDateRange($dateRange, $this); + + match (true) { + $result->hasFailures() && $result->hasSuccesses() => $this->io->warning( + sprintf('%s visits sent to Matomo. %s failed.', $result->successfulVisits, $result->failedVisits), + ), + $result->hasFailures() => $this->io->error( + sprintf('Failed to send %s visits to Matomo.', $result->failedVisits), + ), + $result->hasSuccesses() => $this->io->success( + sprintf('%s visits sent to Matomo.', $result->successfulVisits), + ), + default => $this->io->info('There was no visits matching provided date range.'), + }; + + return ExitCode::EXIT_SUCCESS; + } + + public function success(int $index): void + { + $this->io->write('.'); + } + + public function error(int $index, Throwable $e): void + { + $this->io->write('E'); + if ($this->io->isVerbose()) { + $this->getApplication()?->renderThrowable($e, $this->io); + } + } +} diff --git a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php index a3e8a43c..d3a49c53 100644 --- a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php +++ b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php @@ -55,7 +55,7 @@ abstract class AbstractVisitsListCommand extends Command $rowData = [ 'referer' => $visit->referer, - 'date' => $visit->getDate()->toAtomString(), + 'date' => $visit->date->toAtomString(), 'userAgent' => $visit->userAgent, 'potentialBot' => $visit->potentialBot, 'country' => $visit->getVisitLocation()?->countryName ?? 'Unknown', diff --git a/module/CLI/test/Command/Domain/GetDomainVisitsCommandTest.php b/module/CLI/test/Command/Domain/GetDomainVisitsCommandTest.php index 7f4bd076..6563abc0 100644 --- a/module/CLI/test/Command/Domain/GetDomainVisitsCommandTest.php +++ b/module/CLI/test/Command/Domain/GetDomainVisitsCommandTest.php @@ -60,7 +60,7 @@ class GetDomainVisitsCommandTest extends TestCase +---------+---------------------------+------------+---------+--------+---------------+ | Referer | Date | User agent | Country | City | Short Url | +---------+---------------------------+------------+---------+--------+---------------+ - | foo | {$visit->getDate()->toAtomString()} | bar | Spain | Madrid | the_short_url | + | foo | {$visit->date->toAtomString()} | bar | Spain | Madrid | the_short_url | +---------+---------------------------+------------+---------+--------+---------------+ OUTPUT, diff --git a/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php b/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php new file mode 100644 index 00000000..e3a52733 --- /dev/null +++ b/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php @@ -0,0 +1,135 @@ +visitSender = $this->createMock(MatomoVisitSenderInterface::class); + } + + #[Test] + public function warningDisplayedIfIntegrationIsNotEnabled(): void + { + [$output, $exitCode] = $this->executeCommand(matomoEnabled: false); + + self::assertStringContainsString('Matomo integration is not enabled in this Shlink instance', $output); + self::assertEquals(ExitCode::EXIT_WARNING, $exitCode); + } + + #[Test] + #[TestWith([true])] + #[TestWith([false])] + public function warningIsOnlyDisplayedInInteractiveMode(bool $interactive): void + { + $this->visitSender->method('sendVisitsInDateRange')->willReturn(new SendVisitsResult()); + + [$output] = $this->executeCommand(['y'], ['interactive' => $interactive]); + + if ($interactive) { + self::assertStringContainsString('You are about to send visits', $output); + } else { + self::assertStringNotContainsString('You are about to send visits', $output); + } + } + + #[Test] + #[TestWith([true])] + #[TestWith([false])] + public function canCancelExecutionInInteractiveMode(bool $interactive): void + { + $this->visitSender->expects($this->exactly($interactive ? 0 : 1))->method('sendVisitsInDateRange')->willReturn( + new SendVisitsResult(), + ); + $this->executeCommand(['n'], ['interactive' => $interactive]); + } + + #[Test] + #[TestWith([new SendVisitsResult(), 'There was no visits matching provided date range'])] + #[TestWith([new SendVisitsResult(successfulVisits: 10), '10 visits sent to Matomo.'])] + #[TestWith([new SendVisitsResult(successfulVisits: 2), '2 visits sent to Matomo.'])] + #[TestWith([new SendVisitsResult(failedVisits: 238), 'Failed to send 238 visits to Matomo.'])] + #[TestWith([new SendVisitsResult(failedVisits: 18), 'Failed to send 18 visits to Matomo.'])] + #[TestWith([new SendVisitsResult(successfulVisits: 2, failedVisits: 35), '2 visits sent to Matomo. 35 failed.'])] + #[TestWith([new SendVisitsResult(successfulVisits: 81, failedVisits: 6), '81 visits sent to Matomo. 6 failed.'])] + public function expectedResultIsDisplayed(SendVisitsResult $result, string $expectedResultMessage): void + { + $this->visitSender->expects($this->once())->method('sendVisitsInDateRange')->willReturn($result); + [$output, $exitCode] = $this->executeCommand(['y']); + + self::assertStringContainsString($expectedResultMessage, $output); + self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + } + + #[Test] + public function printsResultOfSendingVisits(): void + { + $this->visitSender->method('sendVisitsInDateRange')->willReturnCallback( + function (DateRange $_, MatomoSendVisitsCommand $command): SendVisitsResult { + // Call it a few times for an easier match of its result in the command putput + $command->success(0); + $command->success(1); + $command->success(2); + $command->error(3, new Exception('Error')); + $command->success(4); + $command->error(5, new Exception('Error')); + + return new SendVisitsResult(); + }, + ); + + [$output] = $this->executeCommand(['y']); + + self::assertStringContainsString('...E.E', $output); + } + + #[Test] + #[TestWith([[], 'All time'])] + #[TestWith([['--since' => '2023-05-01'], 'Since 2023-05-01 00:00:00'])] + #[TestWith([['--until' => '2023-05-01'], 'Until 2023-05-01 00:00:00'])] + #[TestWith([ + ['--since' => '2023-05-01', '--until' => '2024-02-02 23:59:59'], + 'Between 2023-05-01 00:00:00 and 2024-02-02 23:59:59', + ])] + public function providedDateAreParsed(array $args, string $expectedMessage): void + { + [$output] = $this->executeCommand(['n'], args: $args); + self::assertStringContainsString('Resolved date range -> ' . $expectedMessage, $output); + } + + /** + * @return array{string, int, MatomoSendVisitsCommand} + */ + private function executeCommand( + array $input = [], + array $options = [], + array $args = [], + bool $matomoEnabled = true, + ): array { + $command = new MatomoSendVisitsCommand(new MatomoOptions(enabled: $matomoEnabled), $this->visitSender); + $commandTester = CliTestUtils::testerForCommand($command); + $commandTester->setInputs($input); + $commandTester->execute($args, $options); + + $output = $commandTester->getDisplay(); + $exitCode = $commandTester->getStatusCode(); + + return [$output, $exitCode, $command]; + } +} diff --git a/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php index f93ab5ec..ba6735ba 100644 --- a/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php @@ -110,7 +110,7 @@ class GetShortUrlVisitsCommandTest extends TestCase +---------+---------------------------+------------+---------+--------+ | Referer | Date | User agent | Country | City | +---------+---------------------------+------------+---------+--------+ - | foo | {$visit->getDate()->toAtomString()} | bar | Spain | Madrid | + | foo | {$visit->date->toAtomString()} | bar | Spain | Madrid | +---------+---------------------------+------------+---------+--------+ OUTPUT, diff --git a/module/CLI/test/Command/Tag/GetTagVisitsCommandTest.php b/module/CLI/test/Command/Tag/GetTagVisitsCommandTest.php index a2dc059f..9b79f509 100644 --- a/module/CLI/test/Command/Tag/GetTagVisitsCommandTest.php +++ b/module/CLI/test/Command/Tag/GetTagVisitsCommandTest.php @@ -57,7 +57,7 @@ class GetTagVisitsCommandTest extends TestCase +---------+---------------------------+------------+---------+--------+---------------+ | Referer | Date | User agent | Country | City | Short Url | +---------+---------------------------+------------+---------+--------+---------------+ - | foo | {$visit->getDate()->toAtomString()} | bar | Spain | Madrid | the_short_url | + | foo | {$visit->date->toAtomString()} | bar | Spain | Madrid | the_short_url | +---------+---------------------------+------------+---------+--------+---------------+ OUTPUT, diff --git a/module/CLI/test/Command/Visit/GetNonOrphanVisitsCommandTest.php b/module/CLI/test/Command/Visit/GetNonOrphanVisitsCommandTest.php index 439b33bd..0462c2c0 100644 --- a/module/CLI/test/Command/Visit/GetNonOrphanVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/GetNonOrphanVisitsCommandTest.php @@ -56,7 +56,7 @@ class GetNonOrphanVisitsCommandTest extends TestCase +---------+---------------------------+------------+---------+--------+---------------+ | Referer | Date | User agent | Country | City | Short Url | +---------+---------------------------+------------+---------+--------+---------------+ - | foo | {$visit->getDate()->toAtomString()} | bar | Spain | Madrid | the_short_url | + | foo | {$visit->date->toAtomString()} | bar | Spain | Madrid | the_short_url | +---------+---------------------------+------------+---------+--------+---------------+ OUTPUT, diff --git a/module/CLI/test/Command/Visit/GetOrphanVisitsCommandTest.php b/module/CLI/test/Command/Visit/GetOrphanVisitsCommandTest.php index a9e2a50c..29914b61 100644 --- a/module/CLI/test/Command/Visit/GetOrphanVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/GetOrphanVisitsCommandTest.php @@ -54,7 +54,7 @@ class GetOrphanVisitsCommandTest extends TestCase +---------+---------------------------+------------+---------+--------+----------+ | Referer | Date | User agent | Country | City | Type | +---------+---------------------------+------------+---------+--------+----------+ - | foo | {$visit->getDate()->toAtomString()} | bar | Spain | Madrid | base_url | + | foo | {$visit->date->toAtomString()} | bar | Spain | Madrid | base_url | +---------+---------------------------+------------+---------+--------+----------+ OUTPUT, diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 5f3d8fae..5fcc8e44 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; @@ -72,7 +73,7 @@ return [ Visit\Geolocation\VisitLocator::class => ConfigAbstractFactory::class, Visit\Geolocation\VisitToLocationHelper::class => ConfigAbstractFactory::class, Visit\VisitsStatsHelper::class => ConfigAbstractFactory::class, - Visit\Repository\VisitLocationRepository::class => [ + Visit\Repository\VisitIterationRepository::class => [ EntityRepositoryFactory::class, Visit\Entity\Visit::class, ], @@ -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,11 @@ return [ ConfigAbstractFactory::class => [ Matomo\MatomoTrackerBuilder::class => [Matomo\MatomoOptions::class], + Matomo\MatomoVisitSender::class => [ + Matomo\MatomoTrackerBuilder::class, + ShortUrlStringifier::class, + Visit\Repository\VisitIterationRepository::class, + ], ErrorHandler\NotFoundTypeResolverMiddleware::class => ['config.router.base_path'], ErrorHandler\NotFoundTrackerMiddleware::class => [Visit\RequestTracker::class], @@ -143,7 +150,7 @@ return [ ShortUrl\Repository\ShortUrlListRepository::class, Options\UrlShortenerOptions::class, ], - Visit\Geolocation\VisitLocator::class => ['em', Visit\Repository\VisitLocationRepository::class], + Visit\Geolocation\VisitLocator::class => ['em', Visit\Repository\VisitIterationRepository::class], Visit\Geolocation\VisitToLocationHelper::class => [IpLocationResolverInterface::class], Visit\VisitsStatsHelper::class => ['em'], Tag\TagService::class => ['em'], 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/functions/functions.php b/module/Core/functions/functions.php index 5ba45ac2..2e238e99 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -61,6 +61,23 @@ function parseDateRangeFromQuery(array $query, string $startDateName, string $en return buildDateRange($startDate, $endDate); } +function dateRangeToHumanFriendly(?DateRange $dateRange): string +{ + $startDate = $dateRange?->startDate; + $endDate = $dateRange?->endDate; + + return match (true) { + $startDate !== null && $endDate !== null => sprintf( + 'Between %s and %s', + $startDate->toDateTimeString(), + $endDate->toDateTimeString(), + ), + $startDate !== null => sprintf('Since %s', $startDate->toDateTimeString()), + $endDate !== null => sprintf('Until %s', $endDate->toDateTimeString()), + default => 'All time', + }; +} + /** * @return ($date is null ? null : Chronos) */ diff --git a/module/Core/src/Config/PostProcessor/ShortUrlMethodsProcessor.php b/module/Core/src/Config/PostProcessor/ShortUrlMethodsProcessor.php index 42f00889..a73c584d 100644 --- a/module/Core/src/Config/PostProcessor/ShortUrlMethodsProcessor.php +++ b/module/Core/src/Config/PostProcessor/ShortUrlMethodsProcessor.php @@ -4,8 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Config\PostProcessor; -use Fig\Http\Message\RequestMethodInterface; -use Mezzio\Router\Route; use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Util\RedirectStatus; @@ -40,9 +38,7 @@ class ShortUrlMethodsProcessor $redirectStatus = RedirectStatus::tryFrom( $config['redirects']['redirect_status_code'] ?? 0, ) ?? DEFAULT_REDIRECT_STATUS_CODE; - $redirectRoute['allowed_methods'] = $redirectStatus->isLegacyStatus() - ? [RequestMethodInterface::METHOD_GET] - : Route::HTTP_METHOD_ANY; + $redirectRoute['allowed_methods'] = $redirectStatus->allowedHttpMethods(); $config['routes'] = [...$rest, $redirectRoute]; return $config; diff --git a/module/Core/src/EventDispatcher/Matomo/SendVisitToMatomo.php b/module/Core/src/EventDispatcher/Matomo/SendVisitToMatomo.php index d0fa1035..5a85aed4 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->sendVisit($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/Importer/ImportedLinksProcessor.php b/module/Core/src/Importer/ImportedLinksProcessor.php index 7a9c3b92..16da0a09 100644 --- a/module/Core/src/Importer/ImportedLinksProcessor.php +++ b/module/Core/src/Importer/ImportedLinksProcessor.php @@ -139,7 +139,7 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface $importedVisits = 0; foreach ($iterable as $importedOrphanVisit) { // Skip visits which are older than the most recent already imported visit's date - if ($mostRecentOrphanVisit?->getDate()->greaterThanOrEquals(normalizeDate($importedOrphanVisit->date))) { + if ($mostRecentOrphanVisit?->date->greaterThanOrEquals(normalizeDate($importedOrphanVisit->date))) { continue; } 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..d2a4484a --- /dev/null +++ b/module/Core/src/Matomo/MatomoVisitSender.php @@ -0,0 +1,91 @@ +visitIterationRepository->findAllVisits($dateRange); + $successfulVisits = 0; + $failedVisits = 0; + + foreach ($visitsIterator as $index => $visit) { + try { + $this->sendVisit($visit); + $progressTracker?->success($index); + $successfulVisits++; + } catch (Throwable $e) { + $progressTracker?->error($index, $e); + $failedVisits++; + } + } + + return new SendVisitsResult($successfulVisits, $failedVisits); + } + + public function sendVisit(Visit $visit, ?string $originalIpAddress = null): void + { + $tracker = $this->trackerBuilder->buildMatomoTracker(); + + $tracker + ->setUrl($this->resolveUrlToTrack($visit)) + ->setCustomTrackingParameter('type', $visit->type->value) + ->setUserAgent($visit->userAgent) + ->setUrlReferrer($visit->referer) + ->setForceVisitDateTime($visit->date->setTimezone('UTC')->toDateTimeString()); + + $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..e1b1c3cb --- /dev/null +++ b/module/Core/src/Matomo/MatomoVisitSenderInterface.php @@ -0,0 +1,22 @@ + $successfulVisits + * @param int<0, max> $failedVisits + */ + public function __construct(public int $successfulVisits = 0, public int $failedVisits = 0) + { + } + + public function hasSuccesses(): bool + { + return $this->successfulVisits > 0; + } + + public function hasFailures(): bool + { + return $this->failedVisits > 0; + } + + public function count(): int + { + return $this->successfulVisits + $this->failedVisits; + } +} diff --git a/module/Core/src/Matomo/VisitSendingProgressTrackerInterface.php b/module/Core/src/Matomo/VisitSendingProgressTrackerInterface.php new file mode 100644 index 00000000..94686992 --- /dev/null +++ b/module/Core/src/Matomo/VisitSendingProgressTrackerInterface.php @@ -0,0 +1,14 @@ +setMaxResults(1); $visit = $this->visits->matching($criteria)->last(); - return $visit instanceof Visit ? $visit->getDate() : null; + return $visit instanceof Visit ? $visit->date : null; } /** diff --git a/module/Core/src/Util/RedirectStatus.php b/module/Core/src/Util/RedirectStatus.php index f561e212..defea11b 100644 --- a/module/Core/src/Util/RedirectStatus.php +++ b/module/Core/src/Util/RedirectStatus.php @@ -2,6 +2,9 @@ namespace Shlinkio\Shlink\Core\Util; +use Fig\Http\Message\RequestMethodInterface; +use Mezzio\Router\Route; + use function Shlinkio\Shlink\Core\ArrayUtils\contains; enum RedirectStatus: int @@ -16,8 +19,13 @@ enum RedirectStatus: int return contains($this, [self::STATUS_301, self::STATUS_308]); } - public function isLegacyStatus(): bool + /** + * @return array|Route::HTTP_METHOD_ANY + */ + public function allowedHttpMethods(): array|null { - return contains($this, [self::STATUS_301, self::STATUS_302]); + return contains($this, [self::STATUS_301, self::STATUS_302]) + ? [RequestMethodInterface::METHOD_GET] + : Route::HTTP_METHOD_ANY; } } diff --git a/module/Core/src/Visit/Entity/Visit.php b/module/Core/src/Visit/Entity/Visit.php index 86854945..be8400dc 100644 --- a/module/Core/src/Visit/Entity/Visit.php +++ b/module/Core/src/Visit/Entity/Visit.php @@ -29,8 +29,7 @@ class Visit extends AbstractEntity implements JsonSerializable public readonly ?string $remoteAddr = null, public readonly ?string $visitedUrl = null, private ?VisitLocation $visitLocation = null, - // TODO Make public readonly once VisitRepositoryTest does not try to set it - private Chronos $date = new Chronos(), + public readonly Chronos $date = new Chronos(), ) { } @@ -147,14 +146,6 @@ class Visit extends AbstractEntity implements JsonSerializable return $this->type; } - /** - * @internal - */ - public function getDate(): Chronos - { - return $this->date; - } - public function jsonSerialize(): array { $base = [ diff --git a/module/Core/src/Visit/Geolocation/VisitLocator.php b/module/Core/src/Visit/Geolocation/VisitLocator.php index 4b3b8e22..63cb6137 100644 --- a/module/Core/src/Visit/Geolocation/VisitLocator.php +++ b/module/Core/src/Visit/Geolocation/VisitLocator.php @@ -8,14 +8,14 @@ use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Entity\VisitLocation; -use Shlinkio\Shlink\Core\Visit\Repository\VisitLocationRepositoryInterface; +use Shlinkio\Shlink\Core\Visit\Repository\VisitIterationRepositoryInterface; use Shlinkio\Shlink\IpGeolocation\Model\Location; class VisitLocator implements VisitLocatorInterface { public function __construct( private readonly EntityManagerInterface $em, - private readonly VisitLocationRepositoryInterface $repo, + private readonly VisitIterationRepositoryInterface $repo, ) { } diff --git a/module/Core/src/Visit/Repository/VisitLocationRepository.php b/module/Core/src/Visit/Repository/VisitIterationRepository.php similarity index 72% rename from module/Core/src/Visit/Repository/VisitLocationRepository.php rename to module/Core/src/Visit/Repository/VisitIterationRepository.php index 6db1a4f8..cf342611 100644 --- a/module/Core/src/Visit/Repository/VisitLocationRepository.php +++ b/module/Core/src/Visit/Repository/VisitIterationRepository.php @@ -6,9 +6,14 @@ namespace Shlinkio\Shlink\Core\Visit\Repository; use Doctrine\ORM\QueryBuilder; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; +use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType; +use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -class VisitLocationRepository extends EntitySpecificationRepository implements VisitLocationRepositoryInterface +/** + * Allows iterating large amounts of visits in a memory-efficient way, to use in batch processes + */ +class VisitIterationRepository extends EntitySpecificationRepository implements VisitIterationRepositoryInterface { /** * @return iterable @@ -42,9 +47,18 @@ class VisitLocationRepository extends EntitySpecificationRepository implements V /** * @return iterable */ - public function findAllVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable + public function findAllVisits(?DateRange $dateRange = null, int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable { $qb = $this->createQueryBuilder('v'); + if ($dateRange?->startDate !== null) { + $qb->andWhere($qb->expr()->gte('v.date', ':since')) + ->setParameter('since', $dateRange->startDate, ChronosDateTimeType::CHRONOS_DATETIME); + } + if ($dateRange?->endDate !== null) { + $qb->andWhere($qb->expr()->lte('v.date', ':until')) + ->setParameter('until', $dateRange->endDate, ChronosDateTimeType::CHRONOS_DATETIME); + } + return $this->visitsIterableForQuery($qb, $blockSize); } diff --git a/module/Core/src/Visit/Repository/VisitLocationRepositoryInterface.php b/module/Core/src/Visit/Repository/VisitIterationRepositoryInterface.php similarity index 71% rename from module/Core/src/Visit/Repository/VisitLocationRepositoryInterface.php rename to module/Core/src/Visit/Repository/VisitIterationRepositoryInterface.php index 083d61f2..d4ffb864 100644 --- a/module/Core/src/Visit/Repository/VisitLocationRepositoryInterface.php +++ b/module/Core/src/Visit/Repository/VisitIterationRepositoryInterface.php @@ -4,9 +4,10 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Visit\Repository; +use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -interface VisitLocationRepositoryInterface +interface VisitIterationRepositoryInterface { public const DEFAULT_BLOCK_SIZE = 10000; @@ -23,5 +24,5 @@ interface VisitLocationRepositoryInterface /** * @return iterable */ - public function findAllVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable; + public function findAllVisits(?DateRange $dateRange = null, int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable; } diff --git a/module/Core/test-db/Visit/Repository/VisitLocationRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitIterationRepositoryTest.php similarity index 55% rename from module/Core/test-db/Visit/Repository/VisitLocationRepositoryTest.php rename to module/Core/test-db/Visit/Repository/VisitIterationRepositoryTest.php index c5aadf1f..6d3d4b39 100644 --- a/module/Core/test-db/Visit/Repository/VisitLocationRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitIterationRepositoryTest.php @@ -4,27 +4,29 @@ declare(strict_types=1); namespace ShlinkioDbTest\Shlink\Core\Visit\Repository; +use Cake\Chronos\Chronos; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; +use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Entity\VisitLocation; use Shlinkio\Shlink\Core\Visit\Model\Visitor; -use Shlinkio\Shlink\Core\Visit\Repository\VisitLocationRepository; +use Shlinkio\Shlink\Core\Visit\Repository\VisitIterationRepository; use Shlinkio\Shlink\IpGeolocation\Model\Location; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; use function array_map; use function range; -class VisitLocationRepositoryTest extends DatabaseTestCase +class VisitIterationRepositoryTest extends DatabaseTestCase { - private VisitLocationRepository $repo; + private VisitIterationRepository $repo; protected function setUp(): void { $em = $this->getEntityManager(); - $this->repo = new VisitLocationRepository($em, $em->getClassMetadata(Visit::class)); + $this->repo = new VisitIterationRepository($em, $em->getClassMetadata(Visit::class)); } #[Test, DataProvider('provideBlockSize')] @@ -33,7 +35,9 @@ class VisitLocationRepositoryTest extends DatabaseTestCase $shortUrl = ShortUrl::createFake(); $this->getEntityManager()->persist($shortUrl); + $unmodifiedDate = Chronos::now(); for ($i = 0; $i < 6; $i++) { + Chronos::setTestNow($unmodifiedDate->subDays($i)); // Enforce a different day for every visit $visit = Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()); if ($i >= 2) { @@ -44,15 +48,34 @@ class VisitLocationRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($visit); } + Chronos::setTestNow(); $this->getEntityManager()->flush(); $withEmptyLocation = $this->repo->findVisitsWithEmptyLocation($blockSize); $unlocated = $this->repo->findUnlocatedVisits($blockSize); - $all = $this->repo->findAllVisits($blockSize); + $all = $this->repo->findAllVisits(blockSize: $blockSize); + $lastThreeDays = $this->repo->findAllVisits( + dateRange: DateRange::since(Chronos::now()->subDays(2)->startOfDay()), + blockSize: $blockSize, + ); + $firstTwoDays = $this->repo->findAllVisits( + dateRange: DateRange::until(Chronos::now()->subDays(4)->endOfDay()), + blockSize: $blockSize, + ); + $daysInBetween = $this->repo->findAllVisits( + dateRange: DateRange::between( + startDate: Chronos::now()->subDays(5)->startOfDay(), + endDate: Chronos::now()->subDays(2)->endOfDay(), + ), + blockSize: $blockSize, + ); self::assertCount(2, [...$unlocated]); self::assertCount(4, [...$withEmptyLocation]); self::assertCount(6, [...$all]); + self::assertCount(3, [...$lastThreeDays]); + self::assertCount(2, [...$firstTwoDays]); + self::assertCount(4, [...$daysInBetween]); } public static function provideBlockSize(): iterable diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index 1506dd1a..9dc18390 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -6,7 +6,6 @@ namespace ShlinkioDbTest\Shlink\Core\Visit\Repository; use Cake\Chronos\Chronos; use PHPUnit\Framework\Attributes\Test; -use ReflectionObject; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; @@ -371,15 +370,15 @@ class VisitRepositoryTest extends DatabaseTestCase $botsCount = 3; for ($i = 0; $i < 6; $i++) { $this->getEntityManager()->persist($this->setDateOnVisit( - Visit::forBasePath($botsCount < 1 ? Visitor::emptyInstance() : Visitor::botInstance()), + fn () => Visit::forBasePath($botsCount < 1 ? Visitor::emptyInstance() : Visitor::botInstance()), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( - Visit::forInvalidShortUrl(Visitor::emptyInstance()), + fn () => Visit::forInvalidShortUrl(Visitor::emptyInstance()), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( - Visit::forRegularNotFound(Visitor::emptyInstance()), + fn () => Visit::forRegularNotFound(Visitor::emptyInstance()), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); @@ -429,15 +428,15 @@ class VisitRepositoryTest extends DatabaseTestCase for ($i = 0; $i < 6; $i++) { $this->getEntityManager()->persist($this->setDateOnVisit( - Visit::forBasePath(Visitor::emptyInstance()), + fn () => Visit::forBasePath(Visitor::emptyInstance()), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( - Visit::forInvalidShortUrl(Visitor::emptyInstance()), + fn () => Visit::forInvalidShortUrl(Visitor::emptyInstance()), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( - Visit::forRegularNotFound(Visitor::emptyInstance()), + fn () => Visit::forRegularNotFound(Visitor::emptyInstance()), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); } @@ -566,7 +565,7 @@ class VisitRepositoryTest extends DatabaseTestCase { for ($i = 0; $i < $amount; $i++) { $visit = $this->setDateOnVisit( - Visit::forValidShortUrl( + fn () => Visit::forValidShortUrl( $shortUrl, $botsAmount < 1 ? Visitor::emptyInstance() : Visitor::botInstance(), ), @@ -578,12 +577,14 @@ class VisitRepositoryTest extends DatabaseTestCase } } - private function setDateOnVisit(Visit $visit, Chronos $date): Visit + /** + * @param callable(): Visit $createVisit + */ + private function setDateOnVisit(callable $createVisit, Chronos $date): Visit { - $ref = new ReflectionObject($visit); - $dateProp = $ref->getProperty('date'); - $dateProp->setAccessible(true); - $dateProp->setValue($visit, $date); + Chronos::setTestNow($date); + $visit = $createVisit(); + Chronos::setTestNow(); return $visit; } diff --git a/module/Core/test/EventDispatcher/Matomo/SendVisitToMatomoTest.php b/module/Core/test/EventDispatcher/Matomo/SendVisitToMatomoTest.php index cf9f4005..10726273 100644 --- a/module/Core/test/EventDispatcher/Matomo/SendVisitToMatomoTest.php +++ b/module/Core/test/EventDispatcher/Matomo/SendVisitToMatomoTest.php @@ -6,7 +6,6 @@ namespace ShlinkioTest\Shlink\Core\EventDispatcher\Matomo; use Doctrine\ORM\EntityManagerInterface; use Exception; -use MatomoTracker; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; @@ -15,34 +14,28 @@ use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; use Shlinkio\Shlink\Core\EventDispatcher\Matomo\SendVisitToMatomo; use Shlinkio\Shlink\Core\Matomo\MatomoOptions; -use Shlinkio\Shlink\Core\Matomo\MatomoTrackerBuilderInterface; -use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; -use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; -use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; -use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; +use Shlinkio\Shlink\Core\Matomo\MatomoVisitSenderInterface; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -use Shlinkio\Shlink\Core\Visit\Entity\VisitLocation; use Shlinkio\Shlink\Core\Visit\Model\Visitor; -use Shlinkio\Shlink\IpGeolocation\Model\Location; class SendVisitToMatomoTest extends TestCase { private MockObject & EntityManagerInterface $em; private MockObject & LoggerInterface $logger; - private MockObject & MatomoTrackerBuilderInterface $trackerBuilder; + private MockObject & MatomoVisitSenderInterface $visitSender; protected function setUp(): void { $this->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('sendVisit'); $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('sendVisit'); $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('sendVisit')->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('sendVisit')->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/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php index 2dd1c0c7..bd3c82c8 100644 --- a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php +++ b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php @@ -76,7 +76,7 @@ class PublishingUpdatesGeneratorTest extends TestCase 'referer' => '', 'userAgent' => '', 'visitLocation' => null, - 'date' => $visit->getDate()->toAtomString(), + 'date' => $visit->date->toAtomString(), 'potentialBot' => false, 'visitedUrl' => '', ], @@ -100,7 +100,7 @@ class PublishingUpdatesGeneratorTest extends TestCase 'referer' => '', 'userAgent' => '', 'visitLocation' => null, - 'date' => $orphanVisit->getDate()->toAtomString(), + 'date' => $orphanVisit->date->toAtomString(), 'potentialBot' => false, 'visitedUrl' => $orphanVisit->visitedUrl, 'type' => $orphanVisit->type->value, diff --git a/module/Core/test/Matomo/MatomoVisitSenderTest.php b/module/Core/test/Matomo/MatomoVisitSenderTest.php new file mode 100644 index 00000000..816c8eea --- /dev/null +++ b/module/Core/test/Matomo/MatomoVisitSenderTest.php @@ -0,0 +1,173 @@ +trackerBuilder = $this->createMock(MatomoTrackerBuilderInterface::class); + $this->visitIterationRepository = $this->createMock(VisitIterationRepositoryInterface::class); + + $this->visitSender = new MatomoVisitSender( + $this->trackerBuilder, + new ShortUrlStringifier(['hostname' => 's2.test']), + $this->visitIterationRepository, + ); + } + + #[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() ?? ''); + $tracker->expects($this->once())->method('setForceVisitDateTime')->with( + $visit->date->setTimezone('UTC')->toDateTimeString(), + ); + + 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->sendVisit($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'); + $tracker->expects($this->once())->method('setForceVisitDateTime')->with( + $visit->date->setTimezone('UTC')->toDateTimeString(), + ); + + $this->trackerBuilder->expects($this->once())->method('buildMatomoTracker')->willReturn($tracker); + + $this->visitSender->sendVisit($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', + ]; + } + + #[Test] + public function multipleVisitsCanBeSent(): void + { + $dateRange = DateRange::allTime(); + $visitor = Visitor::emptyInstance(); + $bot = Visitor::botInstance(); + + $this->visitIterationRepository->expects($this->once())->method('findAllVisits')->with($dateRange)->willReturn([ + Visit::forBasePath($bot), + Visit::forValidShortUrl(ShortUrl::createFake(), $visitor), + Visit::forInvalidShortUrl($visitor), + ]); + + $tracker = $this->createMock(MatomoTracker::class); + $tracker->method('setUrl')->willReturn($tracker); + $tracker->method('setUserAgent')->willReturn($tracker); + $tracker->method('setUrlReferrer')->willReturn($tracker); + $tracker->method('setCustomTrackingParameter')->willReturn($tracker); + + $callCount = 0; + $this->trackerBuilder->expects($this->exactly(3))->method('buildMatomoTracker')->willReturnCallback( + function () use (&$callCount, $tracker) { + $callCount++; + + if ($callCount === 2) { + throw new Exception('Error'); + } + + return $tracker; + }, + ); + + $result = $this->visitSender->sendVisitsInDateRange($dateRange); + + self::assertEquals(2, $result->successfulVisits); + self::assertEquals(1, $result->failedVisits); + self::assertCount(3, $result); + self::assertTrue($result->hasSuccesses()); + self::assertTrue($result->hasFailures()); + } +} diff --git a/module/Core/test/Visit/Entity/VisitTest.php b/module/Core/test/Visit/Entity/VisitTest.php index d9c50af6..923b2e6b 100644 --- a/module/Core/test/Visit/Entity/VisitTest.php +++ b/module/Core/test/Visit/Entity/VisitTest.php @@ -26,7 +26,7 @@ class VisitTest extends TestCase self::assertEquals([ 'referer' => 'some site', - 'date' => $visit->getDate()->toAtomString(), + 'date' => $visit->date->toAtomString(), 'userAgent' => $userAgent, 'visitLocation' => null, 'potentialBot' => $expectedToBePotentialBot, @@ -58,7 +58,7 @@ class VisitTest extends TestCase $visit = Visit::forBasePath(Visitor::emptyInstance()), [ 'referer' => '', - 'date' => $visit->getDate()->toAtomString(), + 'date' => $visit->date->toAtomString(), 'userAgent' => '', 'visitLocation' => null, 'potentialBot' => false, @@ -74,7 +74,7 @@ class VisitTest extends TestCase )), [ 'referer' => 'bar', - 'date' => $visit->getDate()->toAtomString(), + 'date' => $visit->date->toAtomString(), 'userAgent' => 'foo', 'visitLocation' => null, 'potentialBot' => false, @@ -92,7 +92,7 @@ class VisitTest extends TestCase )->locate($location = VisitLocation::fromGeolocation(Location::emptyInstance())), [ 'referer' => 'referer', - 'date' => $visit->getDate()->toAtomString(), + 'date' => $visit->date->toAtomString(), 'userAgent' => 'user-agent', 'visitLocation' => $location, 'potentialBot' => false, diff --git a/module/Core/test/Visit/Geolocation/VisitLocatorTest.php b/module/Core/test/Visit/Geolocation/VisitLocatorTest.php index 1d3af228..f1d86f63 100644 --- a/module/Core/test/Visit/Geolocation/VisitLocatorTest.php +++ b/module/Core/test/Visit/Geolocation/VisitLocatorTest.php @@ -17,7 +17,7 @@ use Shlinkio\Shlink\Core\Visit\Entity\VisitLocation; use Shlinkio\Shlink\Core\Visit\Geolocation\VisitGeolocationHelperInterface; use Shlinkio\Shlink\Core\Visit\Geolocation\VisitLocator; use Shlinkio\Shlink\Core\Visit\Model\Visitor; -use Shlinkio\Shlink\Core\Visit\Repository\VisitLocationRepositoryInterface; +use Shlinkio\Shlink\Core\Visit\Repository\VisitIterationRepositoryInterface; use Shlinkio\Shlink\IpGeolocation\Model\Location; use function array_map; @@ -30,12 +30,12 @@ class VisitLocatorTest extends TestCase { private VisitLocator $visitService; private MockObject & EntityManager $em; - private MockObject & VisitLocationRepositoryInterface $repo; + private MockObject & VisitIterationRepositoryInterface $repo; protected function setUp(): void { $this->em = $this->createMock(EntityManager::class); - $this->repo = $this->createMock(VisitLocationRepositoryInterface::class); + $this->repo = $this->createMock(VisitIterationRepositoryInterface::class); $this->visitService = new VisitLocator($this->em, $this->repo); } diff --git a/module/Rest/test-api/Fixtures/VisitsFixture.php b/module/Rest/test-api/Fixtures/VisitsFixture.php index 6076f95e..9972e3a8 100644 --- a/module/Rest/test-api/Fixtures/VisitsFixture.php +++ b/module/Rest/test-api/Fixtures/VisitsFixture.php @@ -8,7 +8,6 @@ use Cake\Chronos\Chronos; use Doctrine\Common\DataFixtures\AbstractFixture; use Doctrine\Common\DataFixtures\DependentFixtureInterface; use Doctrine\Persistence\ObjectManager; -use ReflectionObject; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\Visitor; @@ -50,27 +49,31 @@ class VisitsFixture extends AbstractFixture implements DependentFixtureInterface ); $manager->persist($this->setVisitDate( - Visit::forBasePath(new Visitor('shlink-tests-agent', 'https://s.test', '1.2.3.4', '')), + fn () => Visit::forBasePath(new Visitor('shlink-tests-agent', 'https://s.test', '1.2.3.4', '')), '2020-01-01', )); $manager->persist($this->setVisitDate( - Visit::forRegularNotFound(new Visitor('shlink-tests-agent', 'https://s.test/foo/bar', '1.2.3.4', '')), + fn () => Visit::forRegularNotFound( + new Visitor('shlink-tests-agent', 'https://s.test/foo/bar', '1.2.3.4', ''), + ), '2020-02-01', )); $manager->persist($this->setVisitDate( - Visit::forInvalidShortUrl(new Visitor('cf-facebook', 'https://s.test/foo', '1.2.3.4', 'foo.com')), + fn () => Visit::forInvalidShortUrl(new Visitor('cf-facebook', 'https://s.test/foo', '1.2.3.4', 'foo.com')), '2020-03-01', )); $manager->flush(); } - private function setVisitDate(Visit $visit, string $date): Visit + /** + * @param callable(): Visit $createVisit + */ + private function setVisitDate(callable $createVisit, string $date): Visit { - $ref = new ReflectionObject($visit); - $dateProp = $ref->getProperty('date'); - $dateProp->setAccessible(true); - $dateProp->setValue($visit, Chronos::parse($date)); + Chronos::setTestNow($date); + $visit = $createVisit(); + Chronos::setTestNow(); return $visit; }