From 021cecc21648897fb4d9c3b57f0b2c2dd074d51c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 21 Jul 2021 21:09:33 +0200 Subject: [PATCH] Created command that allows configuring not found redirects for every domain --- module/CLI/config/cli.config.php | 1 + module/CLI/config/dependencies.config.php | 2 + .../Command/Domain/DomainRedirectsCommand.php | 114 ++++++++++++++++++ .../src/Command/Domain/ListDomainsCommand.php | 38 +++++- .../Command/Domain/ListDomainsCommandTest.php | 8 +- module/Core/config/dependencies.config.php | 6 +- module/Core/src/Config/NotFoundRedirects.php | 30 +++++ module/Core/src/Domain/DomainService.php | 23 +++- .../src/Domain/DomainServiceInterface.php | 3 + module/Core/src/Domain/Model/DomainItem.php | 26 +++- module/Core/src/Entity/Domain.php | 8 ++ module/Core/test/Domain/DomainServiceTest.php | 28 +++-- .../Action/Domain/ListDomainsActionTest.php | 6 +- 13 files changed, 269 insertions(+), 24 deletions(-) create mode 100644 module/CLI/src/Command/Domain/DomainRedirectsCommand.php create mode 100644 module/Core/src/Config/NotFoundRedirects.php 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->listDomains()->willReturn([ - new DomainItem('foo.com', true), - new DomainItem('bar.com', false), - new DomainItem('baz.com', false), + DomainItem::forDefaultDomain('foo.com', new NotFoundRedirectOptions()), + DomainItem::forExistingDomain(new Domain('bar.com')), + DomainItem::forExistingDomain(new Domain('baz.com')), ]); $this->commandTester->execute([]); diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index a215a871..7f28b14d 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -115,7 +115,11 @@ 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'], diff --git a/module/Core/src/Config/NotFoundRedirects.php b/module/Core/src/Config/NotFoundRedirects.php new file mode 100644 index 00000000..2a1e68d4 --- /dev/null +++ b/module/Core/src/Config/NotFoundRedirects.php @@ -0,0 +1,30 @@ +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 6d99b0a7..b974e6d7 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, ]; } @@ -69,4 +74,14 @@ class DomainService implements DomainServiceInterface 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 be357a22..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; @@ -24,4 +25,6 @@ interface DomainServiceInterface 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/Entity/Domain.php b/module/Core/src/Entity/Domain.php index 73b790c4..cb777a7b 100644 --- a/module/Core/src/Entity/Domain.php +++ b/module/Core/src/Entity/Domain.php @@ -7,6 +7,7 @@ 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, NotFoundRedirectConfigInterface { @@ -57,4 +58,11 @@ class Domain extends AbstractEntity implements JsonSerializable, NotFoundRedirec { 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/test/Domain/DomainServiceTest.php b/module/Core/test/Domain/DomainServiceTest.php index 80326b3c..de58b1b5 100644 --- a/module/Core/test/Domain/DomainServiceTest.php +++ b/module/Core/test/Domain/DomainServiceTest.php @@ -14,6 +14,7 @@ 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 +29,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,7 +51,7 @@ 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'))), @@ -59,36 +60,47 @@ class DomainServiceTest extends TestCase yield 'empty list without API key' => [[], [$default], null]; yield 'one item without API key' => [ [new Domain('bar.com')], - [$default, new DomainItem('bar.com', false)], + [$default, DomainItem::forExistingDomain(new Domain('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)], + [ + $default, + DomainItem::forExistingDomain(new Domain('foo.com')), + DomainItem::forExistingDomain(new Domain('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)], + [$default, DomainItem::forExistingDomain(new Domain('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)], + [ + $default, + DomainItem::forExistingDomain(new Domain('foo.com')), + DomainItem::forExistingDomain(new Domain('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)], + [DomainItem::forExistingDomain(new Domain('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)], + [ + DomainItem::forExistingDomain(new Domain('foo.com')), + DomainItem::forExistingDomain(new Domain('bar.com')), + ], $domainSpecificApiKey, ]; } diff --git a/module/Rest/test/Action/Domain/ListDomainsActionTest.php b/module/Rest/test/Action/Domain/ListDomainsActionTest.php index cbe43895..beff58fd 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(new Domain('baz.com')), ]; $listDomains = $this->domainService->listDomains($apiKey)->willReturn($domains);