From 525a306ec630fa9f28e882fa3583a3089cc9eee7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 28 Oct 2024 08:36:06 +0100 Subject: [PATCH] Create constant representing default domain identifier --- .../src/Command/ShortUrl/ListShortUrlsCommand.php | 3 ++- module/CLI/test-cli/Command/CreateShortUrlTest.php | 3 ++- module/CLI/test-cli/Command/ImportShortUrlsTest.php | 5 +++-- module/Core/src/Domain/Entity/Domain.php | 2 ++ .../ShortUrl/Persistence/ShortUrlsCountFiltering.php | 1 + .../ShortUrl/Repository/ShortUrlListRepository.php | 6 ++++-- module/Core/src/Visit/Repository/VisitRepository.php | 3 ++- module/Core/src/Visit/VisitsStatsHelper.php | 2 +- .../test-db/Visit/Repository/VisitRepositoryTest.php | 12 ++++++------ module/Core/test/Visit/VisitsStatsHelperTest.php | 6 +++--- module/Rest/src/Action/Visit/DomainVisitsAction.php | 3 ++- module/Rest/test-api/Action/DomainVisitsTest.php | 11 ++++++----- .../test/Action/Visit/DomainVisitsActionTest.php | 5 +++-- 13 files changed, 37 insertions(+), 25 deletions(-) diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 6d38ff0f..9fd39d44 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -10,6 +10,7 @@ use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtils; +use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary; @@ -231,7 +232,7 @@ class ListShortUrlsCommand extends Command } if ($input->getOption('show-domain')) { $columnsMap['Domain'] = static fn (array $_, ShortUrl $shortUrl): string => - $shortUrl->getDomain()?->authority ?? 'DEFAULT'; + $shortUrl->getDomain()?->authority ?? Domain::DEFAULT_AUTHORITY; } if ($input->getOption('show-api-key')) { $columnsMap['API Key'] = static fn (array $_, ShortUrl $shortUrl): string => diff --git a/module/CLI/test-cli/Command/CreateShortUrlTest.php b/module/CLI/test-cli/Command/CreateShortUrlTest.php index c2e96611..b07975be 100644 --- a/module/CLI/test-cli/Command/CreateShortUrlTest.php +++ b/module/CLI/test-cli/Command/CreateShortUrlTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\Attributes\Test; use Shlinkio\Shlink\CLI\Command\ShortUrl\CreateShortUrlCommand; use Shlinkio\Shlink\CLI\Command\ShortUrl\ListShortUrlsCommand; use Shlinkio\Shlink\CLI\Util\ExitCode; +use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\TestUtils\CliTest\CliTestCase; class CreateShortUrlTest extends CliTestCase @@ -26,6 +27,6 @@ class CreateShortUrlTest extends CliTestCase self::assertStringContainsString('Generated short URL: http://' . $defaultDomain . '/' . $slug, $output); [$listOutput] = $this->exec([ListShortUrlsCommand::NAME, '--show-domain', '--search-term', $slug]); - self::assertStringContainsString('DEFAULT', $listOutput); + self::assertStringContainsString(Domain::DEFAULT_AUTHORITY, $listOutput); } } diff --git a/module/CLI/test-cli/Command/ImportShortUrlsTest.php b/module/CLI/test-cli/Command/ImportShortUrlsTest.php index 1ed15d7c..40e00cc0 100644 --- a/module/CLI/test-cli/Command/ImportShortUrlsTest.php +++ b/module/CLI/test-cli/Command/ImportShortUrlsTest.php @@ -6,6 +6,7 @@ namespace ShlinkioCliTest\Shlink\CLI\Command; use PHPUnit\Framework\Attributes\Test; use Shlinkio\Shlink\CLI\Command\ShortUrl\ListShortUrlsCommand; +use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Importer\Command\ImportCommand; use Shlinkio\Shlink\TestUtils\CliTest\CliTestCase; @@ -66,10 +67,10 @@ class ImportShortUrlsTest extends CliTestCase [$listOutput1] = $this->exec( [ListShortUrlsCommand::NAME, '--show-domain', '--search-term', 'testing-default-domain-import-1'], ); - self::assertStringContainsString('DEFAULT', $listOutput1); + self::assertStringContainsString(Domain::DEFAULT_AUTHORITY, $listOutput1); [$listOutput1] = $this->exec( [ListShortUrlsCommand::NAME, '--show-domain', '--search-term', 'testing-default-domain-import-2'], ); - self::assertStringContainsString('DEFAULT', $listOutput1); + self::assertStringContainsString(Domain::DEFAULT_AUTHORITY, $listOutput1); } } diff --git a/module/Core/src/Domain/Entity/Domain.php b/module/Core/src/Domain/Entity/Domain.php index b3d2b734..ba3446a7 100644 --- a/module/Core/src/Domain/Entity/Domain.php +++ b/module/Core/src/Domain/Entity/Domain.php @@ -11,6 +11,8 @@ use Shlinkio\Shlink\Core\Config\NotFoundRedirects; class Domain extends AbstractEntity implements JsonSerializable, NotFoundRedirectConfigInterface { + public const DEFAULT_AUTHORITY = 'DEFAULT'; + private function __construct( public readonly string $authority, private ?string $baseUrlRedirect = null, diff --git a/module/Core/src/ShortUrl/Persistence/ShortUrlsCountFiltering.php b/module/Core/src/ShortUrl/Persistence/ShortUrlsCountFiltering.php index 906adc63..15b9d47f 100644 --- a/module/Core/src/ShortUrl/Persistence/ShortUrlsCountFiltering.php +++ b/module/Core/src/ShortUrl/Persistence/ShortUrlsCountFiltering.php @@ -25,6 +25,7 @@ class ShortUrlsCountFiltering public readonly bool $excludePastValidUntil = false, public readonly ?ApiKey $apiKey = null, ?string $defaultDomain = null, + public readonly ?string $domain = null, ) { $this->searchIncludesDefaultDomain = !empty($searchTerm) && !empty($defaultDomain) && str_contains( strtolower($defaultDomain), diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php index 8c49697a..70e9dbff 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php @@ -104,14 +104,13 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh $searchTerm = $filtering->searchTerm; $tags = $filtering->tags; - // Apply search term to every searchable field if not empty if (! empty($searchTerm)) { // Left join with tags only if no tags were provided. In case of tags, an inner join will be done later if (empty($tags)) { $qb->leftJoin('s.tags', 't'); } - // Apply general search conditions + // Apply search term to every "searchable" field $conditions = [ $qb->expr()->like('s.longUrl', ':searchPattern'), $qb->expr()->like('s.shortCode', ':searchPattern'), @@ -142,6 +141,9 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh : $this->joinAllTags($qb, $tags); } + if ($filtering->domain !== null) { + } + if ($filtering->excludeMaxVisitsReached) { $qb->andWhere($qb->expr()->orX( $qb->expr()->isNull('s.maxVisits'), diff --git a/module/Core/src/Visit/Repository/VisitRepository.php b/module/Core/src/Visit/Repository/VisitRepository.php index 0708a4e1..1df109b3 100644 --- a/module/Core/src/Visit/Repository/VisitRepository.php +++ b/module/Core/src/Visit/Repository/VisitRepository.php @@ -8,6 +8,7 @@ use Doctrine\ORM\Query\ResultSetMappingBuilder; use Doctrine\ORM\QueryBuilder; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface; @@ -124,7 +125,7 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo $qb->from(Visit::class, 'v') ->join('v.shortUrl', 's'); - if ($domain === 'DEFAULT') { + if ($domain === Domain::DEFAULT_AUTHORITY) { $qb->where($qb->expr()->isNull('s.domain')); } else { $qb->join('s.domain', 'd') diff --git a/module/Core/src/Visit/VisitsStatsHelper.php b/module/Core/src/Visit/VisitsStatsHelper.php index 7f3e2282..0952670b 100644 --- a/module/Core/src/Visit/VisitsStatsHelper.php +++ b/module/Core/src/Visit/VisitsStatsHelper.php @@ -109,7 +109,7 @@ readonly class VisitsStatsHelper implements VisitsStatsHelperInterface { /** @var DomainRepository $domainRepo */ $domainRepo = $this->em->getRepository(Domain::class); - if ($domain !== 'DEFAULT' && ! $domainRepo->domainExists($domain, $apiKey)) { + if ($domain !== Domain::DEFAULT_AUTHORITY && ! $domainRepo->domainExists($domain, $apiKey)) { throw DomainNotFoundException::fromAuthority($domain); } diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index 9dc18390..8d7579b7 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -227,7 +227,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); self::assertCount(0, $this->repo->findVisitsByDomain('invalid', new VisitsListFiltering())); - self::assertCount(6, $this->repo->findVisitsByDomain('DEFAULT', new VisitsListFiltering())); + self::assertCount(6, $this->repo->findVisitsByDomain(Domain::DEFAULT_AUTHORITY, new VisitsListFiltering())); self::assertCount(3, $this->repo->findVisitsByDomain('s.test', new VisitsListFiltering())); self::assertCount(1, $this->repo->findVisitsByDomain('s.test', new VisitsListFiltering(null, true))); self::assertCount(2, $this->repo->findVisitsByDomain('s.test', new VisitsListFiltering( @@ -236,10 +236,10 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertCount(1, $this->repo->findVisitsByDomain('s.test', new VisitsListFiltering( DateRange::since(Chronos::parse('2016-01-03')), ))); - self::assertCount(2, $this->repo->findVisitsByDomain('DEFAULT', new VisitsListFiltering( + self::assertCount(2, $this->repo->findVisitsByDomain(Domain::DEFAULT_AUTHORITY, new VisitsListFiltering( DateRange::between(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ))); - self::assertCount(4, $this->repo->findVisitsByDomain('DEFAULT', new VisitsListFiltering( + self::assertCount(4, $this->repo->findVisitsByDomain(Domain::DEFAULT_AUTHORITY, new VisitsListFiltering( DateRange::since(Chronos::parse('2016-01-03')), ))); } @@ -251,7 +251,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); self::assertEquals(0, $this->repo->countVisitsByDomain('invalid', new VisitsListFiltering())); - self::assertEquals(6, $this->repo->countVisitsByDomain('DEFAULT', new VisitsListFiltering())); + self::assertEquals(6, $this->repo->countVisitsByDomain(Domain::DEFAULT_AUTHORITY, new VisitsListFiltering())); self::assertEquals(3, $this->repo->countVisitsByDomain('s.test', new VisitsListFiltering())); self::assertEquals(1, $this->repo->countVisitsByDomain('s.test', new VisitsListFiltering(null, true))); self::assertEquals(2, $this->repo->countVisitsByDomain('s.test', new VisitsListFiltering( @@ -260,10 +260,10 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(1, $this->repo->countVisitsByDomain('s.test', new VisitsListFiltering( DateRange::since(Chronos::parse('2016-01-03')), ))); - self::assertEquals(2, $this->repo->countVisitsByDomain('DEFAULT', new VisitsListFiltering( + self::assertEquals(2, $this->repo->countVisitsByDomain(Domain::DEFAULT_AUTHORITY, new VisitsListFiltering( DateRange::between(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ))); - self::assertEquals(4, $this->repo->countVisitsByDomain('DEFAULT', new VisitsListFiltering( + self::assertEquals(4, $this->repo->countVisitsByDomain(Domain::DEFAULT_AUTHORITY, new VisitsListFiltering( DateRange::since(Chronos::parse('2016-01-03')), ))); } diff --git a/module/Core/test/Visit/VisitsStatsHelperTest.php b/module/Core/test/Visit/VisitsStatsHelperTest.php index c1aa0747..61fb1293 100644 --- a/module/Core/test/Visit/VisitsStatsHelperTest.php +++ b/module/Core/test/Visit/VisitsStatsHelperTest.php @@ -240,11 +240,11 @@ class VisitsStatsHelperTest extends TestCase ); $repo2 = $this->createMock(VisitRepository::class); $repo2->method('findVisitsByDomain')->with( - 'DEFAULT', + Domain::DEFAULT_AUTHORITY, $this->isInstanceOf(VisitsListFiltering::class), )->willReturn($list); $repo2->method('countVisitsByDomain')->with( - 'DEFAULT', + Domain::DEFAULT_AUTHORITY, $this->isInstanceOf(VisitsCountFiltering::class), )->willReturn(1); @@ -253,7 +253,7 @@ class VisitsStatsHelperTest extends TestCase [Visit::class, $repo2], ]); - $paginator = $this->helper->visitsForDomain('DEFAULT', new VisitsParams(), $apiKey); + $paginator = $this->helper->visitsForDomain(Domain::DEFAULT_AUTHORITY, new VisitsParams(), $apiKey); self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentPageResults())); } diff --git a/module/Rest/src/Action/Visit/DomainVisitsAction.php b/module/Rest/src/Action/Visit/DomainVisitsAction.php index 4d534202..fc9cf20c 100644 --- a/module/Rest/src/Action/Visit/DomainVisitsAction.php +++ b/module/Rest/src/Action/Visit/DomainVisitsAction.php @@ -9,6 +9,7 @@ use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtils; use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; +use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -39,7 +40,7 @@ class DomainVisitsAction extends AbstractRestAction { $domainParam = $request->getAttribute('domain', ''); if ($domainParam === $this->urlShortenerOptions->defaultDomain) { - return 'DEFAULT'; + return Domain::DEFAULT_AUTHORITY; } return $domainParam; diff --git a/module/Rest/test-api/Action/DomainVisitsTest.php b/module/Rest/test-api/Action/DomainVisitsTest.php index 3a06257b..628b7211 100644 --- a/module/Rest/test-api/Action/DomainVisitsTest.php +++ b/module/Rest/test-api/Action/DomainVisitsTest.php @@ -7,6 +7,7 @@ namespace ShlinkioApiTest\Shlink\Rest\Action; use GuzzleHttp\RequestOptions; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; +use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; use function sprintf; @@ -34,11 +35,11 @@ class DomainVisitsTest extends ApiTestCase public static function provideDomains(): iterable { yield 'example.com with admin API key' => ['valid_api_key', 'example.com', false, 0]; - yield 'DEFAULT with admin API key' => ['valid_api_key', 'DEFAULT', false, 7]; - yield 'DEFAULT with admin API key and no bots' => ['valid_api_key', 'DEFAULT', true, 6]; - yield 'DEFAULT with domain API key' => ['domain_api_key', 'DEFAULT', false, 0]; - yield 'DEFAULT with author API key' => ['author_api_key', 'DEFAULT', false, 5]; - yield 'DEFAULT with author API key and no bots' => ['author_api_key', 'DEFAULT', true, 4]; + yield 'DEFAULT with admin API key' => ['valid_api_key', Domain::DEFAULT_AUTHORITY, false, 7]; + yield 'DEFAULT with admin API key and no bots' => ['valid_api_key', Domain::DEFAULT_AUTHORITY, true, 6]; + yield 'DEFAULT with domain API key' => ['domain_api_key', Domain::DEFAULT_AUTHORITY, false, 0]; + yield 'DEFAULT with author API key' => ['author_api_key', Domain::DEFAULT_AUTHORITY, false, 5]; + yield 'DEFAULT with author API key and no bots' => ['author_api_key', Domain::DEFAULT_AUTHORITY, true, 4]; } #[Test, DataProvider('provideApiKeysAndTags')] diff --git a/module/Rest/test/Action/Visit/DomainVisitsActionTest.php b/module/Rest/test/Action/Visit/DomainVisitsActionTest.php index d60dae2e..d4a16573 100644 --- a/module/Rest/test/Action/Visit/DomainVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/DomainVisitsActionTest.php @@ -12,6 +12,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; +use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\Visit\DomainVisitsAction; @@ -49,7 +50,7 @@ class DomainVisitsActionTest extends TestCase public static function provideDomainAuthorities(): iterable { yield 'no default domain' => ['foo.com', 'foo.com']; - yield 'default domain' => ['the_default.com', 'DEFAULT']; - yield 'DEFAULT keyword' => ['DEFAULT', 'DEFAULT']; + yield 'default domain' => ['the_default.com', Domain::DEFAULT_AUTHORITY]; + yield 'DEFAULT keyword' => ['DEFAULT', Domain::DEFAULT_AUTHORITY]; } }