Refactored method in DomainRepo, as one fo their arguments was no longer used

This commit is contained in:
Alejandro Celaya 2021-12-09 12:43:49 +01:00
parent ee43e68a57
commit 9752abff19
7 changed files with 16 additions and 67 deletions

View file

@ -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.<br/>It also includes the domain redirects, plus the default redirects that will be used for any non-explicitly-configured one.",
"security": [
{
"ApiKey": []

View file

@ -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);
}
/**

View file

@ -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.

View file

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

View file

@ -1,22 +0,0 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Domain\Spec;
use Happyr\DoctrineSpecification\Filter\Filter;
use Happyr\DoctrineSpecification\Spec;
use Happyr\DoctrineSpecification\Specification\BaseSpecification;
class IsNotAuthority extends BaseSpecification
{
public function __construct(private string $authority, ?string $context = null)
{
parent::__construct($context);
}
protected function getSpec(): Filter
{
return Spec::not(Spec::eq('authority', $this->authority));
}
}

View file

@ -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));

View file

@ -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);