From 103af2e2c158d210f6ab94b5d39298cca720ed39 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 4 Jan 2022 12:11:47 +0100 Subject: [PATCH 1/6] Added support for a new tagsMode param when listing short URLs --- module/Core/src/Model/ShortUrlsParams.php | 13 ++++++++ .../Adapter/ShortUrlRepositoryAdapter.php | 2 ++ .../src/Repository/ShortUrlRepository.php | 13 +++++--- .../ShortUrlRepositoryInterface.php | 3 ++ .../Validation/ShortUrlsParamsInputFilter.php | 10 ++++++ .../Repository/ShortUrlRepositoryTest.php | 31 ++++++++++++++----- .../Adapter/ShortUrlRepositoryAdapterTest.php | 6 ++-- 7 files changed, 64 insertions(+), 14 deletions(-) diff --git a/module/Core/src/Model/ShortUrlsParams.php b/module/Core/src/Model/ShortUrlsParams.php index b3761ea8..ac78b807 100644 --- a/module/Core/src/Model/ShortUrlsParams.php +++ b/module/Core/src/Model/ShortUrlsParams.php @@ -14,11 +14,15 @@ use function Shlinkio\Shlink\Core\parseDateField; final class ShortUrlsParams { public const DEFAULT_ITEMS_PER_PAGE = 10; + 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 ShortUrlsOrdering $orderBy; private ?DateRange $dateRange; @@ -63,6 +67,7 @@ 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; } public function page(): int @@ -94,4 +99,12 @@ final class ShortUrlsParams { return $this->dateRange; } + + /** + * @return self::TAGS_MODE_ANY|self::TAGS_MODE_ALL + */ + public function tagsMode(): string + { + return $this->tagsMode; + } } diff --git a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php index 93b69d33..0be5403b 100644 --- a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php +++ b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php @@ -25,6 +25,7 @@ class ShortUrlRepositoryAdapter implements AdapterInterface $offset, $this->params->searchTerm(), $this->params->tags(), + $this->params->tagsMode(), $this->params->orderBy(), $this->params->dateRange(), $this->apiKey?->spec(), @@ -36,6 +37,7 @@ class ShortUrlRepositoryAdapter implements AdapterInterface return $this->repository->countList( $this->params->searchTerm(), $this->params->tags(), + $this->params->tagsMode(), $this->params->dateRange(), $this->apiKey?->spec(), ); diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index e1b9c419..88a79e34 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Repository; use Doctrine\DBAL\LockMode; +use Doctrine\DBAL\Platforms\PostgreSQLPlatform; use Doctrine\ORM\Query\Expr\Join; use Doctrine\ORM\QueryBuilder; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; @@ -15,6 +16,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use function array_column; @@ -32,11 +34,12 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU ?int $offset = null, ?string $searchTerm = null, array $tags = [], + string $tagsMode = ShortUrlsParams::TAGS_MODE_ANY, ?ShortUrlsOrdering $orderBy = null, ?DateRange $dateRange = null, ?Specification $spec = null, ): array { - $qb = $this->createListQueryBuilder($searchTerm, $tags, $dateRange, $spec); + $qb = $this->createListQueryBuilder($searchTerm, $tags, $tagsMode, $dateRange, $spec); $qb->select('DISTINCT s') ->setMaxResults($limit) ->setFirstResult($offset); @@ -77,10 +80,11 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU public function countList( ?string $searchTerm = null, array $tags = [], + string $tagsMode = ShortUrlsParams::TAGS_MODE_ANY, ?DateRange $dateRange = null, ?Specification $spec = null, ): int { - $qb = $this->createListQueryBuilder($searchTerm, $tags, $dateRange, $spec); + $qb = $this->createListQueryBuilder($searchTerm, $tags, $tagsMode, $dateRange, $spec); $qb->select('COUNT(DISTINCT s)'); return (int) $qb->getQuery()->getSingleScalarResult(); @@ -89,6 +93,7 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU private function createListQueryBuilder( ?string $searchTerm, array $tags, + string $tagsMode, ?DateRange $dateRange, ?Specification $spec, ): QueryBuilder { @@ -139,8 +144,8 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU { // When ordering DESC, Postgres puts nulls at the beginning while the rest of supported DB engines put them at // the bottom - $dbPlatform = $this->getEntityManager()->getConnection()->getDatabasePlatform()->getName(); - $ordering = $dbPlatform === 'postgresql' ? 'ASC' : 'DESC'; + $dbPlatform = $this->getEntityManager()->getConnection()->getDatabasePlatform(); + $ordering = $dbPlatform instanceof PostgreSQLPlatform ? 'ASC' : 'DESC'; $dql = <<add($this->createNumericInput(self::ITEMS_PER_PAGE, false, Paginator::ALL_ITEMS)); $this->add($this->createTagsInput(self::TAGS, false)); + + $tagsMode = $this->createInput(self::TAGS_MODE, false); + $tagsMode->getValidatorChain()->attach(new InArray([ + 'haystack' => [ShortUrlsParams::TAGS_MODE_ALL, ShortUrlsParams::TAGS_MODE_ANY], + '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 cff341b3..b7c83a61 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -14,6 +14,7 @@ use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; @@ -127,22 +128,31 @@ class ShortUrlRepositoryTest extends DatabaseTestCase self::assertCount(1, $this->repo->findList(2, 2)); - $result = $this->repo->findList(null, null, null, [], ShortUrlsOrdering::fromRawData([ + $tagsModeAll = ShortUrlsParams::TAGS_MODE_ANY; + $result = $this->repo->findList(null, null, null, [], $tagsModeAll, ShortUrlsOrdering::fromRawData([ 'orderBy' => 'visits-DESC', ])); self::assertCount(3, $result); self::assertSame($bar, $result[0]); - $result = $this->repo->findList(null, null, null, [], null, DateRange::withEndDate(Chronos::now()->subDays(2))); + $result = $this->repo->findList(null, null, null, [], $tagsModeAll, null, DateRange::withEndDate( + Chronos::now()->subDays(2), + )); self::assertCount(1, $result); - self::assertEquals(1, $this->repo->countList(null, [], DateRange::withEndDate(Chronos::now()->subDays(2)))); + self::assertEquals(1, $this->repo->countList(null, [], $tagsModeAll, DateRange::withEndDate( + Chronos::now()->subDays(2), + ))); self::assertSame($foo2, $result[0]); self::assertCount( 2, - $this->repo->findList(null, null, null, [], null, DateRange::withStartDate(Chronos::now()->subDays(2))), + $this->repo->findList(null, null, null, [], $tagsModeAll, null, DateRange::withStartDate( + Chronos::now()->subDays(2), + )), ); - self::assertEquals(2, $this->repo->countList(null, [], DateRange::withStartDate(Chronos::now()->subDays(2)))); + self::assertEquals(2, $this->repo->countList(null, [], $tagsModeAll, DateRange::withStartDate( + Chronos::now()->subDays(2), + ))); } /** @test */ @@ -155,9 +165,14 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - $result = $this->repo->findList(null, null, null, [], ShortUrlsOrdering::fromRawData([ - 'orderBy' => 'longUrl-ASC', - ])); + $result = $this->repo->findList( + null, + null, + null, + [], + ShortUrlsParams::TAGS_MODE_ANY, + ShortUrlsOrdering::fromRawData(['orderBy' => 'longUrl-ASC']), + ); self::assertCount(count($urls), $result); self::assertEquals('a', $result[0]->getLongUrl()); diff --git a/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php b/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php index 33fdb8f6..99195818 100644 --- a/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php @@ -46,7 +46,8 @@ class ShortUrlRepositoryAdapterTest extends TestCase $orderBy = $params->orderBy(); $dateRange = $params->dateRange(); - $this->repo->findList(10, 5, $searchTerm, $tags, $orderBy, $dateRange, null)->shouldBeCalledOnce(); + $this->repo->findList(10, 5, $searchTerm, $tags, ShortUrlsParams::TAGS_MODE_ANY, $orderBy, $dateRange, null) + ->shouldBeCalledOnce(); $adapter->getSlice(5, 10); } @@ -70,7 +71,8 @@ class ShortUrlRepositoryAdapterTest extends TestCase $adapter = new ShortUrlRepositoryAdapter($this->repo->reveal(), $params, $apiKey); $dateRange = $params->dateRange(); - $this->repo->countList($searchTerm, $tags, $dateRange, $apiKey->spec())->shouldBeCalledOnce(); + $this->repo->countList($searchTerm, $tags, ShortUrlsParams::TAGS_MODE_ANY, $dateRange, $apiKey->spec()) + ->shouldBeCalledOnce(); $adapter->getNbResults(); } From 665a3dbcbf5115e9111e886d1add05b42e36de6f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 4 Jan 2022 12:22:36 +0100 Subject: [PATCH 2/6] Documented tagsMode param for short URLs list --- docs/swagger/paths/v1_short-urls.json | 12 +++++++++- .../src/Repository/ShortUrlRepository.php | 7 +++--- .../ShortUrlRepositoryInterface.php | 5 ++-- .../Repository/ShortUrlRepositoryTest.php | 23 +++++++------------ 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index 04afdd3a..6e8bb015 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -49,10 +49,20 @@ } } }, + { + "name": "tagsMode", + "in": "query", + "description": "Tells how the filtering by tags should work, returning short URLs containing \"any\" of the tags, or \"all\" the tags. It's ignored if no tags are provided, and defaults to \"any\" if not provided.", + "required": false, + "schema": { + "type": "string", + "enum": ["any", "all"] + } + }, { "name": "orderBy", "in": "query", - "description": "The field from which you want to order the result. (Since v1.3.0)", + "description": "The field from which you want to order the result.", "required": false, "schema": { "type": "string", diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 88a79e34..aaa0bcca 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -16,7 +16,6 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; -use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use function array_column; @@ -34,7 +33,7 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU ?int $offset = null, ?string $searchTerm = null, array $tags = [], - string $tagsMode = ShortUrlsParams::TAGS_MODE_ANY, + ?string $tagsMode = null, ?ShortUrlsOrdering $orderBy = null, ?DateRange $dateRange = null, ?Specification $spec = null, @@ -80,7 +79,7 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU public function countList( ?string $searchTerm = null, array $tags = [], - string $tagsMode = ShortUrlsParams::TAGS_MODE_ANY, + ?string $tagsMode = null, ?DateRange $dateRange = null, ?Specification $spec = null, ): int { @@ -93,7 +92,7 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU private function createListQueryBuilder( ?string $searchTerm, array $tags, - string $tagsMode, + ?string $tagsMode, ?DateRange $dateRange, ?Specification $spec, ): QueryBuilder { diff --git a/module/Core/src/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/Repository/ShortUrlRepositoryInterface.php index 02f5b5be..c3bd8d7f 100644 --- a/module/Core/src/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/Repository/ShortUrlRepositoryInterface.php @@ -12,7 +12,6 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; -use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificationRepositoryInterface @@ -22,7 +21,7 @@ interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificat ?int $offset = null, ?string $searchTerm = null, array $tags = [], - string $tagsMode = ShortUrlsParams::TAGS_MODE_ANY, + ?string $tagsMode = null, ?ShortUrlsOrdering $orderBy = null, ?DateRange $dateRange = null, ?Specification $spec = null, @@ -31,7 +30,7 @@ interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificat public function countList( ?string $searchTerm = null, array $tags = [], - string $tagsMode = ShortUrlsParams::TAGS_MODE_ANY, + ?string $tagsMode = null, ?DateRange $dateRange = null, ?Specification $spec = null, ): int; diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index b7c83a61..69ef7143 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -14,7 +14,6 @@ use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; -use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; @@ -128,29 +127,28 @@ class ShortUrlRepositoryTest extends DatabaseTestCase self::assertCount(1, $this->repo->findList(2, 2)); - $tagsModeAll = ShortUrlsParams::TAGS_MODE_ANY; - $result = $this->repo->findList(null, null, null, [], $tagsModeAll, ShortUrlsOrdering::fromRawData([ + $result = $this->repo->findList(null, null, null, [], null, ShortUrlsOrdering::fromRawData([ 'orderBy' => 'visits-DESC', ])); self::assertCount(3, $result); self::assertSame($bar, $result[0]); - $result = $this->repo->findList(null, null, null, [], $tagsModeAll, null, DateRange::withEndDate( + $result = $this->repo->findList(null, null, null, [], null, null, DateRange::withEndDate( Chronos::now()->subDays(2), )); self::assertCount(1, $result); - self::assertEquals(1, $this->repo->countList(null, [], $tagsModeAll, DateRange::withEndDate( + self::assertEquals(1, $this->repo->countList(null, [], null, DateRange::withEndDate( Chronos::now()->subDays(2), ))); self::assertSame($foo2, $result[0]); self::assertCount( 2, - $this->repo->findList(null, null, null, [], $tagsModeAll, null, DateRange::withStartDate( + $this->repo->findList(null, null, null, [], null, null, DateRange::withStartDate( Chronos::now()->subDays(2), )), ); - self::assertEquals(2, $this->repo->countList(null, [], $tagsModeAll, DateRange::withStartDate( + self::assertEquals(2, $this->repo->countList(null, [], null, DateRange::withStartDate( Chronos::now()->subDays(2), ))); } @@ -165,14 +163,9 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - $result = $this->repo->findList( - null, - null, - null, - [], - ShortUrlsParams::TAGS_MODE_ANY, - ShortUrlsOrdering::fromRawData(['orderBy' => 'longUrl-ASC']), - ); + $result = $this->repo->findList(null, null, null, [], null, ShortUrlsOrdering::fromRawData([ + 'orderBy' => 'longUrl-ASC', + ])); self::assertCount(count($urls), $result); self::assertEquals('a', $result[0]->getLongUrl()); From d8484e777f73008aca2949e8096f275b2a09c661 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 4 Jan 2022 14:23:21 +0100 Subject: [PATCH 3/6] Added logic to actually filter short URLs by any tag or all tags --- .../Command/ShortUrl/ListShortUrlsCommand.php | 10 +++ .../ShortUrl/ListShortUrlsCommandTest.php | 14 +++- .../src/Repository/ShortUrlRepository.php | 22 +++++-- .../Repository/ShortUrlRepositoryTest.php | 66 +++++++++++++++++++ 4 files changed, 102 insertions(+), 10 deletions(-) diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 83e7bc2e..0c0daf33 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -64,6 +64,12 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand InputOption::VALUE_REQUIRED, 'A comma-separated list of tags to filter results.', ) + ->addOption( + 'including-all-tags', + 'i', + InputOption::VALUE_NONE, + 'If tags is provided, returns only short URLs having ALL tags.', + ) ->addOption( 'order-by', 'o', @@ -115,6 +121,9 @@ 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; $tags = ! empty($tags) ? explode(',', $tags) : []; $all = $input->getOption('all'); $startDate = $this->getStartDateOption($input, $output); @@ -125,6 +134,7 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand $data = [ ShortUrlsParamsInputFilter::SEARCH_TERM => $searchTerm, ShortUrlsParamsInputFilter::TAGS => $tags, + ShortUrlsParamsInputFilter::TAGS_MODE => $tagsMode, ShortUrlsOrdering::ORDER_BY => $orderBy, ShortUrlsParamsInputFilter::START_DATE => $startDate?->toAtomString(), ShortUrlsParamsInputFilter::END_DATE => $endDate?->toAtomString(), diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index e7dae690..97ced44c 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -184,6 +184,7 @@ class ListShortUrlsCommandTest extends TestCase ?int $page, ?string $searchTerm, array $tags, + string $tagsMode, ?string $startDate = null, ?string $endDate = null, ): void { @@ -191,6 +192,7 @@ class ListShortUrlsCommandTest extends TestCase 'page' => $page, 'searchTerm' => $searchTerm, 'tags' => $tags, + 'tagsMode' => $tagsMode, 'startDate' => $startDate !== null ? Chronos::parse($startDate)->toAtomString() : null, 'endDate' => $endDate !== null ? Chronos::parse($endDate)->toAtomString() : null, ]))->willReturn(new Paginator(new ArrayAdapter([]))); @@ -203,20 +205,23 @@ class ListShortUrlsCommandTest extends TestCase public function provideArgs(): iterable { - yield [[], 1, null, []]; - yield [['--page' => $page = 3], $page, null, []]; - yield [['--search-term' => $searchTerm = 'search this'], 1, $searchTerm, []]; + 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 [ ['--page' => $page = 3, '--search-term' => $searchTerm = 'search this', '--tags' => $tags = 'foo,bar'], $page, $searchTerm, explode(',', $tags), + ShortUrlsParams::TAGS_MODE_ANY, ]; yield [ ['--start-date' => $startDate = '2019-01-01'], 1, null, [], + ShortUrlsParams::TAGS_MODE_ANY, $startDate, ]; yield [ @@ -224,6 +229,7 @@ class ListShortUrlsCommandTest extends TestCase 1, null, [], + ShortUrlsParams::TAGS_MODE_ANY, null, $endDate, ]; @@ -232,6 +238,7 @@ class ListShortUrlsCommandTest extends TestCase 1, null, [], + ShortUrlsParams::TAGS_MODE_ANY, $startDate, $endDate, ]; @@ -269,6 +276,7 @@ class ListShortUrlsCommandTest extends TestCase 'page' => 1, 'searchTerm' => null, 'tags' => [], + 'tagsMode' => ShortUrlsParams::TAGS_MODE_ANY, 'startDate' => null, 'endDate' => null, 'orderBy' => null, diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index aaa0bcca..5fe659f2 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -16,6 +16,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use function array_column; @@ -130,8 +131,10 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU // Filter by tags if provided if (! empty($tags)) { - $qb->join('s.tags', 't') - ->andWhere($qb->expr()->in('t.name', $tags)); + $tagsMode = $tagsMode ?? ShortUrlsParams::TAGS_MODE_ANY; + $tagsMode === ShortUrlsParams::TAGS_MODE_ANY + ? $qb->join('s.tags', 't')->andWhere($qb->expr()->in('t.name', $tags)) + : $this->joinAllTags($qb, $tags); } $this->applySpecification($qb, $spec, 's'); @@ -261,11 +264,7 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU return $qb->getQuery()->getOneOrNullResult(); } - foreach ($tags as $index => $tag) { - $alias = 't_' . $index; - $qb->join('s.tags', $alias, Join::WITH, $alias . '.name = :tag' . $index) - ->setParameter('tag' . $index, $tag); - } + $this->joinAllTags($qb, $tags); // If tags where provided, we need an extra join to see the amount of tags that every short URL has, so that we // can discard those that also have more tags, making sure only those fully matching are included. @@ -277,6 +276,15 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU return $qb->getQuery()->getOneOrNullResult(); } + private function joinAllTags(QueryBuilder $qb, array $tags): void + { + foreach ($tags as $index => $tag) { + $alias = 't_' . $index; + $qb->join('s.tags', $alias, Join::WITH, $alias . '.name = :tag' . $index) + ->setParameter('tag' . $index, $tag); + } + } + public function findOneByImportedUrl(ImportedShlinkUrl $url): ?ShortUrl { $qb = $this->createQueryBuilder('s'); diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index 69ef7143..8cb161d4 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -14,6 +14,7 @@ use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; @@ -174,6 +175,71 @@ class ShortUrlRepositoryTest extends DatabaseTestCase self::assertEquals('z', $result[3]->getLongUrl()); } + /** @test */ + public function findListReturnsOnlyThoseWithMatchingTags(): void + { + $shortUrl1 = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ + 'longUrl' => 'foo1', + 'tags' => ['foo', 'bar'], + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl1); + $shortUrl2 = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ + 'longUrl' => 'foo2', + 'tags' => ['foo', 'baz'], + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl2); + $shortUrl3 = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ + 'longUrl' => 'foo3', + 'tags' => ['foo'], + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl3); + $shortUrl4 = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ + 'longUrl' => 'foo4', + 'tags' => ['bar', 'baz'], + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl4); + $shortUrl5 = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ + 'longUrl' => 'foo5', + 'tags' => ['bar', 'baz'], + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl5); + + $this->getEntityManager()->flush(); + + self::assertCount(5, $this->repo->findList(null, null, null, ['foo', 'bar'])); + self::assertCount(5, $this->repo->findList(null, null, null, ['foo', 'bar'], ShortUrlsParams::TAGS_MODE_ANY)); + self::assertCount(1, $this->repo->findList(null, null, null, ['foo', 'bar'], ShortUrlsParams::TAGS_MODE_ALL)); + self::assertEquals(5, $this->repo->countList(null, ['foo', 'bar'])); + self::assertEquals(5, $this->repo->countList(null, ['foo', 'bar'], ShortUrlsParams::TAGS_MODE_ANY)); + self::assertEquals(1, $this->repo->countList(null, ['foo', 'bar'], ShortUrlsParams::TAGS_MODE_ALL)); + + self::assertCount(4, $this->repo->findList(null, null, null, ['bar', 'baz'])); + self::assertCount(4, $this->repo->findList(null, null, null, ['bar', 'baz'], ShortUrlsParams::TAGS_MODE_ANY)); + self::assertCount(2, $this->repo->findList(null, null, null, ['bar', 'baz'], ShortUrlsParams::TAGS_MODE_ALL)); + self::assertEquals(4, $this->repo->countList(null, ['bar', 'baz'])); + self::assertEquals(4, $this->repo->countList(null, ['bar', 'baz'], ShortUrlsParams::TAGS_MODE_ANY)); + self::assertEquals(2, $this->repo->countList(null, ['bar', 'baz'], ShortUrlsParams::TAGS_MODE_ALL)); + + self::assertCount(5, $this->repo->findList(null, null, null, ['foo', 'bar', 'baz'])); + self::assertCount(5, $this->repo->findList( + null, + null, + null, + ['foo', 'bar', 'baz'], + ShortUrlsParams::TAGS_MODE_ANY, + )); + self::assertCount(0, $this->repo->findList( + null, + null, + null, + ['foo', 'bar', 'baz'], + ShortUrlsParams::TAGS_MODE_ALL, + )); + self::assertEquals(5, $this->repo->countList(null, ['foo', 'bar', 'baz'])); + self::assertEquals(5, $this->repo->countList(null, ['foo', 'bar', 'baz'], ShortUrlsParams::TAGS_MODE_ANY)); + self::assertEquals(0, $this->repo->countList(null, ['foo', 'bar', 'baz'], ShortUrlsParams::TAGS_MODE_ALL)); + } + /** @test */ public function shortCodeIsInUseLooksForShortUrlInProperSetOfTables(): void { From 0e25af790d43e21048c49e24e2e224853fab5602 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 4 Jan 2022 14:28:00 +0100 Subject: [PATCH 4/6] Updated changelog --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a56edebc..b0d7c45d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added -* *Nothing* +* [#1274](https://github.com/shlinkio/shlink/issues/1274) Added support to filter short URLs lists by all provided tags. + + The `GET /short-urls` endpoint now accepts a `tagsMode=all` param which will make only short URLs matching **all** the tags in the `tags[]` query param, to be returned. + + The `short-urls:list` command now accepts a `-i`/`--including-all-tags` flag which behaves the same. ### Changed * [#1277](https://github.com/shlinkio/shlink/issues/1277) Reduced docker image size to 45% the original size. From 0447aa07fa58d327117ee761cdeda2587460ac0e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 4 Jan 2022 14:34:31 +0100 Subject: [PATCH 5/6] Added more API tests covering the new tagsMode param on short URLs list --- .../test-api/Action/ListShortUrlsTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index e3526756..e5f0ed55 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -189,6 +189,25 @@ class ListShortUrlsTest extends ApiTestCase yield [['tags' => ['bar']], [ self::SHORT_URL_META, ], 'valid_api_key']; + yield [['tags' => ['foo', 'bar']], [ + self::SHORT_URL_SHLINK_WITH_TITLE, + self::SHORT_URL_META, + self::SHORT_URL_CUSTOM_DOMAIN, + ], 'valid_api_key']; + yield [['tags' => ['foo', 'bar'], 'tagsMode' => 'any'], [ + self::SHORT_URL_SHLINK_WITH_TITLE, + self::SHORT_URL_META, + self::SHORT_URL_CUSTOM_DOMAIN, + ], 'valid_api_key']; + yield [['tags' => ['foo', 'bar'], 'tagsMode' => 'all'], [ + self::SHORT_URL_META, + ], 'valid_api_key']; + yield [['tags' => ['foo', 'bar', 'baz']], [ + self::SHORT_URL_SHLINK_WITH_TITLE, + self::SHORT_URL_META, + self::SHORT_URL_CUSTOM_DOMAIN, + ], 'valid_api_key']; + yield [['tags' => ['foo', 'bar', 'baz'], 'tagsMode' => 'all'], [], 'valid_api_key']; yield [['tags' => ['foo'], 'endDate' => Chronos::parse('2018-12-01')->toAtomString()], [ self::SHORT_URL_SHLINK_WITH_TITLE, ], 'valid_api_key']; From 9dec05f62df73dacbc648f1a5fe59da34a5f035c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 4 Jan 2022 14:42:31 +0100 Subject: [PATCH 6/6] Added API test covering invalid tagsMode --- .../Rest/test-api/Action/ListShortUrlsTest.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index e5f0ed55..6a140279 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -241,4 +241,21 @@ class ListShortUrlsTest extends ApiTestCase 'totalItems' => $itemsCount, ]; } + + /** @test */ + public function errorIsReturnedWhenProvidingInvalidValues(): void + { + $query = ['tagsMode' => 'invalid']; + $resp = $this->callApiWithKey(self::METHOD_GET, '/short-urls', [RequestOptions::QUERY => $query]); + $respPayload = $this->getJsonResponsePayload($resp); + + self::assertEquals(400, $resp->getStatusCode()); + self::assertEquals([ + 'invalidElements' => ['tagsMode'], + 'title' => 'Invalid data', + 'type' => 'INVALID_ARGUMENT', + 'status' => 400, + 'detail' => 'Provided data is not valid', + ], $respPayload); + } }