Use pre-calculated visits counts when listing short URLs

module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php

@@ -13,6 +13,7 @@ use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait;
 use Shlinkio\Shlink\Common\Rest\DataTransformerInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
 use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode;
 use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlsParamsInputFilter;
 use Shlinkio\Shlink\Core\ShortUrl\ShortUrlListServiceInterface;
@@ -23,10 +24,10 @@ use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Style\SymfonyStyle;
 
 use function array_keys;
-use function array_map;
 use function array_pad;
 use function explode;
 use function implode;
+use function Shlinkio\Shlink\Core\ArrayUtils\map;
 use function sprintf;
 
 class ListShortUrlsCommand extends Command
@@ -184,10 +185,10 @@ class ListShortUrlsCommand extends Command
     ): Paginator {
         $shortUrls = $this->shortUrlService->listShortUrls($params);
 
-        $rows = array_map(function (ShortUrl $shortUrl) use ($columnsMap) {
+        $rows = map([...$shortUrls], function (ShortUrlWithVisitsSummary $shortUrl) use ($columnsMap) {
             $rawShortUrl = $this->transformer->transform($shortUrl);
-            return array_map(fn (callable $call) => $call($rawShortUrl, $shortUrl), $columnsMap);
-        }, [...$shortUrls]);
+            return map($columnsMap, fn (callable $call) => $call($rawShortUrl, $shortUrl));
+        });
 
         ShlinkTable::default($output)->render(
             array_keys($columnsMap),

module/Core/src/ShortUrl/Entity/ShortUrl.php

@@ -256,7 +256,7 @@ class ShortUrl extends AbstractEntity
         return true;
     }
 
-    public function toArray(): array
+    public function toArray(?VisitsSummary $precalculatedSummary = null): array
     {
         return [
             'shortCode' => $this->shortCode,
@@ -272,7 +272,7 @@ class ShortUrl extends AbstractEntity
             'title' => $this->title,
             'crawlable' => $this->crawlable,
             'forwardQuery' => $this->forwardQuery,
-            'visitsSummary' => VisitsSummary::fromTotalAndNonBots(
+            'visitsSummary' => $precalculatedSummary ?? VisitsSummary::fromTotalAndNonBots(
                 count($this->visits),
                 count($this->visits->matching(
                     Criteria::create()->where(Criteria::expr()->eq('potentialBot', false)),

module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php

@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\ShortUrl\Model;
+
+use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\Visit\Model\VisitsSummary;
+
+final readonly class ShortUrlWithVisitsSummary
+{
+    private function __construct(public ShortUrl $shortUrl, public VisitsSummary $visitsSummary)
+    {
+    }
+
+    /**
+     * @param array{shortUrl: ShortUrl, visitsCount: string|int, nonBotVisitsCount: string|int} $data
+     */
+    public static function fromArray(array $data): self
+    {
+        return new self($data['shortUrl'], VisitsSummary::fromTotalAndNonBots(
+            (int) $data['visitsCount'],
+            (int) $data['nonBotVisitsCount'],
+        ));
+    }
+
+    public function toArray(): array
+    {
+        return $this->shortUrl->toArray($this->visitsSummary);
+    }
+}

module/Core/src/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapter.php

@@ -11,13 +11,13 @@ use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering;
 use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlListRepositoryInterface;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
-class ShortUrlRepositoryAdapter implements AdapterInterface
+readonly class ShortUrlRepositoryAdapter implements AdapterInterface
 {
     public function __construct(
-        private readonly ShortUrlListRepositoryInterface $repository,
-        private readonly ShortUrlsParams $params,
-        private readonly ?ApiKey $apiKey,
-        private readonly string $defaultDomain,
+        private ShortUrlListRepositoryInterface $repository,
+        private ShortUrlsParams $params,
+        private ?ApiKey $apiKey,
+        private string $defaultDomain,
     ) {
     }
 

module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php

@@ -11,34 +11,39 @@ use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository;
 use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType;
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Model\OrderableField;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
 use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering;
 use Shlinkio\Shlink\Core\Visit\Entity\Visit;
 
-use function array_column;
+use function Shlinkio\Shlink\Core\ArrayUtils\map;
 use function sprintf;
 
 class ShortUrlListRepository extends EntitySpecificationRepository implements ShortUrlListRepositoryInterface
 {
     /**
-     * @return ShortUrl[]
+     * @return ShortUrlWithVisitsSummary[]
      */
     public function findList(ShortUrlsListFiltering $filtering): array
     {
         $qb = $this->createListQueryBuilder($filtering);
-        $qb->select('DISTINCT s')
+        $qb->select('DISTINCT s AS shortUrl', 'SUM(v.count) AS visitsCount', 'SUM(v2.count) AS nonBotVisitsCount')
+           ->addSelect('SUM(v.count)')
+           ->leftJoin('s.visitsCounts', 'v')
+           ->leftJoin('s.visitsCounts', 'v2', Join::WITH, $qb->expr()->andX(
+               $qb->expr()->eq('v.shortUrl', 's'),
+               $qb->expr()->eq('v.potentialBot', 'false'),
+           ))
+           ->groupBy('s')
            ->setMaxResults($filtering->limit)
            ->setFirstResult($filtering->offset);
 
         $this->processOrderByForList($qb, $filtering);
 
+        /** @var array{shortUrl: ShortUrl, visitsCount: string, nonBotVisitsCount: string}[] $result */
         $result = $qb->getQuery()->getResult();
-        if (OrderableField::isVisitsField($filtering->orderBy->field ?? '')) {
-            return array_column($result, 0);
-        }
-
-        return $result;
+        return map($result, static fn (array $s) => ShortUrlWithVisitsSummary::fromArray($s));
     }
 
     private function processOrderByForList(QueryBuilder $qb, ShortUrlsListFiltering $filtering): void
@@ -51,26 +56,12 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh
         }
 
         $order = $filtering->orderBy->direction;
-
         if (OrderableField::isBasicField($fieldName)) {
             $qb->orderBy('s.' . $fieldName, $order);
-        } elseif (OrderableField::isVisitsField($fieldName)) {
-            $leftJoinConditions = [$qb->expr()->eq('v.shortUrl', 's')];
-            if ($fieldName === OrderableField::NON_BOT_VISITS->value) {
-                $leftJoinConditions[] = $qb->expr()->eq('v.potentialBot', 'false');
-            }
-
-            $qb->addSelect('SUM(v.count)')
-               ->leftJoin('s.visitsCounts', 'v', Join::WITH, $qb->expr()->andX(...$leftJoinConditions))
-               ->groupBy('s')
-               ->orderBy('SUM(v.count)', $order);
-
-            // FIXME This query is inefficient.
-            //       Diagnostic: It might need to use a sub-query, as done with the tags list query.
-//            $qb->addSelect('COUNT(DISTINCT v)')
-//               ->leftJoin('s.visits', 'v', Join::WITH, $qb->expr()->andX(...$leftJoinConditions))
-//               ->groupBy('s')
-//               ->orderBy('COUNT(DISTINCT v)', $order);
+        } elseif (OrderableField::VISITS->value === $fieldName) {
+            $qb->orderBy('SUM(v.count)', $order);
+        } elseif (OrderableField::NON_BOT_VISITS->value === $fieldName) {
+            $qb->orderBy('SUM(v2.count)', $order);
         }
     }
 

module/Core/src/ShortUrl/Repository/ShortUrlListRepositoryInterface.php

@@ -4,14 +4,14 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core\ShortUrl\Repository;
 
-use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering;
 
 interface ShortUrlListRepositoryInterface
 {
     /**
-     * @return ShortUrl[]
+     * @return ShortUrlWithVisitsSummary[]
      */
     public function findList(ShortUrlsListFiltering $filtering): array;
 

module/Core/src/ShortUrl/ShortUrlListService.php

@@ -6,22 +6,22 @@ namespace Shlinkio\Shlink\Core\ShortUrl;
 
 use Shlinkio\Shlink\Common\Paginator\Paginator;
 use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
-use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
 use Shlinkio\Shlink\Core\ShortUrl\Paginator\Adapter\ShortUrlRepositoryAdapter;
 use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlListRepositoryInterface;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
-class ShortUrlListService implements ShortUrlListServiceInterface
+readonly class ShortUrlListService implements ShortUrlListServiceInterface
 {
     public function __construct(
-        private readonly ShortUrlListRepositoryInterface $repo,
-        private readonly UrlShortenerOptions $urlShortenerOptions,
+        private ShortUrlListRepositoryInterface $repo,
+        private UrlShortenerOptions $urlShortenerOptions,
     ) {
     }
 
     /**
-     * @return ShortUrl[]|Paginator
+     * @return ShortUrlWithVisitsSummary[]|Paginator
      */
     public function listShortUrls(ShortUrlsParams $params, ?ApiKey $apiKey = null): Paginator
     {

module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php

@@ -7,7 +7,11 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Transformer;
 use Shlinkio\Shlink\Common\Rest\DataTransformerInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
 
+/**
+ * @fixme Do not implement DataTransformerInterface, but a separate interface
+ */
 readonly class ShortUrlDataTransformer implements DataTransformerInterface
 {
     public function __construct(private ShortUrlStringifierInterface $stringifier)
@@ -15,13 +19,14 @@ readonly class ShortUrlDataTransformer implements DataTransformerInterface
     }
 
     /**
-     * @param ShortUrl $shortUrl
+     * @param ShortUrlWithVisitsSummary|ShortUrl $data
      */
-    public function transform($shortUrl): array // phpcs:ignore
+    public function transform($data): array // phpcs:ignore
     {
+        $shortUrl = $data instanceof ShortUrlWithVisitsSummary ? $data->shortUrl : $data;
         return [
             'shortUrl' => $this->stringifier->stringify($shortUrl),
-            ...$shortUrl->toArray(),
+            ...$data->toArray(),
         ];
     }
 }