From 775f58f972d2d5e7176503e923230435a8715007 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 5 Jan 2022 19:12:08 +0100 Subject: [PATCH] Added support for pagination in tags lists --- composer.json | 2 +- .../CLI/src/Command/Tag/ListTagsCommand.php | 3 +- .../test/Command/Tag/ListTagsCommandTest.php | 9 ++++-- module/Core/src/Tag/TagService.php | 23 +++++++++++---- module/Core/src/Tag/TagServiceInterface.php | 10 ++++--- .../Core/test/Service/Tag/TagServiceTest.php | 9 +++--- module/Rest/src/Action/Tag/ListTagsAction.php | 21 +++++++------- module/Rest/test-api/Action/ListTagsTest.php | 10 +++++++ .../test/Action/Tag/ListTagsActionTest.php | 28 +++++++++++++++++-- 9 files changed, 83 insertions(+), 32 deletions(-) diff --git a/composer.json b/composer.json index 4bc4890e..643c658c 100644 --- a/composer.json +++ b/composer.json @@ -48,7 +48,7 @@ "predis/predis": "^1.1", "pugx/shortid-php": "^1.0", "ramsey/uuid": "^4.2", - "shlinkio/shlink-common": "dev-main#5cb4092 as 4.3", + "shlinkio/shlink-common": "dev-main#0d476fd as 4.3", "shlinkio/shlink-config": "^1.5", "shlinkio/shlink-event-dispatcher": "^2.3", "shlinkio/shlink-importer": "^2.5", diff --git a/module/CLI/src/Command/Tag/ListTagsCommand.php b/module/CLI/src/Command/Tag/ListTagsCommand.php index 9eebe36f..7d21613d 100644 --- a/module/CLI/src/Command/Tag/ListTagsCommand.php +++ b/module/CLI/src/Command/Tag/ListTagsCommand.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\CLI\Command\Tag; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; +use Shlinkio\Shlink\Core\Tag\Model\TagsParams; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -38,7 +39,7 @@ class ListTagsCommand extends Command private function getTagsRows(): array { - $tags = $this->tagService->tagsInfo(); + $tags = $this->tagService->tagsInfo(TagsParams::fromRawData([]))->getCurrentPageResults(); if (empty($tags)) { return [['No tags found', '-', '-']]; } diff --git a/module/CLI/test/Command/Tag/ListTagsCommandTest.php b/module/CLI/test/Command/Tag/ListTagsCommandTest.php index f79aa03d..879b2eb7 100644 --- a/module/CLI/test/Command/Tag/ListTagsCommandTest.php +++ b/module/CLI/test/Command/Tag/ListTagsCommandTest.php @@ -4,9 +4,12 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\CLI\Command\Tag; +use Pagerfanta\Adapter\ArrayAdapter; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Tag\ListTagsCommand; +use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; @@ -29,7 +32,7 @@ class ListTagsCommandTest extends TestCase /** @test */ public function noTagsPrintsEmptyMessage(): void { - $tagsInfo = $this->tagService->tagsInfo()->willReturn([]); + $tagsInfo = $this->tagService->tagsInfo(Argument::any())->willReturn(new Paginator(new ArrayAdapter([]))); $this->commandTester->execute([]); $output = $this->commandTester->getDisplay(); @@ -41,10 +44,10 @@ class ListTagsCommandTest extends TestCase /** @test */ public function listOfTagsIsPrinted(): void { - $tagsInfo = $this->tagService->tagsInfo()->willReturn([ + $tagsInfo = $this->tagService->tagsInfo(Argument::any())->willReturn(new Paginator(new ArrayAdapter([ new TagInfo(new Tag('foo'), 10, 2), new TagInfo(new Tag('bar'), 7, 32), - ]); + ]))); $this->commandTester->execute([]); $output = $this->commandTester->getDisplay(); diff --git a/module/Core/src/Tag/TagService.php b/module/Core/src/Tag/TagService.php index c9248520..dd6bda7e 100644 --- a/module/Core/src/Tag/TagService.php +++ b/module/Core/src/Tag/TagService.php @@ -6,6 +6,8 @@ namespace Shlinkio\Shlink\Core\Tag; use Doctrine\ORM; use Happyr\DoctrineSpecification\Spec; +use Pagerfanta\Adapter\ArrayAdapter; +use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\ForbiddenTagOperationException; use Shlinkio\Shlink\Core\Exception\TagConflictException; @@ -14,6 +16,7 @@ use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\Core\Repository\TagRepositoryInterface; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; +use Shlinkio\Shlink\Core\Tag\Model\TagsParams; use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -24,26 +27,34 @@ class TagService implements TagServiceInterface } /** - * @return Tag[] + * @return Tag[]|Paginator */ - public function listTags(?ApiKey $apiKey = null): array + public function listTags(TagsParams $params, ?ApiKey $apiKey = null): Paginator { /** @var TagRepository $repo */ $repo = $this->em->getRepository(Tag::class); - return $repo->match(Spec::andX( + $tags = $repo->match(Spec::andX( Spec::orderBy('name'), new WithApiKeySpecsEnsuringJoin($apiKey), )); + + return (new Paginator(new ArrayAdapter($tags))) + ->setMaxPerPage($params->getItemsPerPage()) + ->setCurrentPage($params->getPage()); } /** - * @return TagInfo[] + * @return TagInfo[]|Paginator */ - public function tagsInfo(?ApiKey $apiKey = null): array + public function tagsInfo(TagsParams $params, ?ApiKey $apiKey = null): Paginator { /** @var TagRepositoryInterface $repo */ $repo = $this->em->getRepository(Tag::class); - return $repo->findTagsWithInfo($apiKey); + $tagsInfo = $repo->findTagsWithInfo($apiKey); + + return (new Paginator(new ArrayAdapter($tagsInfo))) + ->setMaxPerPage($params->getItemsPerPage()) + ->setCurrentPage($params->getPage()); } /** diff --git a/module/Core/src/Tag/TagServiceInterface.php b/module/Core/src/Tag/TagServiceInterface.php index a1aa6122..284fc341 100644 --- a/module/Core/src/Tag/TagServiceInterface.php +++ b/module/Core/src/Tag/TagServiceInterface.php @@ -4,25 +4,27 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Tag; +use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\ForbiddenTagOperationException; use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; +use Shlinkio\Shlink\Core\Tag\Model\TagsParams; use Shlinkio\Shlink\Rest\Entity\ApiKey; interface TagServiceInterface { /** - * @return Tag[] + * @return Tag[]|Paginator */ - public function listTags(?ApiKey $apiKey = null): array; + public function listTags(TagsParams $params, ?ApiKey $apiKey = null): Paginator; /** - * @return TagInfo[] + * @return TagInfo[]|Paginator */ - public function tagsInfo(?ApiKey $apiKey = null): array; + public function tagsInfo(TagsParams $params, ?ApiKey $apiKey = null): Paginator; /** * @param string[] $tagNames diff --git a/module/Core/test/Service/Tag/TagServiceTest.php b/module/Core/test/Service/Tag/TagServiceTest.php index ed8cba29..b1b1ed5d 100644 --- a/module/Core/test/Service/Tag/TagServiceTest.php +++ b/module/Core/test/Service/Tag/TagServiceTest.php @@ -16,6 +16,7 @@ use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; +use Shlinkio\Shlink\Core\Tag\Model\TagsParams; use Shlinkio\Shlink\Core\Tag\TagService; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; @@ -47,9 +48,9 @@ class TagServiceTest extends TestCase $match = $this->repo->match(Argument::cetera())->willReturn($expected); - $result = $this->service->listTags(); + $result = $this->service->listTags(TagsParams::fromRawData([])); - self::assertEquals($expected, $result); + self::assertEquals($expected, $result->getCurrentPageResults()); $match->shouldHaveBeenCalled(); } @@ -63,9 +64,9 @@ class TagServiceTest extends TestCase $find = $this->repo->findTagsWithInfo($apiKey)->willReturn($expected); - $result = $this->service->tagsInfo($apiKey); + $result = $this->service->tagsInfo(TagsParams::fromRawData([]), $apiKey); // TODO Add more cases with params - self::assertEquals($expected, $result); + self::assertEquals($expected, $result->getCurrentPageResults()); $find->shouldHaveBeenCalled(); } diff --git a/module/Rest/src/Action/Tag/ListTagsAction.php b/module/Rest/src/Action/Tag/ListTagsAction.php index 3d34bd19..c4e0e0de 100644 --- a/module/Rest/src/Action/Tag/ListTagsAction.php +++ b/module/Rest/src/Action/Tag/ListTagsAction.php @@ -7,7 +7,9 @@ namespace Shlinkio\Shlink\Rest\Action\Tag; use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; +use Shlinkio\Shlink\Core\Tag\Model\TagsParams; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; @@ -16,6 +18,8 @@ use function Functional\map; class ListTagsAction extends AbstractRestAction { + use PagerfantaUtilsTrait; + protected const ROUTE_PATH = '/tags'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; @@ -28,23 +32,18 @@ class ListTagsAction extends AbstractRestAction $query = $request->getQueryParams(); $withStats = ($query['withStats'] ?? null) === 'true'; $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); + $params = TagsParams::fromRawData($query); if (! $withStats) { return new JsonResponse([ - 'tags' => [ - 'data' => $this->tagService->listTags($apiKey), - ], + 'tags' => $this->serializePaginator($this->tagService->listTags($params, $apiKey)), ]); } - $tagsInfo = $this->tagService->tagsInfo($apiKey); - $data = map($tagsInfo, static fn (TagInfo $info) => $info->tag()->__toString()); + $tagsInfo = $this->tagService->tagsInfo($params, $apiKey); + $rawTags = $this->serializePaginator($tagsInfo, null, 'stats'); + $rawTags['data'] = map($tagsInfo, static fn (TagInfo $info) => $info->tag()->__toString()); - return new JsonResponse([ - 'tags' => [ - 'data' => $data, - 'stats' => $tagsInfo, - ], - ]); + return new JsonResponse(['tags' => $rawTags]); } } diff --git a/module/Rest/test-api/Action/ListTagsTest.php b/module/Rest/test-api/Action/ListTagsTest.php index d82a4f8e..de71182d 100644 --- a/module/Rest/test-api/Action/ListTagsTest.php +++ b/module/Rest/test-api/Action/ListTagsTest.php @@ -7,6 +7,8 @@ namespace ShlinkioApiTest\Shlink\Rest\Action; use GuzzleHttp\RequestOptions; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; +use function count; + class ListTagsTest extends ApiTestCase { /** @@ -17,6 +19,14 @@ class ListTagsTest extends ApiTestCase { $resp = $this->callApiWithKey(self::METHOD_GET, '/tags', [RequestOptions::QUERY => $query], $apiKey); $payload = $this->getJsonResponsePayload($resp); + $itemsCount = count($expectedTags['data']); + $expectedTags['pagination'] = [ + 'currentPage' => 1, + 'pagesCount' => 1, + 'itemsPerPage' => $itemsCount, + 'itemsInCurrentPage' => $itemsCount, + 'totalItems' => $itemsCount, + ]; self::assertEquals(['tags' => $expectedTags], $payload); } diff --git a/module/Rest/test/Action/Tag/ListTagsActionTest.php b/module/Rest/test/Action/Tag/ListTagsActionTest.php index 8b7378fd..504f7b4f 100644 --- a/module/Rest/test/Action/Tag/ListTagsActionTest.php +++ b/module/Rest/test/Action/Tag/ListTagsActionTest.php @@ -6,17 +6,21 @@ namespace ShlinkioTest\Shlink\Rest\Action\Tag; use Laminas\Diactoros\Response\JsonResponse; use Laminas\Diactoros\ServerRequestFactory; +use Pagerfanta\Adapter\ArrayAdapter; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\ServerRequestInterface; +use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\Tag\ListTagsAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; +use function count; + class ListTagsActionTest extends TestCase { use ProphecyTrait; @@ -37,7 +41,10 @@ class ListTagsActionTest extends TestCase public function returnsBaseDataWhenStatsAreNotRequested(array $query): void { $tags = [new Tag('foo'), new Tag('bar')]; - $listTags = $this->tagService->listTags(Argument::type(ApiKey::class))->willReturn($tags); + $tagsCount = count($tags); + $listTags = $this->tagService->listTags(Argument::any(), Argument::type(ApiKey::class))->willReturn( + new Paginator(new ArrayAdapter($tags)), + ); /** @var JsonResponse $resp */ $resp = $this->action->handle($this->requestWithApiKey()->withQueryParams($query)); @@ -46,6 +53,13 @@ class ListTagsActionTest extends TestCase self::assertEquals([ 'tags' => [ 'data' => $tags, + 'pagination' => [ + 'currentPage' => 1, + 'pagesCount' => 1, + 'itemsPerPage' => 10, + 'itemsInCurrentPage' => $tagsCount, + 'totalItems' => $tagsCount, + ], ], ], $payload); $listTags->shouldHaveBeenCalled(); @@ -65,7 +79,10 @@ class ListTagsActionTest extends TestCase new TagInfo(new Tag('foo'), 1, 1), new TagInfo(new Tag('bar'), 3, 10), ]; - $tagsInfo = $this->tagService->tagsInfo(Argument::type(ApiKey::class))->willReturn($stats); + $itemsCount = count($stats); + $tagsInfo = $this->tagService->tagsInfo(Argument::any(), Argument::type(ApiKey::class))->willReturn( + new Paginator(new ArrayAdapter($stats)), + ); $req = $this->requestWithApiKey()->withQueryParams(['withStats' => 'true']); /** @var JsonResponse $resp */ @@ -76,6 +93,13 @@ class ListTagsActionTest extends TestCase 'tags' => [ 'data' => ['foo', 'bar'], 'stats' => $stats, + 'pagination' => [ + 'currentPage' => 1, + 'pagesCount' => 1, + 'itemsPerPage' => 10, + 'itemsInCurrentPage' => $itemsCount, + 'totalItems' => $itemsCount, + ], ], ], $payload); $tagsInfo->shouldHaveBeenCalled();