diff --git a/CHANGELOG.md b/CHANGELOG.md index dc6cdef3..9aeca402 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added +* [#1163](https://github.com/shlinkio/shlink/issues/1163) Allowed setting not-found redirects for default domain in the same way it's done for any other domain. + + This implies a few non-breaking changes: + + * The domains list no longer has the values of `INVALID_SHORT_URL_REDIRECT_TO`, `REGULAR_404_REDIRECT_TO` and `BASE_URL_REDIRECT_TO` on the default domain redirects. + * The `GET /domains` endpoint includes a new `defaultRedirects` property in the response, with the default redirects set via config or env vars. + * The `INVALID_SHORT_URL_REDIRECT_TO`, `REGULAR_404_REDIRECT_TO` and `BASE_URL_REDIRECT_TO` env vars are now deprecated, and should be replaced by `DEFAULT_INVALID_SHORT_URL_REDIRECT`, `DEFAULT_REGULAR_404_REDIRECT` and `DEFAULT_BASE_URL_REDIRECT` respectively. Deprecated ones will continue to work until v3.0.0, where they will be removed. + * [#1204](https://github.com/shlinkio/shlink/issues/1204) Added support for `openswoole` and migrated official docker image to `openswoole`. * [#1242](https://github.com/shlinkio/shlink/issues/1242) Added support to import urls and visits from YOURLS. diff --git a/config/autoload/redirects.global.php b/config/autoload/redirects.global.php index 339ca27d..d2c73884 100644 --- a/config/autoload/redirects.global.php +++ b/config/autoload/redirects.global.php @@ -10,9 +10,10 @@ use const Shlinkio\Shlink\DEFAULT_REDIRECT_STATUS_CODE; return [ 'not_found_redirects' => [ - 'invalid_short_url' => env('INVALID_SHORT_URL_REDIRECT_TO'), - 'regular_404' => env('REGULAR_404_REDIRECT_TO'), - 'base_url' => env('BASE_URL_REDIRECT_TO'), + // Deprecated env vars + 'invalid_short_url' => env('DEFAULT_INVALID_SHORT_URL_REDIRECT', env('INVALID_SHORT_URL_REDIRECT_TO')), + 'regular_404' => env('DEFAULT_REGULAR_404_REDIRECT', env('REGULAR_404_REDIRECT_TO')), + 'base_url' => env('DEFAULT_BASE_URL_REDIRECT', env('BASE_URL_REDIRECT_TO')), ], 'url_shortener' => [ diff --git a/docs/swagger/paths/v2_domains.json b/docs/swagger/paths/v2_domains.json index ef63ee4e..d92677c1 100644 --- a/docs/swagger/paths/v2_domains.json +++ b/docs/swagger/paths/v2_domains.json @@ -4,8 +4,8 @@ "tags": [ "Domains" ], - "summary": "List existing domains", - "description": "Returns the list of all domains ever used, with a flag that tells if they are the default domain", + "summary": "List configured domains", + "description": "Returns the list of all domains that have been either used for some short URL, or have explicitly configured redirects.
It also includes the domain redirects, plus the default redirects that will be used for any non-explicitly-configured one.", "security": [ { "ApiKey": [] @@ -46,6 +46,9 @@ } } } + }, + "defaultRedirects": { + "$ref": "../definitions/NotFoundRedirects.json" } } } @@ -84,7 +87,12 @@ "invalidShortUrlRedirect": "https://example.com/invalid-url" } } - ] + ], + "defaultRedirects": { + "baseUrlRedirect": "https://somewhere.com", + "regular404Redirect": null, + "invalidShortUrlRedirect": null + } } } } diff --git a/docs/swagger/paths/v2_domains_redirects.json b/docs/swagger/paths/v2_domains_redirects.json index d9863dcd..031e1d43 100644 --- a/docs/swagger/paths/v2_domains_redirects.json +++ b/docs/swagger/paths/v2_domains_redirects.json @@ -99,16 +99,6 @@ } } }, - "403": { - "description": "Default domain was provided, and it cannot be edited this way.", - "content": { - "application/problem+json": { - "schema": { - "$ref": "../definitions/Error.json" - } - } - } - }, "500": { "description": "Unexpected error.", "content": { diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 46bb90ef..e06ad727 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -8,7 +8,7 @@ return [ 'cli' => [ 'commands' => [ - Command\ShortUrl\GenerateShortUrlCommand::NAME => Command\ShortUrl\GenerateShortUrlCommand::class, + Command\ShortUrl\CreateShortUrlCommand::NAME => Command\ShortUrl\CreateShortUrlCommand::class, Command\ShortUrl\ResolveUrlCommand::NAME => Command\ShortUrl\ResolveUrlCommand::class, Command\ShortUrl\ListShortUrlsCommand::NAME => Command\ShortUrl\ListShortUrlsCommand::class, Command\ShortUrl\GetVisitsCommand::NAME => Command\ShortUrl\GetVisitsCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index d89a8af2..41d415dc 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -39,7 +39,7 @@ return [ ApiKey\RoleResolver::class => ConfigAbstractFactory::class, - Command\ShortUrl\GenerateShortUrlCommand::class => ConfigAbstractFactory::class, + Command\ShortUrl\CreateShortUrlCommand::class => ConfigAbstractFactory::class, Command\ShortUrl\ResolveUrlCommand::class => ConfigAbstractFactory::class, Command\ShortUrl\ListShortUrlsCommand::class => ConfigAbstractFactory::class, Command\ShortUrl\GetVisitsCommand::class => ConfigAbstractFactory::class, @@ -75,10 +75,11 @@ return [ Util\ProcessRunner::class => [SymfonyCli\Helper\ProcessHelper::class], ApiKey\RoleResolver::class => [DomainService::class], - Command\ShortUrl\GenerateShortUrlCommand::class => [ + Command\ShortUrl\CreateShortUrlCommand::class => [ Service\UrlShortener::class, ShortUrlStringifier::class, 'config.url_shortener.default_short_codes_length', + 'config.url_shortener.domain.hostname', ], Command\ShortUrl\ResolveUrlCommand::class => [Service\ShortUrl\ShortUrlResolver::class], Command\ShortUrl\ListShortUrlsCommand::class => [ diff --git a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php similarity index 89% rename from module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php rename to module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index e43b4ec5..62b50456 100644 --- a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -26,14 +26,17 @@ use function method_exists; use function sprintf; use function str_contains; -class GenerateShortUrlCommand extends BaseCommand +class CreateShortUrlCommand extends BaseCommand { - public const NAME = 'short-url:generate'; + public const NAME = 'short-url:create'; + + private ?SymfonyStyle $io; public function __construct( private UrlShortenerInterface $urlShortener, private ShortUrlStringifierInterface $stringifier, private int $defaultShortCodeLength, + private string $defaultDomain, ) { parent::__construct(); } @@ -42,6 +45,7 @@ class GenerateShortUrlCommand extends BaseCommand { $this ->setName(self::NAME) + ->setAliases(['short-url:generate']) // Deprecated ->setDescription('Generates a short URL for provided long URL and returns it') ->addArgument('longUrl', InputArgument::REQUIRED, 'The long URL to parse') ->addOption( @@ -122,21 +126,33 @@ class GenerateShortUrlCommand extends BaseCommand protected function interact(InputInterface $input, OutputInterface $output): void { - $io = new SymfonyStyle($input, $output); + $this->verifyLongUrlArgument($input, $output); + $this->verifyDomainArgument($input); + } + + private function verifyLongUrlArgument(InputInterface $input, OutputInterface $output): void + { $longUrl = $input->getArgument('longUrl'); if (! empty($longUrl)) { return; } + $io = $this->getIO($input, $output); $longUrl = $io->ask('Which URL do you want to shorten?'); if (! empty($longUrl)) { $input->setArgument('longUrl', $longUrl); } } + 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 { - $io = new SymfonyStyle($input, $output); + $io = $this->getIO($input, $output); $longUrl = $input->getArgument('longUrl'); if (empty($longUrl)) { $io->error('A URL was not provided!'); @@ -196,4 +212,9 @@ class GenerateShortUrlCommand extends BaseCommand return null; } + + private function getIO(InputInterface $input, OutputInterface $output): SymfonyStyle + { + return $this->io ?? ($this->io = new SymfonyStyle($input, $output)); + } } diff --git a/module/CLI/test/Command/Domain/DomainRedirectsCommandTest.php b/module/CLI/test/Command/Domain/DomainRedirectsCommandTest.php index 9801930e..6b6e1036 100644 --- a/module/CLI/test/Command/Domain/DomainRedirectsCommandTest.php +++ b/module/CLI/test/Command/Domain/DomainRedirectsCommandTest.php @@ -126,8 +126,8 @@ class DomainRedirectsCommandTest extends TestCase $listDomains = $this->domainService->listDomains()->willReturn([ DomainItem::forDefaultDomain('default-domain.com', new NotFoundRedirectOptions()), - DomainItem::forExistingDomain(Domain::withAuthority('existing-one.com')), - DomainItem::forExistingDomain(Domain::withAuthority($domainAuthority)), + DomainItem::forNonDefaultDomain(Domain::withAuthority('existing-one.com')), + DomainItem::forNonDefaultDomain(Domain::withAuthority($domainAuthority)), ]); $findDomain = $this->domainService->findByAuthority($domainAuthority)->willReturn($domain); $configureRedirects = $this->domainService->configureNotFoundRedirects( @@ -156,8 +156,8 @@ class DomainRedirectsCommandTest extends TestCase $listDomains = $this->domainService->listDomains()->willReturn([ DomainItem::forDefaultDomain('default-domain.com', new NotFoundRedirectOptions()), - DomainItem::forExistingDomain(Domain::withAuthority('existing-one.com')), - DomainItem::forExistingDomain(Domain::withAuthority('existing-two.com')), + DomainItem::forNonDefaultDomain(Domain::withAuthority('existing-one.com')), + DomainItem::forNonDefaultDomain(Domain::withAuthority('existing-two.com')), ]); $findDomain = $this->domainService->findByAuthority($domainAuthority)->willReturn($domain); $configureRedirects = $this->domainService->configureNotFoundRedirects( diff --git a/module/CLI/test/Command/Domain/ListDomainsCommandTest.php b/module/CLI/test/Command/Domain/ListDomainsCommandTest.php index 13e6d062..6d56ea69 100644 --- a/module/CLI/test/Command/Domain/ListDomainsCommandTest.php +++ b/module/CLI/test/Command/Domain/ListDomainsCommandTest.php @@ -47,8 +47,8 @@ class ListDomainsCommandTest extends TestCase 'base_url' => 'https://foo.com/default/base', 'invalid_short_url' => 'https://foo.com/default/invalid', ])), - DomainItem::forExistingDomain(Domain::withAuthority('bar.com')), - DomainItem::forExistingDomain($bazDomain), + DomainItem::forNonDefaultDomain(Domain::withAuthority('bar.com')), + DomainItem::forNonDefaultDomain($bazDomain), ]); $this->commandTester->execute($input); diff --git a/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php similarity index 79% rename from module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php rename to module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php index 19767dc7..08389d61 100644 --- a/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php @@ -8,7 +8,7 @@ use PHPUnit\Framework\Assert; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\CLI\Command\ShortUrl\GenerateShortUrlCommand; +use Shlinkio\Shlink\CLI\Command\ShortUrl\CreateShortUrlCommand; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; @@ -19,10 +19,12 @@ use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use ShlinkioTest\Shlink\CLI\CliTestUtilsTrait; use Symfony\Component\Console\Tester\CommandTester; -class GenerateShortUrlCommandTest extends TestCase +class CreateShortUrlCommandTest extends TestCase { use CliTestUtilsTrait; + private const DEFAULT_DOMAIN = 'default.com'; + private CommandTester $commandTester; private ObjectProphecy $urlShortener; private ObjectProphecy $stringifier; @@ -33,7 +35,12 @@ class GenerateShortUrlCommandTest extends TestCase $this->stringifier = $this->prophesize(ShortUrlStringifierInterface::class); $this->stringifier->stringify(Argument::type(ShortUrl::class))->willReturn(''); - $command = new GenerateShortUrlCommand($this->urlShortener->reveal(), $this->stringifier->reveal(), 5); + $command = new CreateShortUrlCommand( + $this->urlShortener->reveal(), + $this->stringifier->reveal(), + 5, + self::DEFAULT_DOMAIN, + ); $this->commandTester = $this->testerForCommand($command); } @@ -110,6 +117,34 @@ class GenerateShortUrlCommandTest extends TestCase $stringify->shouldHaveBeenCalledOnce(); } + /** + * @test + * @dataProvider provideDomains + */ + public function properlyProcessesProvidedDomain(array $input, ?string $expectedDomain): void + { + $shorten = $this->urlShortener->shorten( + Argument::that(function (ShortUrlMeta $meta) use ($expectedDomain) { + Assert::assertEquals($expectedDomain, $meta->getDomain()); + return true; + }), + )->willReturn(ShortUrl::createEmpty()); + + $input['longUrl'] = 'http://domain.com/foo/bar'; + $this->commandTester->execute($input); + + self::assertEquals(ExitCodes::EXIT_SUCCESS, $this->commandTester->getStatusCode()); + $shorten->shouldHaveBeenCalledOnce(); + } + + public function provideDomains(): iterable + { + yield 'no domain' => [[], null]; + yield 'non-default domain foo' => [['--domain' => 'foo.com'], 'foo.com']; + yield 'non-default domain bar' => [['-d' => 'bar.com'], 'bar.com']; + yield 'default domain' => [['--domain' => self::DEFAULT_DOMAIN], null]; + } + /** * @test * @dataProvider provideFlags diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 16b84819..fdfecef9 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -119,11 +119,7 @@ return [ ], Service\ShortUrl\ShortUrlResolver::class => ['em'], Service\ShortUrl\ShortCodeHelper::class => ['em'], - Domain\DomainService::class => [ - 'em', - 'config.url_shortener.domain.hostname', - Options\NotFoundRedirectOptions::class, - ], + Domain\DomainService::class => ['em', 'config.url_shortener.domain.hostname'], Util\UrlValidator::class => ['httpClient', Options\UrlShortenerOptions::class], Util\DoctrineBatchHelper::class => ['em'], diff --git a/module/Core/src/Config/EmptyNotFoundRedirectConfig.php b/module/Core/src/Config/EmptyNotFoundRedirectConfig.php new file mode 100644 index 00000000..6ccb3848 --- /dev/null +++ b/module/Core/src/Config/EmptyNotFoundRedirectConfig.php @@ -0,0 +1,38 @@ +em->getRepository(Domain::class); - $domains = $repo->findDomainsWithout($this->defaultDomain, $apiKey); - $mappedDomains = map($domains, fn (Domain $domain) => DomainItem::forExistingDomain($domain)); + [$default, $domains] = $this->defaultDomainAndRest($apiKey); + $mappedDomains = map($domains, fn (Domain $domain) => DomainItem::forNonDefaultDomain($domain)); if ($apiKey?->hasRole(Role::DOMAIN_SPECIFIC)) { return $mappedDomains; } return [ - DomainItem::forDefaultDomain($this->defaultDomain, $this->redirectOptions), + DomainItem::forDefaultDomain($this->defaultDomain, $default ?? new EmptyNotFoundRedirectConfig()), ...$mappedDomains, ]; } + /** + * @return array{Domain|null, Domain[]} + */ + private function defaultDomainAndRest(?ApiKey $apiKey): array + { + /** @var DomainRepositoryInterface $repo */ + $repo = $this->em->getRepository(Domain::class); + $groups = group( + $repo->findDomains($apiKey), + fn (Domain $domain) => $domain->getAuthority() === $this->defaultDomain ? 'default' : 'domains', + ); + + return [first($groups['default'] ?? []), $groups['domains'] ?? []]; + } + /** * @throws DomainNotFoundException */ @@ -62,8 +73,7 @@ class DomainService implements DomainServiceInterface public function findByAuthority(string $authority, ?ApiKey $apiKey = null): ?Domain { - $repo = $this->em->getRepository(Domain::class); - return $repo->findOneByAuthority($authority, $apiKey); + return $this->em->getRepository(Domain::class)->findOneByAuthority($authority, $apiKey); } /** @@ -79,17 +89,12 @@ class DomainService implements DomainServiceInterface /** * @throws DomainNotFoundException - * @throws InvalidDomainException */ public function configureNotFoundRedirects( string $authority, NotFoundRedirects $notFoundRedirects, ?ApiKey $apiKey = null, ): Domain { - if ($authority === $this->defaultDomain) { - throw InvalidDomainException::forDefaultDomainRedirects(); - } - $domain = $this->getPersistedDomain($authority, $apiKey); $domain->configureNotFoundRedirects($notFoundRedirects); diff --git a/module/Core/src/Domain/DomainServiceInterface.php b/module/Core/src/Domain/DomainServiceInterface.php index 7748284d..9ac48e69 100644 --- a/module/Core/src/Domain/DomainServiceInterface.php +++ b/module/Core/src/Domain/DomainServiceInterface.php @@ -8,7 +8,6 @@ use Shlinkio\Shlink\Core\Config\NotFoundRedirects; use Shlinkio\Shlink\Core\Domain\Model\DomainItem; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Exception\DomainNotFoundException; -use Shlinkio\Shlink\Core\Exception\InvalidDomainException; use Shlinkio\Shlink\Rest\Entity\ApiKey; interface DomainServiceInterface @@ -32,7 +31,6 @@ interface DomainServiceInterface /** * @throws DomainNotFoundException If the API key is restricted to one domain and a different one is provided - * @throws InvalidDomainException If default domain is provided */ public function configureNotFoundRedirects( string $authority, diff --git a/module/Core/src/Domain/Model/DomainItem.php b/module/Core/src/Domain/Model/DomainItem.php index 909cca7d..5547fe8d 100644 --- a/module/Core/src/Domain/Model/DomainItem.php +++ b/module/Core/src/Domain/Model/DomainItem.php @@ -18,7 +18,7 @@ final class DomainItem implements JsonSerializable ) { } - public static function forExistingDomain(Domain $domain): self + public static function forNonDefaultDomain(Domain $domain): self { return new self($domain->getAuthority(), $domain, false); } diff --git a/module/Core/src/Domain/Repository/DomainRepository.php b/module/Core/src/Domain/Repository/DomainRepository.php index 1741cea7..4de3ea36 100644 --- a/module/Core/src/Domain/Repository/DomainRepository.php +++ b/module/Core/src/Domain/Repository/DomainRepository.php @@ -8,7 +8,6 @@ use Doctrine\ORM\Query\Expr\Join; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; use Happyr\DoctrineSpecification\Spec; use Shlinkio\Shlink\Core\Domain\Spec\IsDomain; -use Shlinkio\Shlink\Core\Domain\Spec\IsNotAuthority; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Spec\BelongsToApiKey; @@ -20,7 +19,7 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe /** * @return Domain[] */ - public function findDomainsWithout(?string $excludedAuthority, ?ApiKey $apiKey = null): array + public function findDomains(?ApiKey $apiKey = null): array { $qb = $this->createQueryBuilder('d'); $qb->leftJoin(ShortUrl::class, 's', Join::WITH, 's.domain = d') @@ -31,7 +30,7 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe ->orHaving($qb->expr()->isNotNull('d.regular404Redirect')) ->orHaving($qb->expr()->isNotNull('d.invalidShortUrlRedirect')); - $specs = $this->determineExtraSpecs($excludedAuthority, $apiKey); + $specs = $this->determineExtraSpecs($apiKey); foreach ($specs as [$alias, $spec]) { $this->applySpecification($qb, $spec, $alias); } @@ -47,7 +46,7 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe ->setParameter('authority', $authority) ->setMaxResults(1); - $specs = $this->determineExtraSpecs(null, $apiKey); + $specs = $this->determineExtraSpecs($apiKey); foreach ($specs as [$alias, $spec]) { $this->applySpecification($qb, $spec, $alias); } @@ -55,12 +54,8 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe return $qb->getQuery()->getOneOrNullResult(); } - private function determineExtraSpecs(?string $excludedAuthority, ?ApiKey $apiKey): iterable + private function determineExtraSpecs(?ApiKey $apiKey): iterable { - if ($excludedAuthority !== null) { - yield ['d', new IsNotAuthority($excludedAuthority)]; - } - // FIXME The $apiKey->spec() method cannot be used here, as it returns a single spec which assumes the // ShortUrl is the root entity. Here, the Domain is the root entity. // Think on a way to centralize the conditional behavior and make $apiKey->spec() more flexible. diff --git a/module/Core/src/Domain/Repository/DomainRepositoryInterface.php b/module/Core/src/Domain/Repository/DomainRepositoryInterface.php index 123e349d..69e74e5b 100644 --- a/module/Core/src/Domain/Repository/DomainRepositoryInterface.php +++ b/module/Core/src/Domain/Repository/DomainRepositoryInterface.php @@ -14,7 +14,7 @@ interface DomainRepositoryInterface extends ObjectRepository, EntitySpecificatio /** * @return Domain[] */ - public function findDomainsWithout(?string $excludedAuthority, ?ApiKey $apiKey = null): array; + public function findDomains(?ApiKey $apiKey = null): array; public function findOneByAuthority(string $authority, ?ApiKey $apiKey = null): ?Domain; } diff --git a/module/Core/src/Domain/Spec/IsNotAuthority.php b/module/Core/src/Domain/Spec/IsNotAuthority.php deleted file mode 100644 index 0f0f0653..00000000 --- a/module/Core/src/Domain/Spec/IsNotAuthority.php +++ /dev/null @@ -1,22 +0,0 @@ -authority)); - } -} diff --git a/module/Core/src/Exception/InvalidDomainException.php b/module/Core/src/Exception/InvalidDomainException.php deleted file mode 100644 index d41e71ac..00000000 --- a/module/Core/src/Exception/InvalidDomainException.php +++ /dev/null @@ -1,33 +0,0 @@ -detail = $e->getMessage(); - $e->title = self::TITLE; - $e->type = self::TYPE; - $e->status = StatusCodeInterface::STATUS_FORBIDDEN; - - return $e; - } -} diff --git a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php index 1eaf6ea9..382e58dd 100644 --- a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php +++ b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php @@ -50,27 +50,7 @@ class DomainRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - self::assertEquals( - [$barDomain, $bazDomain, $detachedWithRedirects, $fooDomain], - $this->repo->findDomainsWithout(null), - ); - self::assertEquals( - [$barDomain, $bazDomain, $detachedWithRedirects], - $this->repo->findDomainsWithout('foo.com'), - ); - self::assertEquals( - [$bazDomain, $detachedWithRedirects, $fooDomain], - $this->repo->findDomainsWithout('bar.com'), - ); - self::assertEquals( - [$barDomain, $detachedWithRedirects, $fooDomain], - $this->repo->findDomainsWithout('baz.com'), - ); - self::assertEquals( - [$barDomain, $bazDomain, $fooDomain], - $this->repo->findDomainsWithout('detached-with-redirects.com'), - ); - + self::assertEquals([$barDomain, $bazDomain, $detachedWithRedirects, $fooDomain], $this->repo->findDomains()); self::assertEquals($barDomain, $this->repo->findOneByAuthority('bar.com')); self::assertEquals($detachedWithRedirects, $this->repo->findOneByAuthority('detached-with-redirects.com')); self::assertNull($this->repo->findOneByAuthority('does-not-exist.com')); @@ -121,14 +101,11 @@ class DomainRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - self::assertEquals([$fooDomain], $this->repo->findDomainsWithout(null, $fooDomainApiKey)); - self::assertEquals([$barDomain], $this->repo->findDomainsWithout(null, $barDomainApiKey)); - self::assertEquals( - [$detachedWithRedirects], - $this->repo->findDomainsWithout(null, $detachedWithRedirectsApiKey), - ); - self::assertEquals([$bazDomain, $fooDomain], $this->repo->findDomainsWithout(null, $authorApiKey)); - self::assertEquals([], $this->repo->findDomainsWithout(null, $authorAndDomainApiKey)); + self::assertEquals([$fooDomain], $this->repo->findDomains($fooDomainApiKey)); + self::assertEquals([$barDomain], $this->repo->findDomains($barDomainApiKey)); + self::assertEquals([$detachedWithRedirects], $this->repo->findDomains($detachedWithRedirectsApiKey)); + self::assertEquals([$bazDomain, $fooDomain], $this->repo->findDomains($authorApiKey)); + self::assertEquals([], $this->repo->findDomains($authorAndDomainApiKey)); self::assertEquals($fooDomain, $this->repo->findOneByAuthority('foo.com', $authorApiKey)); self::assertNull($this->repo->findOneByAuthority('bar.com', $authorApiKey)); diff --git a/module/Core/test/Config/EmptyNotFoundRedirectConfigTest.php b/module/Core/test/Config/EmptyNotFoundRedirectConfigTest.php new file mode 100644 index 00000000..d1c47e10 --- /dev/null +++ b/module/Core/test/Config/EmptyNotFoundRedirectConfigTest.php @@ -0,0 +1,29 @@ +redirectsConfig = new EmptyNotFoundRedirectConfig(); + } + + /** @test */ + public function allMethodsReturnHardcodedValues(): void + { + self::assertNull($this->redirectsConfig->invalidShortUrlRedirect()); + self::assertFalse($this->redirectsConfig->hasInvalidShortUrlRedirect()); + self::assertNull($this->redirectsConfig->regular404Redirect()); + self::assertFalse($this->redirectsConfig->hasRegular404Redirect()); + self::assertNull($this->redirectsConfig->baseUrlRedirect()); + self::assertFalse($this->redirectsConfig->hasBaseUrlRedirect()); + } +} diff --git a/module/Core/test/Domain/DomainServiceTest.php b/module/Core/test/Domain/DomainServiceTest.php index 159fb6ca..ea3cfe02 100644 --- a/module/Core/test/Domain/DomainServiceTest.php +++ b/module/Core/test/Domain/DomainServiceTest.php @@ -9,14 +9,13 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; +use Shlinkio\Shlink\Core\Config\EmptyNotFoundRedirectConfig; use Shlinkio\Shlink\Core\Config\NotFoundRedirects; use Shlinkio\Shlink\Core\Domain\DomainService; use Shlinkio\Shlink\Core\Domain\Model\DomainItem; use Shlinkio\Shlink\Core\Domain\Repository\DomainRepositoryInterface; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Exception\DomainNotFoundException; -use Shlinkio\Shlink\Core\Exception\InvalidDomainException; -use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -31,7 +30,7 @@ class DomainServiceTest extends TestCase public function setUp(): void { $this->em = $this->prophesize(EntityManagerInterface::class); - $this->domainService = new DomainService($this->em->reveal(), 'default.com', new NotFoundRedirectOptions()); + $this->domainService = new DomainService($this->em->reveal(), 'default.com'); } /** @@ -42,7 +41,7 @@ class DomainServiceTest extends TestCase { $repo = $this->prophesize(DomainRepositoryInterface::class); $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); - $findDomains = $repo->findDomainsWithout('default.com', $apiKey)->willReturn($domains); + $findDomains = $repo->findDomains($apiKey)->willReturn($domains); $result = $this->domainService->listDomains($apiKey); @@ -53,7 +52,7 @@ class DomainServiceTest extends TestCase public function provideExcludedDomains(): iterable { - $default = DomainItem::forDefaultDomain('default.com', new NotFoundRedirectOptions()); + $default = DomainItem::forDefaultDomain('default.com', new EmptyNotFoundRedirectConfig()); $adminApiKey = ApiKey::create(); $domainSpecificApiKey = ApiKey::fromMeta( ApiKeyMeta::withRoles(RoleDefinition::forDomain(Domain::withAuthority('')->setId('123'))), @@ -62,15 +61,15 @@ class DomainServiceTest extends TestCase yield 'empty list without API key' => [[], [$default], null]; yield 'one item without API key' => [ [Domain::withAuthority('bar.com')], - [$default, DomainItem::forExistingDomain(Domain::withAuthority('bar.com'))], + [$default, DomainItem::forNonDefaultDomain(Domain::withAuthority('bar.com'))], null, ]; yield 'multiple items without API key' => [ [Domain::withAuthority('foo.com'), Domain::withAuthority('bar.com')], [ $default, - DomainItem::forExistingDomain(Domain::withAuthority('foo.com')), - DomainItem::forExistingDomain(Domain::withAuthority('bar.com')), + DomainItem::forNonDefaultDomain(Domain::withAuthority('foo.com')), + DomainItem::forNonDefaultDomain(Domain::withAuthority('bar.com')), ], null, ]; @@ -78,15 +77,15 @@ class DomainServiceTest extends TestCase yield 'empty list with admin API key' => [[], [$default], $adminApiKey]; yield 'one item with admin API key' => [ [Domain::withAuthority('bar.com')], - [$default, DomainItem::forExistingDomain(Domain::withAuthority('bar.com'))], + [$default, DomainItem::forNonDefaultDomain(Domain::withAuthority('bar.com'))], $adminApiKey, ]; yield 'multiple items with admin API key' => [ [Domain::withAuthority('foo.com'), Domain::withAuthority('bar.com')], [ $default, - DomainItem::forExistingDomain(Domain::withAuthority('foo.com')), - DomainItem::forExistingDomain(Domain::withAuthority('bar.com')), + DomainItem::forNonDefaultDomain(Domain::withAuthority('foo.com')), + DomainItem::forNonDefaultDomain(Domain::withAuthority('bar.com')), ], $adminApiKey, ]; @@ -94,14 +93,14 @@ class DomainServiceTest extends TestCase yield 'empty list with domain-specific API key' => [[], [], $domainSpecificApiKey]; yield 'one item with domain-specific API key' => [ [Domain::withAuthority('bar.com')], - [DomainItem::forExistingDomain(Domain::withAuthority('bar.com'))], + [DomainItem::forNonDefaultDomain(Domain::withAuthority('bar.com'))], $domainSpecificApiKey, ]; yield 'multiple items with domain-specific API key' => [ [Domain::withAuthority('foo.com'), Domain::withAuthority('bar.com')], [ - DomainItem::forExistingDomain(Domain::withAuthority('foo.com')), - DomainItem::forExistingDomain(Domain::withAuthority('bar.com')), + DomainItem::forNonDefaultDomain(Domain::withAuthority('foo.com')), + DomainItem::forNonDefaultDomain(Domain::withAuthority('bar.com')), ], $domainSpecificApiKey, ]; @@ -214,15 +213,4 @@ class DomainServiceTest extends TestCase yield 'domain not found and author API key' => [null, $authorApiKey]; yield 'domain found and author API key' => [$domain, $authorApiKey]; } - - /** @test */ - public function anExceptionIsThrowsWhenTryingToEditRedirectsForDefaultDomain(): void - { - $this->expectException(InvalidDomainException::class); - $this->expectExceptionMessage( - 'You cannot configure default domain\'s redirects this way. Use the configuration or env vars.', - ); - - $this->domainService->configureNotFoundRedirects('default.com', NotFoundRedirects::withoutRedirects()); - } } diff --git a/module/Core/test/Exception/InvalidDomainExceptionTest.php b/module/Core/test/Exception/InvalidDomainExceptionTest.php deleted file mode 100644 index 06b78ff2..00000000 --- a/module/Core/test/Exception/InvalidDomainExceptionTest.php +++ /dev/null @@ -1,24 +0,0 @@ -getMessage()); - self::assertEquals($expected, $e->getDetail()); - self::assertEquals('Invalid domain', $e->getTitle()); - self::assertEquals('INVALID_DOMAIN', $e->getType()); - self::assertEquals(403, $e->getStatus()); - } -} diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 5e0267d6..98b385b0 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -9,7 +9,7 @@ use Laminas\ServiceManager\Factory\InvokableFactory; use Mezzio\Router\Middleware\ImplicitOptionsMiddleware; use Shlinkio\Shlink\Common\Mercure\LcobucciJwtProvider; use Shlinkio\Shlink\Core\Domain\DomainService; -use Shlinkio\Shlink\Core\Options\AppOptions; +use Shlinkio\Shlink\Core\Options; use Shlinkio\Shlink\Core\Service; use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Core\Tag\TagService; @@ -55,7 +55,7 @@ return [ ConfigAbstractFactory::class => [ ApiKeyService::class => ['em'], - Action\HealthAction::class => ['em', AppOptions::class], + Action\HealthAction::class => ['em', Options\AppOptions::class], Action\MercureInfoAction::class => [LcobucciJwtProvider::class, 'config.mercure'], Action\ShortUrl\CreateShortUrlAction::class => [Service\UrlShortener::class, ShortUrlDataTransformer::class], Action\ShortUrl\SingleStepCreateShortUrlAction::class => [ @@ -81,7 +81,7 @@ return [ Action\Tag\DeleteTagsAction::class => [TagService::class], Action\Tag\CreateTagsAction::class => [TagService::class], Action\Tag\UpdateTagAction::class => [TagService::class], - Action\Domain\ListDomainsAction::class => [DomainService::class], + Action\Domain\ListDomainsAction::class => [DomainService::class, Options\NotFoundRedirectOptions::class], Action\Domain\DomainRedirectsAction::class => [DomainService::class], Middleware\CrossDomainMiddleware::class => ['config.cors'], diff --git a/module/Rest/src/Action/Domain/ListDomainsAction.php b/module/Rest/src/Action/Domain/ListDomainsAction.php index c8f9a475..e50ada16 100644 --- a/module/Rest/src/Action/Domain/ListDomainsAction.php +++ b/module/Rest/src/Action/Domain/ListDomainsAction.php @@ -7,7 +7,9 @@ namespace Shlinkio\Shlink\Rest\Action\Domain; use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use Shlinkio\Shlink\Core\Config\NotFoundRedirects; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; +use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; @@ -16,7 +18,7 @@ class ListDomainsAction extends AbstractRestAction protected const ROUTE_PATH = '/domains'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - public function __construct(private DomainServiceInterface $domainService) + public function __construct(private DomainServiceInterface $domainService, private NotFoundRedirectOptions $options) { } @@ -28,6 +30,7 @@ class ListDomainsAction extends AbstractRestAction return new JsonResponse([ 'domains' => [ 'data' => $domainItems, + 'defaultRedirects' => NotFoundRedirects::fromConfig($this->options), ], ]); } diff --git a/module/Rest/test-api/Action/DomainRedirectsTest.php b/module/Rest/test-api/Action/DomainRedirectsTest.php index 987c09d6..fdeec3b3 100644 --- a/module/Rest/test-api/Action/DomainRedirectsTest.php +++ b/module/Rest/test-api/Action/DomainRedirectsTest.php @@ -9,24 +9,6 @@ use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; class DomainRedirectsTest extends ApiTestCase { - /** @test */ - public function anErrorIsReturnedWhenTryingToEditDefaultDomain(): void - { - $resp = $this->callApiWithKey(self::METHOD_PATCH, '/domains/redirects', [ - RequestOptions::JSON => ['domain' => 'doma.in'], - ]); - $payload = $this->getJsonResponsePayload($resp); - - self::assertEquals(self::STATUS_FORBIDDEN, $resp->getStatusCode()); - self::assertEquals(self::STATUS_FORBIDDEN, $payload['status']); - self::assertEquals('INVALID_DOMAIN', $payload['type']); - self::assertEquals( - 'You cannot configure default domain\'s redirects this way. Use the configuration or env vars.', - $payload['detail'], - ); - self::assertEquals('Invalid domain', $payload['title']); - } - /** * @test * @dataProvider provideInvalidDomains @@ -78,6 +60,14 @@ class DomainRedirectsTest extends ApiTestCase 'regular404Redirect' => 'foo.com', 'invalidShortUrlRedirect' => null, ]]; + yield 'default domain' => [[ + 'domain' => 'doma.in', + 'regular404Redirect' => 'foo-for-default.com', + ], [ + 'baseUrlRedirect' => null, + 'regular404Redirect' => 'foo-for-default.com', + 'invalidShortUrlRedirect' => null, + ]]; yield 'existing domain with redirects' => [[ 'domain' => 'detached-with-redirects.com', 'baseUrlRedirect' => null, diff --git a/module/Rest/test-api/Action/ListDomainsTest.php b/module/Rest/test-api/Action/ListDomainsTest.php index 5f33c20b..54039c41 100644 --- a/module/Rest/test-api/Action/ListDomainsTest.php +++ b/module/Rest/test-api/Action/ListDomainsTest.php @@ -21,6 +21,11 @@ class ListDomainsTest extends ApiTestCase self::assertEquals([ 'domains' => [ 'data' => $expectedDomains, + 'defaultRedirects' => [ + 'baseUrlRedirect' => null, + 'regular404Redirect' => null, + 'invalidShortUrlRedirect' => null, + ], ], ], $respPayload); } diff --git a/module/Rest/test/Action/Domain/ListDomainsActionTest.php b/module/Rest/test/Action/Domain/ListDomainsActionTest.php index 45575cc6..bc852b34 100644 --- a/module/Rest/test/Action/Domain/ListDomainsActionTest.php +++ b/module/Rest/test/Action/Domain/ListDomainsActionTest.php @@ -9,6 +9,7 @@ use Laminas\Diactoros\ServerRequestFactory; use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; +use Shlinkio\Shlink\Core\Config\NotFoundRedirects; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; use Shlinkio\Shlink\Core\Domain\Model\DomainItem; use Shlinkio\Shlink\Core\Entity\Domain; @@ -22,11 +23,13 @@ class ListDomainsActionTest extends TestCase private ListDomainsAction $action; private ObjectProphecy $domainService; + private NotFoundRedirectOptions $options; public function setUp(): void { $this->domainService = $this->prophesize(DomainServiceInterface::class); - $this->action = new ListDomainsAction($this->domainService->reveal()); + $this->options = new NotFoundRedirectOptions(); + $this->action = new ListDomainsAction($this->domainService->reveal(), $this->options); } /** @test */ @@ -35,7 +38,7 @@ class ListDomainsActionTest extends TestCase $apiKey = ApiKey::create(); $domains = [ DomainItem::forDefaultDomain('bar.com', new NotFoundRedirectOptions()), - DomainItem::forExistingDomain(Domain::withAuthority('baz.com')), + DomainItem::forNonDefaultDomain(Domain::withAuthority('baz.com')), ]; $listDomains = $this->domainService->listDomains($apiKey)->willReturn($domains); @@ -46,6 +49,7 @@ class ListDomainsActionTest extends TestCase self::assertEquals([ 'domains' => [ 'data' => $domains, + 'defaultRedirects' => NotFoundRedirects::fromConfig($this->options), ], ], $payload); $listDomains->shouldHaveBeenCalledOnce();