Changed logic to list domains to centralize conditions in service

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,

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'], [
+        ShlinkTable::fromOutput($output)->render(
-            [$this->defaultDomain, 'Yes'],
+            ['Domain', 'Is default'],
-            ...map($regularDomains, fn (Domain $domain) => [$domain->getAuthority(), 'No']),
+            map($regularDomains, fn (DomainItem $domain) => [$domain->toString(), $domain->isDefault() ? 'Yes' : 'No']),
-        ]);
+        );
 
         return ExitCodes::EXIT_SUCCESS;
     }

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([
+        $listDomains = $this->domainService->listDomainsWithout()->willReturn([
-            new Domain('bar.com'),
+            new DomainItem('foo.com', true),
-            new Domain('baz.com'),
+            new DomainItem('bar.com', false),
+            new DomainItem('baz.com', false),
         ]);
 
         $this->commandTester->execute([]);

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'],

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)),
+        ];
     }
 }

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;
 }

module/Core/src/Domain/Model/DomainItem.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Domain\Model;
+
+use JsonSerializable;
+
+final class DomainItem implements JsonSerializable
+{
+    private string $domain;
+    private bool $isDefault;
+
+    public function __construct(string $domain, bool $isDefault)
+    {
+        $this->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;
+    }
+}

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, []];
+        $default = new DomainItem('default.com', true);
-        yield 'foo.com excluded domain' => ['foo.com', []];
+
-        yield 'bar.com excluded domain' => ['bar.com', [new Domain('bar.com')]];
+        yield 'empty list' => [[], [$default]];
-        yield 'baz.com excluded domain' => ['baz.com', [new Domain('foo.com'), new Domain('bar.com')]];
+        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)],
+        ];
     }
 }

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'],

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' => [
+                'data' => $domainItems,
-                    $this->mapDomain($this->defaultDomain, true),
-                    ...map($regularDomains, fn (Domain $domain) => $this->mapDomain($domain->getAuthority())),
-                ],
             ],
         ]);
     }
-
-    private function mapDomain(string $domain, bool $isDefault = false): array
-    {
-        return [
-            'domain' => $domain,
-            'isDefault' => $isDefault,
-        ];
-    }
 }

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([
+        $domains = [
-            new Domain('bar.com'),
+            new DomainItem('bar.com', true),
-            new Domain('baz.com'),
+            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' => [
+                'data' => $domains,
-                    [
-                        'domain' => 'foo.com',
-                        'isDefault' => true,
-                    ],
-                    [
-                        'domain' => 'bar.com',
-                        'isDefault' => false,
-                    ],
-                    [
-                        'domain' => 'baz.com',
-                        'isDefault' => false,
-                    ],
-                ],
             ],
         ], $payload);
         $listDomains->shouldHaveBeenCalledOnce();