diff --git a/CHANGELOG.md b/CHANGELOG.md index 265b419d..c03274f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * *Nothing* ### Fixed -* *Nothing* +* [#1682](https://github.com/shlinkio/shlink/issues/1682) Fixed incorrect case-insensitive checks in short URLs when using Microsoft SQL server. ## [3.5.0] - 2023-01-28 diff --git a/data/migrations/Version20230130090946.php b/data/migrations/Version20230130090946.php new file mode 100644 index 00000000..49e6d9bb --- /dev/null +++ b/data/migrations/Version20230130090946.php @@ -0,0 +1,50 @@ +skipIf(! $this->isMsSql(), 'This only sets MsSQL-specific database options'); + + $shortUrls = $schema->getTable('short_urls'); + $shortCode = $shortUrls->getColumn('short_code'); + // Drop the unique index before changing the collation, as the field is part of this index + $shortUrls->dropIndex('unique_short_code_plus_domain'); + $shortCode->setPlatformOption('collation', 'Latin1_General_CS_AS'); + } + + public function postUp(Schema $schema): void + { + if ($this->isMsSql()) { + // The index needs to be re-created in postUp, but here, we can only use statements run against the + // connection directly + $this->connection->executeStatement( + 'CREATE INDEX unique_short_code_plus_domain ON short_urls (domain_id, short_code);', + ); + } + } + + public function down(Schema $schema): void + { + // No down + } + + public function isTransactional(): bool + { + return ! ($this->connection->getDatabasePlatform() instanceof MySQLPlatform); + } + + private function isMsSql(): bool + { + return $this->connection->getDatabasePlatform() instanceof SQLServerPlatform; + } +} diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index 0328923a..15f1998c 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -39,8 +39,8 @@ class ShortUrl extends AbstractEntity private string $longUrl; private string $shortCode; private Chronos $dateCreated; - /** @var Collection */ - private Collection $visits; + /** @var Collection & Selectable */ + private Collection & Selectable $visits; /** @var Collection */ private Collection $deviceLongUrls; /** @var Collection */ @@ -255,23 +255,19 @@ class ShortUrl extends AbstractEntity public function mostRecentImportedVisitDate(): ?Chronos { - /** @var Selectable $visits */ - $visits = $this->visits; $criteria = Criteria::create()->where(Criteria::expr()->eq('type', VisitType::IMPORTED)) ->orderBy(['id' => 'DESC']) ->setMaxResults(1); + $visit = $this->visits->matching($criteria)->last(); - /** @var Visit|false $visit */ - $visit = $visits->matching($criteria)->last(); - - return $visit === false ? null : $visit->getDate(); + return $visit instanceof Visit ? $visit->getDate() : null; } /** - * @param Collection $visits + * @param Collection & Selectable $visits * @internal */ - public function setVisits(Collection $visits): self + public function setVisits(Collection & Selectable $visits): self { $this->visits = $visits; return $this; diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php index 5f47ffb3..31cac3dc 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace ShlinkioDbTest\Shlink\Core\ShortUrl\Repository; use Cake\Chronos\Chronos; -use Doctrine\DBAL\Platforms\SQLServerPlatform; use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; @@ -61,13 +60,10 @@ class ShortUrlRepositoryTest extends DatabaseTestCase ShortUrlIdentifier::fromShortCodeAndDomain('fOo'), ShortUrlMode::LOOSE, )); - // TODO MS is doing loose checks always, making this fail. - if (! $this->getEntityManager()->getConnection()->getDatabasePlatform() instanceof SQLServerPlatform) { - self::assertNull($this->repo->findOneWithDomainFallback( - ShortUrlIdentifier::fromShortCodeAndDomain('foo'), - ShortUrlMode::STRICT, - )); - } + self::assertNull($this->repo->findOneWithDomainFallback( + ShortUrlIdentifier::fromShortCodeAndDomain('foo'), + ShortUrlMode::STRICT, + )); self::assertSame($regularOne, $this->repo->findOneWithDomainFallback( ShortUrlIdentifier::fromShortCodeAndDomain($withDomainDuplicatingRegular->getShortCode()), ShortUrlMode::STRICT,