From 4b90cf93d37bf8a53c19d6869c4b134272536c8c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 5 Jan 2022 23:44:14 +0100 Subject: [PATCH] Created DB-level paginator for tags with stats --- module/Core/src/Repository/TagRepository.php | 12 +++++++++--- .../Core/src/Repository/TagRepositoryInterface.php | 7 ++++++- .../Adapter/AbstractTagsPaginatorAdapter.php | 4 ++-- .../Paginator/Adapter/TagsInfoPaginatorAdapter.php | 13 +++++++++++++ module/Core/src/Tag/TagService.php | 6 ++---- module/Core/test/Service/Tag/TagServiceTest.php | 6 ++++-- 6 files changed, 36 insertions(+), 12 deletions(-) create mode 100644 module/Core/src/Tag/Paginator/Adapter/TagsInfoPaginatorAdapter.php diff --git a/module/Core/src/Repository/TagRepository.php b/module/Core/src/Repository/TagRepository.php index a6871cfc..de304441 100644 --- a/module/Core/src/Repository/TagRepository.php +++ b/module/Core/src/Repository/TagRepository.php @@ -32,14 +32,20 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito /** * @return TagInfo[] */ - public function findTagsWithInfo(?ApiKey $apiKey = null): array - { + public function findTagsWithInfo( + ?int $limit = null, + ?int $offset = null, + ?string $searchTerm = null, + ?ApiKey $apiKey = null, + ): array { $qb = $this->createQueryBuilder('t'); $qb->select('t AS tag', 'COUNT(DISTINCT s.id) AS shortUrlsCount', 'COUNT(DISTINCT v.id) AS visitsCount') ->leftJoin('t.shortUrls', 's') ->leftJoin('s.visits', 'v') ->groupBy('t') - ->orderBy('t.name', 'ASC'); + ->orderBy('t.name', 'ASC') + ->setMaxResults($limit) + ->setFirstResult($offset); if ($apiKey !== null) { $this->applySpecification($qb, $apiKey->spec(false, 'shortUrls'), 't'); diff --git a/module/Core/src/Repository/TagRepositoryInterface.php b/module/Core/src/Repository/TagRepositoryInterface.php index 924706ff..0158d43d 100644 --- a/module/Core/src/Repository/TagRepositoryInterface.php +++ b/module/Core/src/Repository/TagRepositoryInterface.php @@ -16,7 +16,12 @@ interface TagRepositoryInterface extends ObjectRepository, EntitySpecificationRe /** * @return TagInfo[] */ - public function findTagsWithInfo(?ApiKey $apiKey = null): array; + public function findTagsWithInfo( + ?int $limit = null, + ?int $offset = null, + ?string $searchTerm = null, + ?ApiKey $apiKey = null, + ): array; public function tagExists(string $tag, ?ApiKey $apiKey = null): bool; } diff --git a/module/Core/src/Tag/Paginator/Adapter/AbstractTagsPaginatorAdapter.php b/module/Core/src/Tag/Paginator/Adapter/AbstractTagsPaginatorAdapter.php index f35e492d..f6331f5e 100644 --- a/module/Core/src/Tag/Paginator/Adapter/AbstractTagsPaginatorAdapter.php +++ b/module/Core/src/Tag/Paginator/Adapter/AbstractTagsPaginatorAdapter.php @@ -24,8 +24,8 @@ abstract class AbstractTagsPaginatorAdapter implements AdapterInterface public function getNbResults(): int { return (int) $this->repo->matchSingleScalarResult(Spec::andX( - // FIXME I don't think using Spec::selectNew is the correct thing here, - // but seems to be the only way to use Spec::COUNT + // FIXME I don't think using Spec::selectNew is the correct thing here, ideally it should be Spec::select, + // but seems to be the only way to use Spec::COUNT(...) Spec::selectNew(Tag::class, Spec::COUNT('id', true)), new WithApiKeySpecsEnsuringJoin($this->apiKey), )); diff --git a/module/Core/src/Tag/Paginator/Adapter/TagsInfoPaginatorAdapter.php b/module/Core/src/Tag/Paginator/Adapter/TagsInfoPaginatorAdapter.php new file mode 100644 index 00000000..8d64942d --- /dev/null +++ b/module/Core/src/Tag/Paginator/Adapter/TagsInfoPaginatorAdapter.php @@ -0,0 +1,13 @@ +repo->findTagsWithInfo($length, $offset, null, $this->apiKey); + } +} diff --git a/module/Core/src/Tag/TagService.php b/module/Core/src/Tag/TagService.php index 85955b76..40eb413f 100644 --- a/module/Core/src/Tag/TagService.php +++ b/module/Core/src/Tag/TagService.php @@ -6,7 +6,6 @@ namespace Shlinkio\Shlink\Core\Tag; use Doctrine\ORM; use Pagerfanta\Adapter\AdapterInterface; -use Pagerfanta\Adapter\ArrayAdapter; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\ForbiddenTagOperationException; @@ -17,6 +16,7 @@ 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\Core\Tag\Paginator\Adapter\TagsInfoPaginatorAdapter; use Shlinkio\Shlink\Core\Tag\Paginator\Adapter\TagsPaginatorAdapter; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -43,9 +43,7 @@ class TagService implements TagServiceInterface { /** @var TagRepositoryInterface $repo */ $repo = $this->em->getRepository(Tag::class); - $tagsInfo = $repo->findTagsWithInfo($apiKey); - - return $this->createPaginator(new ArrayAdapter($tagsInfo), $params); + return $this->createPaginator(new TagsInfoPaginatorAdapter($repo, $params, $apiKey), $params); } private function createPaginator(AdapterInterface $adapter, TagsParams $params): Paginator diff --git a/module/Core/test/Service/Tag/TagServiceTest.php b/module/Core/test/Service/Tag/TagServiceTest.php index d291b3c4..eac7495a 100644 --- a/module/Core/test/Service/Tag/TagServiceTest.php +++ b/module/Core/test/Service/Tag/TagServiceTest.php @@ -47,7 +47,7 @@ class TagServiceTest extends TestCase $expected = [new Tag('foo'), new Tag('bar')]; $match = $this->repo->match(Argument::cetera())->willReturn($expected); - $count = $this->repo->matchSingleScalarResult(Argument::cetera())->willReturn(0); + $count = $this->repo->matchSingleScalarResult(Argument::cetera())->willReturn(2); $result = $this->service->listTags(TagsParams::fromRawData([])); @@ -64,12 +64,14 @@ class TagServiceTest extends TestCase { $expected = [new TagInfo(new Tag('foo'), 1, 1), new TagInfo(new Tag('bar'), 3, 10)]; - $find = $this->repo->findTagsWithInfo($apiKey)->willReturn($expected); + $find = $this->repo->findTagsWithInfo(2, 0, null, $apiKey)->willReturn($expected); + $count = $this->repo->matchSingleScalarResult(Argument::cetera())->willReturn(2); $result = $this->service->tagsInfo(TagsParams::fromRawData([]), $apiKey); // TODO Add more cases with params self::assertEquals($expected, $result->getCurrentPageResults()); $find->shouldHaveBeenCalled(); + $count->shouldHaveBeenCalled(); } /**