From 6ada704bc34bc55b3955ebedfc82c370e12f87cf Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 23 Apr 2022 18:56:27 +0200 Subject: [PATCH] Moved TagsMode to its own enum --- .../Command/ShortUrl/ListShortUrlsCommand.php | 5 ++-- .../ShortUrl/ListShortUrlsCommandTest.php | 19 ++++++------ module/Core/src/Model/ShortUrlsParams.php | 24 ++++++++------- .../src/Repository/ShortUrlRepository.php | 6 ++-- module/Core/src/ShortUrl/Model/TagsMode.php | 13 ++++++++ .../Persistence/ShortUrlsCountFiltering.php | 5 ++-- .../Persistence/ShortUrlsListFiltering.php | 3 +- .../Validation/ShortUrlsParamsInputFilter.php | 3 +- .../Repository/ShortUrlRepositoryTest.php | 30 ++++++++----------- .../Adapter/ShortUrlRepositoryAdapterTest.php | 5 ++-- 10 files changed, 64 insertions(+), 49 deletions(-) create mode 100644 module/Core/src/ShortUrl/Model/TagsMode.php diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index ebc9e783..fc0f19a0 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -13,6 +13,7 @@ use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; +use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; use Shlinkio\Shlink\Core\Validation\ShortUrlsParamsInputFilter; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -120,9 +121,7 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand $page = (int) $input->getOption('page'); $searchTerm = $input->getOption('search-term'); $tags = $input->getOption('tags'); - $tagsMode = $input->getOption('including-all-tags') === true - ? ShortUrlsParams::TAGS_MODE_ALL - : ShortUrlsParams::TAGS_MODE_ANY; + $tagsMode = $input->getOption('including-all-tags') === true ? TagsMode::ALL->value : TagsMode::ANY->value; $tags = ! empty($tags) ? explode(',', $tags) : []; $all = $input->getOption('all'); $startDate = $this->getStartDateOption($input, $output); diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index 38d3bcd3..f9d701cb 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -16,6 +16,7 @@ use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; +use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -205,23 +206,23 @@ class ListShortUrlsCommandTest extends TestCase public function provideArgs(): iterable { - yield [[], 1, null, [], ShortUrlsParams::TAGS_MODE_ANY]; - yield [['--page' => $page = 3], $page, null, [], ShortUrlsParams::TAGS_MODE_ANY]; - yield [['--including-all-tags' => true], 1, null, [], ShortUrlsParams::TAGS_MODE_ALL]; - yield [['--search-term' => $searchTerm = 'search this'], 1, $searchTerm, [], ShortUrlsParams::TAGS_MODE_ANY]; + yield [[], 1, null, [], TagsMode::ANY->value]; + yield [['--page' => $page = 3], $page, null, [], TagsMode::ANY->value]; + yield [['--including-all-tags' => true], 1, null, [], TagsMode::ALL->value]; + yield [['--search-term' => $searchTerm = 'search this'], 1, $searchTerm, [], TagsMode::ANY->value]; yield [ ['--page' => $page = 3, '--search-term' => $searchTerm = 'search this', '--tags' => $tags = 'foo,bar'], $page, $searchTerm, explode(',', $tags), - ShortUrlsParams::TAGS_MODE_ANY, + TagsMode::ANY->value, ]; yield [ ['--start-date' => $startDate = '2019-01-01'], 1, null, [], - ShortUrlsParams::TAGS_MODE_ANY, + TagsMode::ANY->value, $startDate, ]; yield [ @@ -229,7 +230,7 @@ class ListShortUrlsCommandTest extends TestCase 1, null, [], - ShortUrlsParams::TAGS_MODE_ANY, + TagsMode::ANY->value, null, $endDate, ]; @@ -238,7 +239,7 @@ class ListShortUrlsCommandTest extends TestCase 1, null, [], - ShortUrlsParams::TAGS_MODE_ANY, + TagsMode::ANY->value, $startDate, $endDate, ]; @@ -276,7 +277,7 @@ class ListShortUrlsCommandTest extends TestCase 'page' => 1, 'searchTerm' => null, 'tags' => [], - 'tagsMode' => ShortUrlsParams::TAGS_MODE_ANY, + 'tagsMode' => TagsMode::ANY->value, 'startDate' => null, 'endDate' => null, 'orderBy' => null, diff --git a/module/Core/src/Model/ShortUrlsParams.php b/module/Core/src/Model/ShortUrlsParams.php index ca4c0954..bd6dc556 100644 --- a/module/Core/src/Model/ShortUrlsParams.php +++ b/module/Core/src/Model/ShortUrlsParams.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Model; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Exception\ValidationException; +use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; use Shlinkio\Shlink\Core\Validation\ShortUrlsParamsInputFilter; use function Shlinkio\Shlink\Common\buildDateRange; @@ -16,16 +17,11 @@ final class ShortUrlsParams public const ORDERABLE_FIELDS = ['longUrl', 'shortCode', 'dateCreated', 'title', 'visits']; public const DEFAULT_ITEMS_PER_PAGE = 10; - // TODO Convert to enum - public const TAGS_MODE_ANY = 'any'; - public const TAGS_MODE_ALL = 'all'; - private int $page; private int $itemsPerPage; private ?string $searchTerm; private array $tags; - /** @var self::TAGS_MODE_ANY|self::TAGS_MODE_ALL */ - private string $tagsMode = self::TAGS_MODE_ANY; + private TagsMode $tagsMode = TagsMode::ANY; private Ordering $orderBy; private ?DateRange $dateRange; @@ -70,7 +66,16 @@ final class ShortUrlsParams $this->itemsPerPage = (int) ( $inputFilter->getValue(ShortUrlsParamsInputFilter::ITEMS_PER_PAGE) ?? self::DEFAULT_ITEMS_PER_PAGE ); - $this->tagsMode = $inputFilter->getValue(ShortUrlsParamsInputFilter::TAGS_MODE) ?? self::TAGS_MODE_ANY; + $this->tagsMode = $this->resolveTagsMode($inputFilter->getValue(ShortUrlsParamsInputFilter::TAGS_MODE)); + } + + private function resolveTagsMode(?string $rawTagsMode): TagsMode + { + if ($rawTagsMode === null) { + return TagsMode::ANY; + } + + return TagsMode::tryFrom($rawTagsMode) ?? TagsMode::ANY; } public function page(): int @@ -103,10 +108,7 @@ final class ShortUrlsParams return $this->dateRange; } - /** - * @return self::TAGS_MODE_ANY|self::TAGS_MODE_ALL - */ - public function tagsMode(): string + public function tagsMode(): TagsMode { return $this->tagsMode; } diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 357b7357..7a3def16 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -15,7 +15,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Model\Ordering; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; -use Shlinkio\Shlink\Core\Model\ShortUrlsParams; +use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; @@ -116,8 +116,8 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU // Filter by tags if provided if (! empty($tags)) { - $tagsMode = $filtering->tagsMode() ?? ShortUrlsParams::TAGS_MODE_ANY; - $tagsMode === ShortUrlsParams::TAGS_MODE_ANY + $tagsMode = $filtering->tagsMode() ?? TagsMode::ANY; + $tagsMode === TagsMode::ANY ? $qb->join('s.tags', 't')->andWhere($qb->expr()->in('t.name', $tags)) : $this->joinAllTags($qb, $tags); } diff --git a/module/Core/src/ShortUrl/Model/TagsMode.php b/module/Core/src/ShortUrl/Model/TagsMode.php new file mode 100644 index 00000000..5ae2d9bb --- /dev/null +++ b/module/Core/src/ShortUrl/Model/TagsMode.php @@ -0,0 +1,13 @@ +tags; } - public function tagsMode(): ?string + public function tagsMode(): ?TagsMode { return $this->tagsMode; } diff --git a/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php b/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php index 089915e3..04645126 100644 --- a/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php +++ b/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Persistence; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Model\Ordering; use Shlinkio\Shlink\Core\Model\ShortUrlsParams; +use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; use Shlinkio\Shlink\Rest\Entity\ApiKey; class ShortUrlsListFiltering extends ShortUrlsCountFiltering @@ -17,7 +18,7 @@ class ShortUrlsListFiltering extends ShortUrlsCountFiltering private Ordering $orderBy, ?string $searchTerm = null, array $tags = [], - ?string $tagsMode = null, + ?TagsMode $tagsMode = null, ?DateRange $dateRange = null, ?ApiKey $apiKey = null, ) { diff --git a/module/Core/src/Validation/ShortUrlsParamsInputFilter.php b/module/Core/src/Validation/ShortUrlsParamsInputFilter.php index 6c0443aa..50953310 100644 --- a/module/Core/src/Validation/ShortUrlsParamsInputFilter.php +++ b/module/Core/src/Validation/ShortUrlsParamsInputFilter.php @@ -9,6 +9,7 @@ use Laminas\Validator\InArray; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Validation; use Shlinkio\Shlink\Core\Model\ShortUrlsParams; +use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; class ShortUrlsParamsInputFilter extends InputFilter { @@ -43,7 +44,7 @@ class ShortUrlsParamsInputFilter extends InputFilter $tagsMode = $this->createInput(self::TAGS_MODE, false); $tagsMode->getValidatorChain()->attach(new InArray([ - 'haystack' => [ShortUrlsParams::TAGS_MODE_ALL, ShortUrlsParams::TAGS_MODE_ANY], + 'haystack' => [TagsMode::ALL->value, TagsMode::ANY->value], 'strict' => InArray::COMPARE_STRICT, ])); $this->add($tagsMode); diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index 4ad89629..05c65ee3 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -14,9 +14,9 @@ use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\Ordering; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; -use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; +use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; @@ -227,7 +227,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase Ordering::emptyInstance(), null, ['foo', 'bar'], - ShortUrlsParams::TAGS_MODE_ANY, + TagsMode::ANY, ))); self::assertCount(1, $this->repo->findList(new ShortUrlsListFiltering( null, @@ -235,15 +235,11 @@ class ShortUrlRepositoryTest extends DatabaseTestCase Ordering::emptyInstance(), null, ['foo', 'bar'], - ShortUrlsParams::TAGS_MODE_ALL, + TagsMode::ALL, ))); self::assertEquals(5, $this->repo->countList(new ShortUrlsCountFiltering(null, ['foo', 'bar']))); - self::assertEquals(5, $this->repo->countList( - new ShortUrlsCountFiltering(null, ['foo', 'bar'], ShortUrlsParams::TAGS_MODE_ANY), - )); - self::assertEquals(1, $this->repo->countList( - new ShortUrlsCountFiltering(null, ['foo', 'bar'], ShortUrlsParams::TAGS_MODE_ALL), - )); + self::assertEquals(5, $this->repo->countList(new ShortUrlsCountFiltering(null, ['foo', 'bar'], TagsMode::ANY))); + self::assertEquals(1, $this->repo->countList(new ShortUrlsCountFiltering(null, ['foo', 'bar'], TagsMode::ALL))); self::assertCount(4, $this->repo->findList( new ShortUrlsListFiltering(null, null, Ordering::emptyInstance(), null, ['bar', 'baz']), @@ -254,7 +250,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase Ordering::emptyInstance(), null, ['bar', 'baz'], - ShortUrlsParams::TAGS_MODE_ANY, + TagsMode::ANY, ))); self::assertCount(2, $this->repo->findList(new ShortUrlsListFiltering( null, @@ -262,14 +258,14 @@ class ShortUrlRepositoryTest extends DatabaseTestCase Ordering::emptyInstance(), null, ['bar', 'baz'], - ShortUrlsParams::TAGS_MODE_ALL, + TagsMode::ALL, ))); self::assertEquals(4, $this->repo->countList(new ShortUrlsCountFiltering(null, ['bar', 'baz']))); self::assertEquals(4, $this->repo->countList( - new ShortUrlsCountFiltering(null, ['bar', 'baz'], ShortUrlsParams::TAGS_MODE_ANY), + new ShortUrlsCountFiltering(null, ['bar', 'baz'], TagsMode::ANY), )); self::assertEquals(2, $this->repo->countList( - new ShortUrlsCountFiltering(null, ['bar', 'baz'], ShortUrlsParams::TAGS_MODE_ALL), + new ShortUrlsCountFiltering(null, ['bar', 'baz'], TagsMode::ALL), )); self::assertCount(5, $this->repo->findList( @@ -281,7 +277,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase Ordering::emptyInstance(), null, ['foo', 'bar', 'baz'], - ShortUrlsParams::TAGS_MODE_ANY, + TagsMode::ANY, ))); self::assertCount(0, $this->repo->findList(new ShortUrlsListFiltering( null, @@ -289,14 +285,14 @@ class ShortUrlRepositoryTest extends DatabaseTestCase Ordering::emptyInstance(), null, ['foo', 'bar', 'baz'], - ShortUrlsParams::TAGS_MODE_ALL, + TagsMode::ALL, ))); self::assertEquals(5, $this->repo->countList(new ShortUrlsCountFiltering(null, ['foo', 'bar', 'baz']))); self::assertEquals(5, $this->repo->countList( - new ShortUrlsCountFiltering(null, ['foo', 'bar', 'baz'], ShortUrlsParams::TAGS_MODE_ANY), + new ShortUrlsCountFiltering(null, ['foo', 'bar', 'baz'], TagsMode::ANY), )); self::assertEquals(0, $this->repo->countList( - new ShortUrlsCountFiltering(null, ['foo', 'bar', 'baz'], ShortUrlsParams::TAGS_MODE_ALL), + new ShortUrlsCountFiltering(null, ['foo', 'bar', 'baz'], TagsMode::ALL), )); } diff --git a/module/Core/test/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php b/module/Core/test/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php index 336526b1..2675b04a 100644 --- a/module/Core/test/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php +++ b/module/Core/test/ShortUrl/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php @@ -10,6 +10,7 @@ use Prophecy\PhpUnit\ProphecyTrait; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; +use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; use Shlinkio\Shlink\Core\ShortUrl\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering; @@ -49,7 +50,7 @@ class ShortUrlRepositoryAdapterTest extends TestCase $dateRange = $params->dateRange(); $this->repo->findList( - new ShortUrlsListFiltering(10, 5, $orderBy, $searchTerm, $tags, ShortUrlsParams::TAGS_MODE_ANY, $dateRange), + new ShortUrlsListFiltering(10, 5, $orderBy, $searchTerm, $tags, TagsMode::ANY, $dateRange), )->shouldBeCalledOnce(); $adapter->getSlice(5, 10); } @@ -75,7 +76,7 @@ class ShortUrlRepositoryAdapterTest extends TestCase $dateRange = $params->dateRange(); $this->repo->countList( - new ShortUrlsCountFiltering($searchTerm, $tags, ShortUrlsParams::TAGS_MODE_ANY, $dateRange, $apiKey), + new ShortUrlsCountFiltering($searchTerm, $tags, TagsMode::ANY, $dateRange, $apiKey), )->shouldBeCalledOnce(); $adapter->getNbResults(); }