diff --git a/CHANGELOG.md b/CHANGELOG.md index b1124864..c7679799 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this This behavior needs to be actively opted in, via installer config options or env vars. +* [#943](https://github.com/shlinkio/shlink/issues/943) Added support to define different "not-found" redirects for every domain handled by Shlink. + + Shlink will continue to allow defining the default values via env vars or config, but afterwards, you can use the `domain:redirects` command to define specific values for every single domain. + ### Changed * [#1118](https://github.com/shlinkio/shlink/issues/1118) Increased phpstan required level to 8. diff --git a/config/autoload/url-shortener.local.php.dist b/config/autoload/url-shortener.local.php.dist index c686137f..f34245fb 100644 --- a/config/autoload/url-shortener.local.php.dist +++ b/config/autoload/url-shortener.local.php.dist @@ -2,13 +2,16 @@ declare(strict_types=1); +$isSwoole = extension_loaded('swoole'); + return [ 'url_shortener' => [ 'domain' => [ 'schema' => 'http', - 'hostname' => 'localhost:8080', + 'hostname' => sprintf('localhost:%s', $isSwoole ? '8080' : '8000'), ], + 'auto_resolve_titles' => true, ], ]; diff --git a/data/migrations/Version20210720143824.php b/data/migrations/Version20210720143824.php new file mode 100644 index 00000000..66e03be5 --- /dev/null +++ b/data/migrations/Version20210720143824.php @@ -0,0 +1,41 @@ +getTable('domains'); + $this->skipIf($domainsTable->hasColumn('base_url_redirect')); + + $this->createRedirectColumn($domainsTable, 'base_url_redirect'); + $this->createRedirectColumn($domainsTable, 'regular_not_found_redirect'); + $this->createRedirectColumn($domainsTable, 'invalid_short_url_redirect'); + } + + private function createRedirectColumn(Table $table, string $columnName): void + { + $table->addColumn($columnName, Types::STRING, [ + 'notnull' => false, + 'default' => null, + ]); + } + + public function down(Schema $schema): void + { + $domainsTable = $schema->getTable('domains'); + $this->skipIf(! $domainsTable->hasColumn('base_url_redirect')); + + $domainsTable->dropColumn('base_url_redirect'); + $domainsTable->dropColumn('regular_not_found_redirect'); + $domainsTable->dropColumn('invalid_short_url_redirect'); + } +} diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 6043833b..46bb90ef 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -27,6 +27,7 @@ return [ Command\Tag\DeleteTagsCommand::NAME => Command\Tag\DeleteTagsCommand::class, Command\Domain\ListDomainsCommand::NAME => Command\Domain\ListDomainsCommand::class, + Command\Domain\DomainRedirectsCommand::NAME => Command\Domain\DomainRedirectsCommand::class, Command\Db\CreateDatabaseCommand::NAME => Command\Db\CreateDatabaseCommand::class, Command\Db\MigrateDatabaseCommand::NAME => Command\Db\MigrateDatabaseCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 5f51d6c2..95ea1bbc 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -61,6 +61,7 @@ return [ Command\Db\MigrateDatabaseCommand::class => ConfigAbstractFactory::class, Command\Domain\ListDomainsCommand::class => ConfigAbstractFactory::class, + Command\Domain\DomainRedirectsCommand::class => ConfigAbstractFactory::class, ], ], @@ -104,6 +105,7 @@ return [ Command\Tag\DeleteTagsCommand::class => [TagService::class], Command\Domain\ListDomainsCommand::class => [DomainService::class], + Command\Domain\DomainRedirectsCommand::class => [DomainService::class], Command\Db\CreateDatabaseCommand::class => [ LockFactory::class, diff --git a/module/CLI/src/Command/Domain/DomainRedirectsCommand.php b/module/CLI/src/Command/Domain/DomainRedirectsCommand.php new file mode 100644 index 00000000..9a97e5fd --- /dev/null +++ b/module/CLI/src/Command/Domain/DomainRedirectsCommand.php @@ -0,0 +1,114 @@ +setName(self::NAME) + ->setDescription('Set specific "not found" redirects for individual domains.') + ->addArgument( + 'domain', + InputArgument::REQUIRED, + 'The domain authority to which you want to set the specific redirects', + ); + } + + protected function interact(InputInterface $input, OutputInterface $output): void + { + /** @var string|null $domain */ + $domain = $input->getArgument('domain'); + if ($domain !== null) { + return; + } + + $io = new SymfonyStyle($input, $output); + $askNewDomain = static fn () => $io->ask('Domain authority for which you want to set specific redirects'); + + /** @var string[] $availableDomains */ + $availableDomains = invoke( + filter($this->domainService->listDomains(), static fn (DomainItem $item) => ! $item->isDefault()), + 'toString', + ); + if (empty($availableDomains)) { + $input->setArgument('domain', $askNewDomain()); + return; + } + + $selectedOption = $io->choice( + 'Select the domain to configure', + [...$availableDomains, 'New domain'], + ); + $input->setArgument('domain', str_contains($selectedOption, 'New domain') ? $askNewDomain() : $selectedOption); + } + + protected function execute(InputInterface $input, OutputInterface $output): ?int + { + $io = new SymfonyStyle($input, $output); + $domainAuthority = $input->getArgument('domain'); + $domain = $this->domainService->findByAuthority($domainAuthority); + + $ask = static function (string $message, ?string $current) use ($io): ?string { + if ($current === null) { + return $io->ask(sprintf('%s (Leave empty for no redirect)', $message)); + } + + $choice = $io->choice($message, [ + sprintf('Keep current one: [%s]', $current), + 'Set new redirect URL', + 'Remove redirect', + ]); + + return match ($choice) { + 'Set new redirect URL' => $io->ask('New redirect URL'), + 'Remove redirect' => null, + default => $current, + }; + }; + + $this->domainService->configureNotFoundRedirects($domainAuthority, new NotFoundRedirects( + $ask( + 'URL to redirect to when a user hits this domain\'s base URL', + $domain?->baseUrlRedirect(), + ), + $ask( + 'URL to redirect to when a user hits a not found URL other than an invalid short URL', + $domain?->regular404Redirect(), + ), + $ask( + 'URL to redirect to when a user hits an invalid short URL', + $domain?->invalidShortUrlRedirect(), + ), + )); + + $io->success(sprintf('"Not found" redirects properly set for "%s"', $domainAuthority)); + + return ExitCodes::EXIT_SUCCESS; + } +} diff --git a/module/CLI/src/Command/Domain/ListDomainsCommand.php b/module/CLI/src/Command/Domain/ListDomainsCommand.php index 6fa25097..5e368170 100644 --- a/module/CLI/src/Command/Domain/ListDomainsCommand.php +++ b/module/CLI/src/Command/Domain/ListDomainsCommand.php @@ -6,10 +6,12 @@ namespace Shlinkio\Shlink\CLI\Command\Domain; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; +use Shlinkio\Shlink\Core\Config\NotFoundRedirectConfigInterface; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; use Shlinkio\Shlink\Core\Domain\Model\DomainItem; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use function Functional\map; @@ -27,18 +29,48 @@ class ListDomainsCommand extends Command { $this ->setName(self::NAME) - ->setDescription('List all domains that have been ever used for some short URL'); + ->setDescription('List all domains that have been ever used for some short URL') + ->addOption( + 'show-redirects', + 'r', + InputOption::VALUE_NONE, + 'Will display an extra column with the information of the "not found" redirects for every domain.', + ); } protected function execute(InputInterface $input, OutputInterface $output): ?int { $domains = $this->domainService->listDomains(); + $showRedirects = $input->getOption('show-redirects'); + $commonFields = ['Domain', 'Is default']; ShlinkTable::fromOutput($output)->render( - ['Domain', 'Is default'], - map($domains, fn (DomainItem $domain) => [$domain->toString(), $domain->isDefault() ? 'Yes' : 'No']), + $showRedirects ? [...$commonFields, '"Not found" redirects'] : $commonFields, + map($domains, function (DomainItem $domain) use ($showRedirects) { + $commonValues = [$domain->toString(), $domain->isDefault() ? 'Yes' : 'No']; + + return $showRedirects + ? [ + ...$commonValues, + $this->notFoundRedirectsToString($domain->notFoundRedirectConfig()), + ] + : $commonValues; + }), ); return ExitCodes::EXIT_SUCCESS; } + + private function notFoundRedirectsToString(NotFoundRedirectConfigInterface $config): string + { + $baseUrl = $config->baseUrlRedirect() ?? 'N/A'; + $regular404 = $config->regular404Redirect() ?? 'N/A'; + $invalidShortUrl = $config->invalidShortUrlRedirect() ?? 'N/A'; + + return <<domainService->getOrCreate('example.com')->willReturn( - (new Domain('example.com'))->setId('1'), + Domain::withAuthority('example.com')->setId('1'), ); $result = $this->resolver->determineRoles($input); @@ -47,7 +47,7 @@ class RoleResolverTest extends TestCase public function provideRoles(): iterable { - $domain = (new Domain('example.com'))->setId('1'); + $domain = Domain::withAuthority('example.com')->setId('1'); $buildInput = function (array $definition): InputInterface { $input = $this->prophesize(InputInterface::class); diff --git a/module/CLI/test/Command/Api/ListKeysCommandTest.php b/module/CLI/test/Command/Api/ListKeysCommandTest.php index fc845ff7..389c6bbd 100644 --- a/module/CLI/test/Command/Api/ListKeysCommandTest.php +++ b/module/CLI/test/Command/Api/ListKeysCommandTest.php @@ -76,11 +76,13 @@ class ListKeysCommandTest extends TestCase [ $apiKey1 = ApiKey::create(), $apiKey2 = $this->apiKeyWithRoles([RoleDefinition::forAuthoredShortUrls()]), - $apiKey3 = $this->apiKeyWithRoles([RoleDefinition::forDomain((new Domain('example.com'))->setId('1'))]), + $apiKey3 = $this->apiKeyWithRoles( + [RoleDefinition::forDomain(Domain::withAuthority('example.com')->setId('1'))], + ), $apiKey4 = ApiKey::create(), $apiKey5 = $this->apiKeyWithRoles([ RoleDefinition::forAuthoredShortUrls(), - RoleDefinition::forDomain((new Domain('example.com'))->setId('1')), + RoleDefinition::forDomain(Domain::withAuthority('example.com')->setId('1')), ]), $apiKey6 = ApiKey::create(), ], diff --git a/module/CLI/test/Command/Domain/DomainRedirectsCommandTest.php b/module/CLI/test/Command/Domain/DomainRedirectsCommandTest.php new file mode 100644 index 00000000..1f8b93ab --- /dev/null +++ b/module/CLI/test/Command/Domain/DomainRedirectsCommandTest.php @@ -0,0 +1,180 @@ +domainService = $this->prophesize(DomainServiceInterface::class); + $this->commandTester = $this->testerForCommand(new DomainRedirectsCommand($this->domainService->reveal())); + } + + /** + * @test + * @dataProvider provideDomains + */ + public function onlyPlainQuestionsAreAskedForNewDomainsAndDomainsWithNoRedirects(?Domain $domain): void + { + $domainAuthority = 'my-domain.com'; + $findDomain = $this->domainService->findByAuthority($domainAuthority)->willReturn($domain); + $configureRedirects = $this->domainService->configureNotFoundRedirects( + $domainAuthority, + new NotFoundRedirects('foo.com', null, 'baz.com'), + )->willReturn(Domain::withAuthority('')); + + $this->commandTester->setInputs(['foo.com', '', 'baz.com']); + $this->commandTester->execute(['domain' => $domainAuthority]); + $output = $this->commandTester->getDisplay(); + + self::assertStringContainsString('[OK] "Not found" redirects properly set for "my-domain.com"', $output); + self::assertStringContainsString('URL to redirect to when a user hits this domain\'s base URL', $output); + self::assertStringContainsString( + 'URL to redirect to when a user hits a not found URL other than an invalid short URL', + $output, + ); + self::assertStringContainsString('URL to redirect to when a user hits an invalid short URL', $output); + self::assertEquals(3, substr_count($output, '(Leave empty for no redirect)')); + $findDomain->shouldHaveBeenCalledOnce(); + $configureRedirects->shouldHaveBeenCalledOnce(); + $this->domainService->listDomains()->shouldNotHaveBeenCalled(); + } + + public function provideDomains(): iterable + { + yield 'no domain' => [null]; + yield 'domain without redirects' => [Domain::withAuthority('')]; + } + + /** @test */ + public function offersNewOptionsForDomainsWithExistingRedirects(): void + { + $domainAuthority = 'example.com'; + $domain = Domain::withAuthority($domainAuthority); + $domain->configureNotFoundRedirects(new NotFoundRedirects('foo.com', 'bar.com', 'baz.com')); + + $findDomain = $this->domainService->findByAuthority($domainAuthority)->willReturn($domain); + $configureRedirects = $this->domainService->configureNotFoundRedirects( + $domainAuthority, + new NotFoundRedirects(null, 'edited.com', 'baz.com'), + )->willReturn($domain); + + $this->commandTester->setInputs(['2', '1', 'edited.com', '0']); + $this->commandTester->execute(['domain' => $domainAuthority]); + $output = $this->commandTester->getDisplay(); + + self::assertStringContainsString('[OK] "Not found" redirects properly set for "example.com"', $output); + self::assertStringContainsString('Keep current one: [bar.com]', $output); + self::assertStringContainsString('Keep current one: [baz.com]', $output); + self::assertStringContainsString('Keep current one: [baz.com]', $output); + self::assertStringNotContainsStringIgnoringCase('(Leave empty for no redirect)', $output); + self::assertEquals(3, substr_count($output, 'Set new redirect URL')); + self::assertEquals(3, substr_count($output, 'Remove redirect')); + $findDomain->shouldHaveBeenCalledOnce(); + $configureRedirects->shouldHaveBeenCalledOnce(); + $this->domainService->listDomains()->shouldNotHaveBeenCalled(); + } + + /** @test */ + public function authorityIsRequestedWhenNotProvidedAndNoOtherDomainsExist(): void + { + $domainAuthority = 'example.com'; + $domain = Domain::withAuthority($domainAuthority); + + $listDomains = $this->domainService->listDomains()->willReturn([]); + $findDomain = $this->domainService->findByAuthority($domainAuthority)->willReturn($domain); + $configureRedirects = $this->domainService->configureNotFoundRedirects( + $domainAuthority, + new NotFoundRedirects(), + )->willReturn($domain); + + $this->commandTester->setInputs([$domainAuthority, '', '', '']); + $this->commandTester->execute([]); + $output = $this->commandTester->getDisplay(); + + self::assertStringContainsString('Domain authority for which you want to set specific redirects', $output); + $listDomains->shouldHaveBeenCalledOnce(); + $findDomain->shouldHaveBeenCalledOnce(); + $configureRedirects->shouldHaveBeenCalledOnce(); + } + + /** @test */ + public function oneOfTheExistingDomainsCanBeSelected(): void + { + $domainAuthority = 'existing-two.com'; + $domain = Domain::withAuthority($domainAuthority); + + $listDomains = $this->domainService->listDomains()->willReturn([ + DomainItem::forDefaultDomain('default-domain.com', new NotFoundRedirectOptions()), + DomainItem::forExistingDomain(Domain::withAuthority('existing-one.com')), + DomainItem::forExistingDomain(Domain::withAuthority($domainAuthority)), + ]); + $findDomain = $this->domainService->findByAuthority($domainAuthority)->willReturn($domain); + $configureRedirects = $this->domainService->configureNotFoundRedirects( + $domainAuthority, + new NotFoundRedirects(), + )->willReturn($domain); + + $this->commandTester->setInputs(['1', '', '', '']); + $this->commandTester->execute([]); + $output = $this->commandTester->getDisplay(); + + self::assertStringNotContainsString('Domain authority for which you want to set specific redirects', $output); + self::assertStringNotContainsString('default-domain.com', $output); + self::assertStringContainsString('existing-one.com', $output); + self::assertStringContainsString($domainAuthority, $output); + $listDomains->shouldHaveBeenCalledOnce(); + $findDomain->shouldHaveBeenCalledOnce(); + $configureRedirects->shouldHaveBeenCalledOnce(); + } + + /** @test */ + public function aNewDomainCanBeCreatedEvenIfOthersAlreadyExist(): void + { + $domainAuthority = 'new-domain.com'; + $domain = Domain::withAuthority($domainAuthority); + + $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')), + ]); + $findDomain = $this->domainService->findByAuthority($domainAuthority)->willReturn($domain); + $configureRedirects = $this->domainService->configureNotFoundRedirects( + $domainAuthority, + new NotFoundRedirects(), + )->willReturn($domain); + + $this->commandTester->setInputs(['2', $domainAuthority, '', '', '']); + $this->commandTester->execute([]); + $output = $this->commandTester->getDisplay(); + + self::assertStringContainsString('Domain authority for which you want to set specific redirects', $output); + self::assertStringNotContainsString('default-domain.com', $output); + self::assertStringContainsString('existing-one.com', $output); + self::assertStringContainsString('existing-two.com', $output); + $listDomains->shouldHaveBeenCalledOnce(); + $findDomain->shouldHaveBeenCalledOnce(); + $configureRedirects->shouldHaveBeenCalledOnce(); + } +} diff --git a/module/CLI/test/Command/Domain/ListDomainsCommandTest.php b/module/CLI/test/Command/Domain/ListDomainsCommandTest.php index 04f7eb5d..9f4be920 100644 --- a/module/CLI/test/Command/Domain/ListDomainsCommandTest.php +++ b/module/CLI/test/Command/Domain/ListDomainsCommandTest.php @@ -8,8 +8,11 @@ use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Domain\ListDomainsCommand; use Shlinkio\Shlink\CLI\Util\ExitCodes; +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; +use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; use ShlinkioTest\Shlink\CLI\CliTestUtilsTrait; use Symfony\Component\Console\Tester\CommandTester; @@ -26,10 +29,38 @@ class ListDomainsCommandTest extends TestCase $this->commandTester = $this->testerForCommand(new ListDomainsCommand($this->domainService->reveal())); } - /** @test */ - public function allDomainsAreProperlyPrinted(): void + /** + * @test + * @dataProvider provideInputsAndOutputs + */ + public function allDomainsAreProperlyPrinted(array $input, string $expectedOutput): void { - $expectedOutput = <<configureNotFoundRedirects(new NotFoundRedirects( + null, + 'https://foo.com/baz-domain/regular', + 'https://foo.com/baz-domain/invalid', + )); + + $listDomains = $this->domainService->listDomains()->willReturn([ + DomainItem::forDefaultDomain('foo.com', new NotFoundRedirectOptions([ + 'base_url' => 'https://foo.com/default/base', + 'invalid_short_url' => 'https://foo.com/default/invalid', + ])), + DomainItem::forExistingDomain(Domain::withAuthority('bar.com')), + DomainItem::forExistingDomain($bazDomain), + ]); + + $this->commandTester->execute($input); + + self::assertEquals($expectedOutput, $this->commandTester->getDisplay()); + self::assertEquals(ExitCodes::EXIT_SUCCESS, $this->commandTester->getStatusCode()); + $listDomains->shouldHaveBeenCalledOnce(); + } + + public function provideInputsAndOutputs(): iterable + { + $withoutRedirectsOutput = <<domainService->listDomains()->willReturn([ - new DomainItem('foo.com', true), - new DomainItem('bar.com', false), - new DomainItem('baz.com', false), - ]); + $withRedirectsOutput = <<commandTester->execute([]); + OUTPUT; - self::assertEquals($expectedOutput, $this->commandTester->getDisplay()); - self::assertEquals(ExitCodes::EXIT_SUCCESS, $this->commandTester->getStatusCode()); - $listDomains->shouldHaveBeenCalledOnce(); + yield 'no args' => [[], $withoutRedirectsOutput]; + yield 'no show redirects' => [['--show-redirects' => false], $withoutRedirectsOutput]; + yield 'show redirects' => [['--show-redirects' => true], $withRedirectsOutput]; } } diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 03147bcb..7f28b14d 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -46,6 +46,8 @@ return [ Util\DoctrineBatchHelper::class => ConfigAbstractFactory::class, Util\RedirectResponseHelper::class => ConfigAbstractFactory::class, + Config\NotFoundRedirectResolver::class => ConfigAbstractFactory::class, + Action\RedirectAction::class => ConfigAbstractFactory::class, Action\PixelAction::class => ConfigAbstractFactory::class, Action\QrCodeAction::class => ConfigAbstractFactory::class, @@ -75,7 +77,8 @@ return [ ErrorHandler\NotFoundTrackerMiddleware::class => [Visit\RequestTracker::class], ErrorHandler\NotFoundRedirectHandler::class => [ NotFoundRedirectOptions::class, - Util\RedirectResponseHelper::class, + Config\NotFoundRedirectResolver::class, + Domain\DomainService::class, ], Options\AppOptions::class => ['config.app_options'], @@ -112,12 +115,18 @@ return [ ], Service\ShortUrl\ShortUrlResolver::class => ['em'], Service\ShortUrl\ShortCodeHelper::class => ['em'], - Domain\DomainService::class => ['em', 'config.url_shortener.domain.hostname'], + Domain\DomainService::class => [ + 'em', + 'config.url_shortener.domain.hostname', + Options\NotFoundRedirectOptions::class, + ], Util\UrlValidator::class => ['httpClient', Options\UrlShortenerOptions::class], Util\DoctrineBatchHelper::class => ['em'], Util\RedirectResponseHelper::class => [Options\UrlShortenerOptions::class], + Config\NotFoundRedirectResolver::class => [Util\RedirectResponseHelper::class], + Action\RedirectAction::class => [ Service\ShortUrl\ShortUrlResolver::class, Visit\RequestTracker::class, diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Domain.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Domain.php index e3d8c3cf..596f41da 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Domain.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Domain.php @@ -24,4 +24,19 @@ return static function (ClassMetadata $metadata, array $emConfig): void { $builder->createField('authority', Types::STRING) ->unique() ->build(); + + $builder->createField('baseUrlRedirect', Types::STRING) + ->columnName('base_url_redirect') + ->nullable() + ->build(); + + $builder->createField('regular404Redirect', Types::STRING) + ->columnName('regular_not_found_redirect') + ->nullable() + ->build(); + + $builder->createField('invalidShortUrlRedirect', Types::STRING) + ->columnName('invalid_short_url_redirect') + ->nullable() + ->build(); }; diff --git a/module/Core/src/Config/NotFoundRedirectConfigInterface.php b/module/Core/src/Config/NotFoundRedirectConfigInterface.php new file mode 100644 index 00000000..bbdfa9c5 --- /dev/null +++ b/module/Core/src/Config/NotFoundRedirectConfigInterface.php @@ -0,0 +1,20 @@ +isBaseUrl() && $config->hasBaseUrlRedirect() => + // @phpstan-ignore-next-line Create custom PHPStan rule + $this->redirectResponseHelper->buildRedirectResponse($config->baseUrlRedirect()), + $notFoundType->isRegularNotFound() && $config->hasRegular404Redirect() => + // @phpstan-ignore-next-line Create custom PHPStan rule + $this->redirectResponseHelper->buildRedirectResponse($config->regular404Redirect()), + $notFoundType->isInvalidShortUrl() && $config->hasInvalidShortUrlRedirect() => + // @phpstan-ignore-next-line Create custom PHPStan rule + $this->redirectResponseHelper->buildRedirectResponse($config->invalidShortUrlRedirect()), + default => null, + }; + } +} diff --git a/module/Core/src/Config/NotFoundRedirectResolverInterface.php b/module/Core/src/Config/NotFoundRedirectResolverInterface.php new file mode 100644 index 00000000..a5c55f3d --- /dev/null +++ b/module/Core/src/Config/NotFoundRedirectResolverInterface.php @@ -0,0 +1,16 @@ +baseUrlRedirect; + } + + public function regular404Redirect(): ?string + { + return $this->regular404Redirect; + } + + public function invalidShortUrlRedirect(): ?string + { + return $this->invalidShortUrlRedirect; + } +} diff --git a/module/Core/src/Domain/DomainService.php b/module/Core/src/Domain/DomainService.php index 1e1ac279..99bade7c 100644 --- a/module/Core/src/Domain/DomainService.php +++ b/module/Core/src/Domain/DomainService.php @@ -5,10 +5,12 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Domain; use Doctrine\ORM\EntityManagerInterface; +use Shlinkio\Shlink\Core\Config\NotFoundRedirects; 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\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Rest\ApiKey\Role; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -16,8 +18,11 @@ use function Functional\map; class DomainService implements DomainServiceInterface { - public function __construct(private EntityManagerInterface $em, private string $defaultDomain) - { + public function __construct( + private EntityManagerInterface $em, + private string $defaultDomain, + private NotFoundRedirectOptions $redirectOptions, + ) { } /** @@ -28,14 +33,14 @@ class DomainService implements DomainServiceInterface /** @var DomainRepositoryInterface $repo */ $repo = $this->em->getRepository(Domain::class); $domains = $repo->findDomainsWithout($this->defaultDomain, $apiKey); - $mappedDomains = map($domains, fn (Domain $domain) => new DomainItem($domain->getAuthority(), false)); + $mappedDomains = map($domains, fn (Domain $domain) => DomainItem::forExistingDomain($domain)); if ($apiKey?->hasRole(Role::DOMAIN_SPECIFIC)) { return $mappedDomains; } return [ - new DomainItem($this->defaultDomain, true), + DomainItem::forDefaultDomain($this->defaultDomain, $this->redirectOptions), ...$mappedDomains, ]; } @@ -54,14 +59,29 @@ class DomainService implements DomainServiceInterface return $domain; } - public function getOrCreate(string $authority): Domain + public function findByAuthority(string $authority): ?Domain { $repo = $this->em->getRepository(Domain::class); - $domain = $repo->findOneBy(['authority' => $authority]) ?? new Domain($authority); + return $repo->findOneBy(['authority' => $authority]); + } + + public function getOrCreate(string $authority): Domain + { + $domain = $this->findByAuthority($authority) ?? Domain::withAuthority($authority); $this->em->persist($domain); $this->em->flush(); return $domain; } + + public function configureNotFoundRedirects(string $authority, NotFoundRedirects $notFoundRedirects): Domain + { + $domain = $this->getOrCreate($authority); + $domain->configureNotFoundRedirects($notFoundRedirects); + + $this->em->flush(); + + return $domain; + } } diff --git a/module/Core/src/Domain/DomainServiceInterface.php b/module/Core/src/Domain/DomainServiceInterface.php index 3588fbc6..802ecc7a 100644 --- a/module/Core/src/Domain/DomainServiceInterface.php +++ b/module/Core/src/Domain/DomainServiceInterface.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Domain; +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; @@ -22,4 +23,8 @@ interface DomainServiceInterface public function getDomain(string $domainId): Domain; public function getOrCreate(string $authority): Domain; + + public function findByAuthority(string $authority): ?Domain; + + public function configureNotFoundRedirects(string $authority, NotFoundRedirects $notFoundRedirects): Domain; } diff --git a/module/Core/src/Domain/Model/DomainItem.php b/module/Core/src/Domain/Model/DomainItem.php index f389f1e7..bad7d5cf 100644 --- a/module/Core/src/Domain/Model/DomainItem.php +++ b/module/Core/src/Domain/Model/DomainItem.php @@ -5,28 +5,48 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Domain\Model; use JsonSerializable; +use Shlinkio\Shlink\Core\Config\NotFoundRedirectConfigInterface; +use Shlinkio\Shlink\Core\Entity\Domain; final class DomainItem implements JsonSerializable { - public function __construct(private string $domain, private bool $isDefault) + private function __construct( + private string $authority, + private NotFoundRedirectConfigInterface $notFoundRedirectConfig, + private bool $isDefault + ) { + } + + public static function forExistingDomain(Domain $domain): self { + return new self($domain->getAuthority(), $domain, false); + } + + public static function forDefaultDomain(string $authority, NotFoundRedirectConfigInterface $config): self + { + return new self($authority, $config, true); } public function jsonSerialize(): array { return [ - 'domain' => $this->domain, + 'domain' => $this->authority, 'isDefault' => $this->isDefault, ]; } public function toString(): string { - return $this->domain; + return $this->authority; } public function isDefault(): bool { return $this->isDefault; } + + public function notFoundRedirectConfig(): NotFoundRedirectConfigInterface + { + return $this->notFoundRedirectConfig; + } } diff --git a/module/Core/src/Domain/Repository/DomainRepository.php b/module/Core/src/Domain/Repository/DomainRepository.php index 2e4f3bb2..e0862558 100644 --- a/module/Core/src/Domain/Repository/DomainRepository.php +++ b/module/Core/src/Domain/Repository/DomainRepository.php @@ -18,8 +18,13 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe public function findDomainsWithout(?string $excludedAuthority, ?ApiKey $apiKey = null): array { $qb = $this->createQueryBuilder('d'); - $qb->join(ShortUrl::class, 's', Join::WITH, 's.domain = d') - ->orderBy('d.authority', 'ASC'); + $qb->leftJoin(ShortUrl::class, 's', Join::WITH, 's.domain = d') + ->orderBy('d.authority', 'ASC') + ->groupBy('d') + ->having($qb->expr()->gt('COUNT(s.id)', '0')) + ->orHaving($qb->expr()->isNotNull('d.baseUrlRedirect')) + ->orHaving($qb->expr()->isNotNull('d.regular404Redirect')) + ->orHaving($qb->expr()->isNotNull('d.invalidShortUrlRedirect')); if ($excludedAuthority !== null) { $qb->where($qb->expr()->neq('d.authority', ':excludedAuthority')) diff --git a/module/Core/src/Entity/Domain.php b/module/Core/src/Entity/Domain.php index ee094576..65ca8ce6 100644 --- a/module/Core/src/Entity/Domain.php +++ b/module/Core/src/Entity/Domain.php @@ -6,13 +6,24 @@ namespace Shlinkio\Shlink\Core\Entity; use JsonSerializable; use Shlinkio\Shlink\Common\Entity\AbstractEntity; +use Shlinkio\Shlink\Core\Config\NotFoundRedirectConfigInterface; +use Shlinkio\Shlink\Core\Config\NotFoundRedirects; -class Domain extends AbstractEntity implements JsonSerializable +class Domain extends AbstractEntity implements JsonSerializable, NotFoundRedirectConfigInterface { - public function __construct(private string $authority) + private ?string $baseUrlRedirect = null; + private ?string $regular404Redirect = null; + private ?string $invalidShortUrlRedirect = null; + + private function __construct(private string $authority) { } + public static function withAuthority(string $authority): self + { + return new self($authority); + } + public function getAuthority(): string { return $this->authority; @@ -22,4 +33,41 @@ class Domain extends AbstractEntity implements JsonSerializable { return $this->getAuthority(); } + + public function invalidShortUrlRedirect(): ?string + { + return $this->invalidShortUrlRedirect; + } + + public function hasInvalidShortUrlRedirect(): bool + { + return $this->invalidShortUrlRedirect !== null; + } + + public function regular404Redirect(): ?string + { + return $this->regular404Redirect; + } + + public function hasRegular404Redirect(): bool + { + return $this->regular404Redirect !== null; + } + + public function baseUrlRedirect(): ?string + { + return $this->baseUrlRedirect; + } + + public function hasBaseUrlRedirect(): bool + { + return $this->baseUrlRedirect !== null; + } + + public function configureNotFoundRedirects(NotFoundRedirects $redirects): void + { + $this->baseUrlRedirect = $redirects->baseUrlRedirect(); + $this->regular404Redirect = $redirects->regular404Redirect(); + $this->invalidShortUrlRedirect = $redirects->invalidShortUrlRedirect(); + } } diff --git a/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php index 93fd4597..44cd2ddd 100644 --- a/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php +++ b/module/Core/src/ErrorHandler/NotFoundRedirectHandler.php @@ -8,15 +8,17 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Shlinkio\Shlink\Core\Config\NotFoundRedirectResolverInterface; +use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Options; -use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; class NotFoundRedirectHandler implements MiddlewareInterface { public function __construct( private Options\NotFoundRedirectOptions $redirectOptions, - private RedirectResponseHelperInterface $redirectResponseHelper + private NotFoundRedirectResolverInterface $redirectResolver, + private DomainServiceInterface $domainService, ) { } @@ -24,26 +26,17 @@ class NotFoundRedirectHandler implements MiddlewareInterface { /** @var NotFoundType $notFoundType */ $notFoundType = $request->getAttribute(NotFoundType::class); + $authority = $request->getUri()->getAuthority(); + $domainSpecificRedirect = $this->resolveDomainSpecificRedirect($authority, $notFoundType); - if ($notFoundType->isBaseUrl() && $this->redirectOptions->hasBaseUrlRedirect()) { - // @phpstan-ignore-next-line Create custom PHPStan rule - return $this->redirectResponseHelper->buildRedirectResponse($this->redirectOptions->getBaseUrlRedirect()); - } + return $domainSpecificRedirect + ?? $this->redirectResolver->resolveRedirectResponse($notFoundType, $this->redirectOptions) + ?? $handler->handle($request); + } - if ($notFoundType->isRegularNotFound() && $this->redirectOptions->hasRegular404Redirect()) { - return $this->redirectResponseHelper->buildRedirectResponse( - // @phpstan-ignore-next-line Create custom PHPStan rule - $this->redirectOptions->getRegular404Redirect(), - ); - } - - if ($notFoundType->isInvalidShortUrl() && $this->redirectOptions->hasInvalidShortUrlRedirect()) { - return $this->redirectResponseHelper->buildRedirectResponse( - // @phpstan-ignore-next-line Create custom PHPStan rule - $this->redirectOptions->getInvalidShortUrlRedirect(), - ); - } - - return $handler->handle($request); + private function resolveDomainSpecificRedirect(string $authority, NotFoundType $notFoundType): ?ResponseInterface + { + $domain = $this->domainService->findByAuthority($authority); + return $domain === null ? null : $this->redirectResolver->resolveRedirectResponse($notFoundType, $domain); } } diff --git a/module/Core/src/Options/NotFoundRedirectOptions.php b/module/Core/src/Options/NotFoundRedirectOptions.php index 1bb3b828..2f2d813b 100644 --- a/module/Core/src/Options/NotFoundRedirectOptions.php +++ b/module/Core/src/Options/NotFoundRedirectOptions.php @@ -5,14 +5,15 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Options; use Laminas\Stdlib\AbstractOptions; +use Shlinkio\Shlink\Core\Config\NotFoundRedirectConfigInterface; -class NotFoundRedirectOptions extends AbstractOptions +class NotFoundRedirectOptions extends AbstractOptions implements NotFoundRedirectConfigInterface { private ?string $invalidShortUrl = null; private ?string $regular404 = null; private ?string $baseUrl = null; - public function getInvalidShortUrlRedirect(): ?string + public function invalidShortUrlRedirect(): ?string { return $this->invalidShortUrl; } @@ -28,7 +29,7 @@ class NotFoundRedirectOptions extends AbstractOptions return $this; } - public function getRegular404Redirect(): ?string + public function regular404Redirect(): ?string { return $this->regular404; } @@ -44,7 +45,7 @@ class NotFoundRedirectOptions extends AbstractOptions return $this; } - public function getBaseUrlRedirect(): ?string + public function baseUrlRedirect(): ?string { return $this->baseUrl; } diff --git a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php index a9456712..c8367b49 100644 --- a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php @@ -41,7 +41,9 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt private function memoizeNewDomain(string $domain): Domain { - return $this->memoizedNewDomains[$domain] = $this->memoizedNewDomains[$domain] ?? new Domain($domain); + return $this->memoizedNewDomains[$domain] = $this->memoizedNewDomains[$domain] ?? Domain::withAuthority( + $domain, + ); } /** diff --git a/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php index 2cda44df..173b530c 100644 --- a/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php @@ -15,7 +15,7 @@ class SimpleShortUrlRelationResolver implements ShortUrlRelationResolverInterfac { public function resolveDomain(?string $domain): ?Domain { - return $domain !== null ? new Domain($domain) : null; + return $domain !== null ? Domain::withAuthority($domain) : null; } /** diff --git a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php index 0f5aa259..9b0270a6 100644 --- a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php +++ b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php @@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Core\Domain\Repository; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; +use Shlinkio\Shlink\Core\Config\NotFoundRedirects; use Shlinkio\Shlink\Core\Domain\Repository\DomainRepository; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; @@ -28,27 +29,47 @@ class DomainRepositoryTest extends DatabaseTestCase /** @test */ public function findDomainsReturnsExpectedResult(): void { - $fooDomain = new Domain('foo.com'); + $fooDomain = Domain::withAuthority('foo.com'); $this->getEntityManager()->persist($fooDomain); $this->getEntityManager()->persist($this->createShortUrl($fooDomain)); - $barDomain = new Domain('bar.com'); + $barDomain = Domain::withAuthority('bar.com'); $this->getEntityManager()->persist($barDomain); $this->getEntityManager()->persist($this->createShortUrl($barDomain)); - $bazDomain = new Domain('baz.com'); + $bazDomain = Domain::withAuthority('baz.com'); $this->getEntityManager()->persist($bazDomain); $this->getEntityManager()->persist($this->createShortUrl($bazDomain)); - $detachedDomain = new Domain('detached.com'); + $detachedDomain = Domain::withAuthority('detached.com'); $this->getEntityManager()->persist($detachedDomain); + $detachedWithRedirects = Domain::withAuthority('detached-with-redirects.com'); + $detachedWithRedirects->configureNotFoundRedirects(new NotFoundRedirects('foo.com', 'bar.com')); + $this->getEntityManager()->persist($detachedWithRedirects); + $this->getEntityManager()->flush(); - self::assertEquals([$barDomain, $bazDomain, $fooDomain], $this->repo->findDomainsWithout(null)); - self::assertEquals([$barDomain, $bazDomain], $this->repo->findDomainsWithout('foo.com')); - self::assertEquals([$bazDomain, $fooDomain], $this->repo->findDomainsWithout('bar.com')); - self::assertEquals([$barDomain, $fooDomain], $this->repo->findDomainsWithout('baz.com')); + 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'), + ); } /** @test */ @@ -59,18 +80,25 @@ class DomainRepositoryTest extends DatabaseTestCase $authorAndDomainApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($authorAndDomainApiKey); - $fooDomain = new Domain('foo.com'); + $fooDomain = Domain::withAuthority('foo.com'); $this->getEntityManager()->persist($fooDomain); $this->getEntityManager()->persist($this->createShortUrl($fooDomain, $authorApiKey)); - $barDomain = new Domain('bar.com'); + $barDomain = Domain::withAuthority('bar.com'); $this->getEntityManager()->persist($barDomain); $this->getEntityManager()->persist($this->createShortUrl($barDomain, $authorAndDomainApiKey)); - $bazDomain = new Domain('baz.com'); + $bazDomain = Domain::withAuthority('baz.com'); $this->getEntityManager()->persist($bazDomain); $this->getEntityManager()->persist($this->createShortUrl($bazDomain, $authorApiKey)); +// $detachedDomain = Domain::withAuthority('detached.com'); +// $this->getEntityManager()->persist($detachedDomain); +// +// $detachedWithRedirects = Domain::withAuthority('detached-with-redirects.com'); +// $detachedWithRedirects->configureNotFoundRedirects(new NotFoundRedirects('foo.com', 'bar.com')); +// $this->getEntityManager()->persist($detachedWithRedirects); + $this->getEntityManager()->flush(); $authorAndDomainApiKey->registerRole(RoleDefinition::forDomain($fooDomain)); @@ -79,12 +107,21 @@ class DomainRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($fooDomainApiKey); $barDomainApiKey = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forDomain($barDomain))); - $this->getEntityManager()->persist($fooDomainApiKey); + $this->getEntityManager()->persist($barDomainApiKey); + +// $detachedWithRedirectsApiKey = ApiKey::fromMeta( +// ApiKeyMeta::withRoles(RoleDefinition::forDomain($detachedWithRedirects)), +// ); +// $this->getEntityManager()->persist($detachedWithRedirectsApiKey); $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)); } diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index 867ff3f2..adc3d67f 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -340,9 +340,9 @@ class ShortUrlRepositoryTest extends DatabaseTestCase { $start = Chronos::parse('2020-03-05 20:18:30'); - $wrongDomain = new Domain('wrong.com'); + $wrongDomain = Domain::withAuthority('wrong.com'); $this->getEntityManager()->persist($wrongDomain); - $rightDomain = new Domain('right.com'); + $rightDomain = Domain::withAuthority('right.com'); $this->getEntityManager()->persist($rightDomain); $this->getEntityManager()->flush(); diff --git a/module/Core/test-db/Repository/TagRepositoryTest.php b/module/Core/test-db/Repository/TagRepositoryTest.php index eea2ed8c..92498d9a 100644 --- a/module/Core/test-db/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Repository/TagRepositoryTest.php @@ -97,7 +97,7 @@ class TagRepositoryTest extends DatabaseTestCase /** @test */ public function tagExistsReturnsExpectedResultBasedOnApiKey(): void { - $domain = new Domain('foo.com'); + $domain = Domain::withAuthority('foo.com'); $this->getEntityManager()->persist($domain); $this->getEntityManager()->flush(); diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index 15fe34f4..9f7859ff 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -222,7 +222,7 @@ class VisitRepositoryTest extends DatabaseTestCase /** @test */ public function countVisitsReturnsExpectedResultBasedOnApiKey(): void { - $domain = new Domain('foo.com'); + $domain = Domain::withAuthority('foo.com'); $this->getEntityManager()->persist($domain); $this->getEntityManager()->flush(); diff --git a/module/Core/test/Config/NotFoundRedirectResolverTest.php b/module/Core/test/Config/NotFoundRedirectResolverTest.php new file mode 100644 index 00000000..fe482a41 --- /dev/null +++ b/module/Core/test/Config/NotFoundRedirectResolverTest.php @@ -0,0 +1,114 @@ +helper = $this->prophesize(RedirectResponseHelperInterface::class); + $this->resolver = new NotFoundRedirectResolver($this->helper->reveal()); + + $this->config = new NotFoundRedirectOptions([ + 'invalidShortUrl' => 'invalidShortUrl', + 'regular404' => 'regular404', + 'baseUrl' => 'baseUrl', + ]); + } + + /** + * @test + * @dataProvider provideRedirects + */ + public function expectedRedirectionIsReturnedDependingOnTheCase( + NotFoundType $notFoundType, + string $expectedRedirectTo, + ): void { + $expectedResp = new Response(); + $buildResp = $this->helper->buildRedirectResponse($expectedRedirectTo)->willReturn($expectedResp); + + $resp = $this->resolver->resolveRedirectResponse($notFoundType, $this->config); + + self::assertSame($expectedResp, $resp); + $buildResp->shouldHaveBeenCalledOnce(); + } + + public function provideRedirects(): iterable + { + yield 'base URL with trailing slash' => [ + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri('/'))), + 'baseUrl', + ]; + yield 'base URL without trailing slash' => [ + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri(''))), + 'baseUrl', + ]; + yield 'regular 404' => [ + $this->notFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri('/foo/bar'))), + 'regular404', + ]; + yield 'invalid short URL' => [ + $this->notFoundType($this->requestForRoute(RedirectAction::class)), + 'invalidShortUrl', + ]; + } + + /** @test */ + public function noResponseIsReturnedIfNoConditionsMatch(): void + { + $notFoundType = $this->notFoundType($this->requestForRoute('foo')); + + $result = $this->resolver->resolveRedirectResponse($notFoundType, $this->config); + + self::assertNull($result); + $this->helper->buildRedirectResponse(Argument::cetera())->shouldNotHaveBeenCalled(); + } + + private function notFoundType(ServerRequestInterface $req): NotFoundType + { + return NotFoundType::fromRequest($req, ''); + } + + private function requestForRoute(string $routeName): ServerRequestInterface + { + return ServerRequestFactory::fromGlobals() + ->withAttribute( + RouteResult::class, + RouteResult::fromRoute( + new Route( + '', + $this->prophesize(MiddlewareInterface::class)->reveal(), + ['GET'], + $routeName, + ), + ), + ) + ->withUri(new Uri('/abc123')); + } +} diff --git a/module/Core/test/Domain/DomainServiceTest.php b/module/Core/test/Domain/DomainServiceTest.php index 80326b3c..b53b4a26 100644 --- a/module/Core/test/Domain/DomainServiceTest.php +++ b/module/Core/test/Domain/DomainServiceTest.php @@ -9,11 +9,13 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; +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\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -28,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'); + $this->domainService = new DomainService($this->em->reveal(), 'default.com', new NotFoundRedirectOptions()); } /** @@ -50,45 +52,56 @@ class DomainServiceTest extends TestCase public function provideExcludedDomains(): iterable { - $default = new DomainItem('default.com', true); + $default = DomainItem::forDefaultDomain('default.com', new NotFoundRedirectOptions()); $adminApiKey = ApiKey::create(); $domainSpecificApiKey = ApiKey::fromMeta( - ApiKeyMeta::withRoles(RoleDefinition::forDomain((new Domain(''))->setId('123'))), + ApiKeyMeta::withRoles(RoleDefinition::forDomain(Domain::withAuthority('')->setId('123'))), ); yield 'empty list without API key' => [[], [$default], null]; yield 'one item without API key' => [ - [new Domain('bar.com')], - [$default, new DomainItem('bar.com', false)], + [Domain::withAuthority('bar.com')], + [$default, DomainItem::forExistingDomain(Domain::withAuthority('bar.com'))], null, ]; yield 'multiple items without API key' => [ - [new Domain('foo.com'), new Domain('bar.com')], - [$default, new DomainItem('foo.com', false), new DomainItem('bar.com', false)], + [Domain::withAuthority('foo.com'), Domain::withAuthority('bar.com')], + [ + $default, + DomainItem::forExistingDomain(Domain::withAuthority('foo.com')), + DomainItem::forExistingDomain(Domain::withAuthority('bar.com')), + ], null, ]; yield 'empty list with admin API key' => [[], [$default], $adminApiKey]; yield 'one item with admin API key' => [ - [new Domain('bar.com')], - [$default, new DomainItem('bar.com', false)], + [Domain::withAuthority('bar.com')], + [$default, DomainItem::forExistingDomain(Domain::withAuthority('bar.com'))], $adminApiKey, ]; yield 'multiple items with admin API key' => [ - [new Domain('foo.com'), new Domain('bar.com')], - [$default, new DomainItem('foo.com', false), new DomainItem('bar.com', false)], + [Domain::withAuthority('foo.com'), Domain::withAuthority('bar.com')], + [ + $default, + DomainItem::forExistingDomain(Domain::withAuthority('foo.com')), + DomainItem::forExistingDomain(Domain::withAuthority('bar.com')), + ], $adminApiKey, ]; yield 'empty list with domain-specific API key' => [[], [], $domainSpecificApiKey]; yield 'one item with domain-specific API key' => [ - [new Domain('bar.com')], - [new DomainItem('bar.com', false)], + [Domain::withAuthority('bar.com')], + [DomainItem::forExistingDomain(Domain::withAuthority('bar.com'))], $domainSpecificApiKey, ]; yield 'multiple items with domain-specific API key' => [ - [new Domain('foo.com'), new Domain('bar.com')], - [new DomainItem('foo.com', false), new DomainItem('bar.com', false)], + [Domain::withAuthority('foo.com'), Domain::withAuthority('bar.com')], + [ + DomainItem::forExistingDomain(Domain::withAuthority('foo.com')), + DomainItem::forExistingDomain(Domain::withAuthority('bar.com')), + ], $domainSpecificApiKey, ]; } @@ -107,7 +120,7 @@ class DomainServiceTest extends TestCase /** @test */ public function getDomainReturnsEntityWhenFound(): void { - $domain = new Domain(''); + $domain = Domain::withAuthority(''); $find = $this->em->find(Domain::class, '123')->willReturn($domain); $result = $this->domainService->getDomain('123'); @@ -139,9 +152,39 @@ class DomainServiceTest extends TestCase $flush->shouldHaveBeenCalledOnce(); } + /** + * @test + * @dataProvider provideFoundDomains + */ + public function configureNotFoundRedirectsConfiguresFetchedDomain(?Domain $foundDomain): void + { + $authority = 'example.com'; + $repo = $this->prophesize(DomainRepositoryInterface::class); + $repo->findOneBy(['authority' => $authority])->willReturn($foundDomain); + $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); + $persist = $this->em->persist($foundDomain ?? Argument::type(Domain::class)); + $flush = $this->em->flush(); + + $result = $this->domainService->configureNotFoundRedirects($authority, new NotFoundRedirects( + 'foo.com', + 'bar.com', + 'baz.com', + )); + + if ($foundDomain !== null) { + self::assertSame($result, $foundDomain); + } + self::assertEquals('foo.com', $result->baseUrlRedirect()); + self::assertEquals('bar.com', $result->regular404Redirect()); + self::assertEquals('baz.com', $result->invalidShortUrlRedirect()); + $getRepo->shouldHaveBeenCalledOnce(); + $persist->shouldHaveBeenCalledOnce(); + $flush->shouldHaveBeenCalledTimes(2); + } + public function provideFoundDomains(): iterable { yield 'domain not found' => [null]; - yield 'domain found' => [new Domain('')]; + yield 'domain found' => [Domain::withAuthority('')]; } } diff --git a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php index f3054f49..0d257d8e 100644 --- a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php +++ b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php @@ -6,21 +6,18 @@ namespace ShlinkioTest\Shlink\Core\ErrorHandler; use Laminas\Diactoros\Response; use Laminas\Diactoros\ServerRequestFactory; -use Laminas\Diactoros\Uri; -use Mezzio\Router\Route; -use Mezzio\Router\RouteResult; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\ServerRequestInterface; -use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; -use Shlinkio\Shlink\Core\Action\RedirectAction; +use Shlinkio\Shlink\Core\Config\NotFoundRedirectResolverInterface; +use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; +use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\ErrorHandler\NotFoundRedirectHandler; use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; -use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; class NotFoundRedirectHandlerTest extends TestCase { @@ -28,93 +25,103 @@ class NotFoundRedirectHandlerTest extends TestCase private NotFoundRedirectHandler $middleware; private NotFoundRedirectOptions $redirectOptions; - private ObjectProphecy $helper; + private ObjectProphecy $resolver; + private ObjectProphecy $domainService; + private ObjectProphecy $next; + private ServerRequestInterface $req; public function setUp(): void { $this->redirectOptions = new NotFoundRedirectOptions(); - $this->helper = $this->prophesize(RedirectResponseHelperInterface::class); - $this->middleware = new NotFoundRedirectHandler($this->redirectOptions, $this->helper->reveal()); + $this->resolver = $this->prophesize(NotFoundRedirectResolverInterface::class); + $this->domainService = $this->prophesize(DomainServiceInterface::class); + + $this->middleware = new NotFoundRedirectHandler( + $this->redirectOptions, + $this->resolver->reveal(), + $this->domainService->reveal(), + ); + + $this->next = $this->prophesize(RequestHandlerInterface::class); + $this->req = ServerRequestFactory::fromGlobals()->withAttribute( + NotFoundType::class, + $this->prophesize(NotFoundType::class)->reveal(), + ); } /** * @test - * @dataProvider provideRedirects + * @dataProvider provideNonRedirectScenarios */ - public function expectedRedirectionIsReturnedDependingOnTheCase( - ServerRequestInterface $request, - string $expectedRedirectTo, - ): void { - $this->redirectOptions->invalidShortUrl = 'invalidShortUrl'; - $this->redirectOptions->regular404 = 'regular404'; - $this->redirectOptions->baseUrl = 'baseUrl'; - + public function nextIsCalledWhenNoRedirectIsResolved(callable $setUp): void + { $expectedResp = new Response(); - $buildResp = $this->helper->buildRedirectResponse($expectedRedirectTo)->willReturn($expectedResp); - $next = $this->prophesize(RequestHandlerInterface::class); - $handle = $next->handle($request)->willReturn(new Response()); + $setUp($this->domainService, $this->resolver); + $handle = $this->next->handle($this->req)->willReturn($expectedResp); - $resp = $this->middleware->process($request, $next->reveal()); + $result = $this->middleware->process($this->req, $this->next->reveal()); - self::assertSame($expectedResp, $resp); - $buildResp->shouldHaveBeenCalledOnce(); - $handle->shouldNotHaveBeenCalled(); - } - - public function provideRedirects(): iterable - { - yield 'base URL with trailing slash' => [ - $this->withNotFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri('/'))), - 'baseUrl', - ]; - yield 'base URL without trailing slash' => [ - $this->withNotFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri(''))), - 'baseUrl', - ]; - yield 'regular 404' => [ - $this->withNotFoundType(ServerRequestFactory::fromGlobals()->withUri(new Uri('/foo/bar'))), - 'regular404', - ]; - yield 'invalid short URL' => [ - $this->withNotFoundType(ServerRequestFactory::fromGlobals() - ->withAttribute( - RouteResult::class, - RouteResult::fromRoute( - new Route( - '', - $this->prophesize(MiddlewareInterface::class)->reveal(), - ['GET'], - RedirectAction::class, - ), - ), - ) - ->withUri(new Uri('/abc123'))), - 'invalidShortUrl', - ]; - } - - /** @test */ - public function nextMiddlewareIsInvokedWhenNotRedirectNeedsToOccur(): void - { - $req = $this->withNotFoundType(ServerRequestFactory::fromGlobals()); - $resp = new Response(); - - $buildResp = $this->helper->buildRedirectResponse(Argument::cetera()); - - $next = $this->prophesize(RequestHandlerInterface::class); - $handle = $next->handle($req)->willReturn($resp); - - $result = $this->middleware->process($req, $next->reveal()); - - self::assertSame($resp, $result); - $buildResp->shouldNotHaveBeenCalled(); + self::assertSame($expectedResp, $result); $handle->shouldHaveBeenCalledOnce(); } - private function withNotFoundType(ServerRequestInterface $req): ServerRequestInterface + public function provideNonRedirectScenarios(): iterable { - $type = NotFoundType::fromRequest($req, ''); - return $req->withAttribute(NotFoundType::class, $type); + yield 'no domain' => [function (ObjectProphecy $domainService, ObjectProphecy $resolver): void { + $domainService->findByAuthority(Argument::cetera()) + ->willReturn(null) + ->shouldBeCalledOnce(); + $resolver->resolveRedirectResponse(Argument::cetera()) + ->willReturn(null) + ->shouldBeCalledOnce(); + }]; + yield 'non-redirecting domain' => [function (ObjectProphecy $domainService, ObjectProphecy $resolver): void { + $domainService->findByAuthority(Argument::cetera()) + ->willReturn(Domain::withAuthority('')) + ->shouldBeCalledOnce(); + $resolver->resolveRedirectResponse(Argument::cetera()) + ->willReturn(null) + ->shouldBeCalledTimes(2); + }]; + } + + /** @test */ + public function globalRedirectIsUsedIfDomainRedirectIsNotFound(): void + { + $expectedResp = new Response(); + + $findDomain = $this->domainService->findByAuthority(Argument::cetera())->willReturn(null); + $resolveRedirect = $this->resolver->resolveRedirectResponse( + Argument::type(NotFoundType::class), + $this->redirectOptions, + )->willReturn($expectedResp); + + $result = $this->middleware->process($this->req, $this->next->reveal()); + + self::assertSame($expectedResp, $result); + $findDomain->shouldHaveBeenCalledOnce(); + $resolveRedirect->shouldHaveBeenCalledOnce(); + $this->next->handle(Argument::cetera())->shouldNotHaveBeenCalled(); + } + + /** @test */ + public function domainRedirectIsUsedIfFound(): void + { + $expectedResp = new Response(); + $domain = Domain::withAuthority(''); + + $findDomain = $this->domainService->findByAuthority(Argument::cetera())->willReturn($domain); + $resolveRedirect = $this->resolver->resolveRedirectResponse( + Argument::type(NotFoundType::class), + $domain, + )->willReturn($expectedResp); + + $result = $this->middleware->process($this->req, $this->next->reveal()); + + self::assertSame($expectedResp, $result); + $findDomain->shouldHaveBeenCalledOnce(); + $resolveRedirect->shouldHaveBeenCalledOnce(); + $this->next->handle(Argument::cetera())->shouldNotHaveBeenCalled(); } } diff --git a/module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php b/module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php index ca3b463f..b30f8cab 100644 --- a/module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php +++ b/module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php @@ -60,7 +60,7 @@ class ShortCodeHelperTest extends TestCase public function provideDomains(): iterable { yield 'no domain' => [null, null]; - yield 'domain' => [new Domain($authority = 'doma.in'), $authority]; + yield 'domain' => [Domain::withAuthority($authority = 'doma.in'), $authority]; } /** @test */ diff --git a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php index aeef3f47..9aaf9495 100644 --- a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php @@ -68,7 +68,7 @@ class PersistenceShortUrlRelationResolverTest extends TestCase $authority = 'doma.in'; yield 'not found domain' => [null, $authority]; - yield 'found domain' => [new Domain($authority), $authority]; + yield 'found domain' => [Domain::withAuthority($authority), $authority]; } /** diff --git a/module/Rest/test-api/Action/ListDomainsTest.php b/module/Rest/test-api/Action/ListDomainsTest.php index cf3167f8..075b6d09 100644 --- a/module/Rest/test-api/Action/ListDomainsTest.php +++ b/module/Rest/test-api/Action/ListDomainsTest.php @@ -32,6 +32,10 @@ class ListDomainsTest extends ApiTestCase 'domain' => 'doma.in', 'isDefault' => true, ], + [ + 'domain' => 'detached-with-redirects.com', + 'isDefault' => false, + ], [ 'domain' => 'example.com', 'isDefault' => false, diff --git a/module/Rest/test-api/Fixtures/DomainFixture.php b/module/Rest/test-api/Fixtures/DomainFixture.php index 576586a6..31e68f21 100644 --- a/module/Rest/test-api/Fixtures/DomainFixture.php +++ b/module/Rest/test-api/Fixtures/DomainFixture.php @@ -6,17 +6,23 @@ namespace ShlinkioApiTest\Shlink\Rest\Fixtures; use Doctrine\Common\DataFixtures\AbstractFixture; use Doctrine\Persistence\ObjectManager; +use Shlinkio\Shlink\Core\Config\NotFoundRedirects; use Shlinkio\Shlink\Core\Entity\Domain; class DomainFixture extends AbstractFixture { public function load(ObjectManager $manager): void { - $domain = new Domain('example.com'); + $domain = Domain::withAuthority('example.com'); $manager->persist($domain); $this->addReference('example_domain', $domain); - $manager->persist(new Domain('this_domain_is_detached.com')); + $manager->persist(Domain::withAuthority('this_domain_is_detached.com')); + + $detachedWithRedirects = Domain::withAuthority('detached-with-redirects.com'); + $detachedWithRedirects->configureNotFoundRedirects(new NotFoundRedirects('foo.com', 'bar.com')); + $manager->persist($detachedWithRedirects); + $manager->flush(); } } diff --git a/module/Rest/test/Action/Domain/ListDomainsActionTest.php b/module/Rest/test/Action/Domain/ListDomainsActionTest.php index cbe43895..45575cc6 100644 --- a/module/Rest/test/Action/Domain/ListDomainsActionTest.php +++ b/module/Rest/test/Action/Domain/ListDomainsActionTest.php @@ -11,6 +11,8 @@ use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; use Shlinkio\Shlink\Core\Domain\Model\DomainItem; +use Shlinkio\Shlink\Core\Entity\Domain; +use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Rest\Action\Domain\ListDomainsAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -32,8 +34,8 @@ class ListDomainsActionTest extends TestCase { $apiKey = ApiKey::create(); $domains = [ - new DomainItem('bar.com', true), - new DomainItem('baz.com', false), + DomainItem::forDefaultDomain('bar.com', new NotFoundRedirectOptions()), + DomainItem::forExistingDomain(Domain::withAuthority('baz.com')), ]; $listDomains = $this->domainService->listDomains($apiKey)->willReturn($domains); diff --git a/module/Rest/test/Middleware/ShortUrl/OverrideDomainMiddlewareTest.php b/module/Rest/test/Middleware/ShortUrl/OverrideDomainMiddlewareTest.php index 9614f8c7..ed1e62d3 100644 --- a/module/Rest/test/Middleware/ShortUrl/OverrideDomainMiddlewareTest.php +++ b/module/Rest/test/Middleware/ShortUrl/OverrideDomainMiddlewareTest.php @@ -82,19 +82,23 @@ class OverrideDomainMiddlewareTest extends TestCase public function provideBodies(): iterable { - yield 'no domain provided' => [new Domain('foo.com'), [], [ShortUrlInputFilter::DOMAIN => 'foo.com']]; + yield 'no domain provided' => [ + Domain::withAuthority('foo.com'), + [], + [ShortUrlInputFilter::DOMAIN => 'foo.com'], + ]; yield 'other domain provided' => [ - new Domain('bar.com'), + Domain::withAuthority('bar.com'), [ShortUrlInputFilter::DOMAIN => 'foo.com'], [ShortUrlInputFilter::DOMAIN => 'bar.com'], ]; yield 'same domain provided' => [ - new Domain('baz.com'), + Domain::withAuthority('baz.com'), [ShortUrlInputFilter::DOMAIN => 'baz.com'], [ShortUrlInputFilter::DOMAIN => 'baz.com'], ]; yield 'more body params' => [ - new Domain('doma.in'), + Domain::withAuthority('doma.in'), [ShortUrlInputFilter::DOMAIN => 'baz.com', 'something' => 'else', 'foo' => 123], [ShortUrlInputFilter::DOMAIN => 'doma.in', 'something' => 'else', 'foo' => 123], ]; @@ -106,7 +110,7 @@ class OverrideDomainMiddlewareTest extends TestCase */ public function setsRequestAttributeWhenMethodIsNotPost(string $method): void { - $domain = new Domain('something.com'); + $domain = Domain::withAuthority('something.com'); $request = $this->requestWithApiKey()->withMethod($method); $hasRole = $this->apiKey->hasRole(Role::DOMAIN_SPECIFIC)->willReturn(true); $getRoleMeta = $this->apiKey->getRoleMeta(Role::DOMAIN_SPECIFIC)->willReturn(['domain_id' => '123']); diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php index addebbcd..de17d8bd 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -55,7 +55,7 @@ class ApiKeyServiceTest extends TestCase yield 'no expiration date or name' => [null, null, []]; yield 'expiration date' => [Chronos::parse('2030-01-01'), null, []]; yield 'roles' => [null, null, [ - RoleDefinition::forDomain((new Domain(''))->setId('123')), + RoleDefinition::forDomain(Domain::withAuthority('')->setId('123')), RoleDefinition::forAuthoredShortUrls(), ]]; yield 'single name' => [null, 'Alice', []];