From 14a7e3bb05f3d5e05fd7c60ec560551f9121c3c3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 28 Oct 2025 10:55:06 +0100 Subject: [PATCH 1/8] Allow tags, orphan and non-orphan visits to be provided a domain filter param --- .../src/Command/Tag/GetTagVisitsCommand.php | 4 +- .../Visit/GetNonOrphanVisitsCommand.php | 4 +- .../AbstractInfinitePaginableListParams.php | 2 +- .../src/Visit/Model/OrphanVisitsParams.php | 9 ++-- .../Visit/Model/WithDomainVisitsParams.php | 31 ++++++++++++++ .../NonOrphanVisitsPaginatorAdapter.php | 14 ++++--- .../Adapter/OrphanVisitsPaginatorAdapter.php | 2 + .../Adapter/TagVisitsPaginatorAdapter.php | 14 ++++--- .../OrphanVisitsCountFiltering.php | 5 ++- .../Persistence/OrphanVisitsListFiltering.php | 3 +- .../WithDomainVisitsCountFiltering.php | 20 +++++++++ .../WithDomainVisitsListFiltering.php | 22 ++++++++++ .../src/Visit/Repository/VisitRepository.php | 10 +++-- .../Repository/VisitRepositoryInterface.php | 10 +++-- module/Core/src/Visit/VisitsStatsHelper.php | 5 ++- .../src/Visit/VisitsStatsHelperInterface.php | 5 ++- .../Visit/Repository/VisitRepositoryTest.php | 42 +++++++++++-------- .../NonOrphanVisitsPaginatorAdapterTest.php | 18 ++++---- .../VisitsForTagPaginatorAdapterTest.php | 12 +++--- .../Core/test/Visit/VisitsStatsHelperTest.php | 14 ++++--- .../Action/Visit/AbstractListVisitsAction.php | 10 +---- .../src/Action/Visit/DomainVisitsAction.php | 3 +- .../Action/Visit/NonOrphanVisitsAction.php | 10 ++--- .../src/Action/Visit/OrphanVisitsAction.php | 10 ++--- .../src/Action/Visit/ShortUrlVisitsAction.php | 3 +- .../Rest/src/Action/Visit/TagVisitsAction.php | 5 ++- 26 files changed, 189 insertions(+), 98 deletions(-) create mode 100644 module/Core/src/Visit/Model/WithDomainVisitsParams.php create mode 100644 module/Core/src/Visit/Persistence/WithDomainVisitsCountFiltering.php create mode 100644 module/Core/src/Visit/Persistence/WithDomainVisitsListFiltering.php diff --git a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php index b3c083bc..2bd900e7 100644 --- a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php +++ b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php @@ -9,7 +9,7 @@ use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -39,7 +39,7 @@ class GetTagVisitsCommand extends AbstractVisitsListCommand protected function getVisitsPaginator(InputInterface $input, DateRange $dateRange): Paginator { $tag = $input->getArgument('tag'); - return $this->visitsHelper->visitsForTag($tag, new VisitsParams($dateRange)); + return $this->visitsHelper->visitsForTag($tag, new WithDomainVisitsParams($dateRange)); } /** diff --git a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php index 445bd36f..692cc45f 100644 --- a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php @@ -8,7 +8,7 @@ use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Symfony\Component\Console\Input\InputInterface; @@ -35,7 +35,7 @@ class GetNonOrphanVisitsCommand extends AbstractVisitsListCommand */ protected function getVisitsPaginator(InputInterface $input, DateRange $dateRange): Paginator { - return $this->visitsHelper->nonOrphanVisits(new VisitsParams($dateRange)); + return $this->visitsHelper->nonOrphanVisits(new WithDomainVisitsParams($dateRange)); } /** diff --git a/module/Core/src/Model/AbstractInfinitePaginableListParams.php b/module/Core/src/Model/AbstractInfinitePaginableListParams.php index 70db853e..4d1f5f89 100644 --- a/module/Core/src/Model/AbstractInfinitePaginableListParams.php +++ b/module/Core/src/Model/AbstractInfinitePaginableListParams.php @@ -8,7 +8,7 @@ use Shlinkio\Shlink\Common\Paginator\Paginator; abstract class AbstractInfinitePaginableListParams { - private const FIRST_PAGE = 1; + private const int FIRST_PAGE = 1; public readonly int $page; public readonly int $itemsPerPage; diff --git a/module/Core/src/Visit/Model/OrphanVisitsParams.php b/module/Core/src/Visit/Model/OrphanVisitsParams.php index 6991928d..bff5b38c 100644 --- a/module/Core/src/Visit/Model/OrphanVisitsParams.php +++ b/module/Core/src/Visit/Model/OrphanVisitsParams.php @@ -9,20 +9,22 @@ use ValueError; use function Shlinkio\Shlink\Core\enumToString; use function sprintf; -final class OrphanVisitsParams extends VisitsParams +final class OrphanVisitsParams extends WithDomainVisitsParams { public function __construct( DateRange|null $dateRange = null, int|null $page = null, int|null $itemsPerPage = null, bool $excludeBots = false, + string|null $domain = null, public readonly OrphanVisitType|null $type = null, ) { - parent::__construct($dateRange, $page, $itemsPerPage, $excludeBots); + parent::__construct($dateRange, $page, $itemsPerPage, $excludeBots, $domain); } - public static function fromVisitsParamsAndRawData(VisitsParams $visitsParams, array $query): self + public static function fromRawData(array $query): self { + $visitsParams = WithDomainVisitsParams::fromRawData($query); $type = $query['type'] ?? null; return new self( @@ -30,6 +32,7 @@ final class OrphanVisitsParams extends VisitsParams page: $visitsParams->page, itemsPerPage: $visitsParams->itemsPerPage, excludeBots: $visitsParams->excludeBots, + domain: $visitsParams->domain, type: $type !== null ? self::parseType($type) : null, ); } diff --git a/module/Core/src/Visit/Model/WithDomainVisitsParams.php b/module/Core/src/Visit/Model/WithDomainVisitsParams.php new file mode 100644 index 00000000..edec3399 --- /dev/null +++ b/module/Core/src/Visit/Model/WithDomainVisitsParams.php @@ -0,0 +1,31 @@ +dateRange, + page: $visitsParams->page, + itemsPerPage: $visitsParams->itemsPerPage, + excludeBots: $visitsParams->excludeBots, + domain: $query['domain'] ?? null, + ); + } +} diff --git a/module/Core/src/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapter.php index 5e3cdbe1..e23cf10c 100644 --- a/module/Core/src/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapter.php @@ -6,9 +6,9 @@ namespace Shlinkio\Shlink\Core\Visit\Paginator\Adapter; use Shlinkio\Shlink\Core\Paginator\Adapter\AbstractCacheableCountPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -17,26 +17,28 @@ class NonOrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAda { public function __construct( private readonly VisitRepositoryInterface $repo, - private readonly VisitsParams $params, + private readonly WithDomainVisitsParams $params, private readonly ApiKey|null $apiKey, ) { } protected function doCount(): int { - return $this->repo->countNonOrphanVisits(new VisitsCountFiltering( + return $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering( $this->params->dateRange, $this->params->excludeBots, $this->apiKey, + $this->params->domain, )); } public function getSlice(int $offset, int $length): iterable { - return $this->repo->findNonOrphanVisits(new VisitsListFiltering( + return $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering( $this->params->dateRange, $this->params->excludeBots, $this->apiKey, + $this->params->domain, $length, $offset, )); diff --git a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php index 899ab831..fbb14c42 100644 --- a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php @@ -28,6 +28,7 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte dateRange: $this->params->dateRange, excludeBots: $this->params->excludeBots, apiKey: $this->apiKey, + domain: $this->params->domain, type: $this->params->type, )); } @@ -38,6 +39,7 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte dateRange: $this->params->dateRange, excludeBots: $this->params->excludeBots, apiKey: $this->apiKey, + domain: $this->params->domain, type: $this->params->type, limit: $length, offset: $offset, diff --git a/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php index 909bd2ba..a76b7524 100644 --- a/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php @@ -6,9 +6,9 @@ namespace Shlinkio\Shlink\Core\Visit\Paginator\Adapter; use Shlinkio\Shlink\Core\Paginator\Adapter\AbstractCacheableCountPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -18,7 +18,7 @@ class TagVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter public function __construct( private readonly VisitRepositoryInterface $visitRepository, private readonly string $tag, - private readonly VisitsParams $params, + private readonly WithDomainVisitsParams $params, private readonly ApiKey|null $apiKey, ) { } @@ -27,10 +27,11 @@ class TagVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { return $this->visitRepository->findVisitsByTag( $this->tag, - new VisitsListFiltering( + new WithDomainVisitsListFiltering( $this->params->dateRange, $this->params->excludeBots, $this->apiKey, + $this->params->domain, $length, $offset, ), @@ -41,10 +42,11 @@ class TagVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { return $this->visitRepository->countVisitsByTag( $this->tag, - new VisitsCountFiltering( + new WithDomainVisitsCountFiltering( $this->params->dateRange, $this->params->excludeBots, $this->apiKey, + $this->params->domain, ), ); } diff --git a/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php b/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php index c09bc5ca..981c26d4 100644 --- a/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php +++ b/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php @@ -8,14 +8,15 @@ use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitType; use Shlinkio\Shlink\Rest\Entity\ApiKey; -class OrphanVisitsCountFiltering extends VisitsCountFiltering +class OrphanVisitsCountFiltering extends WithDomainVisitsCountFiltering { public function __construct( DateRange|null $dateRange = null, bool $excludeBots = false, ApiKey|null $apiKey = null, + string|null $domain = null, public readonly OrphanVisitType|null $type = null, ) { - parent::__construct($dateRange, $excludeBots, $apiKey); + parent::__construct($dateRange, $excludeBots, $apiKey, $domain); } } diff --git a/module/Core/src/Visit/Persistence/OrphanVisitsListFiltering.php b/module/Core/src/Visit/Persistence/OrphanVisitsListFiltering.php index d1e49605..a60a0690 100644 --- a/module/Core/src/Visit/Persistence/OrphanVisitsListFiltering.php +++ b/module/Core/src/Visit/Persistence/OrphanVisitsListFiltering.php @@ -14,10 +14,11 @@ final class OrphanVisitsListFiltering extends OrphanVisitsCountFiltering DateRange|null $dateRange = null, bool $excludeBots = false, ApiKey|null $apiKey = null, + string|null $domain = null, OrphanVisitType|null $type = null, public readonly int|null $limit = null, public readonly int|null $offset = null, ) { - parent::__construct($dateRange, $excludeBots, $apiKey, $type); + parent::__construct($dateRange, $excludeBots, $apiKey, $domain, $type); } } diff --git a/module/Core/src/Visit/Persistence/WithDomainVisitsCountFiltering.php b/module/Core/src/Visit/Persistence/WithDomainVisitsCountFiltering.php new file mode 100644 index 00000000..5d5cc74a --- /dev/null +++ b/module/Core/src/Visit/Persistence/WithDomainVisitsCountFiltering.php @@ -0,0 +1,20 @@ +createVisitsByTagQueryBuilder($tag, $filtering); return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit, $filtering->offset); @@ -173,7 +174,7 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo /** * @return Visit[] */ - public function findNonOrphanVisits(VisitsListFiltering $filtering): array + public function findNonOrphanVisits(WithDomainVisitsListFiltering $filtering): array { $qb = $this->createAllVisitsQueryBuilder($filtering); $qb->andWhere($qb->expr()->isNotNull('v.shortUrl')); @@ -193,8 +194,9 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return (int) $this->matchSingleScalarResult(new CountOfNonOrphanVisits($filtering)); } - private function createAllVisitsQueryBuilder(VisitsListFiltering|OrphanVisitsListFiltering $filtering): QueryBuilder - { + private function createAllVisitsQueryBuilder( + VisitsListFiltering|OrphanVisitsListFiltering|WithDomainVisitsListFiltering $filtering, + ): QueryBuilder { // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later // Since they are not provided by the caller, it's reasonably safe $qb = $this->getEntityManager()->createQueryBuilder(); diff --git a/module/Core/src/Visit/Repository/VisitRepositoryInterface.php b/module/Core/src/Visit/Repository/VisitRepositoryInterface.php index b5590f8c..ca97d12e 100644 --- a/module/Core/src/Visit/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Visit/Repository/VisitRepositoryInterface.php @@ -12,6 +12,8 @@ use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; /** * @extends ObjectRepository @@ -28,9 +30,9 @@ interface VisitRepositoryInterface extends ObjectRepository, EntitySpecification /** * @return Visit[] */ - public function findVisitsByTag(string $tag, VisitsListFiltering $filtering): array; + public function findVisitsByTag(string $tag, WithDomainVisitsListFiltering $filtering): array; - public function countVisitsByTag(string $tag, VisitsCountFiltering $filtering): int; + public function countVisitsByTag(string $tag, WithDomainVisitsCountFiltering $filtering): int; /** * @return Visit[] @@ -49,9 +51,9 @@ interface VisitRepositoryInterface extends ObjectRepository, EntitySpecification /** * @return Visit[] */ - public function findNonOrphanVisits(VisitsListFiltering $filtering): array; + public function findNonOrphanVisits(WithDomainVisitsListFiltering $filtering): array; - public function countNonOrphanVisits(VisitsCountFiltering $filtering): int; + public function countNonOrphanVisits(WithDomainVisitsCountFiltering $filtering): int; public function findMostRecentOrphanVisit(): Visit|null; } diff --git a/module/Core/src/Visit/VisitsStatsHelper.php b/module/Core/src/Visit/VisitsStatsHelper.php index 412decc7..65e710c7 100644 --- a/module/Core/src/Visit/VisitsStatsHelper.php +++ b/module/Core/src/Visit/VisitsStatsHelper.php @@ -23,6 +23,7 @@ use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\Model\VisitsStats; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\DomainVisitsPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\NonOrphanVisitsPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\OrphanVisitsPaginatorAdapter; @@ -88,7 +89,7 @@ readonly class VisitsStatsHelper implements VisitsStatsHelperInterface /** * @inheritDoc */ - public function visitsForTag(string $tag, VisitsParams $params, ApiKey|null $apiKey = null): Paginator + public function visitsForTag(string $tag, WithDomainVisitsParams $params, ApiKey|null $apiKey = null): Paginator { /** @var TagRepository $tagRepo */ $tagRepo = $this->em->getRepository(Tag::class); @@ -130,7 +131,7 @@ readonly class VisitsStatsHelper implements VisitsStatsHelperInterface return $this->createPaginator(new OrphanVisitsPaginatorAdapter($repo, $params, $apiKey), $params); } - public function nonOrphanVisits(VisitsParams $params, ApiKey|null $apiKey = null): Paginator + public function nonOrphanVisits(WithDomainVisitsParams $params, ApiKey|null $apiKey = null): Paginator { /** @var VisitRepository $repo */ $repo = $this->em->getRepository(Visit::class); diff --git a/module/Core/src/Visit/VisitsStatsHelperInterface.php b/module/Core/src/Visit/VisitsStatsHelperInterface.php index 12e58933..418eae61 100644 --- a/module/Core/src/Visit/VisitsStatsHelperInterface.php +++ b/module/Core/src/Visit/VisitsStatsHelperInterface.php @@ -13,6 +13,7 @@ use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\Model\VisitsStats; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Rest\Entity\ApiKey; interface VisitsStatsHelperInterface @@ -33,7 +34,7 @@ interface VisitsStatsHelperInterface * @return Paginator * @throws TagNotFoundException */ - public function visitsForTag(string $tag, VisitsParams $params, ApiKey|null $apiKey = null): Paginator; + public function visitsForTag(string $tag, WithDomainVisitsParams $params, ApiKey|null $apiKey = null): Paginator; /** * @return Paginator @@ -49,5 +50,5 @@ interface VisitsStatsHelperInterface /** * @return Paginator */ - public function nonOrphanVisits(VisitsParams $params, ApiKey|null $apiKey = null): Paginator; + public function nonOrphanVisits(WithDomainVisitsParams $params, ApiKey|null $apiKey = null): Paginator; } diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index fe53629f..755b9bf8 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -22,6 +22,7 @@ use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\OrphanVisitsCountRepository; use Shlinkio\Shlink\Core\Visit\Repository\ShortUrlVisitsCountRepository; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepository; @@ -187,13 +188,13 @@ class VisitRepositoryTest extends DatabaseTestCase $this->createShortUrlsAndVisits(false, [$foo]); $this->getEntityManager()->flush(); - self::assertCount(0, $this->repo->findVisitsByTag('invalid', new VisitsListFiltering())); - self::assertCount(18, $this->repo->findVisitsByTag($foo, new VisitsListFiltering())); - self::assertCount(12, $this->repo->findVisitsByTag($foo, new VisitsListFiltering(null, true))); - self::assertCount(6, $this->repo->findVisitsByTag($foo, new VisitsListFiltering( + self::assertCount(0, $this->repo->findVisitsByTag('invalid', new WithDomainVisitsListFiltering())); + self::assertCount(18, $this->repo->findVisitsByTag($foo, new WithDomainVisitsListFiltering())); + self::assertCount(12, $this->repo->findVisitsByTag($foo, new WithDomainVisitsListFiltering(null, true))); + self::assertCount(6, $this->repo->findVisitsByTag($foo, new WithDomainVisitsListFiltering( DateRange::between(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ))); - self::assertCount(12, $this->repo->findVisitsByTag($foo, new VisitsListFiltering( + self::assertCount(12, $this->repo->findVisitsByTag($foo, new WithDomainVisitsListFiltering( DateRange::since(Chronos::parse('2016-01-03')), ))); } @@ -479,31 +480,38 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - self::assertCount(21, $this->repo->findNonOrphanVisits(new VisitsListFiltering())); - self::assertCount(21, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::allTime()))); - self::assertCount(4, $this->repo->findNonOrphanVisits(new VisitsListFiltering(apiKey: $authoredApiKey))); - self::assertCount(7, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::since( + self::assertCount(21, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering())); + self::assertCount(21, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering( + DateRange::allTime(), + ))); + self::assertCount(4, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering( + apiKey: $authoredApiKey, + ))); + self::assertCount(7, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(DateRange::since( Chronos::parse('2016-01-05')->endOfDay(), )))); - self::assertCount(12, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::until( + self::assertCount(12, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(DateRange::until( Chronos::parse('2016-01-04')->endOfDay(), )))); - self::assertCount(6, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::between( + self::assertCount(6, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(DateRange::between( Chronos::parse('2016-01-03')->startOfDay(), Chronos::parse('2016-01-04')->endOfDay(), )))); - self::assertCount(13, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::between( + self::assertCount(13, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(DateRange::between( Chronos::parse('2016-01-03')->startOfDay(), Chronos::parse('2016-01-08')->endOfDay(), )))); - self::assertCount(3, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::between( + self::assertCount(3, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(DateRange::between( Chronos::parse('2016-01-03')->startOfDay(), Chronos::parse('2016-01-08')->endOfDay(), ), limit: 10, offset: 10))); - self::assertCount(15, $this->repo->findNonOrphanVisits(new VisitsListFiltering(excludeBots: true))); - self::assertCount(10, $this->repo->findNonOrphanVisits(new VisitsListFiltering(limit: 10))); - self::assertCount(1, $this->repo->findNonOrphanVisits(new VisitsListFiltering(limit: 10, offset: 20))); - self::assertCount(5, $this->repo->findNonOrphanVisits(new VisitsListFiltering(limit: 5, offset: 5))); + self::assertCount(15, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(excludeBots: true))); + self::assertCount(10, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(limit: 10))); + self::assertCount(1, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering( + limit: 10, + offset: 20, + ))); + self::assertCount(5, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(limit: 5, offset: 5))); } #[Test] diff --git a/module/Core/test/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapterTest.php b/module/Core/test/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapterTest.php index 2dbaa25a..d021646d 100644 --- a/module/Core/test/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapterTest.php +++ b/module/Core/test/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapterTest.php @@ -10,10 +10,10 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\Visitor; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\NonOrphanVisitsPaginatorAdapter; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -21,13 +21,13 @@ class NonOrphanVisitsPaginatorAdapterTest extends TestCase { private NonOrphanVisitsPaginatorAdapter $adapter; private MockObject & VisitRepositoryInterface $repo; - private VisitsParams $params; + private WithDomainVisitsParams $params; private ApiKey $apiKey; protected function setUp(): void { $this->repo = $this->createMock(VisitRepositoryInterface::class); - $this->params = VisitsParams::fromRawData([]); + $this->params = WithDomainVisitsParams::fromRawData([]); $this->apiKey = ApiKey::create(); $this->adapter = new NonOrphanVisitsPaginatorAdapter($this->repo, $this->params, $this->apiKey); @@ -38,7 +38,7 @@ class NonOrphanVisitsPaginatorAdapterTest extends TestCase { $expectedCount = 5; $this->repo->expects($this->once())->method('countNonOrphanVisits')->with( - new VisitsCountFiltering($this->params->dateRange, $this->params->excludeBots, $this->apiKey), + new WithDomainVisitsCountFiltering($this->params->dateRange, $this->params->excludeBots, $this->apiKey), )->willReturn($expectedCount); $result = $this->adapter->getNbResults(); @@ -55,12 +55,12 @@ class NonOrphanVisitsPaginatorAdapterTest extends TestCase { $visitor = Visitor::empty(); $list = [Visit::forRegularNotFound($visitor), Visit::forInvalidShortUrl($visitor)]; - $this->repo->expects($this->once())->method('findNonOrphanVisits')->with(new VisitsListFiltering( + $this->repo->expects($this->once())->method('findNonOrphanVisits')->with(new WithDomainVisitsListFiltering( $this->params->dateRange, $this->params->excludeBots, $this->apiKey, - $limit, - $offset, + limit: $limit, + offset: $offset, ))->willReturn($list); $result = $this->adapter->getSlice($offset, $limit); diff --git a/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php b/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php index c0cd4d0b..cbecd958 100644 --- a/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php +++ b/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php @@ -8,10 +8,10 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Util\DateRange; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\TagVisitsPaginatorAdapter; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -33,7 +33,7 @@ class VisitsForTagPaginatorAdapterTest extends TestCase $adapter = $this->createAdapter(null); $this->repo->expects($this->exactly($count))->method('findVisitsByTag')->with( 'foo', - new VisitsListFiltering(DateRange::allTime(), false, null, $limit, $offset), + new WithDomainVisitsListFiltering(DateRange::allTime(), limit: $limit, offset: $offset), )->willReturn([]); for ($i = 0; $i < $count; $i++) { @@ -49,7 +49,7 @@ class VisitsForTagPaginatorAdapterTest extends TestCase $adapter = $this->createAdapter($apiKey); $this->repo->expects($this->once())->method('countVisitsByTag')->with( 'foo', - new VisitsCountFiltering(DateRange::allTime(), false, $apiKey), + new WithDomainVisitsCountFiltering(DateRange::allTime(), apiKey: $apiKey), )->willReturn(3); for ($i = 0; $i < $count; $i++) { @@ -59,6 +59,6 @@ class VisitsForTagPaginatorAdapterTest extends TestCase private function createAdapter(ApiKey|null $apiKey): TagVisitsPaginatorAdapter { - return new TagVisitsPaginatorAdapter($this->repo, 'foo', VisitsParams::fromRawData([]), $apiKey); + return new TagVisitsPaginatorAdapter($this->repo, 'foo', WithDomainVisitsParams::fromRawData([]), $apiKey); } } diff --git a/module/Core/test/Visit/VisitsStatsHelperTest.php b/module/Core/test/Visit/VisitsStatsHelperTest.php index 070ea7e6..8d75f523 100644 --- a/module/Core/test/Visit/VisitsStatsHelperTest.php +++ b/module/Core/test/Visit/VisitsStatsHelperTest.php @@ -29,10 +29,12 @@ use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\Model\Visitor; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\Model\VisitsStats; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\OrphanVisitsCountRepository; use Shlinkio\Shlink\Core\Visit\Repository\ShortUrlVisitsCountRepository; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepository; @@ -147,7 +149,7 @@ class VisitsStatsHelperTest extends TestCase $this->expectException(TagNotFoundException::class); - $this->helper->visitsForTag($tag, new VisitsParams(), $apiKey); + $this->helper->visitsForTag($tag, new WithDomainVisitsParams(), $apiKey); } #[Test, DataProviderExternal(ApiKeyDataProviders::class, 'adminApiKeysProvider')] @@ -170,7 +172,7 @@ class VisitsStatsHelperTest extends TestCase [Visit::class, $repo2], ]); - $paginator = $this->helper->visitsForTag($tag, new VisitsParams(), $apiKey); + $paginator = $this->helper->visitsForTag($tag, new WithDomainVisitsParams(), $apiKey); self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentPageResults())); } @@ -265,14 +267,14 @@ class VisitsStatsHelperTest extends TestCase ); $repo = $this->createMock(VisitRepository::class); $repo->expects($this->once())->method('countNonOrphanVisits')->with( - $this->isInstanceOf(VisitsCountFiltering::class), + $this->isInstanceOf(WithDOmainVisitsCountFiltering::class), )->willReturn(count($list)); $repo->expects($this->once())->method('findNonOrphanVisits')->with( - $this->isInstanceOf(VisitsListFiltering::class), + $this->isInstanceOf(WithDOmainVisitsListFiltering::class), )->willReturn($list); $this->em->expects($this->once())->method('getRepository')->with(Visit::class)->willReturn($repo); - $paginator = $this->helper->nonOrphanVisits(new VisitsParams()); + $paginator = $this->helper->nonOrphanVisits(new WithDomainVisitsParams()); self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentPageResults())); } diff --git a/module/Rest/src/Action/Visit/AbstractListVisitsAction.php b/module/Rest/src/Action/Visit/AbstractListVisitsAction.php index b63133fa..6e49c46f 100644 --- a/module/Rest/src/Action/Visit/AbstractListVisitsAction.php +++ b/module/Rest/src/Action/Visit/AbstractListVisitsAction.php @@ -10,7 +10,6 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtils; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -26,9 +25,8 @@ abstract class AbstractListVisitsAction extends AbstractRestAction public function handle(ServerRequestInterface $request): ResponseInterface { - $params = VisitsParams::fromRawData($request->getQueryParams()); $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); - $visits = $this->getVisitsPaginator($request, $params, $apiKey); + $visits = $this->getVisitsPaginator($request, $apiKey); return new JsonResponse(['visits' => PagerfantaUtils::serializePaginator($visits)]); } @@ -36,9 +34,5 @@ abstract class AbstractListVisitsAction extends AbstractRestAction /** * @return Pagerfanta */ - abstract protected function getVisitsPaginator( - ServerRequestInterface $request, - VisitsParams $params, - ApiKey $apiKey, - ): Pagerfanta; + abstract protected function getVisitsPaginator(ServerRequestInterface $request, ApiKey $apiKey): Pagerfanta; } diff --git a/module/Rest/src/Action/Visit/DomainVisitsAction.php b/module/Rest/src/Action/Visit/DomainVisitsAction.php index 0e027955..dd1ad292 100644 --- a/module/Rest/src/Action/Visit/DomainVisitsAction.php +++ b/module/Rest/src/Action/Visit/DomainVisitsAction.php @@ -23,8 +23,9 @@ class DomainVisitsAction extends AbstractListVisitsAction parent::__construct($visitsHelper); } - protected function getVisitsPaginator(Request $request, VisitsParams $params, ApiKey $apiKey): Pagerfanta + protected function getVisitsPaginator(Request $request, ApiKey $apiKey): Pagerfanta { + $params = VisitsParams::fromRawData($request->getQueryParams()); $domain = $this->resolveDomainParam($request); return $this->visitsHelper->visitsForDomain($domain, $params, $apiKey); } diff --git a/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php b/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php index b2f7471b..3bcc9929 100644 --- a/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php @@ -6,18 +6,16 @@ namespace Shlinkio\Shlink\Rest\Action\Visit; use Pagerfanta\Pagerfanta; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Rest\Entity\ApiKey; class NonOrphanVisitsAction extends AbstractListVisitsAction { protected const string ROUTE_PATH = '/visits/non-orphan'; - protected function getVisitsPaginator( - ServerRequestInterface $request, - VisitsParams $params, - ApiKey $apiKey, - ): Pagerfanta { + protected function getVisitsPaginator(ServerRequestInterface $request, ApiKey $apiKey): Pagerfanta + { + $params = WithDomainVisitsParams::fromRawData($request->getQueryParams()); return $this->visitsHelper->nonOrphanVisits($params, $apiKey); } } diff --git a/module/Rest/src/Action/Visit/OrphanVisitsAction.php b/module/Rest/src/Action/Visit/OrphanVisitsAction.php index b3c246ca..4e1f7745 100644 --- a/module/Rest/src/Action/Visit/OrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/OrphanVisitsAction.php @@ -7,19 +7,15 @@ namespace Shlinkio\Shlink\Rest\Action\Visit; use Pagerfanta\Pagerfanta; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Rest\Entity\ApiKey; class OrphanVisitsAction extends AbstractListVisitsAction { protected const string ROUTE_PATH = '/visits/orphan'; - protected function getVisitsPaginator( - ServerRequestInterface $request, - VisitsParams $params, - ApiKey $apiKey, - ): Pagerfanta { - $orphanParams = OrphanVisitsParams::fromVisitsParamsAndRawData($params, $request->getQueryParams()); + protected function getVisitsPaginator(ServerRequestInterface $request, ApiKey $apiKey): Pagerfanta + { + $orphanParams = OrphanVisitsParams::fromRawData($request->getQueryParams()); return $this->visitsHelper->orphanVisits($orphanParams, $apiKey); } } diff --git a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php index d8fc36e9..1d720d35 100644 --- a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php +++ b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php @@ -14,8 +14,9 @@ class ShortUrlVisitsAction extends AbstractListVisitsAction { protected const string ROUTE_PATH = '/short-urls/{shortCode}/visits'; - protected function getVisitsPaginator(Request $request, VisitsParams $params, ApiKey $apiKey): Pagerfanta + protected function getVisitsPaginator(Request $request, ApiKey $apiKey): Pagerfanta { + $params = VisitsParams::fromRawData($request->getQueryParams()); $identifier = ShortUrlIdentifier::fromApiRequest($request); return $this->visitsHelper->visitsForShortUrl($identifier, $params, $apiKey); } diff --git a/module/Rest/src/Action/Visit/TagVisitsAction.php b/module/Rest/src/Action/Visit/TagVisitsAction.php index 07ad7167..bf43edaa 100644 --- a/module/Rest/src/Action/Visit/TagVisitsAction.php +++ b/module/Rest/src/Action/Visit/TagVisitsAction.php @@ -6,15 +6,16 @@ namespace Shlinkio\Shlink\Rest\Action\Visit; use Pagerfanta\Pagerfanta; use Psr\Http\Message\ServerRequestInterface as Request; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Rest\Entity\ApiKey; class TagVisitsAction extends AbstractListVisitsAction { protected const string ROUTE_PATH = '/tags/{tag}/visits'; - protected function getVisitsPaginator(Request $request, VisitsParams $params, ApiKey $apiKey): Pagerfanta + protected function getVisitsPaginator(Request $request, ApiKey $apiKey): Pagerfanta { + $params = WithDomainVisitsParams::fromRawData($request->getQueryParams()); $tag = $request->getAttribute('tag', ''); return $this->visitsHelper->visitsForTag($tag, $params, $apiKey); } From 70e376d569e118559de318c2d1e24af3f383f78d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 29 Oct 2025 08:42:34 +0100 Subject: [PATCH 2/8] Allow domain to be passed to tag:visits, visit:orphan and visit:non-orphan commands --- .../src/Command/Tag/GetTagVisitsCommand.php | 15 +++++++++- .../Visit/GetNonOrphanVisitsCommand.php | 15 +++++++++- .../Command/Visit/GetOrphanVisitsCommand.php | 20 ++++++++++++- module/CLI/src/Input/DomainOption.php | 29 +++++++++++++++++++ 4 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 module/CLI/src/Input/DomainOption.php diff --git a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php index 2bd900e7..bac12ac2 100644 --- a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php +++ b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php @@ -5,8 +5,10 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Tag; use Shlinkio\Shlink\CLI\Command\Visit\AbstractVisitsListCommand; +use Shlinkio\Shlink\CLI\Input\DomainOption; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; @@ -14,15 +16,23 @@ use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; +use function sprintf; + class GetTagVisitsCommand extends AbstractVisitsListCommand { public const string NAME = 'tag:visits'; + private readonly DomainOption $domainOption; + public function __construct( VisitsStatsHelperInterface $visitsHelper, private readonly ShortUrlStringifierInterface $shortUrlStringifier, ) { parent::__construct($visitsHelper); + $this->domainOption = new DomainOption($this, sprintf( + 'Return visits that belong to this domain only. Use %s keyword for visits in default domain', + Domain::DEFAULT_AUTHORITY, + )); } protected function configure(): void @@ -39,7 +49,10 @@ class GetTagVisitsCommand extends AbstractVisitsListCommand protected function getVisitsPaginator(InputInterface $input, DateRange $dateRange): Paginator { $tag = $input->getArgument('tag'); - return $this->visitsHelper->visitsForTag($tag, new WithDomainVisitsParams($dateRange)); + return $this->visitsHelper->visitsForTag($tag, new WithDomainVisitsParams( + dateRange: $dateRange, + domain: $this->domainOption->get($input), + )); } /** diff --git a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php index 692cc45f..1b40d55e 100644 --- a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php @@ -4,23 +4,33 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; +use Shlinkio\Shlink\CLI\Input\DomainOption; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Symfony\Component\Console\Input\InputInterface; +use function sprintf; + class GetNonOrphanVisitsCommand extends AbstractVisitsListCommand { public const string NAME = 'visit:non-orphan'; + private readonly DomainOption $domainOption; + public function __construct( VisitsStatsHelperInterface $visitsHelper, private readonly ShortUrlStringifierInterface $shortUrlStringifier, ) { parent::__construct($visitsHelper); + $this->domainOption = new DomainOption($this, sprintf( + 'Return visits that belong to this domain only. Use %s keyword for visits in default domain', + Domain::DEFAULT_AUTHORITY, + )); } protected function configure(): void @@ -35,7 +45,10 @@ class GetNonOrphanVisitsCommand extends AbstractVisitsListCommand */ protected function getVisitsPaginator(InputInterface $input, DateRange $dateRange): Paginator { - return $this->visitsHelper->nonOrphanVisits(new WithDomainVisitsParams($dateRange)); + return $this->visitsHelper->nonOrphanVisits(new WithDomainVisitsParams( + dateRange: $dateRange, + domain: $this->domainOption->get($input), + )); } /** diff --git a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php index d282d310..0804215a 100644 --- a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php @@ -4,11 +4,14 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; +use Shlinkio\Shlink\CLI\Input\DomainOption; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitType; +use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -19,6 +22,17 @@ class GetOrphanVisitsCommand extends AbstractVisitsListCommand { public const string NAME = 'visit:orphan'; + private readonly DomainOption $domainOption; + + public function __construct(VisitsStatsHelperInterface $visitsHelper) + { + parent::__construct($visitsHelper); + $this->domainOption = new DomainOption($this, sprintf( + 'Return visits that belong to this domain only. Use %s keyword for visits in default domain', + Domain::DEFAULT_AUTHORITY, + )); + } + protected function configure(): void { $this @@ -37,7 +51,11 @@ class GetOrphanVisitsCommand extends AbstractVisitsListCommand { $rawType = $input->getOption('type'); $type = $rawType !== null ? OrphanVisitType::from($rawType) : null; - return $this->visitsHelper->orphanVisits(new OrphanVisitsParams(dateRange: $dateRange, type: $type)); + return $this->visitsHelper->orphanVisits(new OrphanVisitsParams( + dateRange: $dateRange, + domain: $this->domainOption->get($input), + type: $type, + )); } /** diff --git a/module/CLI/src/Input/DomainOption.php b/module/CLI/src/Input/DomainOption.php new file mode 100644 index 00000000..e7a15f52 --- /dev/null +++ b/module/CLI/src/Input/DomainOption.php @@ -0,0 +1,29 @@ +addOption( + name: self::NAME, + shortcut: 'd', + mode: InputOption::VALUE_REQUIRED, + description: $description, + ); + } + + public function get(InputInterface $input): string|null + { + return $input->getOption(self::NAME); + } +} From 9dcc51abde0f517d5080d3e7db169e325958744e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 29 Oct 2025 10:14:28 +0100 Subject: [PATCH 3/8] Allow filtering by domain in VisitRepository::findVisitsByTag --- .../Repository/ShortUrlListRepository.php | 12 +++--- .../src/Visit/Repository/VisitRepository.php | 26 +++++++---- .../Visit/Repository/VisitRepositoryTest.php | 43 +++++++++++++++---- 3 files changed, 57 insertions(+), 24 deletions(-) diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php index 266d19e1..8b720471 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php @@ -159,13 +159,11 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh $qb->andWhere($qb->expr()->notIn('s.id', $subQb->getDQL())); } - if ($filtering->domain !== null) { - if ($filtering->domain === Domain::DEFAULT_AUTHORITY) { - $qb->andWhere($qb->expr()->isNull('s.domain')); - } else { - $qb->andWhere($qb->expr()->eq('d.authority', ':domain')) - ->setParameter('domain', $filtering->domain); - } + if ($filtering->domain === Domain::DEFAULT_AUTHORITY) { + $qb->andWhere($qb->expr()->isNull('s.domain')); + } elseif ($filtering->domain !== null) { + $qb->andWhere($qb->expr()->eq('d.authority', ':domain')) + ->setParameter('domain', $filtering->domain); } if ($filtering->excludeMaxVisitsReached) { diff --git a/module/Core/src/Visit/Repository/VisitRepository.php b/module/Core/src/Visit/Repository/VisitRepository.php index 8702a849..ff2fba32 100644 --- a/module/Core/src/Visit/Repository/VisitRepository.php +++ b/module/Core/src/Visit/Repository/VisitRepository.php @@ -18,6 +18,7 @@ use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Spec\CountOfNonOrphanVisits; use Shlinkio\Shlink\Core\Visit\Spec\CountOfOrphanVisits; @@ -76,7 +77,7 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit, $filtering->offset); } - public function countVisitsByTag(string $tag, VisitsCountFiltering $filtering): int + public function countVisitsByTag(string $tag, WithDomainVisitsCountFiltering $filtering): int { $qb = $this->createVisitsByTagQueryBuilder($tag, $filtering); $qb->select('COUNT(v.id)'); @@ -84,19 +85,29 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return (int) $qb->getQuery()->getSingleScalarResult(); } - private function createVisitsByTagQueryBuilder(string $tag, VisitsCountFiltering $filtering): QueryBuilder + private function createVisitsByTagQueryBuilder(string $tag, WithDomainVisitsCountFiltering $filtering): QueryBuilder { + $conn = $this->getEntityManager()->getConnection(); + // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later. $qb = $this->getEntityManager()->createQueryBuilder(); $qb->from(Visit::class, 'v') ->join('v.shortUrl', 's') ->join('s.tags', 't') - ->where($qb->expr()->eq('t.name', $this->getEntityManager()->getConnection()->quote($tag))); + ->where($qb->expr()->eq('t.name', $conn->quote($tag))); if ($filtering->excludeBots) { $qb->andWhere($qb->expr()->eq('v.potentialBot', 'false')); } + $domain = $filtering->domain; + if ($domain === Domain::DEFAULT_AUTHORITY) { + $qb->andWhere($qb->expr()->isNull('s.domain')); + } elseif ($domain !== null) { + $qb->join('s.domain', 'd') + ->andWhere($qb->expr()->eq('d.authority', $conn->quote($domain))); + } + $this->applyDatesInline($qb, $filtering->dateRange); $this->applySpecification($qb, $filtering->apiKey?->inlinedSpec(), 'v'); @@ -194,11 +205,10 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return (int) $this->matchSingleScalarResult(new CountOfNonOrphanVisits($filtering)); } - private function createAllVisitsQueryBuilder( - VisitsListFiltering|OrphanVisitsListFiltering|WithDomainVisitsListFiltering $filtering, - ): QueryBuilder { - // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later - // Since they are not provided by the caller, it's reasonably safe + private function createAllVisitsQueryBuilder(VisitsCountFiltering $filtering): QueryBuilder + { + // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later. + // Since they are not provided by the caller, it's reasonably safe. $qb = $this->getEntityManager()->createQueryBuilder(); $qb->from(Visit::class, 'v'); diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index 755b9bf8..523db21d 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -22,6 +22,7 @@ use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\OrphanVisitsCountRepository; use Shlinkio\Shlink\Core\Visit\Repository\ShortUrlVisitsCountRepository; @@ -204,21 +205,40 @@ class VisitRepositoryTest extends DatabaseTestCase { $foo = 'foo'; - $this->createShortUrlsAndVisits(false, [$foo]); + $shortUrl1 = ShortUrl::create(ShortUrlCreation::fromRawData([ + ShortUrlInputFilter::LONG_URL => 'https://longUrl', + ShortUrlInputFilter::TAGS => [$foo], + ShortUrlInputFilter::DOMAIN => 'foo.com', + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl1); + $this->createVisitsForShortUrl($shortUrl1, 6); + + $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData([ + ShortUrlInputFilter::LONG_URL => 'https://longUrl', + ShortUrlInputFilter::TAGS => [$foo], + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl2); + $this->createVisitsForShortUrl($shortUrl2, 6); + $this->getEntityManager()->flush(); - $this->createShortUrlsAndVisits(false, [$foo]); - $this->getEntityManager()->flush(); - - self::assertEquals(0, $this->repo->countVisitsByTag('invalid', new VisitsCountFiltering())); - self::assertEquals(12, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering())); - self::assertEquals(8, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering(null, true))); - self::assertEquals(4, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering( + self::assertEquals(0, $this->repo->countVisitsByTag('invalid', new WithDomainVisitsCountFiltering())); + self::assertEquals(12, $this->repo->countVisitsByTag($foo, new WithDomainVisitsCountFiltering())); + self::assertEquals(8, $this->repo->countVisitsByTag($foo, new WithDomainVisitsCountFiltering( + excludeBots: true, + ))); + self::assertEquals(4, $this->repo->countVisitsByTag($foo, new WithDomainVisitsCountFiltering( DateRange::between(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ))); - self::assertEquals(8, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering( + self::assertEquals(8, $this->repo->countVisitsByTag($foo, new WithDomainVisitsCountFiltering( DateRange::since(Chronos::parse('2016-01-03')), ))); + self::assertEquals(6, $this->repo->countVisitsByTag($foo, new WithDomainVisitsCountFiltering( + domain: 'foo.com', + ))); + self::assertEquals(6, $this->repo->countVisitsByTag($foo, new WithDomainVisitsCountFiltering( + domain: Domain::DEFAULT_AUTHORITY, + ))); } #[Test] @@ -534,6 +554,7 @@ class VisitRepositoryTest extends DatabaseTestCase /** * @return array{string, string, ShortUrl} + * @fixme This method does too many things and is not intuitive. It should be removed or simplified */ private function createShortUrlsAndVisits( bool|string $withDomain = true, @@ -566,6 +587,10 @@ class VisitRepositoryTest extends DatabaseTestCase return [$shortCode, $domain, $shortUrl]; } + /** + * @param int $amount - How many visits in total. Defaults to 6 + * @param int $botsAmount - How many of the visits should be bots. Defaults to 2 + */ private function createVisitsForShortUrl(ShortUrl $shortUrl, int $amount = 6, int $botsAmount = 2): void { for ($i = 0; $i < $amount; $i++) { From 94426c7bf4ae0f0c2040562a9041da6eaa0a6260 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 30 Oct 2025 09:04:51 +0100 Subject: [PATCH 4/8] Allow filtering by domain in VisitRepository::findOrphanVisits --- composer.json | 6 +-- .../src/Visit/Repository/VisitRepository.php | 38 +++++++++++++------ .../src/Visit/Spec/CountOfOrphanVisits.php | 37 ------------------ .../Visit/Repository/VisitRepositoryTest.php | 8 +++- 4 files changed, 36 insertions(+), 53 deletions(-) delete mode 100644 module/Core/src/Visit/Spec/CountOfOrphanVisits.php diff --git a/composer.json b/composer.json index 5576c622..ddf090cb 100644 --- a/composer.json +++ b/composer.json @@ -20,9 +20,9 @@ "ext-pdo": "*", "akrabat/ip-address-middleware": "^2.6", "cakephp/chronos": "^3.1", - "doctrine/dbal": "^4.2", - "doctrine/migrations": "^3.8", - "doctrine/orm": "^3.3", + "doctrine/dbal": "^4.3", + "doctrine/migrations": "^3.9", + "doctrine/orm": "^3.5", "donatj/phpuseragentparser": "^1.10", "endroid/qr-code": "^6.0.5", "friendsofphp/proxy-manager-lts": "^1.0", diff --git a/module/Core/src/Visit/Repository/VisitRepository.php b/module/Core/src/Visit/Repository/VisitRepository.php index ff2fba32..e9123743 100644 --- a/module/Core/src/Visit/Repository/VisitRepository.php +++ b/module/Core/src/Visit/Repository/VisitRepository.php @@ -21,7 +21,6 @@ use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Spec\CountOfNonOrphanVisits; -use Shlinkio\Shlink\Core\Visit\Spec\CountOfOrphanVisits; use Shlinkio\Shlink\Rest\ApiKey\Role; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -161,15 +160,7 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return []; } - $qb = $this->createAllVisitsQueryBuilder($filtering); - $qb->andWhere($qb->expr()->isNull('v.shortUrl')); - - // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later - if ($filtering->type) { - $conn = $this->getEntityManager()->getConnection(); - $qb->andWhere($qb->expr()->eq('v.type', $conn->quote($filtering->type->value))); - } - + $qb = $this->createOrphanVisitsQueryBuilder($filtering); return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit, $filtering->offset); } @@ -179,7 +170,32 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return 0; } - return (int) $this->matchSingleScalarResult(new CountOfOrphanVisits($filtering)); + $qb = $this->createOrphanVisitsQueryBuilder($filtering); + $qb->select('COUNT(v.id)'); + + return (int) $qb->getQuery()->getSingleScalarResult(); + } + + private function createOrphanVisitsQueryBuilder(OrphanVisitsCountFiltering $filtering): QueryBuilder + { + $qb = $this->createAllVisitsQueryBuilder($filtering); + $qb->andWhere($qb->expr()->isNull('v.shortUrl')); + + // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later + $conn = $this->getEntityManager()->getConnection(); + + if ($filtering->type) { + $qb->andWhere($qb->expr()->eq('v.type', $conn->quote($filtering->type->value))); + } + + $domain = $filtering->domain; + if ($domain === Domain::DEFAULT_AUTHORITY) { + // TODO + } elseif ($domain !== null) { + $qb->andWhere($qb->expr()->like('v.visitedUrl', $conn->quote('%' . $domain . '%'))); + } + + return $qb; } /** diff --git a/module/Core/src/Visit/Spec/CountOfOrphanVisits.php b/module/Core/src/Visit/Spec/CountOfOrphanVisits.php deleted file mode 100644 index 9d9cab56..00000000 --- a/module/Core/src/Visit/Spec/CountOfOrphanVisits.php +++ /dev/null @@ -1,37 +0,0 @@ -filtering->dateRange), - ]; - - if ($this->filtering->excludeBots) { - $conditions[] = Spec::eq('potentialBot', false); - } - - if ($this->filtering->type) { - $conditions[] = Spec::eq('type', $this->filtering->type->value); - } - - return Spec::countOf(Spec::andX(...$conditions)); - } -} diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index 523db21d..6aa4f7b7 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -399,7 +399,7 @@ class VisitRepositoryTest extends DatabaseTestCase Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( - fn () => Visit::forRegularNotFound(Visitor::empty()), + fn () => Visit::forRegularNotFound(Visitor::fromParams(visitedUrl: 'https://example.com/foo?1=2')), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); @@ -438,6 +438,7 @@ class VisitRepositoryTest extends DatabaseTestCase type: OrphanVisitType::BASE_URL, limit: 4, ))); + self::assertCount(6, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(domain: 'example.com'))); } #[Test] @@ -457,7 +458,7 @@ class VisitRepositoryTest extends DatabaseTestCase Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( - fn () => Visit::forRegularNotFound(Visitor::empty()), + fn () => Visit::forRegularNotFound(Visitor::fromParams(visitedUrl: 'https://example.com/foo/bar')), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); } @@ -486,6 +487,9 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(6, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering( type: OrphanVisitType::REGULAR_404, ))); + self::assertEquals(6, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering( + domain: 'example.com', + ))); } #[Test] From a236f19dc406d9aefd06536ba2b0a6f74a4f0ead Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 30 Oct 2025 10:08:46 +0100 Subject: [PATCH 5/8] Allow filtering by domain in VisitRepository::findNonOrphanVisits --- .../src/Visit/Repository/VisitRepository.php | 33 ++++++++++++---- .../src/Visit/Spec/CountOfNonOrphanVisits.php | 39 ------------------- .../Visit/Repository/VisitRepositoryTest.php | 26 +++++++++---- 3 files changed, 43 insertions(+), 55 deletions(-) delete mode 100644 module/Core/src/Visit/Spec/CountOfNonOrphanVisits.php diff --git a/module/Core/src/Visit/Repository/VisitRepository.php b/module/Core/src/Visit/Repository/VisitRepository.php index e9123743..8b87954e 100644 --- a/module/Core/src/Visit/Repository/VisitRepository.php +++ b/module/Core/src/Visit/Repository/VisitRepository.php @@ -20,7 +20,6 @@ use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; -use Shlinkio\Shlink\Core\Visit\Spec\CountOfNonOrphanVisits; use Shlinkio\Shlink\Rest\ApiKey\Role; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -203,22 +202,40 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo */ public function findNonOrphanVisits(WithDomainVisitsListFiltering $filtering): array { + $qb = $this->createNonOrphanVisitsQueryBuilder($filtering); + return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit, $filtering->offset); + } + + public function countNonOrphanVisits(WithDomainVisitsCountFiltering $filtering): int + { + $qb = $this->createNonOrphanVisitsQueryBuilder($filtering); + $qb->select('COUNT(v.id)'); + + return (int) $qb->getQuery()->getSingleScalarResult(); + } + + private function createNonOrphanVisitsQueryBuilder(WithDomainVisitsCountFiltering $filtering): QueryBuilder + { + $conn = $this->getEntityManager()->getConnection(); $qb = $this->createAllVisitsQueryBuilder($filtering); $qb->andWhere($qb->expr()->isNotNull('v.shortUrl')); $apiKey = $filtering->apiKey; - if (ApiKey::isShortUrlRestricted($apiKey)) { + $domain = $filtering->domain; + if (ApiKey::isShortUrlRestricted($apiKey) || $domain !== null) { $qb->join('v.shortUrl', 's'); } + if ($domain === Domain::DEFAULT_AUTHORITY) { + $qb->andWhere($qb->expr()->isNull('s.domain')); + } elseif ($domain !== null) { + $qb->join('s.domain', 'd') + ->andWhere($qb->expr()->eq('d.authority', $conn->quote($domain))); + } + $this->applySpecification($qb, $apiKey?->inlinedSpec(), 'v'); - return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit, $filtering->offset); - } - - public function countNonOrphanVisits(VisitsCountFiltering $filtering): int - { - return (int) $this->matchSingleScalarResult(new CountOfNonOrphanVisits($filtering)); + return $qb; } private function createAllVisitsQueryBuilder(VisitsCountFiltering $filtering): QueryBuilder diff --git a/module/Core/src/Visit/Spec/CountOfNonOrphanVisits.php b/module/Core/src/Visit/Spec/CountOfNonOrphanVisits.php deleted file mode 100644 index d81cd21b..00000000 --- a/module/Core/src/Visit/Spec/CountOfNonOrphanVisits.php +++ /dev/null @@ -1,39 +0,0 @@ -filtering->dateRange), - ]; - - if ($this->filtering->excludeBots) { - $conditions[] = Spec::eq('potentialBot', false); - } - - $apiKey = $this->filtering->apiKey; - if ($apiKey !== null) { - $conditions[] = new WithApiKeySpecsEnsuringJoin($apiKey, 'shortUrl'); - } - - return Spec::countOf(Spec::andX(...$conditions)); - } -} diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index 6aa4f7b7..1b360005 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -339,13 +339,17 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - self::assertEquals(4 + 5 + 7, $this->repo->countNonOrphanVisits(new VisitsCountFiltering())); + self::assertEquals(4 + 5 + 7, $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering())); self::assertEquals(4 + 5 + 7, $this->countRepo->countNonOrphanVisits(new VisitsCountFiltering())); - self::assertEquals(4, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey1))); + self::assertEquals(4, $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering(apiKey: $apiKey1))); self::assertEquals(4, $this->countRepo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey1))); - self::assertEquals(5 + 7, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey2))); + self::assertEquals(5 + 7, $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering( + apiKey: $apiKey2, + ))); self::assertEquals(5 + 7, $this->countRepo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey2))); - self::assertEquals(4 + 7, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $domainApiKey))); + self::assertEquals(4 + 7, $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering( + apiKey: $domainApiKey, + ))); self::assertEquals(4 + 7, $this->countRepo->countNonOrphanVisits(new VisitsCountFiltering( apiKey: $domainApiKey, ))); @@ -355,21 +359,27 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(0, $this->orphanCountRepo->countOrphanVisits(new OrphanVisitsCountFiltering( apiKey: $noOrphanVisitsApiKey, ))); - self::assertEquals(4, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::since( + self::assertEquals(4, $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering(DateRange::since( Chronos::parse('2016-01-05')->startOfDay(), )))); - self::assertEquals(2, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::since( + self::assertEquals(2, $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering(DateRange::since( Chronos::parse('2016-01-03')->startOfDay(), ), false, $apiKey1))); - self::assertEquals(1, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::since( + self::assertEquals(1, $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering(DateRange::since( Chronos::parse('2016-01-07')->startOfDay(), ), false, $apiKey2))); self::assertEquals(3 + 5, $this->repo->countNonOrphanVisits( - new VisitsCountFiltering(excludeBots: true, apiKey: $apiKey2), + new WithDomainVisitsCountFiltering(excludeBots: true, apiKey: $apiKey2), )); self::assertEquals(3 + 5, $this->countRepo->countNonOrphanVisits( new VisitsCountFiltering(excludeBots: true, apiKey: $apiKey2), )); + self::assertEquals(4 + 7, $this->repo->countNonOrphanVisits( + new WithDomainVisitsCountFiltering(domain: $domain->authority), + )); + self::assertEquals(5, $this->repo->countNonOrphanVisits( + new WithDomainVisitsCountFiltering(domain: Domain::DEFAULT_AUTHORITY), + )); self::assertEquals(4, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering())); self::assertEquals(4, $this->orphanCountRepo->countOrphanVisits(new OrphanVisitsCountFiltering())); self::assertEquals(3, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(excludeBots: true))); From b5f8e8a4cd264fbb045f6f07a5fc7f9a7549c69c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 30 Oct 2025 10:23:00 +0100 Subject: [PATCH 6/8] Document domain param for visits endpoints --- docs/swagger/paths/v2_tags_{tag}_visits.json | 4 ++++ docs/swagger/paths/v2_visits_non-orphan.json | 4 ++++ docs/swagger/paths/v2_visits_orphan.json | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/docs/swagger/paths/v2_tags_{tag}_visits.json b/docs/swagger/paths/v2_tags_{tag}_visits.json index 1f3dabf2..70cfc6e2 100644 --- a/docs/swagger/paths/v2_tags_{tag}_visits.json +++ b/docs/swagger/paths/v2_tags_{tag}_visits.json @@ -64,6 +64,10 @@ "type": "string", "enum": ["true"] } + }, + { + "$ref": "../parameters/domain.json", + "description": "Return visits for short URLs that belong to this domain. Use **DEFAULT** keyword to return visits from default domain." } ], "security": [ diff --git a/docs/swagger/paths/v2_visits_non-orphan.json b/docs/swagger/paths/v2_visits_non-orphan.json index 65b11252..3579030a 100644 --- a/docs/swagger/paths/v2_visits_non-orphan.json +++ b/docs/swagger/paths/v2_visits_non-orphan.json @@ -55,6 +55,10 @@ "type": "string", "enum": ["true"] } + }, + { + "$ref": "../parameters/domain.json", + "description": "Return visits for short URLs that belong to this domain. Use **DEFAULT** keyword to return visits from default domain." } ], "security": [ diff --git a/docs/swagger/paths/v2_visits_orphan.json b/docs/swagger/paths/v2_visits_orphan.json index df2ee0cd..1653e36a 100644 --- a/docs/swagger/paths/v2_visits_orphan.json +++ b/docs/swagger/paths/v2_visits_orphan.json @@ -65,6 +65,10 @@ "type": "string", "enum": ["invalid_short_url", "base_url", "regular_404"] } + }, + { + "$ref": "../parameters/domain.json", + "description": "Return only visits for this domain. Use **DEFAULT** keyword to return visits from default domain." } ], "security": [ From 37088b1a4b016e00d35c6473313c7d6f58f18791 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 31 Oct 2025 08:53:31 +0100 Subject: [PATCH 7/8] Allow filtering orphan visits by domain via DEFAULT keyword --- module/Core/config/dependencies.config.php | 2 +- .../Adapter/OrphanVisitsPaginatorAdapter.php | 4 ++++ .../Visit/Persistence/OrphanVisitsCountFiltering.php | 1 + .../Visit/Persistence/OrphanVisitsListFiltering.php | 3 ++- module/Core/src/Visit/Repository/VisitRepository.php | 5 ++--- module/Core/src/Visit/VisitsStatsHelper.php | 8 ++++++-- .../test-db/Visit/Repository/VisitRepositoryTest.php | 12 +++++++++--- .../Adapter/OrphanVisitsPaginatorAdapterTest.php | 8 +++++++- module/Core/test/Visit/VisitsStatsHelperTest.php | 3 ++- 9 files changed, 34 insertions(+), 12 deletions(-) diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 0ad943e9..5bb534c2 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -168,7 +168,7 @@ return [ ], Visit\Geolocation\VisitLocator::class => ['em', Visit\Repository\VisitIterationRepository::class], Visit\Geolocation\VisitToLocationHelper::class => [IpLocationResolverInterface::class], - Visit\VisitsStatsHelper::class => ['em'], + Visit\VisitsStatsHelper::class => ['em', Config\Options\UrlShortenerOptions::class], Tag\TagService::class => ['em', Tag\Repository\TagRepository::class], ShortUrl\DeleteShortUrlService::class => [ 'em', diff --git a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php index fbb14c42..ad175a79 100644 --- a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Visit\Paginator\Adapter; +use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Paginator\Adapter\AbstractCacheableCountPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; @@ -19,6 +20,7 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte private readonly VisitRepositoryInterface $repo, private readonly OrphanVisitsParams $params, private readonly ApiKey|null $apiKey, + private readonly UrlShortenerOptions $options, ) { } @@ -30,6 +32,7 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte apiKey: $this->apiKey, domain: $this->params->domain, type: $this->params->type, + defaultDomain: $this->options->defaultDomain, )); } @@ -41,6 +44,7 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte apiKey: $this->apiKey, domain: $this->params->domain, type: $this->params->type, + defaultDomain: $this->options->defaultDomain, limit: $length, offset: $offset, )); diff --git a/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php b/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php index 981c26d4..84f7af57 100644 --- a/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php +++ b/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php @@ -16,6 +16,7 @@ class OrphanVisitsCountFiltering extends WithDomainVisitsCountFiltering ApiKey|null $apiKey = null, string|null $domain = null, public readonly OrphanVisitType|null $type = null, + public readonly string $defaultDomain = '', ) { parent::__construct($dateRange, $excludeBots, $apiKey, $domain); } diff --git a/module/Core/src/Visit/Persistence/OrphanVisitsListFiltering.php b/module/Core/src/Visit/Persistence/OrphanVisitsListFiltering.php index a60a0690..23c3d6cb 100644 --- a/module/Core/src/Visit/Persistence/OrphanVisitsListFiltering.php +++ b/module/Core/src/Visit/Persistence/OrphanVisitsListFiltering.php @@ -16,9 +16,10 @@ final class OrphanVisitsListFiltering extends OrphanVisitsCountFiltering ApiKey|null $apiKey = null, string|null $domain = null, OrphanVisitType|null $type = null, + string $defaultDomain = '', public readonly int|null $limit = null, public readonly int|null $offset = null, ) { - parent::__construct($dateRange, $excludeBots, $apiKey, $domain, $type); + parent::__construct($dateRange, $excludeBots, $apiKey, $domain, $type, $defaultDomain); } } diff --git a/module/Core/src/Visit/Repository/VisitRepository.php b/module/Core/src/Visit/Repository/VisitRepository.php index 8b87954e..78ff74b3 100644 --- a/module/Core/src/Visit/Repository/VisitRepository.php +++ b/module/Core/src/Visit/Repository/VisitRepository.php @@ -188,9 +188,8 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo } $domain = $filtering->domain; - if ($domain === Domain::DEFAULT_AUTHORITY) { - // TODO - } elseif ($domain !== null) { + $domain = $domain === Domain::DEFAULT_AUTHORITY ? $filtering->defaultDomain : $domain; + if ($domain !== null) { $qb->andWhere($qb->expr()->like('v.visitedUrl', $conn->quote('%' . $domain . '%'))); } diff --git a/module/Core/src/Visit/VisitsStatsHelper.php b/module/Core/src/Visit/VisitsStatsHelper.php index 65e710c7..b9721f3c 100644 --- a/module/Core/src/Visit/VisitsStatsHelper.php +++ b/module/Core/src/Visit/VisitsStatsHelper.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Visit; use Doctrine\ORM\EntityManagerInterface; use Pagerfanta\Adapter\AdapterInterface; use Shlinkio\Shlink\Common\Paginator\Paginator; +use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Domain\Repository\DomainRepository; use Shlinkio\Shlink\Core\Exception\DomainNotFoundException; @@ -38,7 +39,7 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; readonly class VisitsStatsHelper implements VisitsStatsHelperInterface { - public function __construct(private EntityManagerInterface $em) + public function __construct(private EntityManagerInterface $em, private UrlShortenerOptions $options) { } @@ -128,7 +129,10 @@ readonly class VisitsStatsHelper implements VisitsStatsHelperInterface /** @var VisitRepository $repo */ $repo = $this->em->getRepository(Visit::class); - return $this->createPaginator(new OrphanVisitsPaginatorAdapter($repo, $params, $apiKey), $params); + return $this->createPaginator( + new OrphanVisitsPaginatorAdapter($repo, $params, $apiKey, $this->options), + $params, + ); } public function nonOrphanVisits(WithDomainVisitsParams $params, ApiKey|null $apiKey = null): Paginator diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index 1b360005..56b6175f 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -405,7 +405,7 @@ class VisitRepositoryTest extends DatabaseTestCase Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( - fn () => Visit::forInvalidShortUrl(Visitor::empty()), + fn () => Visit::forInvalidShortUrl(Visitor::fromParams(visitedUrl: 'https://s.test/bar')), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( @@ -449,6 +449,10 @@ class VisitRepositoryTest extends DatabaseTestCase limit: 4, ))); self::assertCount(6, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(domain: 'example.com'))); + self::assertCount(6, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering( + domain: Domain::DEFAULT_AUTHORITY, + defaultDomain: 's.test', + ))); } #[Test] @@ -464,7 +468,7 @@ class VisitRepositoryTest extends DatabaseTestCase Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( - fn () => Visit::forInvalidShortUrl(Visitor::empty()), + fn () => Visit::forInvalidShortUrl(Visitor::fromParams(visitedUrl: 'https://s.test/foo/bar')), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( @@ -497,8 +501,10 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(6, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering( type: OrphanVisitType::REGULAR_404, ))); + self::assertEquals(6, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(domain: 'example.com'))); self::assertEquals(6, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering( - domain: 'example.com', + domain: Domain::DEFAULT_AUTHORITY, + defaultDomain: 's.test', ))); } diff --git a/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php b/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php index b62fa0c6..b3e800a7 100644 --- a/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php +++ b/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\Model\Visitor; @@ -30,7 +31,12 @@ class OrphanVisitsPaginatorAdapterTest extends TestCase $this->params = new OrphanVisitsParams(); $this->apiKey = ApiKey::create(); - $this->adapter = new OrphanVisitsPaginatorAdapter($this->repo, $this->params, $this->apiKey); + $this->adapter = new OrphanVisitsPaginatorAdapter( + $this->repo, + $this->params, + $this->apiKey, + new UrlShortenerOptions(), + ); } #[Test] diff --git a/module/Core/test/Visit/VisitsStatsHelperTest.php b/module/Core/test/Visit/VisitsStatsHelperTest.php index 8d75f523..68aa4310 100644 --- a/module/Core/test/Visit/VisitsStatsHelperTest.php +++ b/module/Core/test/Visit/VisitsStatsHelperTest.php @@ -12,6 +12,7 @@ use PHPUnit\Framework\Attributes\DataProviderExternal; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Domain\Repository\DomainRepository; use Shlinkio\Shlink\Core\Exception\DomainNotFoundException; @@ -54,7 +55,7 @@ class VisitsStatsHelperTest extends TestCase protected function setUp(): void { $this->em = $this->createMock(EntityManagerInterface::class); - $this->helper = new VisitsStatsHelper($this->em); + $this->helper = new VisitsStatsHelper($this->em, new UrlShortenerOptions()); } #[Test, DataProvider('provideCounts')] From e21cea1971d6a3c614b5a076a4c7d81bdb795196 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 1 Nov 2025 09:56:15 +0100 Subject: [PATCH 8/8] Add API tests for visits domain filtering --- .../test-api/Action/NonOrphanVisitsTest.php | 2 ++ .../Rest/test-api/Action/OrphanVisitsTest.php | 14 +++++++--- module/Rest/test-api/Action/TagVisitsTest.php | 26 ++++++++++--------- .../Rest/test-api/Fixtures/VisitsFixture.php | 26 ++++++++++++++----- 4 files changed, 46 insertions(+), 22 deletions(-) diff --git a/module/Rest/test-api/Action/NonOrphanVisitsTest.php b/module/Rest/test-api/Action/NonOrphanVisitsTest.php index 0e69db54..db38c534 100644 --- a/module/Rest/test-api/Action/NonOrphanVisitsTest.php +++ b/module/Rest/test-api/Action/NonOrphanVisitsTest.php @@ -31,5 +31,7 @@ class NonOrphanVisitsTest extends ApiTestCase yield 'bots excluded' => [['excludeBots' => 'true'], 6, 6]; yield 'bots excluded and pagination' => [['excludeBots' => 'true', 'page' => 1, 'itemsPerPage' => 4], 6, 4]; yield 'date filter' => [['startDate' => Chronos::now()->addDays(1)->toAtomString()], 0, 0]; + yield 'domain filter' => [['domain' => 'example.com'], 0, 0]; + yield 'default domain filter' => [['domain' => 'DEFAULT'], 7, 7]; } } diff --git a/module/Rest/test-api/Action/OrphanVisitsTest.php b/module/Rest/test-api/Action/OrphanVisitsTest.php index f9414899..30519c4c 100644 --- a/module/Rest/test-api/Action/OrphanVisitsTest.php +++ b/module/Rest/test-api/Action/OrphanVisitsTest.php @@ -19,7 +19,7 @@ class OrphanVisitsTest extends ApiTestCase 'userAgent' => 'cf-facebook', 'visitLocation' => null, 'potentialBot' => true, - 'visitedUrl' => 'foo.com', + 'visitedUrl' => 'https://example.com/short', 'type' => 'invalid_short_url', 'redirectUrl' => null, ]; @@ -29,7 +29,7 @@ class OrphanVisitsTest extends ApiTestCase 'userAgent' => 'shlink-tests-agent', 'visitLocation' => null, 'potentialBot' => false, - 'visitedUrl' => '', + 'visitedUrl' => 'https://s.test/bar', 'type' => 'regular_404', 'redirectUrl' => null, ]; @@ -39,7 +39,7 @@ class OrphanVisitsTest extends ApiTestCase 'userAgent' => 'shlink-tests-agent', 'visitLocation' => null, 'potentialBot' => false, - 'visitedUrl' => '', + 'visitedUrl' => 'https://s.test/foo', 'type' => 'base_url', 'redirectUrl' => null, ]; @@ -80,6 +80,14 @@ class OrphanVisitsTest extends ApiTestCase 1, [self::INVALID_SHORT_URL], ]; + yield 'example domain only' => [['domain' => 'example.com'], 1, 1, [self::INVALID_SHORT_URL]]; + yield 'default domain only' => [['domain' => 's.test'], 2, 2, [self::REGULAR_NOT_FOUND, self::BASE_URL]]; + yield 'default domain only with DEFAULT keyword' => [ + ['domain' => 'DEFAULT'], + 2, + 2, + [self::REGULAR_NOT_FOUND, self::BASE_URL], + ]; } #[Test] diff --git a/module/Rest/test-api/Action/TagVisitsTest.php b/module/Rest/test-api/Action/TagVisitsTest.php index c51f02fb..7a0c5f45 100644 --- a/module/Rest/test-api/Action/TagVisitsTest.php +++ b/module/Rest/test-api/Action/TagVisitsTest.php @@ -17,11 +17,11 @@ class TagVisitsTest extends ApiTestCase public function expectedVisitsAreReturned( string $apiKey, string $tag, - bool $excludeBots, + array $query, int $expectedVisitsAmount, ): void { $resp = $this->callApiWithKey(self::METHOD_GET, sprintf('/tags/%s/visits', $tag), [ - RequestOptions::QUERY => $excludeBots ? ['excludeBots' => true] : [], + RequestOptions::QUERY => $query, ], $apiKey); $payload = $this->getJsonResponsePayload($resp); @@ -33,16 +33,18 @@ class TagVisitsTest extends ApiTestCase public static function provideTags(): iterable { - yield 'foo with admin API key' => ['valid_api_key', 'foo', false, 5]; - yield 'foo with admin API key and no bots' => ['valid_api_key', 'foo', true, 4]; - yield 'bar with admin API key' => ['valid_api_key', 'bar', false, 2]; - yield 'bar with admin API key and no bots' => ['valid_api_key', 'bar', true, 1]; - yield 'baz with admin API key' => ['valid_api_key', 'baz', false, 0]; - yield 'foo with author API key' => ['author_api_key', 'foo', false, 5]; - yield 'foo with author API key and no bots' => ['author_api_key', 'foo', true, 4]; - yield 'bar with author API key' => ['author_api_key', 'bar', false, 2]; - yield 'bar with author API key and no bots' => ['author_api_key', 'bar', true, 1]; - yield 'foo with domain API key' => ['domain_api_key', 'foo', false, 0]; + yield 'foo with admin API key' => ['valid_api_key', 'foo', [], 5]; + yield 'foo with admin API key and no bots' => ['valid_api_key', 'foo', ['excludeBots' => true], 4]; + yield 'bar with admin API key' => ['valid_api_key', 'bar', [], 2]; + yield 'bar with admin API key and no bots' => ['valid_api_key', 'bar', ['excludeBots' => true], 1]; + yield 'baz with admin API key' => ['valid_api_key', 'baz', [], 0]; + yield 'foo with author API key' => ['author_api_key', 'foo', [], 5]; + yield 'foo with author API key and no bots' => ['author_api_key', 'foo', ['excludeBots' => true], 4]; + yield 'bar with author API key' => ['author_api_key', 'bar', [], 2]; + yield 'bar with author API key and no bots' => ['author_api_key', 'bar', ['excludeBots' => true], 1]; + yield 'foo with domain API key' => ['domain_api_key', 'foo', [], 0]; + yield 'foo with specific domain' => ['valid_api_key', 'foo', ['domain' => 'example.com'], 0]; + yield 'foo with default domain' => ['valid_api_key', 'foo', ['domain' => 'DEFAULT'], 5]; } #[Test, DataProvider('provideApiKeysAndTags')] diff --git a/module/Rest/test-api/Fixtures/VisitsFixture.php b/module/Rest/test-api/Fixtures/VisitsFixture.php index e10b6dab..76954ebf 100644 --- a/module/Rest/test-api/Fixtures/VisitsFixture.php +++ b/module/Rest/test-api/Fixtures/VisitsFixture.php @@ -58,20 +58,32 @@ class VisitsFixture extends AbstractFixture implements DependentFixtureInterface Visitor::fromParams('shlink-tests-agent', 'https://app.shlink.io', ''), )); + // Orphan visits (s.test is the default domain in tests env) $manager->persist($this->setVisitDate( - fn () => Visit::forBasePath(Visitor::fromParams('shlink-tests-agent', 'https://s.test', '1.2.3.4')), + fn () => Visit::forBasePath(Visitor::fromParams( + 'shlink-tests-agent', + 'https://s.test', + '1.2.3.4', + visitedUrl: 'https://s.test/foo', + )), '2020-01-01', )); $manager->persist($this->setVisitDate( - fn () => Visit::forRegularNotFound( - Visitor::fromParams('shlink-tests-agent', 'https://s.test/foo/bar', '1.2.3.4'), - ), + fn () => Visit::forRegularNotFound(Visitor::fromParams( + 'shlink-tests-agent', + 'https://s.test/foo/bar', + '1.2.3.4', + visitedUrl: 'https://s.test/bar', + )), '2020-02-01', )); $manager->persist($this->setVisitDate( - fn () => Visit::forInvalidShortUrl( - Visitor::fromParams('cf-facebook', 'https://s.test/foo', '1.2.3.4', 'foo.com'), - ), + fn () => Visit::forInvalidShortUrl(Visitor::fromParams( + 'cf-facebook', + 'https://s.test/foo', + '1.2.3.4', + visitedUrl: 'https://example.com/short', + )), '2020-03-01', ));