From 33a6c9fda70ebd9c2386f73b8769216622504ec5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 21 Jan 2022 19:52:25 +0100 Subject: [PATCH] Added support to order tags with stats by short URLs or visits count. In a non-performant way --- module/Core/src/Repository/TagRepository.php | 28 ++++- .../test-db/Repository/TagRepositoryTest.php | 107 +++++++++++++++++- 2 files changed, 125 insertions(+), 10 deletions(-) diff --git a/module/Core/src/Repository/TagRepository.php b/module/Core/src/Repository/TagRepository.php index cc952796..7e69dcc9 100644 --- a/module/Core/src/Repository/TagRepository.php +++ b/module/Core/src/Repository/TagRepository.php @@ -16,6 +16,7 @@ use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin; use Shlinkio\Shlink\Rest\ApiKey\Spec\WithInlinedApiKeySpecsEnsuringJoin; use Shlinkio\Shlink\Rest\Entity\ApiKey; +use function Functional\contains; use function Functional\map; use const PHP_INT_MAX; @@ -40,12 +41,19 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito */ public function findTagsWithInfo(?TagsListFiltering $filtering = null): array { + $orderBy = $filtering?->orderBy(); + $orderField = $orderBy?->orderField(); + $orderMainQuery = contains(['shortUrlsCount', 'visitsCount'], $orderField); + $conn = $this->getEntityManager()->getConnection(); $subQb = $this->createQueryBuilder('t'); - $subQb->select('t.id', 't.name') - ->orderBy('t.name', $filtering?->orderBy()?->orderDirection() ?? 'ASC') // TODO Make filed dynamic - ->setMaxResults($filtering?->limit() ?? PHP_INT_MAX) - ->setFirstResult($filtering?->offset() ?? 0); + $subQb->select('t.id', 't.name'); + + if (! $orderMainQuery) { + $subQb->orderBy('t.name', $orderBy?->orderDirection() ?? 'ASC') + ->setMaxResults($filtering?->limit() ?? PHP_INT_MAX) + ->setFirstResult($filtering?->offset() ?? 0); + } $searchTerm = $filtering?->searchTerm(); if ($searchTerm !== null) { @@ -74,7 +82,7 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito ->leftJoin('st', 'short_urls', 's', $nativeQb->expr()->eq('s.id', 'st.short_url_id')) ->leftJoin('st', 'visits', 'v', $nativeQb->expr()->eq('s.id', 'v.short_url_id')) ->groupBy('t.id_0', 't.name_1') - ->orderBy('t.name_1', $filtering?->orderBy()?->orderDirection() ?? 'ASC'); // TODO Make field dynamic + ->orderBy('t.name_1', $orderBy?->orderDirection() ?? 'ASC'); // TODO Make field dynamic // Apply API key role conditions to the native query too, as they will affect the amounts on the aggregates $apiKey?->mapRoles(fn (string $roleName, array $meta) => match ($roleName) { @@ -87,6 +95,16 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito default => $nativeQb, }); + if ($orderMainQuery) { + $nativeQb + ->orderBy( + $orderField === 'shortUrlsCount' ? 'short_urls_count' : 'visits_count', + $orderBy?->orderDirection() ?? 'ASC', + ) + ->setMaxResults($filtering?->limit() ?? PHP_INT_MAX) + ->setFirstResult($filtering?->offset() ?? 0); + } + $rsm = new ResultSetMappingBuilder($this->getEntityManager()); $rsm->addRootEntityFromClassMetadata(Tag::class, 't'); $rsm->addScalarResult('short_urls_count', 'shortUrlsCount'); diff --git a/module/Core/test-db/Repository/TagRepositoryTest.php b/module/Core/test-db/Repository/TagRepositoryTest.php index 9bcfaf2b..0ddd04ab 100644 --- a/module/Core/test-db/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Repository/TagRepositoryTest.php @@ -86,6 +86,15 @@ class TagRepositoryTest extends DatabaseTestCase $shortUrl2 = ShortUrl::fromMeta($metaWithTags($secondUrlTags, null), $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl2, Visitor::emptyInstance())); + + // One of the tags has two extra short URLs, but with no visits + $this->getEntityManager()->persist( + ShortUrl::fromMeta($metaWithTags(['bar'], null), $this->relationResolver), + ); + $this->getEntityManager()->persist( + ShortUrl::fromMeta($metaWithTags(['bar'], null), $this->relationResolver), + ); + $this->getEntityManager()->flush(); $result = $this->repo->findTagsWithInfo($filtering); @@ -102,7 +111,7 @@ class TagRepositoryTest extends DatabaseTestCase self::assertEquals(0, $result[0]->visitsCount()); self::assertEquals($tagNames[3], $result[0]->tag()->__toString()); - self::assertEquals(1, $result[1]->shortUrlsCount()); + self::assertEquals(3, $result[1]->shortUrlsCount()); self::assertEquals(3, $result[1]->visitsCount()); self::assertEquals($tagNames[1], $result[1]->tag()->__toString()); @@ -124,7 +133,7 @@ class TagRepositoryTest extends DatabaseTestCase self::assertEquals(0, $result[0]->visitsCount()); self::assertEquals($tagNames[3], $result[0]->tag()->__toString()); - self::assertEquals(1, $result[1]->shortUrlsCount()); + self::assertEquals(3, $result[1]->shortUrlsCount()); self::assertEquals(3, $result[1]->visitsCount()); self::assertEquals($tagNames[1], $result[1]->tag()->__toString()); }]; @@ -140,7 +149,7 @@ class TagRepositoryTest extends DatabaseTestCase static function (array $result, array $tagNames): void { /** @var TagInfo[] $result */ self::assertCount(2, $result); - self::assertEquals(1, $result[0]->shortUrlsCount()); + self::assertEquals(3, $result[0]->shortUrlsCount()); self::assertEquals(3, $result[0]->visitsCount()); self::assertEquals($tagNames[1], $result[0]->tag()->__toString()); @@ -154,7 +163,7 @@ class TagRepositoryTest extends DatabaseTestCase static function (array $result, array $tagNames): void { /** @var TagInfo[] $result */ self::assertCount(2, $result); - self::assertEquals(1, $result[0]->shortUrlsCount()); + self::assertEquals(3, $result[0]->shortUrlsCount()); self::assertEquals(3, $result[0]->visitsCount()); self::assertEquals($tagNames[1], $result[0]->tag()->__toString()); @@ -176,7 +185,7 @@ class TagRepositoryTest extends DatabaseTestCase self::assertEquals(0, $result[3]->visitsCount()); self::assertEquals($tagNames[3], $result[3]->tag()->__toString()); - self::assertEquals(1, $result[2]->shortUrlsCount()); + self::assertEquals(3, $result[2]->shortUrlsCount()); self::assertEquals(3, $result[2]->visitsCount()); self::assertEquals($tagNames[1], $result[2]->tag()->__toString()); @@ -189,6 +198,94 @@ class TagRepositoryTest extends DatabaseTestCase self::assertEquals($tagNames[0], $result[0]->tag()->__toString()); }, ]; + yield 'short URLs count ASC ordering' => [ + new TagsListFiltering(null, null, null, Ordering::fromTuple(['shortUrlsCount', 'ASC'])), + static function (array $result, array $tagNames): void { + /** @var TagInfo[] $result */ + self::assertCount(4, $result); + self::assertEquals(0, $result[0]->shortUrlsCount()); + self::assertEquals(0, $result[0]->visitsCount()); + self::assertEquals($tagNames[3], $result[0]->tag()->__toString()); + + self::assertEquals(1, $result[1]->shortUrlsCount()); + self::assertEquals(3, $result[1]->visitsCount()); + self::assertEquals($tagNames[2], $result[1]->tag()->__toString()); + + self::assertEquals(2, $result[2]->shortUrlsCount()); + self::assertEquals(4, $result[2]->visitsCount()); + self::assertEquals($tagNames[0], $result[2]->tag()->__toString()); + + self::assertEquals(3, $result[3]->shortUrlsCount()); + self::assertEquals(3, $result[3]->visitsCount()); + self::assertEquals($tagNames[1], $result[3]->tag()->__toString()); + }, + ]; + yield 'short URLs count DESC ordering' => [ + new TagsListFiltering(null, null, null, Ordering::fromTuple(['shortUrlsCount', 'DESC'])), + static function (array $result, array $tagNames): void { + /** @var TagInfo[] $result */ + self::assertCount(4, $result); + self::assertEquals(3, $result[0]->shortUrlsCount()); + self::assertEquals(3, $result[0]->visitsCount()); + self::assertEquals($tagNames[1], $result[0]->tag()->__toString()); + + self::assertEquals(2, $result[1]->shortUrlsCount()); + self::assertEquals(4, $result[1]->visitsCount()); + self::assertEquals($tagNames[0], $result[1]->tag()->__toString()); + + self::assertEquals(1, $result[2]->shortUrlsCount()); + self::assertEquals(3, $result[2]->visitsCount()); + self::assertEquals($tagNames[2], $result[2]->tag()->__toString()); + + self::assertEquals(0, $result[3]->shortUrlsCount()); + self::assertEquals(0, $result[3]->visitsCount()); + self::assertEquals($tagNames[3], $result[3]->tag()->__toString()); + }, + ]; + yield 'visits count ASC ordering' => [ + new TagsListFiltering(null, null, null, Ordering::fromTuple(['visitsCount', 'ASC'])), + static function (array $result, array $tagNames): void { + /** @var TagInfo[] $result */ + self::assertCount(4, $result); + self::assertEquals(0, $result[0]->shortUrlsCount()); + self::assertEquals(0, $result[0]->visitsCount()); + self::assertEquals($tagNames[3], $result[0]->tag()->__toString()); + + self::assertEquals(3, $result[1]->shortUrlsCount()); + self::assertEquals(3, $result[1]->visitsCount()); + self::assertEquals($tagNames[1], $result[1]->tag()->__toString()); + + self::assertEquals(1, $result[2]->shortUrlsCount()); + self::assertEquals(3, $result[2]->visitsCount()); + self::assertEquals($tagNames[2], $result[2]->tag()->__toString()); + + self::assertEquals(2, $result[3]->shortUrlsCount()); + self::assertEquals(4, $result[3]->visitsCount()); + self::assertEquals($tagNames[0], $result[3]->tag()->__toString()); + }, + ]; + yield 'visits count DESC ordering' => [ + new TagsListFiltering(null, null, null, Ordering::fromTuple(['visitsCount', 'DESC'])), + static function (array $result, array $tagNames): void { + /** @var TagInfo[] $result */ + self::assertCount(4, $result); + self::assertEquals(2, $result[0]->shortUrlsCount()); + self::assertEquals(4, $result[0]->visitsCount()); + self::assertEquals($tagNames[0], $result[0]->tag()->__toString()); + + self::assertEquals(3, $result[1]->shortUrlsCount()); + self::assertEquals(3, $result[1]->visitsCount()); + self::assertEquals($tagNames[1], $result[1]->tag()->__toString()); + + self::assertEquals(1, $result[2]->shortUrlsCount()); + self::assertEquals(3, $result[2]->visitsCount()); + self::assertEquals($tagNames[2], $result[2]->tag()->__toString()); + + self::assertEquals(0, $result[3]->shortUrlsCount()); + self::assertEquals(0, $result[3]->visitsCount()); + self::assertEquals($tagNames[3], $result[3]->tag()->__toString()); + }, + ]; yield 'api key' => [new TagsListFiltering(null, null, null, null, ApiKey::fromMeta( ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls()), )), static function (array $result, array $tagNames): void {