Add option to do loosely matches on short URLs when mode is loosely

This commit is contained in:
Alejandro Celaya 2023-01-26 20:45:36 +01:00
parent 05acd4ae88
commit 2f83e90c8b
7 changed files with 61 additions and 32 deletions

View file

@ -136,7 +136,7 @@ return [
Options\DeleteShortUrlsOptions::class, Options\DeleteShortUrlsOptions::class,
ShortUrl\ShortUrlResolver::class, ShortUrl\ShortUrlResolver::class,
], ],
ShortUrl\ShortUrlResolver::class => ['em'], ShortUrl\ShortUrlResolver::class => ['em', Options\UrlShortenerOptions::class],
ShortUrl\Helper\ShortCodeUniquenessHelper::class => ['em', Options\UrlShortenerOptions::class], ShortUrl\Helper\ShortCodeUniquenessHelper::class => ['em', Options\UrlShortenerOptions::class],
Domain\DomainService::class => ['em', 'config.url_shortener.domain.hostname'], Domain\DomainService::class => ['em', 'config.url_shortener.domain.hostname'],

View file

@ -9,8 +9,8 @@ use function str_replace;
class MultiSegmentSlugProcessor class MultiSegmentSlugProcessor
{ {
private const SINGLE_SHORT_CODE_PATTERN = '{shortCode}'; private const SINGLE_SEGMENT_PATTERN = '{shortCode}';
private const MULTI_SHORT_CODE_PATTERN = '{shortCode:.+}'; private const MULTI_SEGMENT_PATTERN = '{shortCode:.+}';
public function __invoke(array $config): array public function __invoke(array $config): array
{ {
@ -21,7 +21,7 @@ class MultiSegmentSlugProcessor
$config['routes'] = map($config['routes'] ?? [], static function (array $route): array { $config['routes'] = map($config['routes'] ?? [], static function (array $route): array {
['path' => $path] = $route; ['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; return $route;
}); });

View file

@ -14,42 +14,41 @@ use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType;
use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode;
use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl;
use function count; use function count;
use function strtolower;
class ShortUrlRepository extends EntitySpecificationRepository implements ShortUrlRepositoryInterface 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 // When ordering DESC, Postgres puts nulls at the beginning while the rest of supported DB engines put them at
// the bottom // the bottom
$dbPlatform = $this->getEntityManager()->getConnection()->getDatabasePlatform(); $dbPlatform = $this->getEntityManager()->getConnection()->getDatabasePlatform();
$ordering = $dbPlatform instanceof PostgreSQLPlatform ? 'ASC' : 'DESC'; $ordering = $dbPlatform instanceof PostgreSQLPlatform ? 'ASC' : 'DESC';
$isStrict = $shortUrlMode === ShortUrlMode::STRICT;
$dql = <<<DQL $qb = $this->createQueryBuilder('s');
SELECT s $qb->leftJoin('s.domain', 'd')
FROM Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl AS s ->where($qb->expr()->eq($isStrict ? 's.shortCode' : 'LOWER(s.shortCode)', ':shortCode'))
LEFT JOIN s.domain AS d ->setParameter('shortCode', $isStrict ? $identifier->shortCode : strtolower($identifier->shortCode))
WHERE s.shortCode = :shortCode ->andWhere($qb->expr()->orX(
AND (s.domain IS NULL OR d.authority = :domain) $qb->expr()->isNull('s.domain'),
ORDER BY s.domain {$ordering} $qb->expr()->eq('d.authority', ':domain')
DQL; ))
->setParameter('domain', $identifier->domain);
$query = $this->getEntityManager()->createQuery($dql); // Since we order by domain, we will have first the URL matching provided domain, followed by the one
$query->setMaxResults(1) // with no domain (if any), so it is safe to fetch 1 max result, and we will get:
->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:
// * The short URL matching both the short code and the domain, or // * The short URL matching both the short code and the domain, or
// * The short URL matching the short code but without any domain, or // * The short URL matching the short code but without any domain, or
// * No short URL at all // * 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 public function findOne(ShortUrlIdentifier $identifier, ?Specification $spec = null): ?ShortUrl

View file

@ -10,11 +10,12 @@ use Happyr\DoctrineSpecification\Specification\Specification;
use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode;
use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl;
interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificationRepositoryInterface 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; public function findOne(ShortUrlIdentifier $identifier, ?Specification $spec = null): ?ShortUrl;

View file

@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\ShortUrl;
use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityManagerInterface;
use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository;
@ -13,8 +14,10 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey;
class ShortUrlResolver implements ShortUrlResolverInterface 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 */ /** @var ShortUrlRepository $shortUrlRepo */
$shortUrlRepo = $this->em->getRepository(ShortUrl::class); $shortUrlRepo = $this->em->getRepository(ShortUrl::class);
$shortUrl = $shortUrlRepo->findOneWithDomainFallback($identifier); $shortUrl = $shortUrlRepo->findOneWithDomainFallback($identifier, $this->urlShortenerOptions->mode);
if (! $shortUrl?->isEnabled()) { if (! $shortUrl?->isEnabled()) {
throw ShortUrlNotFoundException::fromNotFound($identifier); throw ShortUrlNotFoundException::fromNotFound($identifier);
} }

View file

@ -9,6 +9,7 @@ use Shlinkio\Shlink\Core\Domain\Entity\Domain;
use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; 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\Repository\ShortUrlRepository;
use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver;
use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl;
@ -32,7 +33,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
/** @test */ /** @test */
public function findOneWithDomainFallbackReturnsProperData(): void 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); $this->getEntityManager()->persist($regularOne);
$withDomain = ShortUrl::create(ShortUrlCreation::fromRawData( $withDomain = ShortUrl::create(ShortUrlCreation::fromRawData(
@ -41,7 +42,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
$this->getEntityManager()->persist($withDomain); $this->getEntityManager()->persist($withDomain);
$withDomainDuplicatingRegular = ShortUrl::create(ShortUrlCreation::fromRawData( $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); $this->getEntityManager()->persist($withDomainDuplicatingRegular);
@ -49,29 +50,50 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
self::assertSame($regularOne, $this->repo->findOneWithDomainFallback( self::assertSame($regularOne, $this->repo->findOneWithDomainFallback(
ShortUrlIdentifier::fromShortCodeAndDomain($regularOne->getShortCode()), 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( self::assertSame($regularOne, $this->repo->findOneWithDomainFallback(
ShortUrlIdentifier::fromShortCodeAndDomain($withDomainDuplicatingRegular->getShortCode()), ShortUrlIdentifier::fromShortCodeAndDomain($withDomainDuplicatingRegular->getShortCode()),
ShortUrlMode::STRICT,
)); ));
self::assertSame($withDomain, $this->repo->findOneWithDomainFallback( self::assertSame($withDomain, $this->repo->findOneWithDomainFallback(
ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode(), 'example.com'), ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode(), 'example.com'),
ShortUrlMode::STRICT,
)); ));
self::assertSame( self::assertSame(
$withDomainDuplicatingRegular, $withDomainDuplicatingRegular,
$this->repo->findOneWithDomainFallback( $this->repo->findOneWithDomainFallback(
ShortUrlIdentifier::fromShortCodeAndDomain($withDomainDuplicatingRegular->getShortCode(), 's.test'), ShortUrlIdentifier::fromShortCodeAndDomain($withDomainDuplicatingRegular->getShortCode(), 's.test'),
ShortUrlMode::STRICT,
), ),
); );
self::assertSame($regularOne, $this->repo->findOneWithDomainFallback(ShortUrlIdentifier::fromShortCodeAndDomain( self::assertSame($regularOne, $this->repo->findOneWithDomainFallback(ShortUrlIdentifier::fromShortCodeAndDomain(
$withDomainDuplicatingRegular->getShortCode(), $withDomainDuplicatingRegular->getShortCode(),
'other-domain.com', 'other-domain.com',
))); ), ShortUrlMode::STRICT));
self::assertNull($this->repo->findOneWithDomainFallback(ShortUrlIdentifier::fromShortCodeAndDomain('invalid'))); self::assertNull($this->repo->findOneWithDomainFallback(
ShortUrlIdentifier::fromShortCodeAndDomain('invalid'),
ShortUrlMode::STRICT,
));
self::assertNull($this->repo->findOneWithDomainFallback( self::assertNull($this->repo->findOneWithDomainFallback(
ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode()), ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode()),
ShortUrlMode::STRICT,
)); ));
self::assertNull($this->repo->findOneWithDomainFallback( self::assertNull($this->repo->findOneWithDomainFallback(
ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode(), 'other-domain.com'), ShortUrlIdentifier::fromShortCodeAndDomain($withDomain->getShortCode(), 'other-domain.com'),
ShortUrlMode::STRICT,
)); ));
} }

View file

@ -10,9 +10,11 @@ use Doctrine\ORM\EntityManagerInterface;
use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; 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\Repository\ShortUrlRepositoryInterface;
use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolver; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolver;
use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Entity\Visit;
@ -35,7 +37,7 @@ class ShortUrlResolverTest extends TestCase
{ {
$this->em = $this->createMock(EntityManagerInterface::class); $this->em = $this->createMock(EntityManagerInterface::class);
$this->repo = $this->createMock(ShortUrlRepositoryInterface::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( $this->repo->expects($this->once())->method('findOneWithDomainFallback')->with(
ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), ShortUrlIdentifier::fromShortCodeAndDomain($shortCode),
ShortUrlMode::STRICT,
)->willReturn($shortUrl); )->willReturn($shortUrl);
$this->em->expects($this->once())->method('getRepository')->with(ShortUrl::class)->willReturn($this->repo); $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( $this->repo->expects($this->once())->method('findOneWithDomainFallback')->with(
ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), ShortUrlIdentifier::fromShortCodeAndDomain($shortCode),
ShortUrlMode::STRICT,
)->willReturn($shortUrl); )->willReturn($shortUrl);
$this->em->expects($this->once())->method('getRepository')->with(ShortUrl::class)->willReturn($this->repo); $this->em->expects($this->once())->method('getRepository')->with(ShortUrl::class)->willReturn($this->repo);