Make sure short URL domain is resolved as null when default one is provided

This commit is contained in:
Alejandro Celaya 2023-04-22 19:44:04 +02:00
parent de86b62cdd
commit c582eba753
7 changed files with 31 additions and 28 deletions

View file

@ -31,7 +31,6 @@ class CreateShortUrlCommand extends Command
public const NAME = 'short-url:create'; public const NAME = 'short-url:create';
private ?SymfonyStyle $io; private ?SymfonyStyle $io;
private string $defaultDomain;
public function __construct( public function __construct(
private readonly UrlShortenerInterface $urlShortener, private readonly UrlShortenerInterface $urlShortener,
@ -39,7 +38,6 @@ class CreateShortUrlCommand extends Command
private readonly UrlShortenerOptions $options, private readonly UrlShortenerOptions $options,
) { ) {
parent::__construct(); parent::__construct();
$this->defaultDomain = $this->options->domain['hostname'] ?? '';
} }
protected function configure(): void protected function configure(): void
@ -121,7 +119,6 @@ class CreateShortUrlCommand extends Command
protected function interact(InputInterface $input, OutputInterface $output): void protected function interact(InputInterface $input, OutputInterface $output): void
{ {
$this->verifyLongUrlArgument($input, $output); $this->verifyLongUrlArgument($input, $output);
$this->verifyDomainArgument($input);
} }
private function verifyLongUrlArgument(InputInterface $input, OutputInterface $output): void private function verifyLongUrlArgument(InputInterface $input, OutputInterface $output): void
@ -138,12 +135,6 @@ class CreateShortUrlCommand extends Command
} }
} }
private function verifyDomainArgument(InputInterface $input): void
{
$domain = $input->getOption('domain');
$input->setOption('domain', $domain === $this->defaultDomain ? null : $domain);
}
protected function execute(InputInterface $input, OutputInterface $output): ?int protected function execute(InputInterface $input, OutputInterface $output): ?int
{ {
$io = $this->getIO($input, $output); $io = $this->getIO($input, $output);

View file

@ -28,8 +28,6 @@ class CreateShortUrlCommandTest extends TestCase
{ {
use CliTestUtilsTrait; use CliTestUtilsTrait;
private const DEFAULT_DOMAIN = 'default.com';
private CommandTester $commandTester; private CommandTester $commandTester;
private MockObject & UrlShortenerInterface $urlShortener; private MockObject & UrlShortenerInterface $urlShortener;
private MockObject & ShortUrlStringifierInterface $stringifier; private MockObject & ShortUrlStringifierInterface $stringifier;
@ -43,7 +41,7 @@ class CreateShortUrlCommandTest extends TestCase
$this->urlShortener, $this->urlShortener,
$this->stringifier, $this->stringifier,
new UrlShortenerOptions( new UrlShortenerOptions(
domain: ['hostname' => self::DEFAULT_DOMAIN, 'schema' => ''], domain: ['hostname' => 'example.com', 'schema' => ''],
defaultShortCodesLength: 5, defaultShortCodesLength: 5,
), ),
); );
@ -147,9 +145,8 @@ class CreateShortUrlCommandTest extends TestCase
public static function provideDomains(): iterable public static function provideDomains(): iterable
{ {
yield 'no domain' => [[], null]; yield 'no domain' => [[], null];
yield 'non-default domain foo' => [['--domain' => 'foo.com'], 'foo.com']; yield 'domain foo' => [['--domain' => 'foo.com'], 'foo.com'];
yield 'non-default domain bar' => [['-d' => 'bar.com'], 'bar.com']; yield 'domain bar' => [['-d' => 'bar.com'], 'bar.com'];
yield 'default domain' => [['--domain' => self::DEFAULT_DOMAIN], null];
} }
#[Test, DataProvider('provideFlags')] #[Test, DataProvider('provideFlags')]

View file

@ -161,7 +161,7 @@ return [
], ],
Action\RobotsAction::class => [Crawling\CrawlingHelper::class], Action\RobotsAction::class => [Crawling\CrawlingHelper::class],
ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class => ['em'], ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class => ['em', Options\UrlShortenerOptions::class],
ShortUrl\Helper\ShortUrlStringifier::class => ['config.url_shortener.domain', 'config.router.base_path'], ShortUrl\Helper\ShortUrlStringifier::class => ['config.url_shortener.domain', 'config.router.base_path'],
ShortUrl\Helper\ShortUrlTitleResolutionHelper::class => [Util\UrlValidator::class], ShortUrl\Helper\ShortUrlTitleResolutionHelper::class => [Util\UrlValidator::class],
ShortUrl\Helper\ShortUrlRedirectionBuilder::class => [Options\TrackingOptions::class], ShortUrl\Helper\ShortUrlRedirectionBuilder::class => [Options\TrackingOptions::class],

View file

@ -26,4 +26,9 @@ final class UrlShortenerOptions
{ {
return $this->mode === ShortUrlMode::LOOSE; return $this->mode === ShortUrlMode::LOOSE;
} }
public function defaultDomain(): string
{
return $this->domain['hostname'] ?? '';
}
} }

View file

@ -9,6 +9,7 @@ use Doctrine\Common\Collections\Collection;
use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Events; use Doctrine\ORM\Events;
use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Domain\Entity\Domain;
use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
use Shlinkio\Shlink\Core\Tag\Entity\Tag; use Shlinkio\Shlink\Core\Tag\Entity\Tag;
use function Functional\map; use function Functional\map;
@ -21,15 +22,17 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt
/** @var array<string, Tag> */ /** @var array<string, Tag> */
private array $memoizedNewTags = []; private array $memoizedNewTags = [];
public function __construct(private readonly EntityManagerInterface $em) public function __construct(
{ private readonly EntityManagerInterface $em,
private readonly UrlShortenerOptions $options = new UrlShortenerOptions(),
) {
// Registering this as an event listener will make the postFlush method to be called automatically // Registering this as an event listener will make the postFlush method to be called automatically
$this->em->getEventManager()->addEventListener(Events::postFlush, $this); $this->em->getEventManager()->addEventListener(Events::postFlush, $this);
} }
public function resolveDomain(?string $domain): ?Domain public function resolveDomain(?string $domain): ?Domain
{ {
if ($domain === null) { if ($domain === null || $domain === $this->options->defaultDomain()) {
return null; return null;
} }
@ -42,9 +45,7 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt
private function memoizeNewDomain(string $domain): Domain private function memoizeNewDomain(string $domain): Domain
{ {
return $this->memoizedNewDomains[$domain] = $this->memoizedNewDomains[$domain] ?? Domain::withAuthority( return $this->memoizedNewDomains[$domain] ??= Domain::withAuthority($domain);
$domain,
);
} }
/** /**
@ -71,7 +72,7 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt
private function memoizeNewTag(string $tagName): Tag private function memoizeNewTag(string $tagName): Tag
{ {
return $this->memoizedNewTags[$tagName] = $this->memoizedNewTags[$tagName] ?? new Tag($tagName); return $this->memoizedNewTags[$tagName] ??= new Tag($tagName);
} }
public function postFlush(): void public function postFlush(): void

View file

@ -25,7 +25,7 @@ class ShortUrlListService implements ShortUrlListServiceInterface
*/ */
public function listShortUrls(ShortUrlsParams $params, ?ApiKey $apiKey = null): Paginator public function listShortUrls(ShortUrlsParams $params, ?ApiKey $apiKey = null): Paginator
{ {
$defaultDomain = $this->urlShortenerOptions->domain['hostname'] ?? ''; $defaultDomain = $this->urlShortenerOptions->defaultDomain();
$paginator = new Paginator(new ShortUrlRepositoryAdapter($this->repo, $params, $apiKey, $defaultDomain)); $paginator = new Paginator(new ShortUrlRepositoryAdapter($this->repo, $params, $apiKey, $defaultDomain));
$paginator->setMaxPerPage($params->itemsPerPage) $paginator->setMaxPerPage($params->itemsPerPage)
->setCurrentPage($params->page); ->setCurrentPage($params->page);

View file

@ -12,6 +12,7 @@ use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Domain\Entity\Domain;
use Shlinkio\Shlink\Core\Domain\Repository\DomainRepositoryInterface; use Shlinkio\Shlink\Core\Domain\Repository\DomainRepositoryInterface;
use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver;
use Shlinkio\Shlink\Core\Tag\Entity\Tag; use Shlinkio\Shlink\Core\Tag\Entity\Tag;
use Shlinkio\Shlink\Core\Tag\Repository\TagRepositoryInterface; use Shlinkio\Shlink\Core\Tag\Repository\TagRepositoryInterface;
@ -28,14 +29,22 @@ class PersistenceShortUrlRelationResolverTest extends TestCase
$this->em = $this->createMock(EntityManagerInterface::class); $this->em = $this->createMock(EntityManagerInterface::class);
$this->em->method('getEventManager')->willReturn(new EventManager()); $this->em->method('getEventManager')->willReturn(new EventManager());
$this->resolver = new PersistenceShortUrlRelationResolver($this->em); $this->resolver = new PersistenceShortUrlRelationResolver($this->em, new UrlShortenerOptions(
domain: ['schema' => 'https', 'hostname' => 'default.com'],
));
} }
#[Test] #[Test, DataProvider('provideDomainsThatEmpty')]
public function returnsEmptyWhenNoDomainIsProvided(): void public function returnsEmptyInSomeCases(?string $domain): void
{ {
$this->em->expects($this->never())->method('getRepository')->with(Domain::class); $this->em->expects($this->never())->method('getRepository')->with(Domain::class);
self::assertNull($this->resolver->resolveDomain(null)); self::assertNull($this->resolver->resolveDomain($domain));
}
public static function provideDomainsThatEmpty(): iterable
{
yield 'null' => [null];
yield 'default domain' => ['default.com'];
} }
#[Test, DataProvider('provideFoundDomains')] #[Test, DataProvider('provideFoundDomains')]