Merge pull request #1298 from acelaya-forks/feature/filter-all-tags

Feature/filter all tags
This commit is contained in:
Alejandro Celaya 2022-01-04 14:59:22 +01:00 committed by GitHub
commit d3f4263639
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 207 additions and 24 deletions

View file

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

View file

@ -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",

View file

@ -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(),

View file

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

View file

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

View file

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

View file

@ -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 = null,
?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 = null,
?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 {
@ -126,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');
@ -139,8 +146,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 = <<<DQL
SELECT s
@ -257,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.
@ -273,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');

View file

@ -21,6 +21,7 @@ interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificat
?int $offset = null,
?string $searchTerm = null,
array $tags = [],
?string $tagsMode = null,
?ShortUrlsOrdering $orderBy = null,
?DateRange $dateRange = null,
?Specification $spec = null,
@ -29,6 +30,7 @@ interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificat
public function countList(
?string $searchTerm = null,
array $tags = [],
?string $tagsMode = null,
?DateRange $dateRange = null,
?Specification $spec = null,
): int;

View file

@ -5,8 +5,10 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Validation;
use Laminas\InputFilter\InputFilter;
use Laminas\Validator\InArray;
use Shlinkio\Shlink\Common\Paginator\Paginator;
use Shlinkio\Shlink\Common\Validation;
use Shlinkio\Shlink\Core\Model\ShortUrlsParams;
class ShortUrlsParamsInputFilter extends InputFilter
{
@ -18,6 +20,7 @@ class ShortUrlsParamsInputFilter extends InputFilter
public const START_DATE = 'startDate';
public const END_DATE = 'endDate';
public const ITEMS_PER_PAGE = 'itemsPerPage';
public const TAGS_MODE = 'tagsMode';
public function __construct(array $data)
{
@ -36,5 +39,12 @@ class ShortUrlsParamsInputFilter extends InputFilter
$this->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);
}
}

View file

@ -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,30 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
self::assertCount(1, $this->repo->findList(2, 2));
$result = $this->repo->findList(null, null, null, [], 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, [], null, DateRange::withEndDate(Chronos::now()->subDays(2)));
$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, [], DateRange::withEndDate(Chronos::now()->subDays(2))));
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, [], null, DateRange::withStartDate(Chronos::now()->subDays(2))),
$this->repo->findList(null, null, null, [], null, 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, [], null, DateRange::withStartDate(
Chronos::now()->subDays(2),
)));
}
/** @test */
@ -155,7 +164,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
$this->getEntityManager()->flush();
$result = $this->repo->findList(null, null, null, [], ShortUrlsOrdering::fromRawData([
$result = $this->repo->findList(null, null, null, [], null, ShortUrlsOrdering::fromRawData([
'orderBy' => 'longUrl-ASC',
]));
@ -166,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
{

View file

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

View file

@ -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'];
@ -222,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);
}
}