diff --git a/docs/swagger/paths/v2_domains.json b/docs/swagger/paths/v2_domains.json index 40448016..d92677c1 100644 --- a/docs/swagger/paths/v2_domains.json +++ b/docs/swagger/paths/v2_domains.json @@ -4,8 +4,8 @@ "tags": [ "Domains" ], - "summary": "List existing domains", - "description": "Returns the list of all domains ever used, with a flag that tells if they are the default domain", + "summary": "List configured domains", + "description": "Returns the list of all domains that have been either used for some short URL, or have explicitly configured redirects.
It also includes the domain redirects, plus the default redirects that will be used for any non-explicitly-configured one.", "security": [ { "ApiKey": [] diff --git a/module/Core/src/Domain/DomainService.php b/module/Core/src/Domain/DomainService.php index 743312b6..d5e5d88c 100644 --- a/module/Core/src/Domain/DomainService.php +++ b/module/Core/src/Domain/DomainService.php @@ -50,7 +50,7 @@ class DomainService implements DomainServiceInterface /** @var DomainRepositoryInterface $repo */ $repo = $this->em->getRepository(Domain::class); $groups = group( - $repo->findDomainsWithout(null, $apiKey), // FIXME Always called with null as first arg + $repo->findDomains($apiKey), fn (Domain $domain) => $domain->getAuthority() === $this->defaultDomain ? 'default' : 'domains', ); @@ -73,8 +73,7 @@ class DomainService implements DomainServiceInterface public function findByAuthority(string $authority, ?ApiKey $apiKey = null): ?Domain { - $repo = $this->em->getRepository(Domain::class); - return $repo->findOneByAuthority($authority, $apiKey); + return $this->em->getRepository(Domain::class)->findOneByAuthority($authority, $apiKey); } /** diff --git a/module/Core/src/Domain/Repository/DomainRepository.php b/module/Core/src/Domain/Repository/DomainRepository.php index 1741cea7..4de3ea36 100644 --- a/module/Core/src/Domain/Repository/DomainRepository.php +++ b/module/Core/src/Domain/Repository/DomainRepository.php @@ -8,7 +8,6 @@ use Doctrine\ORM\Query\Expr\Join; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; use Happyr\DoctrineSpecification\Spec; use Shlinkio\Shlink\Core\Domain\Spec\IsDomain; -use Shlinkio\Shlink\Core\Domain\Spec\IsNotAuthority; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Spec\BelongsToApiKey; @@ -20,7 +19,7 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe /** * @return Domain[] */ - public function findDomainsWithout(?string $excludedAuthority, ?ApiKey $apiKey = null): array + public function findDomains(?ApiKey $apiKey = null): array { $qb = $this->createQueryBuilder('d'); $qb->leftJoin(ShortUrl::class, 's', Join::WITH, 's.domain = d') @@ -31,7 +30,7 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe ->orHaving($qb->expr()->isNotNull('d.regular404Redirect')) ->orHaving($qb->expr()->isNotNull('d.invalidShortUrlRedirect')); - $specs = $this->determineExtraSpecs($excludedAuthority, $apiKey); + $specs = $this->determineExtraSpecs($apiKey); foreach ($specs as [$alias, $spec]) { $this->applySpecification($qb, $spec, $alias); } @@ -47,7 +46,7 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe ->setParameter('authority', $authority) ->setMaxResults(1); - $specs = $this->determineExtraSpecs(null, $apiKey); + $specs = $this->determineExtraSpecs($apiKey); foreach ($specs as [$alias, $spec]) { $this->applySpecification($qb, $spec, $alias); } @@ -55,12 +54,8 @@ class DomainRepository extends EntitySpecificationRepository implements DomainRe return $qb->getQuery()->getOneOrNullResult(); } - private function determineExtraSpecs(?string $excludedAuthority, ?ApiKey $apiKey): iterable + private function determineExtraSpecs(?ApiKey $apiKey): iterable { - if ($excludedAuthority !== null) { - yield ['d', new IsNotAuthority($excludedAuthority)]; - } - // FIXME The $apiKey->spec() method cannot be used here, as it returns a single spec which assumes the // ShortUrl is the root entity. Here, the Domain is the root entity. // Think on a way to centralize the conditional behavior and make $apiKey->spec() more flexible. diff --git a/module/Core/src/Domain/Repository/DomainRepositoryInterface.php b/module/Core/src/Domain/Repository/DomainRepositoryInterface.php index 123e349d..69e74e5b 100644 --- a/module/Core/src/Domain/Repository/DomainRepositoryInterface.php +++ b/module/Core/src/Domain/Repository/DomainRepositoryInterface.php @@ -14,7 +14,7 @@ interface DomainRepositoryInterface extends ObjectRepository, EntitySpecificatio /** * @return Domain[] */ - public function findDomainsWithout(?string $excludedAuthority, ?ApiKey $apiKey = null): array; + public function findDomains(?ApiKey $apiKey = null): array; public function findOneByAuthority(string $authority, ?ApiKey $apiKey = null): ?Domain; } diff --git a/module/Core/src/Domain/Spec/IsNotAuthority.php b/module/Core/src/Domain/Spec/IsNotAuthority.php deleted file mode 100644 index 0f0f0653..00000000 --- a/module/Core/src/Domain/Spec/IsNotAuthority.php +++ /dev/null @@ -1,22 +0,0 @@ -authority)); - } -} diff --git a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php index 1eaf6ea9..382e58dd 100644 --- a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php +++ b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php @@ -50,27 +50,7 @@ class DomainRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - self::assertEquals( - [$barDomain, $bazDomain, $detachedWithRedirects, $fooDomain], - $this->repo->findDomainsWithout(null), - ); - self::assertEquals( - [$barDomain, $bazDomain, $detachedWithRedirects], - $this->repo->findDomainsWithout('foo.com'), - ); - self::assertEquals( - [$bazDomain, $detachedWithRedirects, $fooDomain], - $this->repo->findDomainsWithout('bar.com'), - ); - self::assertEquals( - [$barDomain, $detachedWithRedirects, $fooDomain], - $this->repo->findDomainsWithout('baz.com'), - ); - self::assertEquals( - [$barDomain, $bazDomain, $fooDomain], - $this->repo->findDomainsWithout('detached-with-redirects.com'), - ); - + self::assertEquals([$barDomain, $bazDomain, $detachedWithRedirects, $fooDomain], $this->repo->findDomains()); self::assertEquals($barDomain, $this->repo->findOneByAuthority('bar.com')); self::assertEquals($detachedWithRedirects, $this->repo->findOneByAuthority('detached-with-redirects.com')); self::assertNull($this->repo->findOneByAuthority('does-not-exist.com')); @@ -121,14 +101,11 @@ class DomainRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - self::assertEquals([$fooDomain], $this->repo->findDomainsWithout(null, $fooDomainApiKey)); - self::assertEquals([$barDomain], $this->repo->findDomainsWithout(null, $barDomainApiKey)); - self::assertEquals( - [$detachedWithRedirects], - $this->repo->findDomainsWithout(null, $detachedWithRedirectsApiKey), - ); - self::assertEquals([$bazDomain, $fooDomain], $this->repo->findDomainsWithout(null, $authorApiKey)); - self::assertEquals([], $this->repo->findDomainsWithout(null, $authorAndDomainApiKey)); + self::assertEquals([$fooDomain], $this->repo->findDomains($fooDomainApiKey)); + self::assertEquals([$barDomain], $this->repo->findDomains($barDomainApiKey)); + self::assertEquals([$detachedWithRedirects], $this->repo->findDomains($detachedWithRedirectsApiKey)); + self::assertEquals([$bazDomain, $fooDomain], $this->repo->findDomains($authorApiKey)); + self::assertEquals([], $this->repo->findDomains($authorAndDomainApiKey)); self::assertEquals($fooDomain, $this->repo->findOneByAuthority('foo.com', $authorApiKey)); self::assertNull($this->repo->findOneByAuthority('bar.com', $authorApiKey)); diff --git a/module/Core/test/Domain/DomainServiceTest.php b/module/Core/test/Domain/DomainServiceTest.php index 71922fe3..ea3cfe02 100644 --- a/module/Core/test/Domain/DomainServiceTest.php +++ b/module/Core/test/Domain/DomainServiceTest.php @@ -41,7 +41,7 @@ class DomainServiceTest extends TestCase { $repo = $this->prophesize(DomainRepositoryInterface::class); $getRepo = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); - $findDomains = $repo->findDomainsWithout(null, $apiKey)->willReturn($domains); + $findDomains = $repo->findDomains($apiKey)->willReturn($domains); $result = $this->domainService->listDomains($apiKey);