mirrors/shlink
1
0
Fork 0
mirror of https://github.com/shlinkio/shlink.git synced 2025-03-14 04:00:57 +03:00
Code Issues Projects Releases Packages Wiki Activity

Standardized ordering field handling and added validation for short URLs list

Browse source
This commit is contained in:
Alejandro Celaya 2022-01-09 11:23:27 +01:00
parent d0c9f5a776
commit 2abcaf02e2
12 changed files with 68 additions and 92 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
composer.json
View file

@ -48,7 +48,7 @@
"predis/predis": "^1.1",
"pugx/shortid-php": "^1.0",
"ramsey/uuid": "^4.2",
"shlinkio/shlink-common": "^4.3",
"shlinkio/shlink-common": "dev-main#cbcff58 as 4.4",
"shlinkio/shlink-config": "^1.5",
"shlinkio/shlink-event-dispatcher": "^2.3",
"shlinkio/shlink-importer": "^2.5",

3
module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php
View file

@ -11,7 +11,6 @@ use Shlinkio\Shlink\Common\Paginator\Paginator;
use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait;
use Shlinkio\Shlink\Common\Rest\DataTransformerInterface;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering;
use Shlinkio\Shlink\Core\Model\ShortUrlsParams;
use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface;
use Shlinkio\Shlink\Core\Validation\ShortUrlsParamsInputFilter;
@ -135,7 +134,7 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand
ShortUrlsParamsInputFilter::SEARCH_TERM => $searchTerm,
ShortUrlsParamsInputFilter::TAGS => $tags,
ShortUrlsParamsInputFilter::TAGS_MODE => $tagsMode,
ShortUrlsOrdering::ORDER_BY => $orderBy,
ShortUrlsParamsInputFilter::ORDER_BY => $orderBy,
ShortUrlsParamsInputFilter::START_DATE => $startDate?->toAtomString(),
ShortUrlsParamsInputFilter::END_DATE => $endDate?->toAtomString(),
];

8
module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php
View file

@ -263,10 +263,10 @@ class ListShortUrlsCommandTest extends TestCase
public function provideOrderBy(): iterable
{
yield [[], null];
yield [['--order-by' => 'foo'], 'foo'];
yield [['--order-by' => 'foo,ASC'], 'foo-ASC'];
yield [['--order-by' => 'bar,DESC'], 'bar-DESC'];
yield [['--order-by' => 'baz-DESC'], 'baz-DESC'];
yield [['--order-by' => 'visits'], 'visits'];
yield [['--order-by' => 'longUrl,ASC'], 'longUrl-ASC'];
yield [['--order-by' => 'shortCode,DESC'], 'shortCode-DESC'];
yield [['--order-by' => 'title-DESC'], 'title-DESC'];
}
/** @test */

35
module/Core/src/Model/Ordering.php Normal file
View file

@ -0,0 +1,35 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Model;
final class Ordering
{
private const DEFAULT_DIR = 'ASC';
private function __construct(private ?string $field, private string $dir)
{
}
public static function fromTuple(array $props): self
{
[$field, $dir] = $props;
return new self($field, $dir ?? self::DEFAULT_DIR);
}
public function orderField(): ?string
{
return $this->field;
}
public function orderDirection(): string
{
return $this->dir;
}
public function hasOrderField(): bool
{
return $this->field !== null;
}
}

60
module/Core/src/Model/ShortUrlsOrdering.php
View file

@ -1,60 +0,0 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Model;
use Shlinkio\Shlink\Core\Exception\ValidationException;
use function array_pad;
use function explode;
final class ShortUrlsOrdering
{
public const ORDER_BY = 'orderBy';
private const DEFAULT_ORDER_DIRECTION = 'ASC';
private ?string $orderField = null;
private string $orderDirection = self::DEFAULT_ORDER_DIRECTION;
/**
* @throws ValidationException
*/
public static function fromRawData(array $query): self
{
$instance = new self();
$instance->validateAndInit($query);
return $instance;
}
/**
* @throws ValidationException
*/
private function validateAndInit(array $data): void
{
$orderBy = $data[self::ORDER_BY] ?? null;
if ($orderBy === null) {
return;
}
[$field, $dir] = array_pad(explode('-', $orderBy), 2, null);
$this->orderField = $field;
$this->orderDirection = $dir ?? self::DEFAULT_ORDER_DIRECTION;
}
public function orderField(): ?string
{
return $this->orderField;
}
public function orderDirection(): string
{
return $this->orderDirection;
}
public function hasOrderField(): bool
{
return $this->orderField !== null;
}
}

7
module/Core/src/Model/ShortUrlsParams.php
View file

@ -13,6 +13,7 @@ use function Shlinkio\Shlink\Core\parseDateField;
final class ShortUrlsParams
{
public const ORDERABLE_FIELDS = ['longUrl', 'shortCode', 'dateCreated', 'title', 'visits'];
public const DEFAULT_ITEMS_PER_PAGE = 10;
public const TAGS_MODE_ANY = 'any';
public const TAGS_MODE_ALL = 'all';
@ -23,7 +24,7 @@ final class ShortUrlsParams
private array $tags;
/** @var self::TAGS_MODE_ANY|self::TAGS_MODE_ALL */
private string $tagsMode = self::TAGS_MODE_ANY;
private ShortUrlsOrdering $orderBy;
private Ordering $orderBy;
private ?DateRange $dateRange;
private function __construct()
@ -63,7 +64,7 @@ final class ShortUrlsParams
parseDateField($inputFilter->getValue(ShortUrlsParamsInputFilter::START_DATE)),
parseDateField($inputFilter->getValue(ShortUrlsParamsInputFilter::END_DATE)),
);
$this->orderBy = ShortUrlsOrdering::fromRawData($query);
$this->orderBy = Ordering::fromTuple($inputFilter->getValue(ShortUrlsParamsInputFilter::ORDER_BY));
$this->itemsPerPage = (int) (
$inputFilter->getValue(ShortUrlsParamsInputFilter::ITEMS_PER_PAGE) ?? self::DEFAULT_ITEMS_PER_PAGE
);
@ -90,7 +91,7 @@ final class ShortUrlsParams
return $this->tags;
}
public function orderBy(): ShortUrlsOrdering
public function orderBy(): Ordering
{
return $this->orderBy;
}

9
module/Core/src/Repository/ShortUrlRepository.php
View file

@ -13,9 +13,9 @@ use Happyr\DoctrineSpecification\Specification\Specification;
use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType;
use Shlinkio\Shlink\Common\Util\DateRange;
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\ShortUrlsOrdering;
use Shlinkio\Shlink\Core\Model\ShortUrlsParams;
use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl;
@ -35,7 +35,7 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU
?string $searchTerm = null,
array $tags = [],
?string $tagsMode = null,
?ShortUrlsOrdering $orderBy = null,
?Ordering $orderBy = null,
?DateRange $dateRange = null,
?Specification $spec = null,
): array {
@ -53,13 +53,14 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU
return $qb->orderBy('s.dateCreated', 'DESC')->getQuery()->getResult();
}
private function processOrderByForList(QueryBuilder $qb, ShortUrlsOrdering $orderBy): array
private function processOrderByForList(QueryBuilder $qb, Ordering $orderBy): array
{
$fieldName = $orderBy->orderField();
$order = $orderBy->orderDirection();
if ($fieldName === 'visits') {
// FIXME This query is inefficient. Debug it.
// FIXME This query is inefficient.
// Diagnostic: It might need to use a sub-query, as done with the tags list query.
$qb->addSelect('COUNT(DISTINCT v) AS totalVisits')
->leftJoin('s.visits', 'v')
->groupBy('s')

4
module/Core/src/Repository/ShortUrlRepositoryInterface.php
View file

@ -9,9 +9,9 @@ use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepositoryInterfa
use Happyr\DoctrineSpecification\Specification\Specification;
use Shlinkio\Shlink\Common\Util\DateRange;
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\ShortUrlsOrdering;
use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl;
interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificationRepositoryInterface
@ -22,7 +22,7 @@ interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificat
?string $searchTerm = null,
array $tags = [],
?string $tagsMode = null,
?ShortUrlsOrdering $orderBy = null,
?Ordering $orderBy = null,
?DateRange $dateRange = null,
?Specification $spec = null,
): array;

3
module/Core/src/Validation/ShortUrlsParamsInputFilter.php
View file

@ -21,6 +21,7 @@ class ShortUrlsParamsInputFilter extends InputFilter
public const END_DATE = 'endDate';
public const ITEMS_PER_PAGE = 'itemsPerPage';
public const TAGS_MODE = 'tagsMode';
public const ORDER_BY = 'orderBy';
public function __construct(array $data)
{
@ -46,5 +47,7 @@ class ShortUrlsParamsInputFilter extends InputFilter
'strict' => InArray::COMPARE_STRICT,
]));
$this->add($tagsMode);
$this->add($this->createOrderByInput(self::ORDER_BY, ShortUrlsParams::ORDERABLE_FIELDS));
}
}

10
module/Core/test-db/Repository/ShortUrlRepositoryTest.php
View file

@ -11,9 +11,9 @@ use Shlinkio\Shlink\Common\Util\DateRange;
use Shlinkio\Shlink\Core\Entity\Domain;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
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\ShortUrlsOrdering;
use Shlinkio\Shlink\Core\Model\ShortUrlsParams;
use Shlinkio\Shlink\Core\Model\Visitor;
use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
@ -128,9 +128,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
self::assertCount(1, $this->repo->findList(2, 2));
$result = $this->repo->findList(null, null, null, [], null, ShortUrlsOrdering::fromRawData([
'orderBy' => 'visits-DESC',
]));
$result = $this->repo->findList(null, null, null, [], null, Ordering::fromTuple(['visits', 'DESC']));
self::assertCount(3, $result);
self::assertSame($bar, $result[0]);
@ -164,9 +162,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
$this->getEntityManager()->flush();
$result = $this->repo->findList(null, null, null, [], null, ShortUrlsOrdering::fromRawData([
'orderBy' => 'longUrl-ASC',
]));
$result = $this->repo->findList(null, null, null, [], null, Ordering::fromTuple(['longUrl', 'ASC']));
self::assertCount(count($urls), $result);
self::assertEquals('a', $result[0]->getLongUrl());

10
module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php
View file

@ -82,11 +82,11 @@ class ShortUrlRepositoryAdapterTest extends TestCase
yield ['search'];
yield ['search', []];
yield ['search', ['foo', 'bar']];
yield ['search', ['foo', 'bar'], null, null, 'order'];
yield ['search', ['foo', 'bar'], Chronos::now()->toAtomString(), null, 'order'];
yield ['search', ['foo', 'bar'], null, Chronos::now()->toAtomString(), 'order'];
yield ['search', ['foo', 'bar'], Chronos::now()->toAtomString(), Chronos::now()->toAtomString(), 'order'];
yield [null, ['foo', 'bar'], Chronos::now()->toAtomString(), null, 'order'];
yield ['search', ['foo', 'bar'], null, null, 'longUrl'];
yield ['search', ['foo', 'bar'], Chronos::now()->toAtomString(), null, 'longUrl'];
yield ['search', ['foo', 'bar'], null, Chronos::now()->toAtomString(), 'longUrl'];
yield ['search', ['foo', 'bar'], Chronos::now()->toAtomString(), Chronos::now()->toAtomString(), 'longUrl'];
yield [null, ['foo', 'bar'], Chronos::now()->toAtomString(), null, 'longUrl'];
yield [null, ['foo', 'bar'], Chronos::now()->toAtomString()];
yield [null, ['foo', 'bar'], Chronos::now()->toAtomString(), Chronos::now()->toAtomString()];
}

9
module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php
View file

@ -6,7 +6,7 @@ namespace ShlinkioTest\Shlink\Rest\Action\ShortUrl;
use Cake\Chronos\Chronos;
use Laminas\Diactoros\Response\JsonResponse;
use Laminas\Diactoros\ServerRequest;
use Laminas\Diactoros\ServerRequestFactory;
use Pagerfanta\Adapter\ArrayAdapter;
use PHPUnit\Framework\TestCase;
use Prophecy\PhpUnit\ProphecyTrait;
@ -52,7 +52,8 @@ class ListShortUrlsActionTest extends TestCase
?string $endDate = null,
): void {
$apiKey = ApiKey::create();
$request = (new ServerRequest())->withQueryParams($query)->withAttribute(ApiKey::class, $apiKey);
$request = ServerRequestFactory::fromGlobals()->withQueryParams($query)
->withAttribute(ApiKey::class, $apiKey);
$listShortUrls = $this->service->listShortUrls(ShortUrlsParams::fromRawData([
'page' => $expectedPage,
'searchTerm' => $expectedSearchTerm,
@ -81,10 +82,10 @@ class ListShortUrlsActionTest extends TestCase
yield [['page' => '8'], 8, null, [], null];
yield [['searchTerm' => $searchTerm = 'foo'], 1, $searchTerm, [], null];
yield [['tags' => $tags = ['foo','bar']], 1, null, $tags, null];
yield [['orderBy' => $orderBy = 'something'], 1, null, [], $orderBy];
yield [['orderBy' => $orderBy = 'longUrl'], 1, null, [], $orderBy];
yield [[
'page' => '2',
'orderBy' => $orderBy = 'something',
'orderBy' => $orderBy = 'visits',
'tags' => $tags = ['one', 'two'],
], 2, null, $tags, $orderBy];
yield [