Feature/slotted counts

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*

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",
@@ -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",

config/autoload/entity-manager.global.php

@@ -2,8 +2,10 @@
 
 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\ShortUrlVisitsCountTracker;
 
 use function Shlinkio\Shlink\Core\ArrayUtils\contains;
 
@@ -60,6 +62,10 @@
  'proxies_dir' => 'data/proxies',
  'load_mappings_using_functional_style' => true,
  'default_repository_classname' => EntitySpecificationRepository::class,
+ 'listeners' => [
+ Events::onFlush => [ShortUrlVisitsCountTracker::class],
+ Events::postFlush => [ShortUrlVisitsCountTracker::class],
+ ],
  ],
  'connection' => $resolveConnection(),
  ],

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',
  ],

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

@@ -13,6 +13,7 @@
 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\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
@@ -176,6 +177,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int
  return ExitCode::EXIT_SUCCESS;
  }
 
+ /**
+ * @param array<string, callable(array $serializedShortUrl, ShortUrl $shortUrl): ?string> $columnsMap
+ */
  private function renderPage(
  OutputInterface $output,
  array $columnsMap,
@@ -184,10 +188,10 @@ private function renderPage(
  ): Paginator {
  $shortUrls = $this->shortUrlService->listShortUrls($params);
 
- $rows = array_map(function (ShortUrl $shortUrl) use ($columnsMap) {
- $rawShortUrl = $this->transformer->transform($shortUrl);
- return array_map(fn (callable $call) => $call($rawShortUrl, $shortUrl), $columnsMap);
- }, [...$shortUrls]);
+ $rows = map([...$shortUrls], function (ShortUrlWithVisitsSummary $shortUrl) use ($columnsMap) {
+ $serializedShortUrl = $this->transformer->transform($shortUrl);
+ return map($columnsMap, fn (callable $call) => $call($serializedShortUrl, $shortUrl->shortUrl));
+ });
 
  ShlinkTable::default($output)->render(
  array_keys($columnsMap),
@@ -209,6 +213,9 @@ private function processOrderBy(InputInterface $input): ?string
  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];

module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php

@@ -16,6 +16,7 @@
 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 @@ public function loadingMorePagesCallsListMoreTimes(): void
  // 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 @@ public function havingMorePagesButAnsweringNoCallsListJustOnce(): void
  // 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 @@ public function provideOptionalFlagsMakesNewColumnsToBeIncluded(
  $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']);

module/Core/config/dependencies.config.php

@@ -76,6 +76,7 @@
  EntityRepositoryFactory::class,
  Visit\Entity\Visit::class,
  ],
+ Visit\Listener\ShortUrlVisitsCountTracker::class => InvokableFactory::class,
 
  Util\DoctrineBatchHelper::class => ConfigAbstractFactory::class,
  Util\RedirectResponseHelper::class => ConfigAbstractFactory::class,

module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php

@@ -67,6 +67,11 @@
  ->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')

module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.ShortUrlVisitsCount.php

@@ -0,0 +1,44 @@
+<?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)
+ ->option('default', 1)
+ ->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();
+
+ $builder->addUniqueConstraint(['short_url_id', 'potential_bot', 'slot_id'], 'UQ_slot_per_short_url');
+};

module/Core/migrations/Version20240306132518.php

@@ -0,0 +1,65 @@
+<?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,
+ ]);
+
+ $table->addUniqueIndex(['short_url_id', 'potential_bot', 'slot_id'], 'UQ_slot_per_short_url');
+ }
+
+ 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);
+ }
+}

module/Core/migrations/Version20240318084804.php

@@ -0,0 +1,58 @@
+<?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();
+
+ if ($botsCount > 0) {
+ $this->insertCount($shortUrlId, $botsCount, potentialBot: true);
+ }
+ if ($nonBotsCount > 0) {
+ $this->insertCount($shortUrlId, $nonBotsCount, potentialBot: false);
+ }
+ }
+ }
+
+ private function insertCount(string|int $shortUrlId, string|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();
+ }
+}

module/Core/src/EventDispatcher/LocateVisit.php

@@ -17,7 +17,7 @@
 use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface;
 use Throwable;
 
-class LocateVisit
+readonly class LocateVisit
 {
  public function __construct(
  private IpLocationResolverInterface $ipLocationResolver,

module/Core/src/Model/Ordering.php

@@ -10,10 +10,15 @@
  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)
  {
  }
 
+ public static function none(): self
+ {
+ return new self();
+ }
+
  /**
  * @param array{string|null, string|null} $props
  */
@@ -23,11 +28,6 @@ public static function fromTuple(array $props): self
  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);