From 48a8290e920f1e9aa9c31b0c170d3655adca0974 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 10 Feb 2024 17:51:42 +0100 Subject: [PATCH] Allow type filter property for orphan visits list --- docker-compose.yml | 2 +- docs/swagger/paths/v2_visits_orphan.json | 58 +++++++++++++++++++ .../Command/Visit/GetOrphanVisitsCommand.php | 4 +- .../src/Visit/Model/OrphanVisitsParams.php | 53 +++++++++++++++++ module/Core/src/Visit/Model/VisitsParams.php | 2 +- .../Adapter/OrphanVisitsPaginatorAdapter.php | 6 +- module/Core/src/Visit/VisitsStatsHelper.php | 3 +- .../src/Visit/VisitsStatsHelperInterface.php | 3 +- .../OrphanVisitsPaginatorAdapterTest.php | 6 +- .../Core/test/Visit/VisitsStatsHelperTest.php | 3 +- .../src/Action/Visit/OrphanVisitsAction.php | 4 +- .../Action/Visit/OrphanVisitsActionTest.php | 16 ++++- 12 files changed, 144 insertions(+), 16 deletions(-) create mode 100644 module/Core/src/Visit/Model/OrphanVisitsParams.php 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..3b89e339 100644 --- a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php @@ -7,7 +7,7 @@ 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 Symfony\Component\Console\Input\InputInterface; class GetOrphanVisitsCommand extends AbstractVisitsListCommand @@ -23,7 +23,7 @@ class GetOrphanVisitsCommand extends AbstractVisitsListCommand protected function getVisitsPaginator(InputInterface $input, DateRange $dateRange): Paginator { - return $this->visitsHelper->orphanVisits(new VisitsParams($dateRange)); + return $this->visitsHelper->orphanVisits(new OrphanVisitsParams($dateRange)); } /** diff --git a/module/Core/src/Visit/Model/OrphanVisitsParams.php b/module/Core/src/Visit/Model/OrphanVisitsParams.php new file mode 100644 index 00000000..45efd6bb --- /dev/null +++ b/module/Core/src/Visit/Model/OrphanVisitsParams.php @@ -0,0 +1,53 @@ +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, + implode('", "', enumValues(OrphanVisitType::class)), + ), + ]); + } + } +} 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 1f38ab6a..863460a1 100644 --- a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php @@ -5,7 +5,7 @@ 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\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; @@ -15,7 +15,7 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte { public function __construct( private readonly VisitRepositoryInterface $repo, - private readonly VisitsParams $params, + private readonly OrphanVisitsParams $params, private readonly ?ApiKey $apiKey, ) { } @@ -26,6 +26,7 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte dateRange: $this->params->dateRange, excludeBots: $this->params->excludeBots, apiKey: $this->apiKey, + type: $this->params->type, )); } @@ -35,6 +36,7 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte 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/VisitsStatsHelper.php b/module/Core/src/Visit/VisitsStatsHelper.php index 23ea8fc2..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; @@ -117,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/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php b/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php index 623482a6..3e50faf0 100644 --- a/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php +++ b/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php @@ -9,8 +9,8 @@ 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\OrphanVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering; @@ -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); diff --git a/module/Core/test/Visit/VisitsStatsHelperTest.php b/module/Core/test/Visit/VisitsStatsHelperTest.php index 3af7b739..b023bc1c 100644 --- a/module/Core/test/Visit/VisitsStatsHelperTest.php +++ b/module/Core/test/Visit/VisitsStatsHelperTest.php @@ -23,6 +23,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\Visitor; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\Model\VisitsStats; @@ -260,7 +261,7 @@ class VisitsStatsHelperTest extends TestCase )->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/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']), + ); + } }