diff --git a/CHANGELOG.md b/CHANGELOG.md index 3093f52b..c5798e4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added * [#1868](https://github.com/shlinkio/shlink/issues/1868) Add support for [docker compose secrets](https://docs.docker.com/compose/use-secrets/) to the docker image. +* [#1979](https://github.com/shlinkio/shlink/issues/1979) Allow orphan visits lists to be filtered by type. + + This is supported both by the `GET /visits/orphan` API endpoint via `type=...` query param, and by the `visit:orphan` CLI command, via `--type` flag. ### Changed * [#1935](https://github.com/shlinkio/shlink/issues/1935) Replace dependency on abandoned `php-middleware/request-id` with userland simple middleware. diff --git a/docker-compose.yml b/docker-compose.yml index f33693ad..71194fbe 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -199,7 +199,7 @@ services: shlink_swagger_ui: container_name: shlink_swagger_ui - image: swaggerapi/swagger-ui:v5.10.3 + image: swaggerapi/swagger-ui:v5.11.3 ports: - "8005:8080" volumes: diff --git a/docs/swagger/paths/v2_visits_orphan.json b/docs/swagger/paths/v2_visits_orphan.json index fe799934..df2ee0cd 100644 --- a/docs/swagger/paths/v2_visits_orphan.json +++ b/docs/swagger/paths/v2_visits_orphan.json @@ -55,6 +55,16 @@ "type": "string", "enum": ["true"] } + }, + { + "name": "type", + "in": "query", + "description": "The type of visits to return. All visits are returned when not provided.", + "required": false, + "schema": { + "type": "string", + "enum": ["invalid_short_url", "base_url", "regular_404"] + } } ], "security": [ @@ -137,6 +147,54 @@ } } }, + "400": { + "description": "Provided query arguments are invalid.", + "content": { + "application/problem+json": { + "schema": { + "type": "object", + "allOf": [ + { + "$ref": "../definitions/Error.json" + }, + { + "type": "object", + "required": ["invalidElements"], + "properties": { + "invalidElements": { + "type": "array", + "items": { + "type": "string", + "enum": ["type"] + } + } + } + } + ] + }, + "examples": { + "API v3 and newer": { + "value": { + "title": "Invalid data", + "type": "https://shlink.io/api/error/invalid-data", + "detail": "Provided data is not valid", + "status": 400, + "invalidElements": ["type"] + } + }, + "Previous to API v3": { + "value": { + "title": "Invalid data", + "type": "INVALID_ARGUMENT", + "detail": "Provided data is not valid", + "status": 400, + "invalidElements": ["type"] + } + } + } + } + } + }, "default": { "description": "Unexpected error.", "content": { diff --git a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php index 618a35cd..7beae19a 100644 --- a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php @@ -7,8 +7,13 @@ namespace Shlinkio\Shlink\CLI\Command\Visit; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitType; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; + +use function Shlinkio\Shlink\Core\enumToString; +use function sprintf; class GetOrphanVisitsCommand extends AbstractVisitsListCommand { @@ -18,12 +23,18 @@ class GetOrphanVisitsCommand extends AbstractVisitsListCommand { $this ->setName(self::NAME) - ->setDescription('Returns the list of orphan visits.'); + ->setDescription('Returns the list of orphan visits.') + ->addOption('type', 't', InputOption::VALUE_REQUIRED, sprintf( + 'Return visits only with this type. One of %s', + enumToString(OrphanVisitType::class), + )); } protected function getVisitsPaginator(InputInterface $input, DateRange $dateRange): Paginator { - return $this->visitsHelper->orphanVisits(new VisitsParams($dateRange)); + $rawType = $input->getOption('type'); + $type = $rawType !== null ? OrphanVisitType::from($rawType) : null; + return $this->visitsHelper->orphanVisits(new OrphanVisitsParams(dateRange: $dateRange, type: $type)); } /** diff --git a/module/CLI/test/Command/Visit/GetOrphanVisitsCommandTest.php b/module/CLI/test/Command/Visit/GetOrphanVisitsCommandTest.php index b90e6af6..a9e2a50c 100644 --- a/module/CLI/test/Command/Visit/GetOrphanVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/GetOrphanVisitsCommandTest.php @@ -6,12 +6,15 @@ namespace ShlinkioTest\Shlink\CLI\Command\Visit; use Pagerfanta\Adapter\ArrayAdapter; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\Visit\GetOrphanVisitsCommand; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Entity\VisitLocation; +use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitType; use Shlinkio\Shlink\Core\Visit\Model\Visitor; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\IpGeolocation\Model\Location; @@ -30,16 +33,20 @@ class GetOrphanVisitsCommandTest extends TestCase } #[Test] - public function outputIsProperlyGenerated(): void + #[TestWith([[], false])] + #[TestWith([['--type' => OrphanVisitType::BASE_URL->value], true])] + public function outputIsProperlyGenerated(array $args, bool $includesType): void { $visit = Visit::forBasePath(new Visitor('bar', 'foo', '', ''))->locate( VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), ); - $this->visitsHelper->expects($this->once())->method('orphanVisits')->withAnyParameters()->willReturn( - new Paginator(new ArrayAdapter([$visit])), - ); + $this->visitsHelper->expects($this->once())->method('orphanVisits')->with($this->callback( + fn (OrphanVisitsParams $param) => ( + ($includesType && $param->type !== null) || (!$includesType && $param->type === null) + ), + ))->willReturn(new Paginator(new ArrayAdapter([$visit]))); - $this->commandTester->execute([]); + $this->commandTester->execute($args); $output = $this->commandTester->getDisplay(); self::assertEquals( diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index d07bc9e2..f26cb84f 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -20,6 +20,7 @@ use function array_keys; use function array_map; use function array_reduce; use function date_default_timezone_get; +use function implode; use function is_array; use function print_r; use function Shlinkio\Shlink\Common\buildDateRange; @@ -182,3 +183,11 @@ function enumValues(string $enum): array $cache[$enum] = array_map(static fn (BackedEnum $type) => (string) $type->value, $enum::cases()) ); } + +/** + * @param class-string $enum + */ +function enumToString(string $enum): string +{ + return sprintf('["%s"]', implode('", "', enumValues($enum))); +} diff --git a/module/Core/src/Visit/Model/OrphanVisitType.php b/module/Core/src/Visit/Model/OrphanVisitType.php new file mode 100644 index 00000000..4150a959 --- /dev/null +++ b/module/Core/src/Visit/Model/OrphanVisitType.php @@ -0,0 +1,12 @@ +dateRange, + page: $visitsParams->page, + itemsPerPage: $visitsParams->itemsPerPage, + excludeBots: $visitsParams->excludeBots, + type: $type !== null ? self::parseType($type) : null, + ); + } + + private static function parseType(string $type): OrphanVisitType + { + try { + return OrphanVisitType::from($type); + } catch (ValueError) { + throw ValidationException::fromArray([ + 'type' => sprintf( + '%s is not a valid orphan visit type. Expected one of %s', + $type, + enumToString(OrphanVisitType::class), + ), + ]); + } + } +} diff --git a/module/Core/src/Visit/Model/VisitType.php b/module/Core/src/Visit/Model/VisitType.php index 5352c2f1..bac726c3 100644 --- a/module/Core/src/Visit/Model/VisitType.php +++ b/module/Core/src/Visit/Model/VisitType.php @@ -8,7 +8,7 @@ enum VisitType: string { case VALID_SHORT_URL = 'valid_short_url'; case IMPORTED = 'imported'; - case INVALID_SHORT_URL = 'invalid_short_url'; - case BASE_URL = 'base_url'; - case REGULAR_404 = 'regular_404'; + case INVALID_SHORT_URL = OrphanVisitType::INVALID_SHORT_URL->value; + case BASE_URL = OrphanVisitType::BASE_URL->value; + case REGULAR_404 = OrphanVisitType::REGULAR_404->value; } diff --git a/module/Core/src/Visit/Model/VisitsParams.php b/module/Core/src/Visit/Model/VisitsParams.php index 90ca4770..10713131 100644 --- a/module/Core/src/Visit/Model/VisitsParams.php +++ b/module/Core/src/Visit/Model/VisitsParams.php @@ -9,7 +9,7 @@ use Shlinkio\Shlink\Core\Model\AbstractInfinitePaginableListParams; use function Shlinkio\Shlink\Core\parseDateRangeFromQuery; -final class VisitsParams extends AbstractInfinitePaginableListParams +class VisitsParams extends AbstractInfinitePaginableListParams { public readonly DateRange $dateRange; diff --git a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php index e871d125..863460a1 100644 --- a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php @@ -5,9 +5,9 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Visit\Paginator\Adapter; use Shlinkio\Shlink\Core\Paginator\Adapter\AbstractCacheableCountPaginatorAdapter; -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\OrphanVisitsParams; +use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -15,26 +15,28 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte { public function __construct( private readonly VisitRepositoryInterface $repo, - private readonly VisitsParams $params, + private readonly OrphanVisitsParams $params, private readonly ?ApiKey $apiKey, ) { } protected function doCount(): int { - return $this->repo->countOrphanVisits(new VisitsCountFiltering( + return $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering( dateRange: $this->params->dateRange, excludeBots: $this->params->excludeBots, apiKey: $this->apiKey, + type: $this->params->type, )); } public function getSlice(int $offset, int $length): iterable { - return $this->repo->findOrphanVisits(new VisitsListFiltering( + return $this->repo->findOrphanVisits(new OrphanVisitsListFiltering( dateRange: $this->params->dateRange, excludeBots: $this->params->excludeBots, apiKey: $this->apiKey, + type: $this->params->type, limit: $length, offset: $offset, )); diff --git a/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php b/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php new file mode 100644 index 00000000..88676df8 --- /dev/null +++ b/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php @@ -0,0 +1,21 @@ +apiKey?->hasRole(Role::NO_ORPHAN_VISITS)) { return []; @@ -146,10 +148,17 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo $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))); + } + return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit, $filtering->offset); } - public function countOrphanVisits(VisitsCountFiltering $filtering): int + public function countOrphanVisits(OrphanVisitsCountFiltering $filtering): int { if ($filtering->apiKey?->hasRole(Role::NO_ORPHAN_VISITS)) { return 0; @@ -176,7 +185,7 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return (int) $this->matchSingleScalarResult(new CountOfNonOrphanVisits($filtering)); } - private function createAllVisitsQueryBuilder(VisitsListFiltering $filtering): QueryBuilder + private function createAllVisitsQueryBuilder(VisitsListFiltering|OrphanVisitsListFiltering $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 diff --git a/module/Core/src/Visit/Repository/VisitRepositoryInterface.php b/module/Core/src/Visit/Repository/VisitRepositoryInterface.php index 4e53db2b..9904181b 100644 --- a/module/Core/src/Visit/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Visit/Repository/VisitRepositoryInterface.php @@ -8,6 +8,8 @@ use Doctrine\Persistence\ObjectRepository; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepositoryInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Visit\Entity\Visit; +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; @@ -37,9 +39,9 @@ interface VisitRepositoryInterface extends ObjectRepository, EntitySpecification /** * @return Visit[] */ - public function findOrphanVisits(VisitsListFiltering $filtering): array; + public function findOrphanVisits(OrphanVisitsListFiltering $filtering): array; - public function countOrphanVisits(VisitsCountFiltering $filtering): int; + public function countOrphanVisits(OrphanVisitsCountFiltering $filtering): int; /** * @return Visit[] diff --git a/module/Core/src/Visit/Spec/CountOfOrphanVisits.php b/module/Core/src/Visit/Spec/CountOfOrphanVisits.php index 106350c6..9d9cab56 100644 --- a/module/Core/src/Visit/Spec/CountOfOrphanVisits.php +++ b/module/Core/src/Visit/Spec/CountOfOrphanVisits.php @@ -8,11 +8,11 @@ use Happyr\DoctrineSpecification\Spec; use Happyr\DoctrineSpecification\Specification\BaseSpecification; use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Core\Spec\InDateRange; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering; class CountOfOrphanVisits extends BaseSpecification { - public function __construct(private VisitsCountFiltering $filtering) + public function __construct(private readonly OrphanVisitsCountFiltering $filtering) { parent::__construct(); } @@ -28,6 +28,10 @@ class CountOfOrphanVisits extends BaseSpecification $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/src/Visit/VisitsStatsHelper.php b/module/Core/src/Visit/VisitsStatsHelper.php index bdd2fd3b..480435c1 100644 --- a/module/Core/src/Visit/VisitsStatsHelper.php +++ b/module/Core/src/Visit/VisitsStatsHelper.php @@ -18,6 +18,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\Tag\Entity\Tag; use Shlinkio\Shlink\Core\Tag\Repository\TagRepository; 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\Paginator\Adapter\DomainVisitsPaginatorAdapter; @@ -25,6 +26,7 @@ use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\NonOrphanVisitsPaginatorAdapter use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\OrphanVisitsPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\ShortUrlVisitsPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\TagVisitsPaginatorAdapter; +use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepository; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; @@ -42,13 +44,13 @@ class VisitsStatsHelper implements VisitsStatsHelperInterface $visitsRepo = $this->em->getRepository(Visit::class); return new VisitsStats( - nonOrphanVisitsTotal: $visitsRepo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($apiKey)), - orphanVisitsTotal: $visitsRepo->countOrphanVisits(VisitsCountFiltering::withApiKey($apiKey)), + nonOrphanVisitsTotal: $visitsRepo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey)), + orphanVisitsTotal: $visitsRepo->countOrphanVisits(new OrphanVisitsCountFiltering(apiKey: $apiKey)), nonOrphanVisitsNonBots: $visitsRepo->countNonOrphanVisits( new VisitsCountFiltering(excludeBots: true, apiKey: $apiKey), ), orphanVisitsNonBots: $visitsRepo->countOrphanVisits( - new VisitsCountFiltering(excludeBots: true, apiKey: $apiKey), + new OrphanVisitsCountFiltering(excludeBots: true, apiKey: $apiKey), ), ); } @@ -116,7 +118,7 @@ class VisitsStatsHelper implements VisitsStatsHelperInterface /** * @return Visit[]|Paginator */ - public function orphanVisits(VisitsParams $params, ?ApiKey $apiKey = null): Paginator + public function orphanVisits(OrphanVisitsParams $params, ?ApiKey $apiKey = null): Paginator { /** @var VisitRepositoryInterface $repo */ $repo = $this->em->getRepository(Visit::class); diff --git a/module/Core/src/Visit/VisitsStatsHelperInterface.php b/module/Core/src/Visit/VisitsStatsHelperInterface.php index 71173553..265174ed 100644 --- a/module/Core/src/Visit/VisitsStatsHelperInterface.php +++ b/module/Core/src/Visit/VisitsStatsHelperInterface.php @@ -10,6 +10,7 @@ use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; 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\Rest\Entity\ApiKey; @@ -43,7 +44,7 @@ interface VisitsStatsHelperInterface /** * @return Visit[]|Paginator */ - public function orphanVisits(VisitsParams $params, ?ApiKey $apiKey = null): Paginator; + public function orphanVisits(OrphanVisitsParams $params, ?ApiKey $apiKey = null): Paginator; /** * @return Visit[]|Paginator diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index cca71a14..90496e1e 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -15,7 +15,10 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; use Shlinkio\Shlink\Core\Visit\Entity\Visit; +use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitType; use Shlinkio\Shlink\Core\Visit\Model\Visitor; +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\Repository\VisitRepository; @@ -305,10 +308,12 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); self::assertEquals(4 + 5 + 7, $this->repo->countNonOrphanVisits(new VisitsCountFiltering())); - self::assertEquals(4, $this->repo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($apiKey1))); - self::assertEquals(5 + 7, $this->repo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($apiKey2))); - self::assertEquals(4 + 7, $this->repo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($domainApiKey))); - self::assertEquals(0, $this->repo->countOrphanVisits(VisitsCountFiltering::withApiKey($noOrphanVisitsApiKey))); + self::assertEquals(4, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey1))); + self::assertEquals(5 + 7, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey2))); + self::assertEquals(4 + 7, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $domainApiKey))); + self::assertEquals(0, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering( + apiKey: $noOrphanVisitsApiKey, + ))); self::assertEquals(4, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::since( Chronos::parse('2016-01-05')->startOfDay(), )))); @@ -319,8 +324,8 @@ class VisitRepositoryTest extends DatabaseTestCase Chronos::parse('2016-01-07')->startOfDay(), ), false, $apiKey2))); self::assertEquals(3 + 5, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(null, true, $apiKey2))); - self::assertEquals(4, $this->repo->countOrphanVisits(new VisitsCountFiltering())); - self::assertEquals(3, $this->repo->countOrphanVisits(new VisitsCountFiltering(null, true))); + self::assertEquals(4, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering())); + self::assertEquals(3, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(excludeBots: true))); } #[Test] @@ -353,27 +358,36 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - self::assertCount(0, $this->repo->findOrphanVisits(new VisitsListFiltering(apiKey: $noOrphanVisitsApiKey))); - self::assertCount(18, $this->repo->findOrphanVisits(new VisitsListFiltering())); - self::assertCount(15, $this->repo->findOrphanVisits(new VisitsListFiltering(null, true))); - self::assertCount(5, $this->repo->findOrphanVisits(new VisitsListFiltering(null, false, null, 5))); - self::assertCount(10, $this->repo->findOrphanVisits(new VisitsListFiltering(null, false, null, 15, 8))); - self::assertCount(9, $this->repo->findOrphanVisits(new VisitsListFiltering( - DateRange::since(Chronos::parse('2020-01-04')), - false, - null, - 15, + self::assertCount(0, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering( + apiKey: $noOrphanVisitsApiKey, ))); - self::assertCount(2, $this->repo->findOrphanVisits(new VisitsListFiltering( - DateRange::between(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')), - false, - null, - 6, - 4, + self::assertCount(18, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering())); + self::assertCount(15, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(excludeBots: true))); + self::assertCount(5, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(limit: 5))); + self::assertCount(10, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(limit: 15, offset: 8))); + self::assertCount(9, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering( + dateRange: DateRange::since(Chronos::parse('2020-01-04')), + limit: 15, ))); - self::assertCount(3, $this->repo->findOrphanVisits(new VisitsListFiltering( + self::assertCount(2, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering( + dateRange: DateRange::between(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')), + limit: 6, + offset: 4, + ))); + self::assertCount(2, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering( + dateRange: DateRange::between(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')), + type: OrphanVisitType::INVALID_SHORT_URL, + ))); + self::assertCount(3, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering( DateRange::until(Chronos::parse('2020-01-01')), ))); + self::assertCount(6, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering( + type: OrphanVisitType::REGULAR_404, + ))); + self::assertCount(4, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering( + type: OrphanVisitType::BASE_URL, + limit: 4, + ))); } #[Test] @@ -400,17 +414,27 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - self::assertEquals(18, $this->repo->countOrphanVisits(new VisitsCountFiltering())); - self::assertEquals(18, $this->repo->countOrphanVisits(new VisitsCountFiltering(DateRange::allTime()))); + self::assertEquals(18, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering())); + self::assertEquals(18, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(DateRange::allTime()))); self::assertEquals(9, $this->repo->countOrphanVisits( - new VisitsCountFiltering(DateRange::since(Chronos::parse('2020-01-04'))), + new OrphanVisitsCountFiltering(DateRange::since(Chronos::parse('2020-01-04'))), )); - self::assertEquals(6, $this->repo->countOrphanVisits(new VisitsCountFiltering( + self::assertEquals(6, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering( DateRange::between(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')), ))); self::assertEquals(3, $this->repo->countOrphanVisits( - new VisitsCountFiltering(DateRange::until(Chronos::parse('2020-01-01'))), + new OrphanVisitsCountFiltering(DateRange::until(Chronos::parse('2020-01-01'))), )); + self::assertEquals(2, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering( + dateRange: DateRange::between(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')), + type: OrphanVisitType::BASE_URL, + ))); + self::assertEquals(6, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering( + type: OrphanVisitType::INVALID_SHORT_URL, + ))); + self::assertEquals(6, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering( + type: OrphanVisitType::REGULAR_404, + ))); } #[Test] diff --git a/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php b/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php index 04e3f84c..3e50faf0 100644 --- a/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php +++ b/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php @@ -9,11 +9,11 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Visit\Entity\Visit; +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\Paginator\Adapter\OrphanVisitsPaginatorAdapter; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -21,13 +21,13 @@ class OrphanVisitsPaginatorAdapterTest extends TestCase { private OrphanVisitsPaginatorAdapter $adapter; private MockObject & VisitRepositoryInterface $repo; - private VisitsParams $params; + private OrphanVisitsParams $params; private ApiKey $apiKey; protected function setUp(): void { $this->repo = $this->createMock(VisitRepositoryInterface::class); - $this->params = VisitsParams::fromRawData([]); + $this->params = OrphanVisitsParams::fromRawData([]); $this->apiKey = ApiKey::create(); $this->adapter = new OrphanVisitsPaginatorAdapter($this->repo, $this->params, $this->apiKey); @@ -38,7 +38,7 @@ class OrphanVisitsPaginatorAdapterTest extends TestCase { $expectedCount = 5; $this->repo->expects($this->once())->method('countOrphanVisits')->with( - new VisitsCountFiltering($this->params->dateRange, apiKey: $this->apiKey), + new OrphanVisitsCountFiltering($this->params->dateRange, apiKey: $this->apiKey), )->willReturn($expectedCount); $result = $this->adapter->getNbResults(); @@ -55,12 +55,12 @@ class OrphanVisitsPaginatorAdapterTest extends TestCase { $visitor = Visitor::emptyInstance(); $list = [Visit::forRegularNotFound($visitor), Visit::forInvalidShortUrl($visitor)]; - $this->repo->expects($this->once())->method('findOrphanVisits')->with(new VisitsListFiltering( - $this->params->dateRange, - $this->params->excludeBots, - $this->apiKey, - $limit, - $offset, + $this->repo->expects($this->once())->method('findOrphanVisits')->with(new OrphanVisitsListFiltering( + dateRange: $this->params->dateRange, + excludeBots: $this->params->excludeBots, + apiKey: $this->apiKey, + limit: $limit, + offset: $offset, ))->willReturn($list); $result = $this->adapter->getSlice($offset, $limit); diff --git a/module/Core/test/Visit/VisitsStatsHelperTest.php b/module/Core/test/Visit/VisitsStatsHelperTest.php index dd11fdef..b023bc1c 100644 --- a/module/Core/test/Visit/VisitsStatsHelperTest.php +++ b/module/Core/test/Visit/VisitsStatsHelperTest.php @@ -23,9 +23,12 @@ use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\Tag\Entity\Tag; use Shlinkio\Shlink\Core\Tag\Repository\TagRepository; use Shlinkio\Shlink\Core\Visit\Entity\Visit; +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\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\Repository\VisitRepository; @@ -251,14 +254,14 @@ class VisitsStatsHelperTest extends TestCase $list = array_map(static fn () => Visit::forBasePath(Visitor::emptyInstance()), range(0, 3)); $repo = $this->createMock(VisitRepository::class); $repo->expects($this->once())->method('countOrphanVisits')->with( - $this->isInstanceOf(VisitsCountFiltering::class), + $this->isInstanceOf(OrphanVisitsCountFiltering::class), )->willReturn(count($list)); $repo->expects($this->once())->method('findOrphanVisits')->with( - $this->isInstanceOf(VisitsListFiltering::class), + $this->isInstanceOf(OrphanVisitsListFiltering::class), )->willReturn($list); $this->em->expects($this->once())->method('getRepository')->with(Visit::class)->willReturn($repo); - $paginator = $this->helper->orphanVisits(new VisitsParams()); + $paginator = $this->helper->orphanVisits(new OrphanVisitsParams()); self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentPageResults())); } diff --git a/module/Rest/src/Action/Visit/OrphanVisitsAction.php b/module/Rest/src/Action/Visit/OrphanVisitsAction.php index c7adf3a1..57244197 100644 --- a/module/Rest/src/Action/Visit/OrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/OrphanVisitsAction.php @@ -9,7 +9,7 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; @@ -29,7 +29,7 @@ class OrphanVisitsAction extends AbstractRestAction public function handle(ServerRequestInterface $request): ResponseInterface { - $params = VisitsParams::fromRawData($request->getQueryParams()); + $params = OrphanVisitsParams::fromRawData($request->getQueryParams()); $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); $visits = $this->visitsHelper->orphanVisits($params, $apiKey); diff --git a/module/Rest/test-api/Action/OrphanVisitsTest.php b/module/Rest/test-api/Action/OrphanVisitsTest.php index 2c8b2479..cf7cee0f 100644 --- a/module/Rest/test-api/Action/OrphanVisitsTest.php +++ b/module/Rest/test-api/Action/OrphanVisitsTest.php @@ -8,6 +8,7 @@ use GuzzleHttp\RequestOptions; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use Shlinkio\Shlink\Common\Paginator\Paginator; +use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitType; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; class OrphanVisitsTest extends ApiTestCase @@ -68,6 +69,23 @@ class OrphanVisitsTest extends ApiTestCase 1, [self::REGULAR_NOT_FOUND], ]; + yield 'base_url only' => [['type' => OrphanVisitType::BASE_URL->value], 1, 1, [self::BASE_URL]]; + yield 'regular_404 only' => [['type' => OrphanVisitType::REGULAR_404->value], 1, 1, [self::REGULAR_NOT_FOUND]]; + yield 'invalid_short_url only' => [ + ['type' => OrphanVisitType::INVALID_SHORT_URL->value], + 1, + 1, + [self::INVALID_SHORT_URL], + ]; + } + + #[Test] + public function errorIsReturnedForInvalidType(): void + { + $resp = $this->callApiWithKey(self::METHOD_GET, '/visits/orphan', [ + RequestOptions::QUERY => ['type' => 'invalid'], + ]); + self::assertEquals(400, $resp->getStatusCode()); } #[Test] diff --git a/module/Rest/test/Action/Visit/OrphanVisitsActionTest.php b/module/Rest/test/Action/Visit/OrphanVisitsActionTest.php index da660d0e..efa14caa 100644 --- a/module/Rest/test/Action/Visit/OrphanVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/OrphanVisitsActionTest.php @@ -12,9 +12,10 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; +use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Visit\Entity\Visit; +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\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\Visit\OrphanVisitsAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -41,7 +42,7 @@ class OrphanVisitsActionTest extends TestCase $visitor = Visitor::emptyInstance(); $visits = [Visit::forInvalidShortUrl($visitor), Visit::forRegularNotFound($visitor)]; $this->visitsHelper->expects($this->once())->method('orphanVisits')->with( - $this->isInstanceOf(VisitsParams::class), + $this->isInstanceOf(OrphanVisitsParams::class), )->willReturn(new Paginator(new ArrayAdapter($visits))); $visitsAmount = count($visits); $this->orphanVisitTransformer->expects($this->exactly($visitsAmount))->method('transform')->with( @@ -57,4 +58,15 @@ class OrphanVisitsActionTest extends TestCase self::assertCount($visitsAmount, $payload['visits']['data']); self::assertEquals(200, $response->getStatusCode()); } + + #[Test] + public function exceptionIsThrownIfInvalidDataIsProvided(): void + { + $this->expectException(ValidationException::class); + $this->action->handle( + ServerRequestFactory::fromGlobals() + ->withAttribute(ApiKey::class, ApiKey::create()) + ->withQueryParams(['type' => 'invalidType']), + ); + } }