From a01e0ba337d9c4f5622777b4e0e6377d4505c104 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 4 Jan 2021 15:02:37 +0100 Subject: [PATCH] Changed logic to list domains to centralize conditions in service --- module/CLI/config/dependencies.config.php | 2 +- .../src/Command/Domain/ListDomainsCommand.php | 16 ++++---- .../Command/Domain/ListDomainsCommandTest.php | 11 +++--- module/Core/config/dependencies.config.php | 2 +- module/Core/src/Domain/DomainService.php | 18 +++++++-- .../src/Domain/DomainServiceInterface.php | 6 +-- module/Core/src/Domain/Model/DomainItem.php | 37 +++++++++++++++++++ module/Core/test/Domain/DomainServiceTest.php | 21 +++++++---- module/Rest/config/dependencies.config.php | 2 +- .../src/Action/Domain/ListDomainsAction.php | 22 ++--------- .../Action/Domain/ListDomainsActionTest.php | 26 ++++--------- 11 files changed, 93 insertions(+), 70 deletions(-) create mode 100644 module/Core/src/Domain/Model/DomainItem.php diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 199d29ef..313d0022 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -87,7 +87,7 @@ return [ Command\Tag\RenameTagCommand::class => [TagService::class], Command\Tag\DeleteTagsCommand::class => [TagService::class], - Command\Domain\ListDomainsCommand::class => [DomainService::class, 'config.url_shortener.domain.hostname'], + Command\Domain\ListDomainsCommand::class => [DomainService::class], Command\Db\CreateDatabaseCommand::class => [ LockFactory::class, diff --git a/module/CLI/src/Command/Domain/ListDomainsCommand.php b/module/CLI/src/Command/Domain/ListDomainsCommand.php index 0368f1dd..b05ad429 100644 --- a/module/CLI/src/Command/Domain/ListDomainsCommand.php +++ b/module/CLI/src/Command/Domain/ListDomainsCommand.php @@ -7,7 +7,7 @@ namespace Shlinkio\Shlink\CLI\Command\Domain; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; -use Shlinkio\Shlink\Core\Entity\Domain; +use Shlinkio\Shlink\Core\Domain\Model\DomainItem; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -19,13 +19,11 @@ class ListDomainsCommand extends Command public const NAME = 'domain:list'; private DomainServiceInterface $domainService; - private string $defaultDomain; - public function __construct(DomainServiceInterface $domainService, string $defaultDomain) + public function __construct(DomainServiceInterface $domainService) { parent::__construct(); $this->domainService = $domainService; - $this->defaultDomain = $defaultDomain; } protected function configure(): void @@ -37,12 +35,12 @@ class ListDomainsCommand extends Command protected function execute(InputInterface $input, OutputInterface $output): ?int { - $regularDomains = $this->domainService->listDomainsWithout($this->defaultDomain); + $regularDomains = $this->domainService->listDomainsWithout(); - ShlinkTable::fromOutput($output)->render(['Domain', 'Is default'], [ - [$this->defaultDomain, 'Yes'], - ...map($regularDomains, fn (Domain $domain) => [$domain->getAuthority(), 'No']), - ]); + ShlinkTable::fromOutput($output)->render( + ['Domain', 'Is default'], + map($regularDomains, fn (DomainItem $domain) => [$domain->toString(), $domain->isDefault() ? 'Yes' : 'No']), + ); return ExitCodes::EXIT_SUCCESS; } diff --git a/module/CLI/test/Command/Domain/ListDomainsCommandTest.php b/module/CLI/test/Command/Domain/ListDomainsCommandTest.php index 500fed7f..05e35c0b 100644 --- a/module/CLI/test/Command/Domain/ListDomainsCommandTest.php +++ b/module/CLI/test/Command/Domain/ListDomainsCommandTest.php @@ -10,7 +10,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Domain\ListDomainsCommand; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; -use Shlinkio\Shlink\Core\Entity\Domain; +use Shlinkio\Shlink\Core\Domain\Model\DomainItem; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -25,7 +25,7 @@ class ListDomainsCommandTest extends TestCase { $this->domainService = $this->prophesize(DomainServiceInterface::class); - $command = new ListDomainsCommand($this->domainService->reveal(), 'foo.com'); + $command = new ListDomainsCommand($this->domainService->reveal()); $app = new Application(); $app->add($command); @@ -45,9 +45,10 @@ class ListDomainsCommandTest extends TestCase +---------+------------+ OUTPUT; - $listDomains = $this->domainService->listDomainsWithout('foo.com')->willReturn([ - new Domain('bar.com'), - new Domain('baz.com'), + $listDomains = $this->domainService->listDomainsWithout()->willReturn([ + new DomainItem('foo.com', true), + new DomainItem('bar.com', false), + new DomainItem('baz.com', false), ]); $this->commandTester->execute([]); diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 94b5858a..a843a0a2 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -88,7 +88,7 @@ return [ ], Service\ShortUrl\ShortUrlResolver::class => ['em'], Service\ShortUrl\ShortCodeHelper::class => ['em'], - Domain\DomainService::class => ['em'], + 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/Domain/DomainService.php b/module/Core/src/Domain/DomainService.php index d7575361..12211208 100644 --- a/module/Core/src/Domain/DomainService.php +++ b/module/Core/src/Domain/DomainService.php @@ -5,25 +5,35 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Domain; use Doctrine\ORM\EntityManagerInterface; +use Shlinkio\Shlink\Core\Domain\Model\DomainItem; use Shlinkio\Shlink\Core\Domain\Repository\DomainRepositoryInterface; use Shlinkio\Shlink\Core\Entity\Domain; +use function Functional\map; + class DomainService implements DomainServiceInterface { private EntityManagerInterface $em; + private string $defaultDomain; - public function __construct(EntityManagerInterface $em) + public function __construct(EntityManagerInterface $em, string $defaultDomain) { $this->em = $em; + $this->defaultDomain = $defaultDomain; } /** - * @return Domain[] + * @return DomainItem[] */ - public function listDomainsWithout(?string $excludeDomain = null): array + public function listDomainsWithout(): array { /** @var DomainRepositoryInterface $repo */ $repo = $this->em->getRepository(Domain::class); - return $repo->findDomainsWithout($excludeDomain); + $domains = $repo->findDomainsWithout($this->defaultDomain); + + return [ + new DomainItem($this->defaultDomain, true), + ...map($domains, fn (Domain $domain) => new DomainItem($domain->getAuthority(), false)), + ]; } } diff --git a/module/Core/src/Domain/DomainServiceInterface.php b/module/Core/src/Domain/DomainServiceInterface.php index 3e56c69c..2cff914f 100644 --- a/module/Core/src/Domain/DomainServiceInterface.php +++ b/module/Core/src/Domain/DomainServiceInterface.php @@ -4,12 +4,12 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Domain; -use Shlinkio\Shlink\Core\Entity\Domain; +use Shlinkio\Shlink\Core\Domain\Model\DomainItem; interface DomainServiceInterface { /** - * @return Domain[] + * @return DomainItem[] */ - public function listDomainsWithout(?string $excludeDomain = null): array; + public function listDomainsWithout(): array; } diff --git a/module/Core/src/Domain/Model/DomainItem.php b/module/Core/src/Domain/Model/DomainItem.php new file mode 100644 index 00000000..4006b186 --- /dev/null +++ b/module/Core/src/Domain/Model/DomainItem.php @@ -0,0 +1,37 @@ +domain = $domain; + $this->isDefault = $isDefault; + } + + public function jsonSerialize(): array + { + return [ + 'domain' => $this->domain, + 'isDefault' => $this->isDefault, + ]; + } + + public function toString(): string + { + return $this->domain; + } + + public function isDefault(): bool + { + return $this->isDefault; + } +} diff --git a/module/Core/test/Domain/DomainServiceTest.php b/module/Core/test/Domain/DomainServiceTest.php index 906088ea..4192745f 100644 --- a/module/Core/test/Domain/DomainServiceTest.php +++ b/module/Core/test/Domain/DomainServiceTest.php @@ -9,6 +9,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; 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; @@ -22,20 +23,20 @@ class DomainServiceTest extends TestCase public function setUp(): void { $this->em = $this->prophesize(EntityManagerInterface::class); - $this->domainService = new DomainService($this->em->reveal()); + $this->domainService = new DomainService($this->em->reveal(), 'default.com'); } /** * @test * @dataProvider provideExcludedDomains */ - public function listDomainsWithoutDelegatesIntoRepository(?string $excludedDomain, array $expectedResult): void + public function listDomainsWithoutDelegatesIntoRepository(array $domains, array $expectedResult): void { $repo = $this->prophesize(DomainRepositoryInterface::class); $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); - $findDomains = $repo->findDomainsWithout($excludedDomain)->willReturn($expectedResult); + $findDomains = $repo->findDomainsWithout('default.com')->willReturn($domains); - $result = $this->domainService->listDomainsWithout($excludedDomain); + $result = $this->domainService->listDomainsWithout(); self::assertEquals($expectedResult, $result); $getRepo->shouldHaveBeenCalledOnce(); @@ -44,9 +45,13 @@ class DomainServiceTest extends TestCase public function provideExcludedDomains(): iterable { - yield 'no excluded domain' => [null, []]; - yield 'foo.com excluded domain' => ['foo.com', []]; - yield 'bar.com excluded domain' => ['bar.com', [new Domain('bar.com')]]; - yield 'baz.com excluded domain' => ['baz.com', [new Domain('foo.com'), new Domain('bar.com')]]; + $default = new DomainItem('default.com', true); + + yield 'empty list' => [[], [$default]]; + yield 'one item' => [[new Domain('bar.com')], [$default, new DomainItem('bar.com', false)]]; + yield 'multiple items' => [ + [new Domain('foo.com'), new Domain('bar.com')], + [$default, new DomainItem('foo.com', false), new DomainItem('bar.com', false)], + ]; } } diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 8c1cdb8e..6f94556d 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -74,7 +74,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, 'config.url_shortener.domain.hostname'], + Action\Domain\ListDomainsAction::class => [DomainService::class], Middleware\CrossDomainMiddleware::class => ['config.cors'], Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class => ['config.url_shortener.domain.hostname'], diff --git a/module/Rest/src/Action/Domain/ListDomainsAction.php b/module/Rest/src/Action/Domain/ListDomainsAction.php index 7362123a..b3bb10bd 100644 --- a/module/Rest/src/Action/Domain/ListDomainsAction.php +++ b/module/Rest/src/Action/Domain/ListDomainsAction.php @@ -8,44 +8,28 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; -use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; -use function Functional\map; - class ListDomainsAction extends AbstractRestAction { protected const ROUTE_PATH = '/domains'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; private DomainServiceInterface $domainService; - private string $defaultDomain; - public function __construct(DomainServiceInterface $domainService, string $defaultDomain) + public function __construct(DomainServiceInterface $domainService) { $this->domainService = $domainService; - $this->defaultDomain = $defaultDomain; } public function handle(ServerRequestInterface $request): ResponseInterface { - $regularDomains = $this->domainService->listDomainsWithout($this->defaultDomain); + $domainItems = $this->domainService->listDomainsWithout(); return new JsonResponse([ 'domains' => [ - 'data' => [ - $this->mapDomain($this->defaultDomain, true), - ...map($regularDomains, fn (Domain $domain) => $this->mapDomain($domain->getAuthority())), - ], + 'data' => $domainItems, ], ]); } - - private function mapDomain(string $domain, bool $isDefault = false): array - { - return [ - 'domain' => $domain, - 'isDefault' => $isDefault, - ]; - } } diff --git a/module/Rest/test/Action/Domain/ListDomainsActionTest.php b/module/Rest/test/Action/Domain/ListDomainsActionTest.php index 6750d105..940bb2fb 100644 --- a/module/Rest/test/Action/Domain/ListDomainsActionTest.php +++ b/module/Rest/test/Action/Domain/ListDomainsActionTest.php @@ -10,7 +10,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; -use Shlinkio\Shlink\Core\Entity\Domain; +use Shlinkio\Shlink\Core\Domain\Model\DomainItem; use Shlinkio\Shlink\Rest\Action\Domain\ListDomainsAction; class ListDomainsActionTest extends TestCase @@ -29,10 +29,11 @@ class ListDomainsActionTest extends TestCase /** @test */ public function domainsAreProperlyListed(): void { - $listDomains = $this->domainService->listDomainsWithout('foo.com')->willReturn([ - new Domain('bar.com'), - new Domain('baz.com'), - ]); + $domains = [ + new DomainItem('bar.com', true), + new DomainItem('baz.com', false), + ]; + $listDomains = $this->domainService->listDomainsWithout()->willReturn($domains); /** @var JsonResponse $resp */ $resp = $this->action->handle(ServerRequestFactory::fromGlobals()); @@ -40,20 +41,7 @@ class ListDomainsActionTest extends TestCase self::assertEquals([ 'domains' => [ - 'data' => [ - [ - 'domain' => 'foo.com', - 'isDefault' => true, - ], - [ - 'domain' => 'bar.com', - 'isDefault' => false, - ], - [ - 'domain' => 'baz.com', - 'isDefault' => false, - ], - ], + 'data' => $domains, ], ], $payload); $listDomains->shouldHaveBeenCalledOnce();