From 17d37a062a010124ff39a84388a475cee4b64343 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Wed, 20 Mar 2024 08:33:52 +0100
Subject: [PATCH 01/19] Add new table to track short URL visits counts

---
 ...o.Shlink.Core.ShortUrl.Entity.ShortUrl.php |  5 ++
 ....Core.Visit.Entity.ShortUrlVisitsCount.php | 41 ++++++++++++
 .../Core/migrations/Version20240306132518.php | 63 +++++++++++++++++++
 .../Core/migrations/Version20240318084804.php | 59 +++++++++++++++++
 module/Core/src/ShortUrl/Entity/ShortUrl.php  | 18 +++---
 .../Repository/ShortUrlListRepository.php     | 13 ++--
 .../src/Visit/Entity/ShortUrlVisitsCount.php  | 19 ++++++
 .../PublishingUpdatesGeneratorTest.php        | 14 ++++-
 8 files changed, 216 insertions(+), 16 deletions(-)
 create mode 100644 module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.ShortUrlVisitsCount.php
 create mode 100644 module/Core/migrations/Version20240306132518.php
 create mode 100644 module/Core/migrations/Version20240318084804.php
 create mode 100644 module/Core/src/Visit/Entity/ShortUrlVisitsCount.php

diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php
index 358ee6bd..b159da13 100644
--- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php
+++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php
@@ -67,6 +67,11 @@ return static function (ClassMetadata $metadata, array $emConfig): void {
             ->fetchExtraLazy()
             ->build();
 
+    $builder->createOneToMany('visitsCounts', Visit\Entity\ShortUrlVisitsCount::class)
+            ->mappedBy('shortUrl')
+            ->fetchExtraLazy() // TODO Check if this makes sense
+            ->build();
+
     $builder->createManyToMany('tags', Tag\Entity\Tag::class)
             ->setJoinTable(determineTableName('short_urls_in_tags', $emConfig))
             ->addInverseJoinColumn('tag_id', 'id', onDelete: 'CASCADE')
diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.ShortUrlVisitsCount.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.ShortUrlVisitsCount.php
new file mode 100644
index 00000000..f65be80a
--- /dev/null
+++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.ShortUrlVisitsCount.php
@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core;
+
+use Doctrine\DBAL\Types\Types;
+use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder;
+use Doctrine\ORM\Mapping\ClassMetadata;
+
+return static function (ClassMetadata $metadata, array $emConfig): void {
+    $builder = new ClassMetadataBuilder($metadata);
+
+    $builder->setTable(determineTableName('short_url_visits_counts', $emConfig));
+
+    $builder->createField('id', Types::BIGINT)
+            ->columnName('id')
+            ->makePrimaryKey()
+            ->generatedValue('IDENTITY')
+            ->option('unsigned', true)
+            ->build();
+
+    $builder->createField('potentialBot', Types::BOOLEAN)
+            ->columnName('potential_bot')
+            ->option('default', false)
+            ->build();
+
+    $builder->createField('count', Types::BIGINT)
+            ->columnName('count')
+            ->option('unsigned', true)
+            ->build();
+
+    $builder->createField('slotId', Types::INTEGER)
+            ->columnName('slot_id')
+            ->option('unsigned', true)
+            ->build();
+
+    $builder->createManyToOne('shortUrl', ShortUrl\Entity\ShortUrl::class)
+            ->addJoinColumn('short_url_id', 'id', onDelete: 'CASCADE')
+            ->build();
+};
diff --git a/module/Core/migrations/Version20240306132518.php b/module/Core/migrations/Version20240306132518.php
new file mode 100644
index 00000000..21847b81
--- /dev/null
+++ b/module/Core/migrations/Version20240306132518.php
@@ -0,0 +1,63 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkMigrations;
+
+use Doctrine\DBAL\Platforms\MySQLPlatform;
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\DBAL\Types\Types;
+use Doctrine\Migrations\AbstractMigration;
+
+/**
+ * Create the new short_url_visits_counts table that will track visit counts per short URL
+ */
+final class Version20240306132518 extends AbstractMigration
+{
+    public function up(Schema $schema): void
+    {
+        $this->skipIf($schema->hasTable('short_url_visits_counts'));
+
+        $table = $schema->createTable('short_url_visits_counts');
+        $table->addColumn('id', Types::BIGINT, [
+            'unsigned' => true,
+            'autoincrement' => true,
+            'notnull' => true,
+        ]);
+        $table->setPrimaryKey(['id']);
+
+        $table->addColumn('short_url_id', Types::BIGINT, [
+            'unsigned' => true,
+            'notnull' => true,
+        ]);
+        $table->addForeignKeyConstraint('short_urls', ['short_url_id'], ['id'], [
+            'onDelete' => 'CASCADE',
+            'onUpdate' => 'RESTRICT',
+        ]);
+
+        $table->addColumn('potential_bot', Types::BOOLEAN, ['default' => false]);
+
+        $table->addColumn('slot_id', Types::INTEGER, [
+            'unsigned' => true,
+            'notnull' => true,
+            'default' => 1,
+        ]);
+
+        $table->addColumn('count', Types::BIGINT, [
+            'unsigned' => true,
+            'notnull' => true,
+            'default' => 1,
+        ]);
+    }
+
+    public function down(Schema $schema): void
+    {
+        $this->skipIf(! $schema->hasTable('short_url_visits_counts'));
+        $schema->dropTable('short_url_visits_counts');
+    }
+
+    public function isTransactional(): bool
+    {
+        return ! ($this->connection->getDatabasePlatform() instanceof MySQLPlatform);
+    }
+}
diff --git a/module/Core/migrations/Version20240318084804.php b/module/Core/migrations/Version20240318084804.php
new file mode 100644
index 00000000..6b906107
--- /dev/null
+++ b/module/Core/migrations/Version20240318084804.php
@@ -0,0 +1,59 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkMigrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+/**
+ * Create initial entries in the short_url_visits_counts table for existing visits
+ */
+final class Version20240318084804 extends AbstractMigration
+{
+    public function up(Schema $schema): void
+    {
+        $qb = $this->connection->createQueryBuilder();
+        $result = $qb->select('id')
+                     ->from('short_urls')
+                     ->executeQuery();
+
+        while ($shortUrlId = $result->fetchOne()) {
+            $visitsQb = $this->connection->createQueryBuilder();
+            $visitsQb->select('COUNT(id)')
+                     ->from('visits')
+                     ->where($visitsQb->expr()->eq('short_url_id', ':short_url_id'))
+                     ->andWhere($visitsQb->expr()->eq('potential_bot', ':potential_bot'))
+                     ->setParameter('short_url_id', $shortUrlId);
+
+            $botsCount = $visitsQb->setParameter('potential_bot', '1')->executeQuery()->fetchOne();
+            $nonBotsCount = $visitsQb->setParameter('potential_bot', '0')->executeQuery()->fetchOne();
+
+            $this->connection->createQueryBuilder()
+                 ->insert('short_url_visits_counts')
+                 ->values([
+                     'short_url_id' => ':short_url_id',
+                     'count' => ':count',
+                     'potential_bot' => '1',
+                 ])
+                 ->setParameters([
+                     'short_url_id' => $shortUrlId,
+                     'count' => $botsCount,
+                 ])
+                 ->executeStatement();
+            $this->connection->createQueryBuilder()
+                 ->insert('short_url_visits_counts')
+                 ->values([
+                     'short_url_id' => ':short_url_id',
+                     'count' => ':count',
+                     'potential_bot' => '0',
+                 ])
+                 ->setParameters([
+                     'short_url_id' => $shortUrlId,
+                     'count' => $nonBotsCount,
+                 ])
+                 ->executeStatement();
+        }
+    }
+}
diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php
index 474d5afc..cc7ebc85 100644
--- a/module/Core/src/ShortUrl/Entity/ShortUrl.php
+++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php
@@ -19,6 +19,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\SimpleShortUrlRelationResolver;
 use Shlinkio\Shlink\Core\Tag\Entity\Tag;
+use Shlinkio\Shlink\Core\Visit\Entity\ShortUrlVisitsCount;
 use Shlinkio\Shlink\Core\Visit\Entity\Visit;
 use Shlinkio\Shlink\Core\Visit\Model\VisitsSummary;
 use Shlinkio\Shlink\Core\Visit\Model\VisitType;
@@ -37,6 +38,7 @@ class ShortUrl extends AbstractEntity
     /**
      * @param Collection<int, Tag> $tags
      * @param Collection<int, Visit> & Selectable $visits
+     * @param Collection<int, ShortUrlVisitsCount> & Selectable $visitsCounts
      */
     private function __construct(
         private string $longUrl,
@@ -44,6 +46,7 @@ class ShortUrl extends AbstractEntity
         private Chronos $dateCreated = new Chronos(),
         private Collection $tags = new ArrayCollection(),
         private Collection & Selectable $visits = new ArrayCollection(),
+        private Collection & Selectable $visitsCounts = new ArrayCollection(),
         private ?Chronos $validSince = null,
         private ?Chronos $validUntil = null,
         private ?int $maxVisits = null,
@@ -179,16 +182,16 @@ class ShortUrl extends AbstractEntity
         return $this->shortCode;
     }
 
-    public function getDateCreated(): Chronos
-    {
-        return $this->dateCreated;
-    }
-
     public function getDomain(): ?Domain
     {
         return $this->domain;
     }
 
+    public function forwardQuery(): bool
+    {
+        return $this->forwardQuery;
+    }
+
     public function reachedVisits(int $visitsAmount): bool
     {
         return count($this->visits) >= $visitsAmount;
@@ -214,11 +217,6 @@ class ShortUrl extends AbstractEntity
         return $this;
     }
 
-    public function forwardQuery(): bool
-    {
-        return $this->forwardQuery;
-    }
-
     /**
      * @throws ShortCodeCannotBeRegeneratedException
      */
diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
index d6f7e421..2f638c73 100644
--- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
+++ b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
@@ -60,12 +60,17 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh
                 $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);
+//            $qb->addSelect('COUNT(DISTINCT v)')
+//               ->leftJoin('s.visits', 'v', Join::WITH, $qb->expr()->andX(...$leftJoinConditions))
+//               ->groupBy('s')
+//               ->orderBy('COUNT(DISTINCT v)', $order);
         }
     }
 
diff --git a/module/Core/src/Visit/Entity/ShortUrlVisitsCount.php b/module/Core/src/Visit/Entity/ShortUrlVisitsCount.php
new file mode 100644
index 00000000..ff3580b3
--- /dev/null
+++ b/module/Core/src/Visit/Entity/ShortUrlVisitsCount.php
@@ -0,0 +1,19 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Visit\Entity;
+
+use Shlinkio\Shlink\Common\Entity\AbstractEntity;
+use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
+
+class ShortUrlVisitsCount extends AbstractEntity
+{
+    public function __construct(
+        private readonly ShortUrl $shortUrl,
+        private readonly bool $potentialBot = false,
+        private readonly int $slotId = 1,
+        private readonly string $count = '1',
+    ) {
+    }
+}
diff --git a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php
index 032c3263..2dd1c0c7 100644
--- a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php
+++ b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php
@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace ShlinkioTest\Shlink\Core\EventDispatcher;
 
+use Cake\Chronos\Chronos;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\TestCase;
@@ -22,14 +23,23 @@ use Shlinkio\Shlink\Core\Visit\Model\VisitType;
 class PublishingUpdatesGeneratorTest extends TestCase
 {
     private PublishingUpdatesGenerator $generator;
+    private Chronos $now;
 
     protected function setUp(): void
     {
+        $this->now = Chronos::now();
+        Chronos::setTestNow($this->now);
+
         $this->generator = new PublishingUpdatesGenerator(
             new ShortUrlDataTransformer(new ShortUrlStringifier([])),
         );
     }
 
+    protected function tearDown(): void
+    {
+        Chronos::setTestNow();
+    }
+
     #[Test, DataProvider('provideMethod')]
     public function visitIsProperlySerializedIntoUpdate(string $method, string $expectedTopic, ?string $title): void
     {
@@ -49,7 +59,7 @@ class PublishingUpdatesGeneratorTest extends TestCase
                 'shortCode' => $shortUrl->getShortCode(),
                 'shortUrl' => 'http:/' . $shortUrl->getShortCode(),
                 'longUrl' => 'https://longUrl',
-                'dateCreated' => $shortUrl->getDateCreated()->toAtomString(),
+                'dateCreated' => $this->now->toAtomString(),
                 'tags' => [],
                 'meta' => [
                     'validSince' => null,
@@ -123,7 +133,7 @@ class PublishingUpdatesGeneratorTest extends TestCase
             'shortCode' => $shortUrl->getShortCode(),
             'shortUrl' => 'http:/' . $shortUrl->getShortCode(),
             'longUrl' => 'https://longUrl',
-            'dateCreated' => $shortUrl->getDateCreated()->toAtomString(),
+            'dateCreated' => $this->now->toAtomString(),
             'tags' => [],
             'meta' => [
                 'validSince' => null,

From f678873e9fc8e5b7646a83ee6ccce276a8a5d358 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Thu, 21 Mar 2024 08:47:39 +0100
Subject: [PATCH 02/19] Use pre-calculated visits counts when listing short
 URLs

---
 .../Command/ShortUrl/ListShortUrlsCommand.php |  9 ++--
 module/Core/src/ShortUrl/Entity/ShortUrl.php  |  4 +-
 .../Model/ShortUrlWithVisitsSummary.php       | 31 +++++++++++++
 .../Adapter/ShortUrlRepositoryAdapter.php     | 10 ++---
 .../Repository/ShortUrlListRepository.php     | 43 ++++++++-----------
 .../ShortUrlListRepositoryInterface.php       |  4 +-
 .../Core/src/ShortUrl/ShortUrlListService.php | 10 ++---
 .../Transformer/ShortUrlDataTransformer.php   | 11 +++--
 8 files changed, 75 insertions(+), 47 deletions(-)
 create mode 100644 module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php

diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php
index c4346f14..b03e0312 100644
--- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php
+++ b/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),
diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php
index cc7ebc85..ac21b34b 100644
--- a/module/Core/src/ShortUrl/Entity/ShortUrl.php
+++ b/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)),
diff --git a/module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php b/module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php
new file mode 100644
index 00000000..8244cde4
--- /dev/null
+++ b/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);
+    }
+}
diff --git a/module/Core/src/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapter.php b/module/Core/src/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapter.php
index 83ce8bd9..56f8e5a5 100644
--- a/module/Core/src/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapter.php
+++ b/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,
     ) {
     }
 
diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
index 2f638c73..0bcf7974 100644
--- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
+++ b/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);
         }
     }
 
diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepositoryInterface.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepositoryInterface.php
index 130e0db7..db3f8017 100644
--- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepositoryInterface.php
+++ b/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;
 
diff --git a/module/Core/src/ShortUrl/ShortUrlListService.php b/module/Core/src/ShortUrl/ShortUrlListService.php
index 60f56554..d86c4988 100644
--- a/module/Core/src/ShortUrl/ShortUrlListService.php
+++ b/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
     {
diff --git a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php
index 09b9436b..413f5a69 100644
--- a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php
+++ b/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(),
         ];
     }
 }

From 3c89d252d26618f7c78ca1bd78868757dcc85891 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Thu, 21 Mar 2024 08:56:13 +0100
Subject: [PATCH 03/19] Simplify logic to match order by for short URL lists

---
 .../Repository/ShortUrlListRepository.php     | 21 +++++++------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
index 0bcf7974..32bcdc28 100644
--- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
+++ b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
@@ -48,21 +48,16 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh
 
     private function processOrderByForList(QueryBuilder $qb, ShortUrlsListFiltering $filtering): void
     {
-        // With no explicit order by, fallback to dateCreated-DESC
         $fieldName = $filtering->orderBy->field;
-        if ($fieldName === null) {
-            $qb->orderBy('s.dateCreated', 'DESC');
-            return;
-        }
-
         $order = $filtering->orderBy->direction;
-        if (OrderableField::isBasicField($fieldName)) {
-            $qb->orderBy('s.' . $fieldName, $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);
-        }
+
+        match (true) {
+            // With no explicit order by, fallback to dateCreated-DESC
+            $fieldName === null => $qb->orderBy('s.dateCreated', 'DESC'),
+            $fieldName === OrderableField::VISITS->value => $qb->orderBy('SUM(v.count)', $order),
+            $fieldName === OrderableField::NON_BOT_VISITS->value => $qb->orderBy('SUM(v2.count)', $order),
+            default => $qb->orderBy('s.' . $fieldName, $order),
+        };
     }
 
     public function countList(ShortUrlsCountFiltering $filtering): int

From 7d415e40b2e9f3b0bce1cc393b453fc03ca88035 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Fri, 22 Mar 2024 08:45:52 +0100
Subject: [PATCH 04/19] Add unique index in short_url_visits_counts

---
 module/Core/migrations/Version20240306132518.php | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/module/Core/migrations/Version20240306132518.php b/module/Core/migrations/Version20240306132518.php
index 21847b81..a2960e9b 100644
--- a/module/Core/migrations/Version20240306132518.php
+++ b/module/Core/migrations/Version20240306132518.php
@@ -48,6 +48,8 @@ final class Version20240306132518 extends AbstractMigration
             'notnull' => true,
             'default' => 1,
         ]);
+
+        $table->addUniqueIndex(['short_url_id', 'potential_bot', 'slot_id'], 'UQ_slot_per_short_url');
     }
 
     public function down(Schema $schema): void

From 7afd3fd6a2cb883e37b4a71d7b240b26bc63d099 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Mon, 25 Mar 2024 19:22:13 +0100
Subject: [PATCH 05/19] Load visits and nonBotVisits via sub-queries in
 ShortUrlListRepository

---
 ....Core.Visit.Entity.ShortUrlVisitsCount.php |  2 +
 .../Core/migrations/Version20240318084804.php | 47 +++++++++----------
 .../Model/ShortUrlWithVisitsSummary.php       |  6 +--
 .../Repository/ShortUrlListRepository.php     | 37 ++++++++++-----
 4 files changed, 53 insertions(+), 39 deletions(-)

diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.ShortUrlVisitsCount.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.ShortUrlVisitsCount.php
index f65be80a..8e06f5c0 100644
--- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.ShortUrlVisitsCount.php
+++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.ShortUrlVisitsCount.php
@@ -38,4 +38,6 @@ return static function (ClassMetadata $metadata, array $emConfig): void {
     $builder->createManyToOne('shortUrl', ShortUrl\Entity\ShortUrl::class)
             ->addJoinColumn('short_url_id', 'id', onDelete: 'CASCADE')
             ->build();
+
+    $builder->addUniqueConstraint(['short_url_id', 'potential_bot', 'slot_id'], 'UQ_slot_per_short_url');
 };
diff --git a/module/Core/migrations/Version20240318084804.php b/module/Core/migrations/Version20240318084804.php
index 6b906107..a9501a2a 100644
--- a/module/Core/migrations/Version20240318084804.php
+++ b/module/Core/migrations/Version20240318084804.php
@@ -30,30 +30,29 @@ final class Version20240318084804 extends AbstractMigration
             $botsCount = $visitsQb->setParameter('potential_bot', '1')->executeQuery()->fetchOne();
             $nonBotsCount = $visitsQb->setParameter('potential_bot', '0')->executeQuery()->fetchOne();
 
-            $this->connection->createQueryBuilder()
-                 ->insert('short_url_visits_counts')
-                 ->values([
-                     'short_url_id' => ':short_url_id',
-                     'count' => ':count',
-                     'potential_bot' => '1',
-                 ])
-                 ->setParameters([
-                     'short_url_id' => $shortUrlId,
-                     'count' => $botsCount,
-                 ])
-                 ->executeStatement();
-            $this->connection->createQueryBuilder()
-                 ->insert('short_url_visits_counts')
-                 ->values([
-                     'short_url_id' => ':short_url_id',
-                     'count' => ':count',
-                     'potential_bot' => '0',
-                 ])
-                 ->setParameters([
-                     'short_url_id' => $shortUrlId,
-                     'count' => $nonBotsCount,
-                 ])
-                 ->executeStatement();
+            if ($botsCount > 0) {
+                $this->insertCount($shortUrlId, $botsCount, potentialBot: true);
+            }
+            if ($nonBotsCount > 0) {
+                $this->insertCount($shortUrlId, $nonBotsCount, potentialBot: false);
+            }
         }
     }
+
+    private function insertCount(string $shortUrlId, int $count, bool $potentialBot): void
+    {
+        $this->connection->createQueryBuilder()
+            ->insert('short_url_visits_counts')
+            ->values([
+                'short_url_id' => ':short_url_id',
+                'count' => ':count',
+                'potential_bot' => ':potential_bot',
+            ])
+            ->setParameters([
+                'short_url_id' => $shortUrlId,
+                'count' => $count,
+                'potential_bot' => $potentialBot ? '1' : '0',
+            ])
+            ->executeStatement();
+    }
 }
diff --git a/module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php b/module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php
index 8244cde4..79bdb526 100644
--- a/module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php
+++ b/module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php
@@ -14,13 +14,13 @@ final readonly class ShortUrlWithVisitsSummary
     }
 
     /**
-     * @param array{shortUrl: ShortUrl, visitsCount: string|int, nonBotVisitsCount: string|int} $data
+     * @param array{shortUrl: ShortUrl, visits: string|int, nonBotVisits: string|int} $data
      */
     public static function fromArray(array $data): self
     {
         return new self($data['shortUrl'], VisitsSummary::fromTotalAndNonBots(
-            (int) $data['visitsCount'],
-            (int) $data['nonBotVisitsCount'],
+            (int) $data['visits'],
+            (int) $data['nonBotVisits'],
         ));
     }
 
diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
index 32bcdc28..0c0c3df3 100644
--- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
+++ b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
@@ -15,6 +15,7 @@ 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\ShortUrlVisitsCount;
 use Shlinkio\Shlink\Core\Visit\Entity\Visit;
 
 use function Shlinkio\Shlink\Core\ArrayUtils\map;
@@ -27,21 +28,33 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh
      */
     public function findList(ShortUrlsListFiltering $filtering): array
     {
+        $buildVisitsSubQuery = function (string $alias, bool $excludingBots): string {
+            $vqb = $this->getEntityManager()->createQueryBuilder();
+            $vqb->select('SUM(' . $alias . '.count)')
+                ->from(ShortUrlVisitsCount::class, $alias)
+                ->where($vqb->expr()->eq($alias . '.shortUrl', 's'));
+
+            if ($excludingBots) {
+                $vqb->andWhere($vqb->expr()->eq($alias . '.potentialBot', ':potentialBot'));
+            }
+
+            return $vqb->getDQL();
+        };
+
         $qb = $this->createListQueryBuilder($filtering);
-        $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')
+        $qb->select(
+            'DISTINCT s AS shortUrl',
+            '(' . $buildVisitsSubQuery('v', excludingBots: false) . ') AS ' . OrderableField::VISITS->value,
+            '(' . $buildVisitsSubQuery('v2', excludingBots: true) . ') AS ' . OrderableField::NON_BOT_VISITS->value,
+        )
            ->setMaxResults($filtering->limit)
-           ->setFirstResult($filtering->offset);
+           ->setFirstResult($filtering->offset)
+           // This param is used in one of the sub-queries, but needs to set in the parent query
+           ->setParameter('potentialBot', 0);
 
         $this->processOrderByForList($qb, $filtering);
 
-        /** @var array{shortUrl: ShortUrl, visitsCount: string, nonBotVisitsCount: string}[] $result */
+        /** @var array{shortUrl: ShortUrl, visits: string, nonBotVisits: string}[] $result */
         $result = $qb->getQuery()->getResult();
         return map($result, static fn (array $s) => ShortUrlWithVisitsSummary::fromArray($s));
     }
@@ -54,8 +67,8 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh
         match (true) {
             // With no explicit order by, fallback to dateCreated-DESC
             $fieldName === null => $qb->orderBy('s.dateCreated', 'DESC'),
-            $fieldName === OrderableField::VISITS->value => $qb->orderBy('SUM(v.count)', $order),
-            $fieldName === OrderableField::NON_BOT_VISITS->value => $qb->orderBy('SUM(v2.count)', $order),
+            $fieldName === OrderableField::VISITS->value,
+            $fieldName === OrderableField::NON_BOT_VISITS->value => $qb->orderBy($fieldName, $order),
             default => $qb->orderBy('s.' . $fieldName, $order),
         };
     }

From 6074f4475dd9fd7b30c608067897dc44bb757f18 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Tue, 26 Mar 2024 08:56:06 +0100
Subject: [PATCH 06/19] Add preFlush listener to track visits counts

---
 config/autoload/entity-manager.global.php     |   7 +
 module/Core/config/dependencies.config.php    |   1 +
 .../ShortUrlVisitsCountPreFlushListener.php   | 146 ++++++++++++++++++
 3 files changed, 154 insertions(+)
 create mode 100644 module/Core/src/Visit/Listener/ShortUrlVisitsCountPreFlushListener.php

diff --git a/config/autoload/entity-manager.global.php b/config/autoload/entity-manager.global.php
index 3eb43edf..eae7e8a9 100644
--- a/config/autoload/entity-manager.global.php
+++ b/config/autoload/entity-manager.global.php
@@ -2,9 +2,11 @@
 
 declare(strict_types=1);
 
+use Doctrine\ORM\Events;
 use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository;
 use Shlinkio\Shlink\Core\Config\EnvVars;
 
+use Shlinkio\Shlink\Core\Visit\Listener\ShortUrlVisitsCountPreFlushListener;
 use function Shlinkio\Shlink\Core\ArrayUtils\contains;
 
 return (static function (): array {
@@ -60,6 +62,11 @@ return (static function (): array {
                 'proxies_dir' => 'data/proxies',
                 'load_mappings_using_functional_style' => true,
                 'default_repository_classname' => EntitySpecificationRepository::class,
+                'listeners' => [
+                    Events::preFlush => [
+                        ShortUrlVisitsCountPreFlushListener::class,
+                    ],
+                ],
             ],
             'connection' => $resolveConnection(),
         ],
diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php
index d75c6bb8..7d3bf763 100644
--- a/module/Core/config/dependencies.config.php
+++ b/module/Core/config/dependencies.config.php
@@ -76,6 +76,7 @@ return [
                 EntityRepositoryFactory::class,
                 Visit\Entity\Visit::class,
             ],
+            Visit\Listener\ShortUrlVisitsCountPreFlushListener::class => InvokableFactory::class,
 
             Util\DoctrineBatchHelper::class => ConfigAbstractFactory::class,
             Util\RedirectResponseHelper::class => ConfigAbstractFactory::class,
diff --git a/module/Core/src/Visit/Listener/ShortUrlVisitsCountPreFlushListener.php b/module/Core/src/Visit/Listener/ShortUrlVisitsCountPreFlushListener.php
new file mode 100644
index 00000000..6a006e54
--- /dev/null
+++ b/module/Core/src/Visit/Listener/ShortUrlVisitsCountPreFlushListener.php
@@ -0,0 +1,146 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Visit\Listener;
+
+use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Exception;
+use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
+use Doctrine\DBAL\Platforms\SQLitePlatform;
+use Doctrine\DBAL\Platforms\SQLServerPlatform;
+use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\Event\PreFlushEventArgs;
+use Shlinkio\Shlink\Core\Visit\Entity\Visit;
+
+use function rand;
+
+final readonly class ShortUrlVisitsCountPreFlushListener
+{
+    /**
+     * @throws Exception
+     */
+    public function preFlush(PreFlushEventArgs $args): void
+    {
+        $em = $args->getObjectManager();
+        $entitiesToBeCreated = $em->getUnitOfWork()->getScheduledEntityInsertions();
+
+        foreach ($entitiesToBeCreated as $entity) {
+            $this->trackVisitCount($em, $entity);
+        }
+    }
+
+    /**
+     * @throws Exception
+     */
+    private function trackVisitCount(EntityManagerInterface $em, object $entity): void
+    {
+        // This is not a non-orphan visit
+        if (!$entity instanceof Visit || $entity->shortUrl === null) {
+            return;
+        }
+        $visit = $entity;
+
+        // The short URL is not persisted yet
+        $shortUrlId = $visit->shortUrl->getId();
+        if ($shortUrlId === null || $shortUrlId === '') {
+            return;
+        }
+
+        $isBot = $visit->potentialBot;
+        $conn = $em->getConnection();
+        $platformClass = $conn->getDatabasePlatform();
+
+        match ($platformClass::class) {
+            PostgreSQLPlatform::class => $this->incrementForPostgres($conn, $shortUrlId, $isBot),
+            SQLitePlatform::class, SQLServerPlatform::class => $this->incrementForOthers($conn, $shortUrlId, $isBot),
+            default => $this->incrementForMySQL($conn, $shortUrlId, $isBot),
+        };
+    }
+
+    /**
+     * @throws Exception
+     */
+    private function incrementForMySQL(Connection $conn, string $shortUrlId, bool $potentialBot): void
+    {
+        $this->incrementWithPreparedStatement($conn, $shortUrlId, $potentialBot, <<<QUERY
+            INSERT INTO short_url_visits_counts (short_url_id, potential_bot, slot_id, count)
+            VALUES (:short_url_id, :potential_bot, RAND() * 100, 1)
+            ON DUPLICATE KEY UPDATE count = count + 1;
+            QUERY);
+    }
+
+    /**
+     * @throws Exception
+     */
+    private function incrementForPostgres(Connection $conn, string $shortUrlId, bool $potentialBot): void
+    {
+        $this->incrementWithPreparedStatement($conn, $shortUrlId, $potentialBot, <<<QUERY
+            INSERT INTO short_url_visits_counts (short_url_id, potential_bot, slot_id, count)
+            VALUES (:short_url_id, :potential_bot, random() * 100, 1)
+            ON CONFLICT (short_url_id, potential_bot, slot_id) DO UPDATE
+              SET count = short_url_visits_counts.count + 1;
+            QUERY);
+    }
+
+    /**
+     * @throws Exception
+     */
+    private function incrementWithPreparedStatement(
+        Connection $conn,
+        string $shortUrlId,
+        bool $potentialBot,
+        string $query,
+    ): void {
+        $statement = $conn->prepare($query);
+        $statement->bindValue('short_url_id', $shortUrlId);
+        $statement->bindValue('potential_bot', $potentialBot ? 1 : 0);
+        $statement->executeStatement();
+    }
+
+    /**
+     * @throws Exception
+     */
+    private function incrementForOthers(Connection $conn, string $shortUrlId, bool $potentialBot): void
+    {
+        $slotId = rand(1, 100);
+
+        // For engines without a specific UPSERT syntax, do a regular locked select followed by an insert or update
+        $qb = $conn->createQueryBuilder();
+        $qb->select('id')
+            ->from('short_url_visits_counts')
+            ->where($qb->expr()->and(
+                $qb->expr()->eq('short_url_id', ':short_url_id'),
+                $qb->expr()->eq('potential_bot', ':potential_bot'),
+                $qb->expr()->eq('slot_id', ':slot_id'),
+            ))
+            ->setParameter('short_url_id', $shortUrlId)
+            ->setParameter('potential_bot', $potentialBot)
+            ->setParameter('slot_id', $slotId)
+            ->forUpdate()
+            ->setMaxResults(1);
+
+        $resultSet = $qb->executeQuery()->fetchOne();
+        $writeQb = ! $resultSet
+            ? $conn->createQueryBuilder()
+                ->insert('short_url_visits_counts')
+                ->values([
+                    'short_url_id' => ':short_url_id',
+                    'potential_bot' => ':potential_bot',
+                    'slot_id' => ':slot_id',
+                ])
+            : $conn->createQueryBuilder()
+                ->update('short_url_visits_counts')
+                ->set('count', 'count + 1')
+                ->where($qb->expr()->and(
+                    $qb->expr()->eq('short_url_id', ':short_url_id'),
+                    $qb->expr()->eq('potential_bot', ':potential_bot'),
+                    $qb->expr()->eq('slot_id', ':slot_id'),
+                ));
+
+        $writeQb->setParameter('short_url_id', $shortUrlId)
+            ->setParameter('potential_bot', $potentialBot)
+            ->setParameter('slot_id', $slotId)
+            ->executeStatement();
+    }
+}

From 054eb426136f02a8a26730161912953c81bc916d Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Tue, 26 Mar 2024 09:06:47 +0100
Subject: [PATCH 07/19] Remove no-longer used methods in OrderableField enum

---
 composer.json                                     |  4 ++++
 module/Core/src/ShortUrl/Model/OrderableField.php | 15 ---------------
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/composer.json b/composer.json
index a35dd727..2179a749 100644
--- a/composer.json
+++ b/composer.json
@@ -129,6 +129,10 @@
         "test:db:postgres": "DB_DRIVER=postgres composer test:db:sqlite",
         "test:db:ms": "DB_DRIVER=mssql composer test:db:sqlite",
         "test:api": "bin/test/run-api-tests.sh",
+        "test:api:sqlite": "DB_DRIVER=sqlite composer test:api",
+        "test:api:mysql": "DB_DRIVER=mysql composer test:api",
+        "test:api:maria": "DB_DRIVER=maria composer test:api",
+        "test:api:mssql": "DB_DRIVER=mssql composer test:api",
         "test:api:ci": "GENERATE_COVERAGE=yes composer test:api && vendor/bin/phpcov merge build/coverage-api --php build/coverage-api.cov && rm build/coverage-api/*.cov",
         "test:api:pretty": "GENERATE_COVERAGE=yes composer test:api && vendor/bin/phpcov merge build/coverage-api --html build/coverage-api/coverage-html && rm build/coverage-api/*.cov",
         "test:cli": "APP_ENV=test DB_DRIVER=maria TEST_ENV=cli php vendor/bin/phpunit --order-by=random --colors=always --testdox -c phpunit-cli.xml",
diff --git a/module/Core/src/ShortUrl/Model/OrderableField.php b/module/Core/src/ShortUrl/Model/OrderableField.php
index 685f6f12..a4053742 100644
--- a/module/Core/src/ShortUrl/Model/OrderableField.php
+++ b/module/Core/src/ShortUrl/Model/OrderableField.php
@@ -2,8 +2,6 @@
 
 namespace Shlinkio\Shlink\Core\ShortUrl\Model;
 
-use function Shlinkio\Shlink\Core\ArrayUtils\contains;
-
 enum OrderableField: string
 {
     case LONG_URL = 'longUrl';
@@ -12,17 +10,4 @@ enum OrderableField: string
     case TITLE = 'title';
     case VISITS = 'visits';
     case NON_BOT_VISITS = 'nonBotVisits';
-
-    public static function isBasicField(string $value): bool
-    {
-        return contains(
-            $value,
-            [self::LONG_URL->value, self::SHORT_CODE->value, self::DATE_CREATED->value, self::TITLE->value],
-        );
-    }
-
-    public static function isVisitsField(string $value): bool
-    {
-        return $value === self::VISITS->value || $value === self::NON_BOT_VISITS->value;
-    }
 }

From 6fbb5a380ddab02a1a0d639524710fadaab4f246 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Tue, 26 Mar 2024 09:24:55 +0100
Subject: [PATCH 08/19] Add missing default value for short url visits count

---
 ....Core.Visit.Entity.ShortUrlVisitsCount.php |  1 +
 .../ShortUrlVisitsCountPreFlushListener.php   | 45 ++++++++++---------
 2 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.ShortUrlVisitsCount.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.ShortUrlVisitsCount.php
index 8e06f5c0..d4a8546b 100644
--- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.ShortUrlVisitsCount.php
+++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.ShortUrlVisitsCount.php
@@ -28,6 +28,7 @@ return static function (ClassMetadata $metadata, array $emConfig): void {
     $builder->createField('count', Types::BIGINT)
             ->columnName('count')
             ->option('unsigned', true)
+            ->option('default', 1)
             ->build();
 
     $builder->createField('slotId', Types::INTEGER)
diff --git a/module/Core/src/Visit/Listener/ShortUrlVisitsCountPreFlushListener.php b/module/Core/src/Visit/Listener/ShortUrlVisitsCountPreFlushListener.php
index 6a006e54..74812e44 100644
--- a/module/Core/src/Visit/Listener/ShortUrlVisitsCountPreFlushListener.php
+++ b/module/Core/src/Visit/Listener/ShortUrlVisitsCountPreFlushListener.php
@@ -108,17 +108,20 @@ final readonly class ShortUrlVisitsCountPreFlushListener
         // For engines without a specific UPSERT syntax, do a regular locked select followed by an insert or update
         $qb = $conn->createQueryBuilder();
         $qb->select('id')
-            ->from('short_url_visits_counts')
-            ->where($qb->expr()->and(
-                $qb->expr()->eq('short_url_id', ':short_url_id'),
-                $qb->expr()->eq('potential_bot', ':potential_bot'),
-                $qb->expr()->eq('slot_id', ':slot_id'),
-            ))
-            ->setParameter('short_url_id', $shortUrlId)
-            ->setParameter('potential_bot', $potentialBot)
-            ->setParameter('slot_id', $slotId)
-            ->forUpdate()
-            ->setMaxResults(1);
+           ->from('short_url_visits_counts')
+           ->where($qb->expr()->and(
+               $qb->expr()->eq('short_url_id', ':short_url_id'),
+               $qb->expr()->eq('potential_bot', ':potential_bot'),
+               $qb->expr()->eq('slot_id', ':slot_id'),
+           ))
+           ->setParameter('short_url_id', $shortUrlId)
+           ->setParameter('potential_bot', $potentialBot)
+           ->setParameter('slot_id', $slotId)
+           ->setMaxResults(1);
+
+        if ($conn->getDatabasePlatform()::class === SQLServerPlatform::class) {
+            $qb->forUpdate();
+        }
 
         $resultSet = $qb->executeQuery()->fetchOne();
         $writeQb = ! $resultSet
@@ -130,17 +133,17 @@ final readonly class ShortUrlVisitsCountPreFlushListener
                     'slot_id' => ':slot_id',
                 ])
             : $conn->createQueryBuilder()
-                ->update('short_url_visits_counts')
-                ->set('count', 'count + 1')
-                ->where($qb->expr()->and(
-                    $qb->expr()->eq('short_url_id', ':short_url_id'),
-                    $qb->expr()->eq('potential_bot', ':potential_bot'),
-                    $qb->expr()->eq('slot_id', ':slot_id'),
-                ));
+                   ->update('short_url_visits_counts')
+                   ->set('count', 'count + 1')
+                   ->where($qb->expr()->and(
+                       $qb->expr()->eq('short_url_id', ':short_url_id'),
+                       $qb->expr()->eq('potential_bot', ':potential_bot'),
+                       $qb->expr()->eq('slot_id', ':slot_id'),
+                   ));
 
         $writeQb->setParameter('short_url_id', $shortUrlId)
-            ->setParameter('potential_bot', $potentialBot)
-            ->setParameter('slot_id', $slotId)
-            ->executeStatement();
+                ->setParameter('potential_bot', $potentialBot)
+                ->setParameter('slot_id', $slotId)
+                ->executeStatement();
     }
 }

From b236354fc78b3f66dc653b7703823a112f5cd3cc Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Tue, 26 Mar 2024 09:42:25 +0100
Subject: [PATCH 09/19] Fix order in which entities are flushed in
 ShortUrlListRepositoryTest

---
 config/autoload/entity-manager.global.php     |   2 +-
 module/Core/src/Model/Ordering.php            |   2 +-
 .../Persistence/ShortUrlsListFiltering.php    |   6 +-
 .../Repository/ShortUrlListRepositoryTest.php | 194 ++++++++----------
 4 files changed, 86 insertions(+), 118 deletions(-)

diff --git a/config/autoload/entity-manager.global.php b/config/autoload/entity-manager.global.php
index eae7e8a9..fef50b56 100644
--- a/config/autoload/entity-manager.global.php
+++ b/config/autoload/entity-manager.global.php
@@ -5,8 +5,8 @@ declare(strict_types=1);
 use Doctrine\ORM\Events;
 use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository;
 use Shlinkio\Shlink\Core\Config\EnvVars;
-
 use Shlinkio\Shlink\Core\Visit\Listener\ShortUrlVisitsCountPreFlushListener;
+
 use function Shlinkio\Shlink\Core\ArrayUtils\contains;
 
 return (static function (): array {
diff --git a/module/Core/src/Model/Ordering.php b/module/Core/src/Model/Ordering.php
index 69a1429a..b56c0ea8 100644
--- a/module/Core/src/Model/Ordering.php
+++ b/module/Core/src/Model/Ordering.php
@@ -10,7 +10,7 @@ final readonly class Ordering
     private const ASC_DIR = 'ASC';
     private const DEFAULT_DIR = self::ASC_DIR;
 
-    private function __construct(public ?string $field, public string $direction)
+    public function __construct(public ?string $field = null, public string $direction = self::DEFAULT_DIR)
     {
     }
 
diff --git a/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php b/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php
index db8b9a70..589947dd 100644
--- a/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php
+++ b/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php
@@ -13,9 +13,9 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey;
 class ShortUrlsListFiltering extends ShortUrlsCountFiltering
 {
     public function __construct(
-        public readonly ?int $limit,
-        public readonly ?int $offset,
-        public readonly Ordering $orderBy,
+        public readonly ?int $limit = null,
+        public readonly ?int $offset = null,
+        public readonly Ordering $orderBy = new Ordering(),
         ?string $searchTerm = null,
         array $tags = [],
         ?TagsMode $tagsMode = null,
diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php
index 315491d8..91e15737 100644
--- a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php
+++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php
@@ -13,6 +13,7 @@ use Shlinkio\Shlink\Core\Model\Ordering;
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Model\OrderableField;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation;
+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;
@@ -25,6 +26,7 @@ use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase;
 use function array_map;
 use function count;
 use function range;
+use function Shlinkio\Shlink\Core\ArrayUtils\map;
 
 class ShortUrlListRepositoryTest extends DatabaseTestCase
 {
@@ -60,6 +62,18 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase
         $this->getEntityManager()->persist($foo);
 
         $bar = ShortUrl::withLongUrl('https://bar');
+        $this->getEntityManager()->persist($bar);
+
+        $foo2 = ShortUrl::withLongUrl('https://foo_2');
+        $ref = new ReflectionObject($foo2);
+        $dateProp = $ref->getProperty('dateCreated');
+        $dateProp->setAccessible(true);
+        $dateProp->setValue($foo2, Chronos::now()->subDays(5));
+        $this->getEntityManager()->persist($foo2);
+
+        // Flush short URLs first
+        $this->getEntityManager()->flush();
+
         $visits = array_map(function () use ($bar) {
             $visit = Visit::forValidShortUrl($bar, Visitor::botInstance());
             $this->getEntityManager()->persist($visit);
@@ -67,9 +81,6 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase
             return $visit;
         }, range(0, 5));
         $bar->setVisits(new ArrayCollection($visits));
-        $this->getEntityManager()->persist($bar);
-
-        $foo2 = ShortUrl::withLongUrl('https://foo_2');
         $visits2 = array_map(function () use ($foo2) {
             $visit = Visit::forValidShortUrl($foo2, Visitor::emptyInstance());
             $this->getEntityManager()->persist($visit);
@@ -77,68 +88,59 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase
             return $visit;
         }, range(0, 3));
         $foo2->setVisits(new ArrayCollection($visits2));
-        $ref = new ReflectionObject($foo2);
-        $dateProp = $ref->getProperty('dateCreated');
-        $dateProp->setAccessible(true);
-        $dateProp->setValue($foo2, Chronos::now()->subDays(5));
-        $this->getEntityManager()->persist($foo2);
 
+        // Flush visits afterwards
         $this->getEntityManager()->flush();
 
-        $result = $this->repo->findList(
-            new ShortUrlsListFiltering(null, null, Ordering::none(), 'foo', ['bar']),
-        );
+        $result = $this->repo->findList(new ShortUrlsListFiltering(searchTerm: 'foo', tags: ['bar']));
         self::assertCount(1, $result);
         self::assertEquals(1, $this->repo->countList(new ShortUrlsCountFiltering('foo', ['bar'])));
-        self::assertSame($foo, $result[0]);
+        self::assertSame($foo, $result[0]->shortUrl);
 
         // Assert searched text also applies to tags
-        $result = $this->repo->findList(new ShortUrlsListFiltering(null, null, Ordering::none(), 'bar'));
+        $result = $this->repo->findList(new ShortUrlsListFiltering(searchTerm: 'bar'));
         self::assertCount(2, $result);
         self::assertEquals(2, $this->repo->countList(new ShortUrlsCountFiltering('bar')));
-        self::assertContains($foo, $result);
+        self::assertContains($foo, map($result, fn (ShortUrlWithVisitsSummary $s) => $s->shortUrl));
 
-        $result = $this->repo->findList(new ShortUrlsListFiltering(null, null, Ordering::none()));
+        $result = $this->repo->findList(new ShortUrlsListFiltering());
         self::assertCount(3, $result);
 
-        $result = $this->repo->findList(new ShortUrlsListFiltering(2, null, Ordering::none()));
+        $result = $this->repo->findList(new ShortUrlsListFiltering(limit: 2));
         self::assertCount(2, $result);
 
-        $result = $this->repo->findList(new ShortUrlsListFiltering(2, 1, Ordering::none()));
+        $result = $this->repo->findList(new ShortUrlsListFiltering(limit: 2, offset: 1));
         self::assertCount(2, $result);
 
-        self::assertCount(1, $this->repo->findList(new ShortUrlsListFiltering(2, 2, Ordering::none())));
+        self::assertCount(1, $this->repo->findList(new ShortUrlsListFiltering(limit: 2, offset: 2)));
 
-        $result = $this->repo->findList(
-            new ShortUrlsListFiltering(null, null, Ordering::fromFieldDesc(OrderableField::VISITS->value)),
-        );
-        self::assertCount(3, $result);
-        self::assertSame($bar, $result[0]);
-
-        $result = $this->repo->findList(
-            new ShortUrlsListFiltering(null, null, Ordering::fromFieldDesc(OrderableField::NON_BOT_VISITS->value)),
-        );
-        self::assertCount(3, $result);
-        self::assertSame($foo2, $result[0]);
-
-        $result = $this->repo->findList(
-            new ShortUrlsListFiltering(null, null, Ordering::none(), null, [], null, DateRange::until(
-                Chronos::now()->subDays(2),
-            )),
-        );
-        self::assertCount(1, $result);
-        self::assertEquals(1, $this->repo->countList(new ShortUrlsCountFiltering(null, [], null, DateRange::until(
-            Chronos::now()->subDays(2),
-        ))));
-        self::assertSame($foo2, $result[0]);
-
-        self::assertCount(2, $this->repo->findList(
-            new ShortUrlsListFiltering(null, null, Ordering::none(), null, [], null, DateRange::since(
-                Chronos::now()->subDays(2),
-            )),
+        $result = $this->repo->findList(new ShortUrlsListFiltering(
+            orderBy: Ordering::fromFieldDesc(OrderableField::VISITS->value),
         ));
+        self::assertCount(3, $result);
+        self::assertSame($bar, $result[0]->shortUrl);
+
+        // FIXME Check why this assertion fails
+//        $result = $this->repo->findList(new ShortUrlsListFiltering(
+//            orderBy: Ordering::fromFieldDesc(OrderableField::NON_BOT_VISITS->value),
+//        ));
+//        self::assertCount(3, $result);
+//        self::assertSame($foo2, $result[0]->shortUrl);
+
+        $result = $this->repo->findList(new ShortUrlsListFiltering(
+            dateRange: DateRange::until(Chronos::now()->subDays(2)),
+        ));
+        self::assertCount(1, $result);
+        self::assertEquals(1, $this->repo->countList(new ShortUrlsCountFiltering(
+            dateRange: DateRange::until(Chronos::now()->subDays(2)),
+        )));
+        self::assertSame($foo2, $result[0]->shortUrl);
+
+        self::assertCount(2, $this->repo->findList(new ShortUrlsListFiltering(
+            dateRange: DateRange::since(Chronos::now()->subDays(2)),
+        )));
         self::assertEquals(2, $this->repo->countList(
-            new ShortUrlsCountFiltering(null, [], null, DateRange::since(Chronos::now()->subDays(2))),
+            new ShortUrlsCountFiltering(dateRange: DateRange::since(Chronos::now()->subDays(2))),
         ));
     }
 
@@ -152,15 +154,13 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase
 
         $this->getEntityManager()->flush();
 
-        $result = $this->repo->findList(
-            new ShortUrlsListFiltering(null, null, Ordering::fromFieldAsc('longUrl')),
-        );
+        $result = $this->repo->findList(new ShortUrlsListFiltering(orderBy: Ordering::fromFieldAsc('longUrl')));
 
         self::assertCount(count($urls), $result);
-        self::assertEquals('https://a', $result[0]->getLongUrl());
-        self::assertEquals('https://b', $result[1]->getLongUrl());
-        self::assertEquals('https://c', $result[2]->getLongUrl());
-        self::assertEquals('https://z', $result[3]->getLongUrl());
+        self::assertEquals('https://a', $result[0]->shortUrl->getLongUrl());
+        self::assertEquals('https://b', $result[1]->shortUrl->getLongUrl());
+        self::assertEquals('https://c', $result[2]->shortUrl->getLongUrl());
+        self::assertEquals('https://z', $result[3]->shortUrl->getLongUrl());
     }
 
     #[Test]
@@ -194,81 +194,55 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase
 
         $this->getEntityManager()->flush();
 
-        self::assertCount(5, $this->repo->findList(
-            new ShortUrlsListFiltering(null, null, Ordering::none(), null, ['foo', 'bar']),
-        ));
+        self::assertCount(5, $this->repo->findList(new ShortUrlsListFiltering(tags: ['foo', 'bar'])));
         self::assertCount(5, $this->repo->findList(new ShortUrlsListFiltering(
-            null,
-            null,
-            Ordering::none(),
-            null,
-            ['foo', 'bar'],
-            TagsMode::ANY,
+            tags: ['foo', 'bar'],
+            tagsMode: TagsMode::ANY,
         )));
         self::assertCount(1, $this->repo->findList(new ShortUrlsListFiltering(
-            null,
-            null,
-            Ordering::none(),
-            null,
-            ['foo', 'bar'],
-            TagsMode::ALL,
+            tags: ['foo', 'bar'],
+            tagsMode: TagsMode::ALL,
         )));
-        self::assertEquals(5, $this->repo->countList(new ShortUrlsCountFiltering(null, ['foo', 'bar'])));
-        self::assertEquals(5, $this->repo->countList(new ShortUrlsCountFiltering(null, ['foo', 'bar'], TagsMode::ANY)));
-        self::assertEquals(1, $this->repo->countList(new ShortUrlsCountFiltering(null, ['foo', 'bar'], TagsMode::ALL)));
-
-        self::assertCount(4, $this->repo->findList(
-            new ShortUrlsListFiltering(null, null, Ordering::none(), null, ['bar', 'baz']),
+        self::assertEquals(5, $this->repo->countList(new ShortUrlsCountFiltering(tags: ['foo', 'bar'])));
+        self::assertEquals(5, $this->repo->countList(
+            new ShortUrlsCountFiltering(tags: ['foo', 'bar'], tagsMode: TagsMode::ANY),
         ));
+        self::assertEquals(1, $this->repo->countList(
+            new ShortUrlsCountFiltering(tags: ['foo', 'bar'], tagsMode: TagsMode::ALL),
+        ));
+
+        self::assertCount(4, $this->repo->findList(new ShortUrlsListFiltering(tags: ['bar', 'baz'])));
         self::assertCount(4, $this->repo->findList(new ShortUrlsListFiltering(
-            null,
-            null,
-            Ordering::none(),
-            null,
-            ['bar', 'baz'],
-            TagsMode::ANY,
+            tags: ['bar', 'baz'],
+            tagsMode: TagsMode::ANY,
         )));
         self::assertCount(2, $this->repo->findList(new ShortUrlsListFiltering(
-            null,
-            null,
-            Ordering::none(),
-            null,
-            ['bar', 'baz'],
-            TagsMode::ALL,
+            tags: ['bar', 'baz'],
+            tagsMode: TagsMode::ALL,
         )));
-        self::assertEquals(4, $this->repo->countList(new ShortUrlsCountFiltering(null, ['bar', 'baz'])));
+        self::assertEquals(4, $this->repo->countList(new ShortUrlsCountFiltering(tags: ['bar', 'baz'])));
         self::assertEquals(4, $this->repo->countList(
-            new ShortUrlsCountFiltering(null, ['bar', 'baz'], TagsMode::ANY),
+            new ShortUrlsCountFiltering(tags: ['bar', 'baz'], tagsMode: TagsMode::ANY),
         ));
         self::assertEquals(2, $this->repo->countList(
-            new ShortUrlsCountFiltering(null, ['bar', 'baz'], TagsMode::ALL),
+            new ShortUrlsCountFiltering(tags: ['bar', 'baz'], tagsMode: TagsMode::ALL),
         ));
 
-        self::assertCount(5, $this->repo->findList(
-            new ShortUrlsListFiltering(null, null, Ordering::none(), null, ['foo', 'bar', 'baz']),
-        ));
+        self::assertCount(5, $this->repo->findList(new ShortUrlsListFiltering(tags: ['foo', 'bar', 'baz'])));
         self::assertCount(5, $this->repo->findList(new ShortUrlsListFiltering(
-            null,
-            null,
-            Ordering::none(),
-            null,
-            ['foo', 'bar', 'baz'],
-            TagsMode::ANY,
+            tags: ['foo', 'bar', 'baz'],
+            tagsMode: TagsMode::ANY,
         )));
         self::assertCount(0, $this->repo->findList(new ShortUrlsListFiltering(
-            null,
-            null,
-            Ordering::none(),
-            null,
-            ['foo', 'bar', 'baz'],
-            TagsMode::ALL,
+            tags: ['foo', 'bar', 'baz'],
+            tagsMode: TagsMode::ALL,
         )));
-        self::assertEquals(5, $this->repo->countList(new ShortUrlsCountFiltering(null, ['foo', 'bar', 'baz'])));
+        self::assertEquals(5, $this->repo->countList(new ShortUrlsCountFiltering(tags: ['foo', 'bar', 'baz'])));
         self::assertEquals(5, $this->repo->countList(
-            new ShortUrlsCountFiltering(null, ['foo', 'bar', 'baz'], TagsMode::ANY),
+            new ShortUrlsCountFiltering(tags: ['foo', 'bar', 'baz'], tagsMode: TagsMode::ANY),
         ));
         self::assertEquals(0, $this->repo->countList(
-            new ShortUrlsCountFiltering(null, ['foo', 'bar', 'baz'], TagsMode::ALL),
+            new ShortUrlsCountFiltering(tags: ['foo', 'bar', 'baz'], tagsMode: TagsMode::ALL),
         ));
     }
 
@@ -294,9 +268,6 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase
         $this->getEntityManager()->flush();
 
         $buildFiltering = static fn (string $searchTerm) => new ShortUrlsListFiltering(
-            null,
-            null,
-            Ordering::none(),
             searchTerm: $searchTerm,
             defaultDomain: 'deFaulT-domain.com',
         );
@@ -339,9 +310,6 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase
 
         $filtering = static fn (bool $excludeMaxVisitsReached, bool $excludePastValidUntil) =>
         new ShortUrlsListFiltering(
-            null,
-            null,
-            Ordering::none(),
             excludeMaxVisitsReached: $excludeMaxVisitsReached,
             excludePastValidUntil: $excludePastValidUntil,
         );

From 3d7b1ca79960f018694b16f42277eb71c4d3b1c3 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Tue, 26 Mar 2024 23:26:44 +0100
Subject: [PATCH 10/19] Move from preFlush to onFlush + postFlush

---
 config/autoload/entity-manager.global.php     |  7 +++----
 module/Core/config/dependencies.config.php    |  2 +-
 ...ner.php => ShortUrlVisitsCountTracker.php} | 21 +++++++++++++++----
 .../Repository/ShortUrlListRepositoryTest.php | 21 +++++++------------
 4 files changed, 29 insertions(+), 22 deletions(-)
 rename module/Core/src/Visit/Listener/{ShortUrlVisitsCountPreFlushListener.php => ShortUrlVisitsCountTracker.php} (88%)

diff --git a/config/autoload/entity-manager.global.php b/config/autoload/entity-manager.global.php
index fef50b56..84233915 100644
--- a/config/autoload/entity-manager.global.php
+++ b/config/autoload/entity-manager.global.php
@@ -5,7 +5,7 @@ declare(strict_types=1);
 use Doctrine\ORM\Events;
 use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository;
 use Shlinkio\Shlink\Core\Config\EnvVars;
-use Shlinkio\Shlink\Core\Visit\Listener\ShortUrlVisitsCountPreFlushListener;
+use Shlinkio\Shlink\Core\Visit\Listener\ShortUrlVisitsCountTracker;
 
 use function Shlinkio\Shlink\Core\ArrayUtils\contains;
 
@@ -63,9 +63,8 @@ return (static function (): array {
                 'load_mappings_using_functional_style' => true,
                 'default_repository_classname' => EntitySpecificationRepository::class,
                 'listeners' => [
-                    Events::preFlush => [
-                        ShortUrlVisitsCountPreFlushListener::class,
-                    ],
+                    Events::onFlush => [ShortUrlVisitsCountTracker::class],
+                    Events::postFlush => [ShortUrlVisitsCountTracker::class],
                 ],
             ],
             'connection' => $resolveConnection(),
diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php
index 7d3bf763..951c7b52 100644
--- a/module/Core/config/dependencies.config.php
+++ b/module/Core/config/dependencies.config.php
@@ -76,7 +76,7 @@ return [
                 EntityRepositoryFactory::class,
                 Visit\Entity\Visit::class,
             ],
-            Visit\Listener\ShortUrlVisitsCountPreFlushListener::class => InvokableFactory::class,
+            Visit\Listener\ShortUrlVisitsCountTracker::class => InvokableFactory::class,
 
             Util\DoctrineBatchHelper::class => ConfigAbstractFactory::class,
             Util\RedirectResponseHelper::class => ConfigAbstractFactory::class,
diff --git a/module/Core/src/Visit/Listener/ShortUrlVisitsCountPreFlushListener.php b/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
similarity index 88%
rename from module/Core/src/Visit/Listener/ShortUrlVisitsCountPreFlushListener.php
rename to module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
index 74812e44..8d79c330 100644
--- a/module/Core/src/Visit/Listener/ShortUrlVisitsCountPreFlushListener.php
+++ b/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
@@ -10,20 +10,33 @@ use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
 use Doctrine\DBAL\Platforms\SQLitePlatform;
 use Doctrine\DBAL\Platforms\SQLServerPlatform;
 use Doctrine\ORM\EntityManagerInterface;
-use Doctrine\ORM\Event\PreFlushEventArgs;
+use Doctrine\ORM\Event\OnFlushEventArgs;
+use Doctrine\ORM\Event\PostFlushEventArgs;
 use Shlinkio\Shlink\Core\Visit\Entity\Visit;
 
 use function rand;
 
-final readonly class ShortUrlVisitsCountPreFlushListener
+final class ShortUrlVisitsCountTracker
 {
+    /** @var object[] */
+    private array $entitiesToBeCreated = [];
+
+    public function onFlush(OnFlushEventArgs $args): void
+    {
+        // Track entities that are going to be created during this flush operation
+        $this->entitiesToBeCreated = $args->getObjectManager()->getUnitOfWork()->getScheduledEntityInsertions();
+    }
+
     /**
      * @throws Exception
      */
-    public function preFlush(PreFlushEventArgs $args): void
+    public function postFlush(PostFlushEventArgs $args): void
     {
         $em = $args->getObjectManager();
-        $entitiesToBeCreated = $em->getUnitOfWork()->getScheduledEntityInsertions();
+        $entitiesToBeCreated = $this->entitiesToBeCreated;
+
+        // Reset tracked entities until next flush operation
+        $this->entitiesToBeCreated = [];
 
         foreach ($entitiesToBeCreated as $entity) {
             $this->trackVisitCount($em, $entity);
diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php
index 91e15737..cad0569d 100644
--- a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php
+++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php
@@ -62,18 +62,6 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase
         $this->getEntityManager()->persist($foo);
 
         $bar = ShortUrl::withLongUrl('https://bar');
-        $this->getEntityManager()->persist($bar);
-
-        $foo2 = ShortUrl::withLongUrl('https://foo_2');
-        $ref = new ReflectionObject($foo2);
-        $dateProp = $ref->getProperty('dateCreated');
-        $dateProp->setAccessible(true);
-        $dateProp->setValue($foo2, Chronos::now()->subDays(5));
-        $this->getEntityManager()->persist($foo2);
-
-        // Flush short URLs first
-        $this->getEntityManager()->flush();
-
         $visits = array_map(function () use ($bar) {
             $visit = Visit::forValidShortUrl($bar, Visitor::botInstance());
             $this->getEntityManager()->persist($visit);
@@ -81,6 +69,9 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase
             return $visit;
         }, range(0, 5));
         $bar->setVisits(new ArrayCollection($visits));
+        $this->getEntityManager()->persist($bar);
+
+        $foo2 = ShortUrl::withLongUrl('https://foo_2');
         $visits2 = array_map(function () use ($foo2) {
             $visit = Visit::forValidShortUrl($foo2, Visitor::emptyInstance());
             $this->getEntityManager()->persist($visit);
@@ -88,8 +79,12 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase
             return $visit;
         }, range(0, 3));
         $foo2->setVisits(new ArrayCollection($visits2));
+        $ref = new ReflectionObject($foo2);
+        $dateProp = $ref->getProperty('dateCreated');
+        $dateProp->setAccessible(true);
+        $dateProp->setValue($foo2, Chronos::now()->subDays(5));
+        $this->getEntityManager()->persist($foo2);
 
-        // Flush visits afterwards
         $this->getEntityManager()->flush();
 
         $result = $this->repo->findList(new ShortUrlsListFiltering(searchTerm: 'foo', tags: ['bar']));

From 10e941cea6043f3e9d15be3775d2b007b9b9c388 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Wed, 27 Mar 2024 09:15:21 +0100
Subject: [PATCH 11/19] Add missing COALESCE when summing visits counts

---
 module/Core/src/Model/Ordering.php                    | 10 +++++-----
 .../ShortUrl/Repository/ShortUrlListRepository.php    |  4 ++--
 .../src/Visit/Listener/ShortUrlVisitsCountTracker.php |  4 ++--
 .../Repository/ShortUrlListRepositoryTest.php         | 11 +++++------
 4 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/module/Core/src/Model/Ordering.php b/module/Core/src/Model/Ordering.php
index b56c0ea8..e1b91510 100644
--- a/module/Core/src/Model/Ordering.php
+++ b/module/Core/src/Model/Ordering.php
@@ -14,6 +14,11 @@ final readonly class Ordering
     {
     }
 
+    public static function none(): self
+    {
+        return new self();
+    }
+
     /**
      * @param array{string|null, string|null} $props
      */
@@ -23,11 +28,6 @@ final readonly class Ordering
         return new self($field, $dir ?? self::DEFAULT_DIR);
     }
 
-    public static function none(): self
-    {
-        return new self(null, self::DEFAULT_DIR);
-    }
-
     public static function fromFieldAsc(string $field): self
     {
         return new self($field, self::ASC_DIR);
diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
index 0c0c3df3..790a3dbb 100644
--- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
+++ b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
@@ -30,7 +30,7 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh
     {
         $buildVisitsSubQuery = function (string $alias, bool $excludingBots): string {
             $vqb = $this->getEntityManager()->createQueryBuilder();
-            $vqb->select('SUM(' . $alias . '.count)')
+            $vqb->select('COALESCE(SUM(' . $alias . '.count), 0)')
                 ->from(ShortUrlVisitsCount::class, $alias)
                 ->where($vqb->expr()->eq($alias . '.shortUrl', 's'));
 
@@ -50,7 +50,7 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh
            ->setMaxResults($filtering->limit)
            ->setFirstResult($filtering->offset)
            // This param is used in one of the sub-queries, but needs to set in the parent query
-           ->setParameter('potentialBot', 0);
+           ->setParameter('potentialBot', false);
 
         $this->processOrderByForList($qb, $filtering);
 
diff --git a/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php b/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
index 8d79c330..f64974dd 100644
--- a/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
+++ b/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
@@ -128,7 +128,7 @@ final class ShortUrlVisitsCountTracker
                $qb->expr()->eq('slot_id', ':slot_id'),
            ))
            ->setParameter('short_url_id', $shortUrlId)
-           ->setParameter('potential_bot', $potentialBot)
+           ->setParameter('potential_bot', $potentialBot ? '1' : '0')
            ->setParameter('slot_id', $slotId)
            ->setMaxResults(1);
 
@@ -155,7 +155,7 @@ final class ShortUrlVisitsCountTracker
                    ));
 
         $writeQb->setParameter('short_url_id', $shortUrlId)
-                ->setParameter('potential_bot', $potentialBot)
+                ->setParameter('potential_bot', $potentialBot ? '1' : '0')
                 ->setParameter('slot_id', $slotId)
                 ->executeStatement();
     }
diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php
index cad0569d..95924956 100644
--- a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php
+++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php
@@ -115,12 +115,11 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase
         self::assertCount(3, $result);
         self::assertSame($bar, $result[0]->shortUrl);
 
-        // FIXME Check why this assertion fails
-//        $result = $this->repo->findList(new ShortUrlsListFiltering(
-//            orderBy: Ordering::fromFieldDesc(OrderableField::NON_BOT_VISITS->value),
-//        ));
-//        self::assertCount(3, $result);
-//        self::assertSame($foo2, $result[0]->shortUrl);
+        $result = $this->repo->findList(new ShortUrlsListFiltering(
+            orderBy: Ordering::fromFieldDesc(OrderableField::NON_BOT_VISITS->value),
+        ));
+        self::assertCount(3, $result);
+        self::assertSame($foo2, $result[0]->shortUrl);
 
         $result = $this->repo->findList(new ShortUrlsListFiltering(
             dateRange: DateRange::until(Chronos::now()->subDays(2)),

From 8417498f08eea372b1e68f52444223b22127b5d0 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Wed, 27 Mar 2024 09:33:19 +0100
Subject: [PATCH 12/19] Fixes on static check and unit tests

---
 .../Command/ShortUrl/ListShortUrlsCommand.php   | 10 ++++++++--
 .../ShortUrl/ListShortUrlsCommandTest.php       | 17 ++++++++++-------
 .../Model/ShortUrlWithVisitsSummary.php         |  7 ++++++-
 .../ShortUrl/ShortUrlListServiceInterface.php   |  4 ++--
 .../Listener/ShortUrlVisitsCountTracker.php     |  8 ++++----
 5 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php
index b03e0312..4e3a7706 100644
--- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php
+++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php
@@ -177,6 +177,9 @@ class ListShortUrlsCommand extends Command
         return ExitCode::EXIT_SUCCESS;
     }
 
+    /**
+     * @param array<string, callable(array $serializedShortUrl, ShortUrl $shortUrl): ?string> $columnsMap
+     */
     private function renderPage(
         OutputInterface $output,
         array $columnsMap,
@@ -186,8 +189,8 @@ class ListShortUrlsCommand extends Command
         $shortUrls = $this->shortUrlService->listShortUrls($params);
 
         $rows = map([...$shortUrls], function (ShortUrlWithVisitsSummary $shortUrl) use ($columnsMap) {
-            $rawShortUrl = $this->transformer->transform($shortUrl);
-            return map($columnsMap, fn (callable $call) => $call($rawShortUrl, $shortUrl));
+            $serializedShortUrl = $this->transformer->transform($shortUrl);
+            return map($columnsMap, fn (callable $call) => $call($serializedShortUrl, $shortUrl->shortUrl));
         });
 
         ShlinkTable::default($output)->render(
@@ -210,6 +213,9 @@ class ListShortUrlsCommand extends Command
         return $dir === null ? $field : sprintf('%s-%s', $field, $dir);
     }
 
+    /**
+     * @return array<string, callable(array $serializedShortUrl, ShortUrl $shortUrl): ?string>
+     */
     private function resolveColumnsMap(InputInterface $input): array
     {
         $pickProp = static fn (string $prop): callable => static fn (array $shortUrl) => $shortUrl[$prop];
diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php
index 6859ec13..6016da1c 100644
--- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php
+++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php
@@ -16,6 +16,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation;
 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\ShortUrlListServiceInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer;
@@ -47,7 +48,7 @@ class ListShortUrlsCommandTest extends TestCase
         // The paginator will return more than one page
         $data = [];
         for ($i = 0; $i < 50; $i++) {
-            $data[] = ShortUrl::withLongUrl('https://url_' . $i);
+            $data[] = ShortUrlWithVisitsSummary::fromShortUrl(ShortUrl::withLongUrl('https://url_' . $i));
         }
 
         $this->shortUrlService->expects($this->exactly(3))->method('listShortUrls')->withAnyParameters()
@@ -69,7 +70,7 @@ class ListShortUrlsCommandTest extends TestCase
         // The paginator will return more than one page
         $data = [];
         for ($i = 0; $i < 30; $i++) {
-            $data[] = ShortUrl::withLongUrl('https://url_' . $i);
+            $data[] = ShortUrlWithVisitsSummary::fromShortUrl(ShortUrl::withLongUrl('https://url_' . $i));
         }
 
         $this->shortUrlService->expects($this->once())->method('listShortUrls')->with(
@@ -111,11 +112,13 @@ class ListShortUrlsCommandTest extends TestCase
         $this->shortUrlService->expects($this->once())->method('listShortUrls')->with(
             ShortUrlsParams::emptyInstance(),
         )->willReturn(new Paginator(new ArrayAdapter([
-            ShortUrl::create(ShortUrlCreation::fromRawData([
-                'longUrl' => 'https://foo.com',
-                'tags' => ['foo', 'bar', 'baz'],
-                'apiKey' => $apiKey,
-            ])),
+            ShortUrlWithVisitsSummary::fromShortUrl(
+                ShortUrl::create(ShortUrlCreation::fromRawData([
+                    'longUrl' => 'https://foo.com',
+                    'tags' => ['foo', 'bar', 'baz'],
+                    'apiKey' => $apiKey,
+                ])),
+            ),
         ])));
 
         $this->commandTester->setInputs(['y']);
diff --git a/module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php b/module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php
index 79bdb526..50efaaee 100644
--- a/module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php
+++ b/module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php
@@ -9,7 +9,7 @@ use Shlinkio\Shlink\Core\Visit\Model\VisitsSummary;
 
 final readonly class ShortUrlWithVisitsSummary
 {
-    private function __construct(public ShortUrl $shortUrl, public VisitsSummary $visitsSummary)
+    private function __construct(public ShortUrl $shortUrl, private ?VisitsSummary $visitsSummary = null)
     {
     }
 
@@ -24,6 +24,11 @@ final readonly class ShortUrlWithVisitsSummary
         ));
     }
 
+    public static function fromShortUrl(ShortUrl $shortUrl): self
+    {
+        return new self($shortUrl);
+    }
+
     public function toArray(): array
     {
         return $this->shortUrl->toArray($this->visitsSummary);
diff --git a/module/Core/src/ShortUrl/ShortUrlListServiceInterface.php b/module/Core/src/ShortUrl/ShortUrlListServiceInterface.php
index ef7b31c2..ffbb1374 100644
--- a/module/Core/src/ShortUrl/ShortUrlListServiceInterface.php
+++ b/module/Core/src/ShortUrl/ShortUrlListServiceInterface.php
@@ -5,14 +5,14 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core\ShortUrl;
 
 use Shlinkio\Shlink\Common\Paginator\Paginator;
-use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams;
+use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 interface ShortUrlListServiceInterface
 {
     /**
-     * @return ShortUrl[]|Paginator
+     * @return ShortUrlWithVisitsSummary[]|Paginator
      */
     public function listShortUrls(ShortUrlsParams $params, ?ApiKey $apiKey = null): Paginator;
 }
diff --git a/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php b/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
index f64974dd..25df0b83 100644
--- a/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
+++ b/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
@@ -48,14 +48,14 @@ final class ShortUrlVisitsCountTracker
      */
     private function trackVisitCount(EntityManagerInterface $em, object $entity): void
     {
-        // This is not a non-orphan visit
-        if (!$entity instanceof Visit || $entity->shortUrl === null) {
+        // This is not a visit
+        if (!$entity instanceof Visit) {
             return;
         }
         $visit = $entity;
 
-        // The short URL is not persisted yet
-        $shortUrlId = $visit->shortUrl->getId();
+        // The short URL is not persisted yet or this is an orphan visit
+        $shortUrlId = $visit->shortUrl?->getId();
         if ($shortUrlId === null || $shortUrlId === '') {
             return;
         }

From cef30c8e2d943fa61eaf9dbd7e22f26d994add39 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Wed, 27 Mar 2024 19:08:00 +0100
Subject: [PATCH 13/19] Fix type in Version20240318084804

---
 module/Core/migrations/Version20240318084804.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/module/Core/migrations/Version20240318084804.php b/module/Core/migrations/Version20240318084804.php
index a9501a2a..228fa474 100644
--- a/module/Core/migrations/Version20240318084804.php
+++ b/module/Core/migrations/Version20240318084804.php
@@ -39,7 +39,7 @@ final class Version20240318084804 extends AbstractMigration
         }
     }
 
-    private function insertCount(string $shortUrlId, int $count, bool $potentialBot): void
+    private function insertCount(string|int $shortUrlId, int $count, bool $potentialBot): void
     {
         $this->connection->createQueryBuilder()
             ->insert('short_url_visits_counts')

From 4a05c4be400e89edfc85e8ac38f127721be26790 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Wed, 27 Mar 2024 19:14:41 +0100
Subject: [PATCH 14/19] Wrap visits tracking in transaction

---
 module/Core/src/Visit/VisitsTracker.php      | 18 ++++++++++--------
 module/Core/test/Visit/VisitsTrackerTest.php |  2 ++
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/module/Core/src/Visit/VisitsTracker.php b/module/Core/src/Visit/VisitsTracker.php
index 9e4b88df..dd520e8f 100644
--- a/module/Core/src/Visit/VisitsTracker.php
+++ b/module/Core/src/Visit/VisitsTracker.php
@@ -12,12 +12,12 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Visit\Entity\Visit;
 use Shlinkio\Shlink\Core\Visit\Model\Visitor;
 
-class VisitsTracker implements VisitsTrackerInterface
+readonly class VisitsTracker implements VisitsTrackerInterface
 {
     public function __construct(
-        private readonly ORM\EntityManagerInterface $em,
-        private readonly EventDispatcherInterface $eventDispatcher,
-        private readonly TrackingOptions $options,
+        private ORM\EntityManagerInterface $em,
+        private EventDispatcherInterface $eventDispatcher,
+        private TrackingOptions $options,
     ) {
     }
 
@@ -71,10 +71,12 @@ class VisitsTracker implements VisitsTrackerInterface
             return;
         }
 
-        $visit = $createVisit($visitor->normalizeForTrackingOptions($this->options));
-        $this->em->persist($visit);
-        $this->em->flush();
+        $this->em->wrapInTransaction(function () use ($createVisit, $visitor): void {
+            $visit = $createVisit($visitor->normalizeForTrackingOptions($this->options));
+            $this->em->persist($visit);
+            $this->em->flush();
 
-        $this->eventDispatcher->dispatch(new UrlVisited($visit->getId(), $visitor->remoteAddress));
+            $this->eventDispatcher->dispatch(new UrlVisited($visit->getId(), $visitor->remoteAddress));
+        });
     }
 }
diff --git a/module/Core/test/Visit/VisitsTrackerTest.php b/module/Core/test/Visit/VisitsTrackerTest.php
index 32cd10a8..d6cbf4bf 100644
--- a/module/Core/test/Visit/VisitsTrackerTest.php
+++ b/module/Core/test/Visit/VisitsTrackerTest.php
@@ -25,6 +25,8 @@ class VisitsTrackerTest extends TestCase
     protected function setUp(): void
     {
         $this->em = $this->createMock(EntityManager::class);
+        $this->em->method('wrapInTransaction')->willReturnCallback(fn (callable $callback) => $callback());
+
         $this->eventDispatcher = $this->createMock(EventDispatcherInterface::class);
     }
 

From da922fb2a7a1fbce5641d5d10ee90ee6ed7b7e86 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Thu, 28 Mar 2024 09:43:54 +0100
Subject: [PATCH 15/19] Add ShortUrlVisitsCountTrackerTest

---
 .../src/Visit/Entity/ShortUrlVisitsCount.php  |  4 +-
 .../Listener/ShortUrlVisitsCountTracker.php   | 24 +++---
 .../ShortUrlVisitsCountTrackerTest.php        | 76 +++++++++++++++++++
 phpunit-db.xml                                |  1 +
 phpunit.xml.dist                              |  1 +
 5 files changed, 91 insertions(+), 15 deletions(-)
 create mode 100644 module/Core/test-db/Visit/Listener/ShortUrlVisitsCountTrackerTest.php

diff --git a/module/Core/src/Visit/Entity/ShortUrlVisitsCount.php b/module/Core/src/Visit/Entity/ShortUrlVisitsCount.php
index ff3580b3..d513cfe8 100644
--- a/module/Core/src/Visit/Entity/ShortUrlVisitsCount.php
+++ b/module/Core/src/Visit/Entity/ShortUrlVisitsCount.php
@@ -12,8 +12,8 @@ class ShortUrlVisitsCount extends AbstractEntity
     public function __construct(
         private readonly ShortUrl $shortUrl,
         private readonly bool $potentialBot = false,
-        private readonly int $slotId = 1,
-        private readonly string $count = '1',
+        public readonly int $slotId = 1,
+        public readonly string $count = '1',
     ) {
     }
 }
diff --git a/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php b/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
index 25df0b83..f62ddb3d 100644
--- a/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
+++ b/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
@@ -136,8 +136,9 @@ final class ShortUrlVisitsCountTracker
             $qb->forUpdate();
         }
 
-        $resultSet = $qb->executeQuery()->fetchOne();
-        $writeQb = ! $resultSet
+        $visitsCountId = $qb->executeQuery()->fetchOne();
+
+        $writeQb = ! $visitsCountId
             ? $conn->createQueryBuilder()
                 ->insert('short_url_visits_counts')
                 ->values([
@@ -145,18 +146,15 @@ final class ShortUrlVisitsCountTracker
                     'potential_bot' => ':potential_bot',
                     'slot_id' => ':slot_id',
                 ])
-            : $conn->createQueryBuilder()
-                   ->update('short_url_visits_counts')
-                   ->set('count', 'count + 1')
-                   ->where($qb->expr()->and(
-                       $qb->expr()->eq('short_url_id', ':short_url_id'),
-                       $qb->expr()->eq('potential_bot', ':potential_bot'),
-                       $qb->expr()->eq('slot_id', ':slot_id'),
-                   ));
-
-        $writeQb->setParameter('short_url_id', $shortUrlId)
+                ->setParameter('short_url_id', $shortUrlId)
                 ->setParameter('potential_bot', $potentialBot ? '1' : '0')
                 ->setParameter('slot_id', $slotId)
-                ->executeStatement();
+            : $conn->createQueryBuilder()
+                ->update('short_url_visits_counts')
+                ->set('count', 'count + 1')
+                ->where($qb->expr()->eq('id', ':visits_count_id'))
+                ->setParameter('visits_count_id', $visitsCountId);
+
+        $writeQb->executeStatement();
     }
 }
diff --git a/module/Core/test-db/Visit/Listener/ShortUrlVisitsCountTrackerTest.php b/module/Core/test-db/Visit/Listener/ShortUrlVisitsCountTrackerTest.php
new file mode 100644
index 00000000..bfb5616f
--- /dev/null
+++ b/module/Core/test-db/Visit/Listener/ShortUrlVisitsCountTrackerTest.php
@@ -0,0 +1,76 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioDbTest\Shlink\Core\Visit\Listener;
+
+use Doctrine\ORM\EntityRepository;
+use PHPUnit\Framework\Attributes\Test;
+use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\Visit\Entity\ShortUrlVisitsCount;
+use Shlinkio\Shlink\Core\Visit\Entity\Visit;
+use Shlinkio\Shlink\Core\Visit\Model\Visitor;
+use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase;
+
+use function array_filter;
+use function array_values;
+
+class ShortUrlVisitsCountTrackerTest extends DatabaseTestCase
+{
+    private EntityRepository $repo;
+
+    protected function setUp(): void
+    {
+        $this->repo = $this->getEntityManager()->getRepository(ShortUrlVisitsCount::class);
+    }
+
+    #[Test]
+    public function createsNewEntriesWhenNoneExist(): void
+    {
+        $shortUrl = ShortUrl::createFake();
+        $this->getEntityManager()->persist($shortUrl);
+
+        $visit = Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance());
+        $this->getEntityManager()->persist($visit);
+        $this->getEntityManager()->flush();
+
+        /** @var ShortUrlVisitsCount[] $result */
+        $result = $this->repo->findBy(['shortUrl' => $shortUrl]);
+
+        self::assertCount(1, $result);
+        self::assertEquals('1', $result[0]->count);
+        self::assertGreaterThanOrEqual(0, $result[0]->slotId);
+        self::assertLessThan(100, $result[0]->slotId);
+    }
+
+    #[Test]
+    public function editsExistingEntriesWhenAlreadyExist(): void
+    {
+        $shortUrl = ShortUrl::createFake();
+        $this->getEntityManager()->persist($shortUrl);
+
+        for ($i = 0; $i < 100; $i++) {
+            $this->getEntityManager()->persist(new ShortUrlVisitsCount($shortUrl, slotId: $i));
+        }
+        $this->getEntityManager()->flush();
+
+        $visit = Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance());
+        $this->getEntityManager()->persist($visit);
+        $this->getEntityManager()->flush();
+
+        // Clear entity manager to force it to get fresh data from the database
+        // This is needed because the tracker inserts natively, bypassing the entity manager
+        $this->getEntityManager()->clear();
+
+        /** @var ShortUrlVisitsCount[] $result */
+        $result = $this->repo->findBy(['shortUrl' => $shortUrl]);
+        $itemsWithCountBiggerThanOnce = array_values(array_filter(
+            $result,
+            static fn (ShortUrlVisitsCount $item) => ((int) $item->count) > 1,
+        ));
+
+        self::assertCount(100, $result);
+        self::assertCount(1, $itemsWithCountBiggerThanOnce);
+        self::assertEquals('2', $itemsWithCountBiggerThanOnce[0]->count);
+    }
+}
diff --git a/phpunit-db.xml b/phpunit-db.xml
index b883d8ca..3c5ffb64 100644
--- a/phpunit-db.xml
+++ b/phpunit-db.xml
@@ -20,6 +20,7 @@
             <directory>./module/*/src/Spec</directory>
             <directory>./module/*/src/**/Spec</directory>
             <directory>./module/*/src/**/**/Spec</directory>
+            <file>./module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php</file>
         </include>
     </source>
 </phpunit>
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
index 9c85d2c4..4364c82c 100644
--- a/phpunit.xml.dist
+++ b/phpunit.xml.dist
@@ -30,6 +30,7 @@
             <directory>./module/Core/src/Spec</directory>
             <directory>./module/Core/src/**/Spec</directory>
             <directory>./module/Core/src/**/**/Spec</directory>
+            <file>./module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php</file>
         </exclude>
     </source>
 </phpunit>

From c4fd3a74c564bc84a062bee972a54102e5e0976c Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Thu, 28 Mar 2024 16:10:56 +0100
Subject: [PATCH 16/19] Fix type hint in migration

---
 composer.json                                               | 2 +-
 module/Core/migrations/Version20240318084804.php            | 2 +-
 .../Visit/Listener/ShortUrlVisitsCountTrackerTest.php       | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/composer.json b/composer.json
index 2179a749..8d8f33f9 100644
--- a/composer.json
+++ b/composer.json
@@ -46,7 +46,7 @@
         "shlinkio/shlink-common": "^6.0",
         "shlinkio/shlink-config": "^3.0",
         "shlinkio/shlink-event-dispatcher": "^4.0",
-        "shlinkio/shlink-importer": "^5.3",
+        "shlinkio/shlink-importer": "^5.3.1",
         "shlinkio/shlink-installer": "^9.0",
         "shlinkio/shlink-ip-geolocation": "^4.0",
         "shlinkio/shlink-json": "^1.1",
diff --git a/module/Core/migrations/Version20240318084804.php b/module/Core/migrations/Version20240318084804.php
index 228fa474..6ff25fac 100644
--- a/module/Core/migrations/Version20240318084804.php
+++ b/module/Core/migrations/Version20240318084804.php
@@ -39,7 +39,7 @@ final class Version20240318084804 extends AbstractMigration
         }
     }
 
-    private function insertCount(string|int $shortUrlId, int $count, bool $potentialBot): void
+    private function insertCount(string|int $shortUrlId, string|int $count, bool $potentialBot): void
     {
         $this->connection->createQueryBuilder()
             ->insert('short_url_visits_counts')
diff --git a/module/Core/test-db/Visit/Listener/ShortUrlVisitsCountTrackerTest.php b/module/Core/test-db/Visit/Listener/ShortUrlVisitsCountTrackerTest.php
index bfb5616f..29f5d5d1 100644
--- a/module/Core/test-db/Visit/Listener/ShortUrlVisitsCountTrackerTest.php
+++ b/module/Core/test-db/Visit/Listener/ShortUrlVisitsCountTrackerTest.php
@@ -40,7 +40,7 @@ class ShortUrlVisitsCountTrackerTest extends DatabaseTestCase
         self::assertCount(1, $result);
         self::assertEquals('1', $result[0]->count);
         self::assertGreaterThanOrEqual(0, $result[0]->slotId);
-        self::assertLessThan(100, $result[0]->slotId);
+        self::assertLessThanOrEqual(100, $result[0]->slotId);
     }
 
     #[Test]
@@ -49,7 +49,7 @@ class ShortUrlVisitsCountTrackerTest extends DatabaseTestCase
         $shortUrl = ShortUrl::createFake();
         $this->getEntityManager()->persist($shortUrl);
 
-        for ($i = 0; $i < 100; $i++) {
+        for ($i = 0; $i <= 100; $i++) {
             $this->getEntityManager()->persist(new ShortUrlVisitsCount($shortUrl, slotId: $i));
         }
         $this->getEntityManager()->flush();
@@ -69,7 +69,7 @@ class ShortUrlVisitsCountTrackerTest extends DatabaseTestCase
             static fn (ShortUrlVisitsCount $item) => ((int) $item->count) > 1,
         ));
 
-        self::assertCount(100, $result);
+        self::assertCount(101, $result);
         self::assertCount(1, $itemsWithCountBiggerThanOnce);
         self::assertEquals('2', $itemsWithCountBiggerThanOnce[0]->count);
     }

From ab96297e58d92ea588d6c3b7d205aa289f4190bd Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Thu, 28 Mar 2024 17:06:18 +0100
Subject: [PATCH 17/19] Make sure VisitsTracker wraps as little operations as
 possible in the transaction

---
 module/Core/src/EventDispatcher/LocateVisit.php |  2 +-
 module/Core/src/Visit/VisitsTracker.php         | 11 +++++++----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/module/Core/src/EventDispatcher/LocateVisit.php b/module/Core/src/EventDispatcher/LocateVisit.php
index aa6afed8..6f7fb7e8 100644
--- a/module/Core/src/EventDispatcher/LocateVisit.php
+++ b/module/Core/src/EventDispatcher/LocateVisit.php
@@ -17,7 +17,7 @@ use Shlinkio\Shlink\IpGeolocation\Model\Location;
 use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
 use Throwable;
 
-class LocateVisit
+readonly class LocateVisit
 {
     public function __construct(
         private IpLocationResolverInterface $ipLocationResolver,
diff --git a/module/Core/src/Visit/VisitsTracker.php b/module/Core/src/Visit/VisitsTracker.php
index dd520e8f..f2e26493 100644
--- a/module/Core/src/Visit/VisitsTracker.php
+++ b/module/Core/src/Visit/VisitsTracker.php
@@ -71,12 +71,15 @@ readonly class VisitsTracker implements VisitsTrackerInterface
             return;
         }
 
-        $this->em->wrapInTransaction(function () use ($createVisit, $visitor): void {
-            $visit = $createVisit($visitor->normalizeForTrackingOptions($this->options));
+        $visit = $createVisit($visitor->normalizeForTrackingOptions($this->options));
+
+        // Wrap persisting and flushing the visit in a transaction, so that the ShortUrlVisitsCountTracker performs
+        // changes inside that very same transaction atomically
+        $this->em->wrapInTransaction(function () use ($visit): void {
             $this->em->persist($visit);
             $this->em->flush();
-
-            $this->eventDispatcher->dispatch(new UrlVisited($visit->getId(), $visitor->remoteAddress));
         });
+
+        $this->eventDispatcher->dispatch(new UrlVisited($visit->getId(), $visitor->remoteAddress));
     }
 }

From 1331b3f87c6e67156c2a3c5a1ca8ee7404e13e2a Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Thu, 28 Mar 2024 17:24:00 +0100
Subject: [PATCH 18/19] Fix RabbitMQ dev port

---
 config/autoload/rabbit.local.php.dist | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/autoload/rabbit.local.php.dist b/config/autoload/rabbit.local.php.dist
index b758528e..d19f82c6 100644
--- a/config/autoload/rabbit.local.php.dist
+++ b/config/autoload/rabbit.local.php.dist
@@ -7,7 +7,7 @@ return [
     'rabbitmq' => [
         'enabled' => true,
         'host' => 'shlink_rabbitmq',
-        'port' => '5673',
+        'port' => '5672',
         'user' => 'rabbit',
         'password' => 'rabbit',
     ],

From 8cb5d44dc9c877e47524e128a372e7b215333d26 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Thu, 28 Mar 2024 17:27:49 +0100
Subject: [PATCH 19/19] Update changelog

---
 CHANGELOG.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8157c47f..7b5f5cff 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 
 ### Changed
 * [#2034](https://github.com/shlinkio/shlink/issues/2034) Modernize entities, using constructor property promotion and readonly wherever possible.
+* [#2036](https://github.com/shlinkio/shlink/issues/2036) Deep performance improvement when listing short URLs ordered by visits counts.
+
+  This has been achieved by introducing a new table which tracks slotted visits counts. We can then `SUM` all counts for certain visit, avoiding `COUNT(visits)` aggregates which are less performant when there are a lot of visits.
 
 ### Deprecated
 * *Nothing*