From 2f83e90c8ba07edbcc948fa8efe6972d719689d1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 26 Jan 2023 20:45:36 +0100 Subject: [PATCH] Add option to do loosely matches on short URLs when mode is loosely --- module/Core/config/dependencies.config.php | 2 +- .../MultiSegmentSlugProcessor.php | 6 +-- .../Repository/ShortUrlRepository.php | 37 +++++++++---------- .../ShortUrlRepositoryInterface.php | 3 +- module/Core/src/ShortUrl/ShortUrlResolver.php | 9 +++-- .../Repository/ShortUrlRepositoryTest.php | 30 +++++++++++++-- .../test/ShortUrl/ShortUrlResolverTest.php | 6 ++- 7 files changed, 61 insertions(+), 32 deletions(-) diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 40985f42..4555bff1 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -136,7 +136,7 @@ return [ Options\DeleteShortUrlsOptions::class, ShortUrl\ShortUrlResolver::class, ], - ShortUrl\ShortUrlResolver::class => ['em'], + ShortUrl\ShortUrlResolver::class => ['em', Options\UrlShortenerOptions::class], ShortUrl\Helper\ShortCodeUniquenessHelper::class => ['em', Options\UrlShortenerOptions::class], Domain\DomainService::class => ['em', 'config.url_shortener.domain.hostname'], diff --git a/module/Core/src/Config/PostProcessor/MultiSegmentSlugProcessor.php b/module/Core/src/Config/PostProcessor/MultiSegmentSlugProcessor.php index b84491f6..33945063 100644 --- a/module/Core/src/Config/PostProcessor/MultiSegmentSlugProcessor.php +++ b/module/Core/src/Config/PostProcessor/MultiSegmentSlugProcessor.php @@ -9,8 +9,8 @@ use function str_replace; class MultiSegmentSlugProcessor { - private const SINGLE_SHORT_CODE_PATTERN = '{shortCode}'; - private const MULTI_SHORT_CODE_PATTERN = '{shortCode:.+}'; + private const SINGLE_SEGMENT_PATTERN = '{shortCode}'; + private const MULTI_SEGMENT_PATTERN = '{shortCode:.+}'; public function __invoke(array $config): array { @@ -21,7 +21,7 @@ class MultiSegmentSlugProcessor $config['routes'] = map($config['routes'] ?? [], static function (array $route): array { ['path' => $path] = $route; - $route['path'] = str_replace(self::SINGLE_SHORT_CODE_PATTERN, self::MULTI_SHORT_CODE_PATTERN, $path); + $route['path'] = str_replace(self::SINGLE_SEGMENT_PATTERN, self::MULTI_SEGMENT_PATTERN, $path); return $route; }); diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php index ee2f7389..f8e384e2 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php @@ -14,42 +14,41 @@ use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use function count; +use function strtolower; class ShortUrlRepository extends EntitySpecificationRepository implements ShortUrlRepositoryInterface { - public function findOneWithDomainFallback(ShortUrlIdentifier $identifier): ?ShortUrl + public function findOneWithDomainFallback(ShortUrlIdentifier $identifier, ShortUrlMode $shortUrlMode): ?ShortUrl { // When ordering DESC, Postgres puts nulls at the beginning while the rest of supported DB engines put them at // the bottom $dbPlatform = $this->getEntityManager()->getConnection()->getDatabasePlatform(); $ordering = $dbPlatform instanceof PostgreSQLPlatform ? 'ASC' : 'DESC'; + $isStrict = $shortUrlMode === ShortUrlMode::STRICT; - $dql = <<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); - $query = $this->getEntityManager()->createQuery($dql); - $query->setMaxResults(1) - ->setParameters([ - 'shortCode' => $identifier->shortCode, - 'domain' => $identifier->domain, - ]); - - // Since we ordered 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: + // 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) + ->setMaxResults(1); - return $query->getOneOrNullResult(); + return $qb->getQuery()->getOneOrNullResult(); } public function findOne(ShortUrlIdentifier $identifier, ?Specification $spec = null): ?ShortUrl diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php index 18a4ec71..8af53cb9 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php @@ -10,11 +10,12 @@ use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificationRepositoryInterface { - public function findOneWithDomainFallback(ShortUrlIdentifier $identifier): ?ShortUrl; + public function findOneWithDomainFallback(ShortUrlIdentifier $identifier, ShortUrlMode $shortUrlMode): ?ShortUrl; public function findOne(ShortUrlIdentifier $identifier, ?Specification $spec = null): ?ShortUrl; diff --git a/module/Core/src/ShortUrl/ShortUrlResolver.php b/module/Core/src/ShortUrl/ShortUrlResolver.php index 20ec930b..2c4f7bdc 100644 --- a/module/Core/src/ShortUrl/ShortUrlResolver.php +++ b/module/Core/src/ShortUrl/ShortUrlResolver.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\ShortUrl; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository; @@ -13,8 +14,10 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; class ShortUrlResolver implements ShortUrlResolverInterface { - public function __construct(private readonly EntityManagerInterface $em) - { + public function __construct( + private readonly EntityManagerInterface $em, + private readonly UrlShortenerOptions $urlShortenerOptions, + ) { } /** @@ -39,7 +42,7 @@ class ShortUrlResolver implements ShortUrlResolverInterface { /** @var ShortUrlRepository $shortUrlRepo */ $shortUrlRepo = $this->em->getRepository(ShortUrl::class); - $shortUrl = $shortUrlRepo->findOneWithDomainFallback($identifier); + $shortUrl = $shortUrlRepo->findOneWithDomainFallback($identifier, $this->urlShortenerOptions->mode); if (! $shortUrl?->isEnabled()) { throw ShortUrlNotFoundException::fromNotFound($identifier); } diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php index 0d90675a..b5633ee6 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php @@ -9,6 +9,7 @@ use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; @@ -32,7 +33,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase /** @test */ public function findOneWithDomainFallbackReturnsProperData(): void { - $regularOne = ShortUrl::create(ShortUrlCreation::fromRawData(['customSlug' => 'foo', 'longUrl' => 'foo'])); + $regularOne = ShortUrl::create(ShortUrlCreation::fromRawData(['customSlug' => 'Foo', 'longUrl' => 'foo'])); $this->getEntityManager()->persist($regularOne); $withDomain = ShortUrl::create(ShortUrlCreation::fromRawData( @@ -41,7 +42,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($withDomain); $withDomainDuplicatingRegular = ShortUrl::create(ShortUrlCreation::fromRawData( - ['domain' => 's.test', 'customSlug' => 'foo', 'longUrl' => 'foo_with_domain'], + ['domain' => 's.test', 'customSlug' => 'Foo', 'longUrl' => 'foo_with_domain'], )); $this->getEntityManager()->persist($withDomainDuplicatingRegular); @@ -49,29 +50,50 @@ class ShortUrlRepositoryTest extends DatabaseTestCase self::assertSame($regularOne, $this->repo->findOneWithDomainFallback( ShortUrlIdentifier::fromShortCodeAndDomain($regularOne->getShortCode()), + ShortUrlMode::STRICT, )); + self::assertSame($regularOne, $this->repo->findOneWithDomainFallback( + ShortUrlIdentifier::fromShortCodeAndDomain('foo'), + ShortUrlMode::LOOSELY, + )); + self::assertSame($regularOne, $this->repo->findOneWithDomainFallback( + ShortUrlIdentifier::fromShortCodeAndDomain('fOo'), + ShortUrlMode::LOOSELY, + )); +// self::assertNull($this->repo->findOneWithDomainFallback( // TODO MS is doing loosely checks always +// ShortUrlIdentifier::fromShortCodeAndDomain('foo'), +// ShortUrlMode::STRICT, +// )); self::assertSame($regularOne, $this->repo->findOneWithDomainFallback( ShortUrlIdentifier::fromShortCodeAndDomain($withDomainDuplicatingRegular->getShortCode()), + ShortUrlMode::STRICT, )); self::assertSame($withDomain, $this->repo->findOneWithDomainFallback( ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode(), 'example.com'), + ShortUrlMode::STRICT, )); self::assertSame( $withDomainDuplicatingRegular, $this->repo->findOneWithDomainFallback( ShortUrlIdentifier::fromShortCodeAndDomain($withDomainDuplicatingRegular->getShortCode(), 's.test'), + ShortUrlMode::STRICT, ), ); self::assertSame($regularOne, $this->repo->findOneWithDomainFallback(ShortUrlIdentifier::fromShortCodeAndDomain( $withDomainDuplicatingRegular->getShortCode(), 'other-domain.com', - ))); - self::assertNull($this->repo->findOneWithDomainFallback(ShortUrlIdentifier::fromShortCodeAndDomain('invalid'))); + ), ShortUrlMode::STRICT)); + self::assertNull($this->repo->findOneWithDomainFallback( + ShortUrlIdentifier::fromShortCodeAndDomain('invalid'), + ShortUrlMode::STRICT, + )); self::assertNull($this->repo->findOneWithDomainFallback( ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode()), + ShortUrlMode::STRICT, )); self::assertNull($this->repo->findOneWithDomainFallback( ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode(), 'other-domain.com'), + ShortUrlMode::STRICT, )); } diff --git a/module/Core/test/ShortUrl/ShortUrlResolverTest.php b/module/Core/test/ShortUrl/ShortUrlResolverTest.php index 177e432e..9c42fefb 100644 --- a/module/Core/test/ShortUrl/ShortUrlResolverTest.php +++ b/module/Core/test/ShortUrl/ShortUrlResolverTest.php @@ -10,9 +10,11 @@ use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolver; use Shlinkio\Shlink\Core\Visit\Entity\Visit; @@ -35,7 +37,7 @@ class ShortUrlResolverTest extends TestCase { $this->em = $this->createMock(EntityManagerInterface::class); $this->repo = $this->createMock(ShortUrlRepositoryInterface::class); - $this->urlResolver = new ShortUrlResolver($this->em); + $this->urlResolver = new ShortUrlResolver($this->em, new UrlShortenerOptions()); } /** @@ -83,6 +85,7 @@ class ShortUrlResolverTest extends TestCase $this->repo->expects($this->once())->method('findOneWithDomainFallback')->with( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), + ShortUrlMode::STRICT, )->willReturn($shortUrl); $this->em->expects($this->once())->method('getRepository')->with(ShortUrl::class)->willReturn($this->repo); @@ -101,6 +104,7 @@ class ShortUrlResolverTest extends TestCase $this->repo->expects($this->once())->method('findOneWithDomainFallback')->with( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), + ShortUrlMode::STRICT, )->willReturn($shortUrl); $this->em->expects($this->once())->method('getRepository')->with(ShortUrl::class)->willReturn($this->repo);