From 745255736a26e51216c6e1d5d302ee15895940ac Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 14 Feb 2025 10:20:50 +0100 Subject: [PATCH 1/3] Simplify query to find short URL when domain is null --- .../Repository/ShortUrlRepository.php | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php index bb6abea2..56921ef0 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php @@ -31,24 +31,33 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU $ordering = $dbPlatform instanceof PostgreSQLPlatform ? 'ASC' : 'DESC'; $isStrict = $shortUrlMode === ShortUrlMode::STRICT; - $qb = $this->createQueryBuilder('s'); - $qb->leftJoin('s.domain', 'd') - ->where($qb->expr()->eq($isStrict ? 's.shortCode' : 'LOWER(s.shortCode)', ':shortCode')) - ->setParameter('shortCode', $isStrict ? $identifier->shortCode : strtolower($identifier->shortCode)) - ->andWhere($qb->expr()->orX( - $qb->expr()->isNull('s.domain'), - $qb->expr()->eq('d.authority', ':domain'), - )) - ->setParameter('domain', $identifier->domain); + // FIXME The `LOWER(s.shortCode)` condition in non-strict mode drops performance dramatically. + // Investigate if the case-insensitive check can be done natively by the DB engine. - // Since we order by domain, we will have first the URL matching provided domain, followed by the one - // with no domain (if any), so it is safe to fetch 1 max result, and we will get: - // * The short URL matching both the short code and the domain, or - // * The short URL matching the short code but without any domain, or - // * No short URL at all - $qb->orderBy('s.domain', $ordering) + $qb = $this->createQueryBuilder('s'); + $qb->where($qb->expr()->eq($isStrict ? 's.shortCode' : 'LOWER(s.shortCode)', ':shortCode')) + ->setParameter('shortCode', $isStrict ? $identifier->shortCode : strtolower($identifier->shortCode)) ->setMaxResults(1); + // If $domain is null, do not join with domains nor do $qb->expr()->eq('d.authority', ':domain') + $domain = $identifier->domain; + if ($domain === null) { + $qb->andWhere($qb->expr()->isNull('s.domain')); + } else { + $qb->leftJoin('s.domain', 'd') + ->andWhere($qb->expr()->orX( + $qb->expr()->isNull('s.domain'), + $qb->expr()->eq('d.authority', ':domain'), + )) + ->setParameter('domain', $domain) + // Since we order by domain, we will have first the URL matching provided domain, followed by the one + // with no domain (if any), so it is safe to fetch 1 max result, and we will get: + // * The short URL matching both the short code and the domain, or + // * The short URL matching the short code but without any domain, or + // * No short URL at all + ->orderBy('s.domain', $ordering); + } + return $qb->getQuery()->getOneOrNullResult(); } From f3da345bf34036f80fb30eee74ed4b3d948bf0a5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 15 Feb 2025 11:17:14 +0100 Subject: [PATCH 2/3] Fix unique_short_code_plus_domain index in Microsoft SQL --- .../Core/migrations/Version20250215100756.php | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 module/Core/migrations/Version20250215100756.php diff --git a/module/Core/migrations/Version20250215100756.php b/module/Core/migrations/Version20250215100756.php new file mode 100644 index 00000000..77e9599e --- /dev/null +++ b/module/Core/migrations/Version20250215100756.php @@ -0,0 +1,43 @@ +skipIf(! $this->isMicrosoftSql()); + + // Drop the existing unique index + $shortUrls = $schema->getTable('short_urls'); + $shortUrls->dropIndex('unique_short_code_plus_domain'); + } + + public function postUp(Schema $schema): void + { + // The only way to get the index properly generated is by hardcoding the SQL. + // Since this migration is run Microsoft SQL only, it is safe to use this approach. + $this->connection->executeStatement( + 'CREATE UNIQUE INDEX unique_short_code_plus_domain ON short_urls (short_code, domain_id);', + ); + } + + private function isMicrosoftSql(): bool + { + return $this->connection->getDatabasePlatform() instanceof SQLServerPlatform; + } +} From 053e1f307381ee86fea21b1bf17f2c171a7b9670 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 15 Feb 2025 11:19:30 +0100 Subject: [PATCH 3/3] Update changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8d3d2ce..8bbfd348 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## [4.4.3] - 2025-02-15 ### Added * *Nothing* @@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this In the future, Shlink will allow you to define trusted proxies, to avoid other potential side effects because of this reversing of the list. * [#2354](https://github.com/shlinkio/shlink/issues/2354) Fix error "NOSCRIPT No matching script. Please use EVAL" thrown when creating a lock in redis. +* [#2319](https://github.com/shlinkio/shlink/issues/2319) Fix unique index for `short_code` and `domain_id` in `short_urls` table not being used in Microsoft SQL engines for rows where `domain_id` is `null`. ## [4.4.2] - 2025-01-29 ### Added