Simplify how limits are applied to tags query

module/Core/src/Tag/Model/OrderableField.php

@@ -15,12 +15,6 @@ enum OrderableField: string
     /** @deprecated Use VISITS instead */
     case VISITS_COUNT = 'visitsCount';
 
-    public static function isAggregateField(string $field): bool
-    {
-        $parsed = self::tryFrom($field);
-        return $parsed !== null && $parsed !== self::TAG;
-    }
-
     public static function toSnakeCaseValidField(?string $field): string
     {
         $parsed = $field !== null ? self::tryFrom($field) : self::VISITS;

module/Core/src/Tag/Repository/TagRepository.php

@@ -18,6 +18,7 @@ use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 use function Functional\map;
+use function Functional\each;
 
 use const PHP_INT_MAX;
 
@@ -43,23 +44,23 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
     {
         $orderField = $filtering?->orderBy?->field;
         $orderDir = $filtering?->orderBy?->direction;
-        $orderMainQuery = $orderField !== null && OrderableField::isAggregateField($orderField);
+        $apiKey = $filtering?->apiKey;
 
         $conn = $this->getEntityManager()->getConnection();
         $tagsSubQb = $conn->createQueryBuilder();
+
+        // For admins and when no API key is present, we'll return tags which are not linked to any short URL
+        $joiningMethod = $apiKey === null || $apiKey->isAdmin() ? 'leftJoin' : 'join';
         $tagsSubQb
             ->select('t.id', 't.name', 'COUNT(DISTINCT s.id) AS short_urls_count')
             ->from('tags', 't')
-            ->leftJoin('t', 'short_urls_in_tags', 'st', $tagsSubQb->expr()->eq('st.tag_id', 't.id'))
+            ->{$joiningMethod}('t', 'short_urls_in_tags', 'st', $tagsSubQb->expr()->eq('st.tag_id', 't.id'))
-            ->leftJoin('st', 'short_urls', 's', $tagsSubQb->expr()->eq('st.short_url_id', 's.id'))
+            ->{$joiningMethod}('st', 'short_urls', 's', $tagsSubQb->expr()->eq('st.short_url_id', 's.id'))
             ->groupBy('t.id', 't.name');
 
-        if (! $orderMainQuery) {
+        $searchTerm = $filtering?->searchTerm;
-            $tagsSubQb
+        if ($searchTerm !== null) {
-                ->orderBy('t.name', $orderDir ?? 'ASC')
+            $tagsSubQb->andWhere($tagsSubQb->expr()->like('t.name', $conn->quote('%' . $searchTerm . '%')));
-                ->setMaxResults($filtering?->limit ?? PHP_INT_MAX)
-                ->setFirstResult($filtering?->offset ?? 0);
-            // TODO Check if applying limit/offset ot visits sub-queries is needed with large amounts of tags
         }
 
         $buildVisitsSubQb = static function (bool $excludeBots, string $aggregateAlias) use ($conn) {
@@ -82,14 +83,8 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
         $allVisitsSubQb = $buildVisitsSubQb(false, 'visits');
         $nonBotVisitsSubQb = $buildVisitsSubQb(true, 'non_bot_visits');
 
-        $searchTerm = $filtering?->searchTerm;
+        // Apply API key specification to all sub-queries
-        if ($searchTerm !== null) {
+        $applyApiKeyToNativeQb = static fn (NativeQueryBuilder $qb) =>
-            $tagsSubQb->andWhere($tagsSubQb->expr()->like('t.name', $conn->quote('%' . $searchTerm . '%')));
-            // TODO Check if applying this to all sub-queries makes it faster or slower
-        }
-
-        $apiKey = $filtering?->apiKey;
-        $applyApiKeyToNativeQb = static fn (?ApiKey $apiKey, NativeQueryBuilder $qb) =>
             $apiKey?->mapRoles(static fn (Role $role, array $meta) => match ($role) {
                 Role::DOMAIN_SPECIFIC => $qb->andWhere(
                     $qb->expr()->eq('s.domain_id', $conn->quote(Role::domainIdFromMeta($meta))),
@@ -98,11 +93,7 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
                     $qb->expr()->eq('s.author_api_key_id', $conn->quote($apiKey->getId())),
                 ),
             });
-
+        each([$tagsSubQb, $allVisitsSubQb, $nonBotVisitsSubQb], $applyApiKeyToNativeQb);
-        // Apply API key specification to all sub-queries
-        $applyApiKeyToNativeQb($apiKey, $tagsSubQb);
-        $applyApiKeyToNativeQb($apiKey, $allVisitsSubQb);
-        $applyApiKeyToNativeQb($apiKey, $nonBotVisitsSubQb);
 
         // A native query builder needs to be used here, because DQL and ORM query builders do not support
         // sub-queries at "from" and "join" level.
@@ -110,7 +101,6 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
         $mainQb = $conn->createQueryBuilder();
         $mainQb
             ->select(
-                't.id AS id',
                 't.name AS name',
                 'COALESCE(v.visits, 0) AS visits', // COALESCE required for postgres to properly order
                 'COALESCE(v2.non_bot_visits, 0) AS non_bot_visits',
@@ -121,18 +111,20 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
             ->leftJoin('t', '(' . $nonBotVisitsSubQb->getSQL() . ')', 'v2', $mainQb->expr()->eq(
                 't.id',
                 'v2.tag_id',
-            ));
+            ))
+            ->setMaxResults($filtering?->limit ?? PHP_INT_MAX)
+            ->setFirstResult($filtering?->offset ?? 0);
 
-        if ($orderMainQuery) {
+        $orderByTag = $orderField == null || $orderField === OrderableField::TAG->value;
+        if ($orderByTag) {
+            $mainQb->orderBy('t.name', $orderDir ?? 'ASC');
+        } else {
             $mainQb
                 ->orderBy(OrderableField::toSnakeCaseValidField($orderField), $orderDir ?? 'ASC')
-                ->setMaxResults($filtering?->limit ?? PHP_INT_MAX)
+                // Add ordering by tag name, as a fallback in case of same amount
-                ->setFirstResult($filtering?->offset ?? 0);
+                ->addOrderBy('t.name', 'ASC');
         }
 
-        // Add ordering by tag name, as a fallback in case of same amount, or as default ordering
-        $mainQb->addOrderBy('t.name', $orderMainQuery || $orderDir === null ? 'ASC' : $orderDir);
-
         $rsm = new ResultSetMappingBuilder($this->getEntityManager());
         $rsm->addScalarResult('name', 'tag');
         $rsm->addScalarResult('visits', 'visits');