diff --git a/CHANGELOG.md b/CHANGELOG.md index 74ee51ab..93cb803a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,29 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). +## [Unreleased] + +#### Added + +* *Nothing* + +#### Changed + +* [#577](https://github.com/shlinkio/shlink/issues/577) Wrapped params used to customize short URL lists into a DTO with implicit validation. + +#### Deprecated + +* *Nothing* + +#### Removed + +* *Nothing* + +#### Fixed + +* *Nothing* + + ## 2.0.3 - 2020-01-27 #### Added diff --git a/composer.json b/composer.json index 19b3f557..0f259095 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,7 @@ "phly/phly-event-dispatcher": "^1.0", "predis/predis": "^1.1", "pugx/shortid-php": "^0.5", - "shlinkio/shlink-common": "^2.5", + "shlinkio/shlink-common": "dev-master#e6658205370260df72f295df15364ac569ff702c as 2.6.0", "shlinkio/shlink-event-dispatcher": "^1.3", "shlinkio/shlink-installer": "^4.0.1", "shlinkio/shlink-ip-geolocation": "^1.3.1", diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index c4251ed9..98f006d4 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -4,16 +4,17 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; -use Cake\Chronos\Chronos; use Laminas\Paginator\Paginator; use Shlinkio\Shlink\CLI\Command\Util\AbstractWithDateRangeCommand; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Util\PaginatorUtilsTrait; -use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; +use Shlinkio\Shlink\Core\Validation\ShortUrlsParamsInputFilter; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; @@ -108,7 +109,14 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand $orderBy = $this->processOrderBy($input); do { - $result = $this->renderPage($output, $page, $searchTerm, $tags, $showTags, $startDate, $endDate, $orderBy); + $result = $this->renderPage($output, $showTags, ShortUrlsParams::fromRawData([ + ShortUrlsParamsInputFilter::PAGE => $page, + ShortUrlsParamsInputFilter::SEARCH_TERM => $searchTerm, + ShortUrlsParamsInputFilter::TAGS => $tags, + ShortUrlsOrdering::ORDER_BY => $orderBy, + ShortUrlsParamsInputFilter::START_DATE => $startDate !== null ? $startDate->toAtomString() : null, + ShortUrlsParamsInputFilter::END_DATE => $endDate !== null ? $endDate->toAtomString() : null, + ])); $page++; $continue = $this->isLastPage($result) @@ -122,26 +130,9 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand return ExitCodes::EXIT_SUCCESS; } - /** - * @param string|array|null $orderBy - */ - private function renderPage( - OutputInterface $output, - int $page, - ?string $searchTerm, - array $tags, - bool $showTags, - ?Chronos $startDate, - ?Chronos $endDate, - $orderBy - ): Paginator { - $result = $this->shortUrlService->listShortUrls( - $page, - $searchTerm, - $tags, - $orderBy, - new DateRange($startDate, $endDate), - ); + private function renderPage(OutputInterface $output, bool $showTags, ShortUrlsParams $params): Paginator + { + $result = $this->shortUrlService->listShortUrls($params); $headers = ['Short code', 'Short URL', 'Long URL', 'Date created', 'Visits count']; if ($showTags) { diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index d4182e27..2e315581 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -11,8 +11,8 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\ShortUrl\ListShortUrlsCommand; -use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -64,7 +64,7 @@ class ListShortUrlsCommandTest extends TestCase $data[] = new ShortUrl('url_' . $i); } - $this->shortUrlService->listShortUrls(1, null, [], null, new DateRange()) + $this->shortUrlService->listShortUrls(ShortUrlsParams::emptyInstance()) ->willReturn(new Paginator(new ArrayAdapter($data))) ->shouldBeCalledOnce(); @@ -85,7 +85,7 @@ class ListShortUrlsCommandTest extends TestCase public function passingPageWillMakeListStartOnThatPage(): void { $page = 5; - $this->shortUrlService->listShortUrls($page, null, [], null, new DateRange()) + $this->shortUrlService->listShortUrls(ShortUrlsParams::fromRawData(['page' => $page])) ->willReturn(new Paginator(new ArrayAdapter())) ->shouldBeCalledOnce(); @@ -96,7 +96,7 @@ class ListShortUrlsCommandTest extends TestCase /** @test */ public function ifTagsFlagIsProvidedTagsColumnIsIncluded(): void { - $this->shortUrlService->listShortUrls(1, null, [], null, new DateRange()) + $this->shortUrlService->listShortUrls(ShortUrlsParams::emptyInstance()) ->willReturn(new Paginator(new ArrayAdapter())) ->shouldBeCalledOnce(); @@ -115,10 +115,16 @@ class ListShortUrlsCommandTest extends TestCase ?int $page, ?string $searchTerm, array $tags, - ?DateRange $dateRange + ?string $startDate = null, + ?string $endDate = null ): void { - $listShortUrls = $this->shortUrlService->listShortUrls($page, $searchTerm, $tags, null, $dateRange) - ->willReturn(new Paginator(new ArrayAdapter())); + $listShortUrls = $this->shortUrlService->listShortUrls(ShortUrlsParams::fromRawData([ + 'page' => $page, + 'searchTerm' => $searchTerm, + 'tags' => $tags, + 'startDate' => $startDate !== null ? Chronos::parse($startDate)->toAtomString() : null, + 'endDate' => $endDate !== null ? Chronos::parse($endDate)->toAtomString() : null, + ]))->willReturn(new Paginator(new ArrayAdapter())); $this->commandTester->setInputs(['n']); $this->commandTester->execute($commandArgs); @@ -128,36 +134,37 @@ class ListShortUrlsCommandTest extends TestCase public function provideArgs(): iterable { - yield [[], 1, null, [], new DateRange()]; - yield [['--page' => $page = 3], $page, null, [], new DateRange()]; - yield [['--searchTerm' => $searchTerm = 'search this'], 1, $searchTerm, [], new DateRange()]; + yield [[], 1, null, []]; + yield [['--page' => $page = 3], $page, null, []]; + yield [['--searchTerm' => $searchTerm = 'search this'], 1, $searchTerm, []]; yield [ ['--page' => $page = 3, '--searchTerm' => $searchTerm = 'search this', '--tags' => $tags = 'foo,bar'], $page, $searchTerm, explode(',', $tags), - new DateRange(), ]; yield [ ['--startDate' => $startDate = '2019-01-01'], 1, null, [], - new DateRange(Chronos::parse($startDate)), + $startDate, ]; yield [ ['--endDate' => $endDate = '2020-05-23'], 1, null, [], - new DateRange(null, Chronos::parse($endDate)), + null, + $endDate, ]; yield [ ['--startDate' => $startDate = '2019-01-01', '--endDate' => $endDate = '2020-05-23'], 1, null, [], - new DateRange(Chronos::parse($startDate), Chronos::parse($endDate)), + $startDate, + $endDate, ]; } @@ -168,8 +175,9 @@ class ListShortUrlsCommandTest extends TestCase */ public function orderByIsProperlyComputed(array $commandArgs, $expectedOrderBy): void { - $listShortUrls = $this->shortUrlService->listShortUrls(1, null, [], $expectedOrderBy, new DateRange()) - ->willReturn(new Paginator(new ArrayAdapter())); + $listShortUrls = $this->shortUrlService->listShortUrls(ShortUrlsParams::fromRawData([ + 'orderBy' => $expectedOrderBy, + ]))->willReturn(new Paginator(new ArrayAdapter())); $this->commandTester->setInputs(['n']); $this->commandTester->execute($commandArgs); diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 92d40fe1..1384549f 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -4,6 +4,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core; +use Cake\Chronos\Chronos; +use DateTimeInterface; use PUGX\Shortid\Factory as ShortIdFactory; function generateRandomShortCode(int $length = 5): string @@ -16,3 +18,24 @@ function generateRandomShortCode(int $length = 5): string $alphabet = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'; return $shortIdFactory->generate($length, $alphabet)->serialize(); } + +function parseDateFromQuery(array $query, string $dateName): ?Chronos +{ + return ! isset($query[$dateName]) || empty($query[$dateName]) ? null : Chronos::parse($query[$dateName]); +} + +/** + * @param string|DateTimeInterface|Chronos|null $date + */ +function parseDateField($date): ?Chronos +{ + if ($date === null || $date instanceof Chronos) { + return $date; + } + + if ($date instanceof DateTimeInterface) { + return Chronos::instance($date); + } + + return Chronos::parse($date); +} diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index f0c487b7..27c8e624 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -5,11 +5,11 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Model; use Cake\Chronos\Chronos; -use DateTimeInterface; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter; use function array_key_exists; +use function Shlinkio\Shlink\Core\parseDateField; final class ShortUrlMeta { @@ -34,30 +34,28 @@ final class ShortUrlMeta } /** - * @param array $data * @throws ValidationException */ public static function fromRawData(array $data): self { $instance = new self(); - $instance->validate($data); + $instance->validateAndInit($data); return $instance; } /** - * @param array $data * @throws ValidationException */ - private function validate(array $data): void + private function validateAndInit(array $data): void { $inputFilter = new ShortUrlMetaInputFilter($data); if (! $inputFilter->isValid()) { throw ValidationException::fromInputFilter($inputFilter); } - $this->validSince = $this->parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE)); + $this->validSince = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE)); $this->validSincePropWasProvided = array_key_exists(ShortUrlMetaInputFilter::VALID_SINCE, $data); - $this->validUntil = $this->parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL)); + $this->validUntil = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL)); $this->validUntilPropWasProvided = array_key_exists(ShortUrlMetaInputFilter::VALID_UNTIL, $data); $this->customSlug = $inputFilter->getValue(ShortUrlMetaInputFilter::CUSTOM_SLUG); $maxVisits = $inputFilter->getValue(ShortUrlMetaInputFilter::MAX_VISITS); @@ -67,22 +65,6 @@ final class ShortUrlMeta $this->domain = $inputFilter->getValue(ShortUrlMetaInputFilter::DOMAIN); } - /** - * @param string|DateTimeInterface|Chronos|null $date - */ - private function parseDateField($date): ?Chronos - { - if ($date === null || $date instanceof Chronos) { - return $date; - } - - if ($date instanceof DateTimeInterface) { - return Chronos::instance($date); - } - - return Chronos::parse($date); - } - public function getValidSince(): ?Chronos { return $this->validSince; diff --git a/module/Core/src/Model/ShortUrlsOrdering.php b/module/Core/src/Model/ShortUrlsOrdering.php new file mode 100644 index 00000000..00c30a54 --- /dev/null +++ b/module/Core/src/Model/ShortUrlsOrdering.php @@ -0,0 +1,68 @@ +validateAndInit($query); + + return $instance; + } + + /** + * @throws ValidationException + */ + private function validateAndInit(array $data): void + { + /** @var string|array|null $orderBy */ + $orderBy = $data[self::ORDER_BY] ?? null; + if ($orderBy === null) { + return; + } + + $isArray = is_array($orderBy); + if (! $isArray && $orderBy !== null && ! is_string($orderBy)) { + throw ValidationException::fromArray([ + 'orderBy' => '"Order by" must be an array, string or null', + ]); + } + + $this->orderField = $isArray ? key($orderBy) : $orderBy; + $this->orderDirection = $isArray ? $orderBy[$this->orderField] : 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; + } +} diff --git a/module/Core/src/Model/ShortUrlsParams.php b/module/Core/src/Model/ShortUrlsParams.php new file mode 100644 index 00000000..cbd74bec --- /dev/null +++ b/module/Core/src/Model/ShortUrlsParams.php @@ -0,0 +1,85 @@ +validateAndInit($query); + + return $instance; + } + + /** + * @throws ValidationException + */ + private function validateAndInit(array $query): void + { + $inputFilter = new ShortUrlsParamsInputFilter($query); + if (! $inputFilter->isValid()) { + throw ValidationException::fromInputFilter($inputFilter); + } + + $this->page = (int) ($inputFilter->getValue(ShortUrlsParamsInputFilter::PAGE) ?? 1); + $this->searchTerm = $inputFilter->getValue(ShortUrlsParamsInputFilter::SEARCH_TERM); + $this->tags = (array) $inputFilter->getValue(ShortUrlsParamsInputFilter::TAGS); + $this->dateRange = new DateRange( + parseDateField($inputFilter->getValue(ShortUrlsParamsInputFilter::START_DATE)), + parseDateField($inputFilter->getValue(ShortUrlsParamsInputFilter::END_DATE)), + ); + $this->orderBy = ShortUrlsOrdering::fromRawData($query); + } + + public function page(): int + { + return $this->page; + } + + public function searchTerm(): ?string + { + return $this->searchTerm; + } + + public function tags(): array + { + return $this->tags; + } + + public function orderBy(): ShortUrlsOrdering + { + return $this->orderBy; + } + + public function dateRange(): ?DateRange + { + return $this->dateRange; + } +} diff --git a/module/Core/src/Model/VisitsParams.php b/module/Core/src/Model/VisitsParams.php index 98fcbe82..041aed9f 100644 --- a/module/Core/src/Model/VisitsParams.php +++ b/module/Core/src/Model/VisitsParams.php @@ -4,9 +4,10 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Model; -use Cake\Chronos\Chronos; use Shlinkio\Shlink\Common\Util\DateRange; +use function Shlinkio\Shlink\Core\parseDateFromQuery; + final class VisitsParams { private const FIRST_PAGE = 1; @@ -34,21 +35,13 @@ final class VisitsParams public static function fromRawData(array $query): self { - $startDate = self::getDateQueryParam($query, 'startDate'); - $endDate = self::getDateQueryParam($query, 'endDate'); - return new self( - new DateRange($startDate, $endDate), + new DateRange(parseDateFromQuery($query, 'startDate'), parseDateFromQuery($query, 'endDate')), (int) ($query['page'] ?? 1), isset($query['itemsPerPage']) ? (int) $query['itemsPerPage'] : null, ); } - private static function getDateQueryParam(array $query, string $key): ?Chronos - { - return ! isset($query[$key]) || empty($query[$key]) ? null : Chronos::parse($query[$key]); - } - public function getDateRange(): DateRange { return $this->dateRange; diff --git a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php index d15d925c..a4cf3190 100644 --- a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php +++ b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php @@ -5,38 +5,20 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Paginator\Adapter; use Laminas\Paginator\Adapter\AdapterInterface; -use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; -use function strip_tags; -use function trim; - class ShortUrlRepositoryAdapter implements AdapterInterface { public const ITEMS_PER_PAGE = 10; private ShortUrlRepositoryInterface $repository; - private ?string $searchTerm; - /** @var null|array|string */ - private $orderBy; - private array $tags; - private ?DateRange $dateRange; + private ShortUrlsParams $params; - /** - * @param string|array|null $orderBy - */ - public function __construct( - ShortUrlRepositoryInterface $repository, - ?string $searchTerm = null, - array $tags = [], - $orderBy = null, - ?DateRange $dateRange = null - ) { + public function __construct(ShortUrlRepositoryInterface $repository, ShortUrlsParams $params) + { $this->repository = $repository; - $this->searchTerm = $searchTerm !== null ? trim(strip_tags($searchTerm)) : null; - $this->orderBy = $orderBy; - $this->tags = $tags; - $this->dateRange = $dateRange; + $this->params = $params; } /** @@ -50,10 +32,10 @@ class ShortUrlRepositoryAdapter implements AdapterInterface return $this->repository->findList( $itemCountPerPage, $offset, - $this->searchTerm, - $this->tags, - $this->orderBy, - $this->dateRange, + $this->params->searchTerm(), + $this->params->tags(), + $this->params->orderBy(), + $this->params->dateRange(), ); } @@ -68,6 +50,10 @@ class ShortUrlRepositoryAdapter implements AdapterInterface */ public function count(): int { - return $this->repository->countList($this->searchTerm, $this->tags, $this->dateRange); + return $this->repository->countList( + $this->params->searchTerm(), + $this->params->tags(), + $this->params->dateRange(), + ); } } diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index a334a838..cd96acfc 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -8,18 +8,16 @@ use Doctrine\ORM\EntityRepository; use Doctrine\ORM\QueryBuilder; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; use function array_column; use function array_key_exists; use function Functional\contains; -use function is_array; -use function key; class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryInterface { /** * @param string[] $tags - * @param string|array|null $orderBy * @return ShortUrl[] */ public function findList( @@ -27,7 +25,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI ?int $offset = null, ?string $searchTerm = null, array $tags = [], - $orderBy = null, + ?ShortUrlsOrdering $orderBy = null, ?DateRange $dateRange = null ): array { $qb = $this->createListQueryBuilder($searchTerm, $tags, $dateRange); @@ -42,7 +40,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI } // In case the ordering has been specified, the query could be more complex. Process it - if ($orderBy !== null) { + if ($orderBy !== null && $orderBy->hasOrderField()) { return $this->processOrderByForList($qb, $orderBy); } @@ -51,14 +49,10 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI return $qb->getQuery()->getResult(); } - /** - * @param string|array|null $orderBy - */ - private function processOrderByForList(QueryBuilder $qb, $orderBy): array + private function processOrderByForList(QueryBuilder $qb, ShortUrlsOrdering $orderBy): array { - $isArray = is_array($orderBy); - $fieldName = $isArray ? key($orderBy) : $orderBy; - $order = $isArray ? $orderBy[$fieldName] : 'ASC'; + $fieldName = $orderBy->orderField(); + $order = $orderBy->orderDirection(); if (contains(['visits', 'visitsCount', 'visitCount'], $fieldName)) { $qb->addSelect('COUNT(DISTINCT v) AS totalVisits') diff --git a/module/Core/src/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/Repository/ShortUrlRepositoryInterface.php index 8695021a..d0acdf1a 100644 --- a/module/Core/src/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/Repository/ShortUrlRepositoryInterface.php @@ -7,18 +7,16 @@ namespace Shlinkio\Shlink\Core\Repository; use Doctrine\Persistence\ObjectRepository; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; interface ShortUrlRepositoryInterface extends ObjectRepository { - /** - * @param string|array|null $orderBy - */ public function findList( ?int $limit = null, ?int $offset = null, ?string $searchTerm = null, array $tags = [], - $orderBy = null, + ?ShortUrlsOrdering $orderBy = null, ?DateRange $dateRange = null ): array; diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index 8733cddb..dbfb1db6 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -6,10 +6,10 @@ namespace Shlinkio\Shlink\Core\Service; use Doctrine\ORM; use Laminas\Paginator\Paginator; -use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\Service\ShortUrl\FindShortCodeTrait; @@ -28,23 +28,15 @@ class ShortUrlService implements ShortUrlServiceInterface } /** - * @param string[] $tags - * @param array|string|null $orderBy - * * @return ShortUrl[]|Paginator */ - public function listShortUrls( - int $page = 1, - ?string $searchQuery = null, - array $tags = [], - $orderBy = null, - ?DateRange $dateRange = null - ) { + public function listShortUrls(ShortUrlsParams $params): Paginator + { /** @var ShortUrlRepository $repo */ $repo = $this->em->getRepository(ShortUrl::class); - $paginator = new Paginator(new ShortUrlRepositoryAdapter($repo, $searchQuery, $tags, $orderBy, $dateRange)); + $paginator = new Paginator(new ShortUrlRepositoryAdapter($repo, $params)); $paginator->setItemCountPerPage(ShortUrlRepositoryAdapter::ITEMS_PER_PAGE) - ->setCurrentPageNumber($page); + ->setCurrentPageNumber($params->page()); return $paginator; } diff --git a/module/Core/src/Service/ShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrlServiceInterface.php index 310ba6b3..e431c51b 100644 --- a/module/Core/src/Service/ShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrlServiceInterface.php @@ -5,26 +5,17 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service; use Laminas\Paginator\Paginator; -use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; interface ShortUrlServiceInterface { /** - * @param string[] $tags - * @param array|string|null $orderBy - * * @return ShortUrl[]|Paginator */ - public function listShortUrls( - int $page = 1, - ?string $searchQuery = null, - array $tags = [], - $orderBy = null, - ?DateRange $dateRange = null - ); + public function listShortUrls(ShortUrlsParams $params): Paginator; /** * @param string[] $tags diff --git a/module/Core/src/Validation/ShortUrlMetaInputFilter.php b/module/Core/src/Validation/ShortUrlMetaInputFilter.php index eca97a13..187ec66f 100644 --- a/module/Core/src/Validation/ShortUrlMetaInputFilter.php +++ b/module/Core/src/Validation/ShortUrlMetaInputFilter.php @@ -20,12 +20,10 @@ class ShortUrlMetaInputFilter extends InputFilter public const FIND_IF_EXISTS = 'findIfExists'; public const DOMAIN = 'domain'; - public function __construct(?array $data = null) + public function __construct(array $data) { $this->initialize(); - if ($data !== null) { - $this->setData($data); - } + $this->setData($data); } private function initialize(): void diff --git a/module/Core/src/Validation/ShortUrlsParamsInputFilter.php b/module/Core/src/Validation/ShortUrlsParamsInputFilter.php new file mode 100644 index 00000000..b3e6db2d --- /dev/null +++ b/module/Core/src/Validation/ShortUrlsParamsInputFilter.php @@ -0,0 +1,45 @@ +initialize(); + $this->setData($data); + } + + private function initialize(): void + { + $this->add($this->createDateInput(self::START_DATE, false)); + $this->add($this->createDateInput(self::END_DATE, false)); + + $this->add($this->createInput(self::SEARCH_TERM, false)); + + $page = $this->createInput(self::PAGE, false); + $page->getValidatorChain()->attach(new Validator\Digits()) + ->attach(new Validator\GreaterThan(['min' => 1, 'inclusive' => true])); + $this->add($page); + + $tags = $this->createArrayInput(self::TAGS, false); + $tags->getFilterChain()->attach(new Filter\StringToLower()) + ->attach(new Validation\SluggerFilter()); + $this->add($tags); + } +} diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index f742c6eb..4af2fd61 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -13,6 +13,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; @@ -122,7 +123,9 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->assertCount(1, $this->repo->findList(2, 2)); - $result = $this->repo->findList(null, null, null, [], ['visits' => 'DESC']); + $result = $this->repo->findList(null, null, null, [], ShortUrlsOrdering::fromRawData([ + 'orderBy' => ['visits' => 'DESC'], + ])); $this->assertCount(3, $result); $this->assertSame($bar, $result[0]); @@ -148,7 +151,9 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - $result = $this->repo->findList(null, null, null, [], ['longUrl' => 'ASC']); + $result = $this->repo->findList(null, null, null, [], ShortUrlsOrdering::fromRawData([ + 'orderBy' => ['longUrl' => 'ASC'], + ])); $this->assertCount(count($urls), $result); $this->assertEquals('a', $result[0]->getLongUrl()); diff --git a/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php b/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php index dd2fa24a..eb094c25 100644 --- a/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php @@ -7,7 +7,7 @@ namespace ShlinkioTest\Shlink\Core\Paginator\Adapter; use Cake\Chronos\Chronos; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; @@ -21,17 +21,26 @@ class ShortUrlRepositoryAdapterTest extends TestCase } /** - * @param string|array|null $orderBy * @test * @dataProvider provideFilteringArgs */ public function getItemsFallsBackToFindList( ?string $searchTerm = null, array $tags = [], - ?DateRange $dateRange = null, - $orderBy = null + ?string $startDate = null, + ?string $endDate = null, + ?string $orderBy = null ): void { - $adapter = new ShortUrlRepositoryAdapter($this->repo->reveal(), $searchTerm, $tags, $orderBy, $dateRange); + $params = ShortUrlsParams::fromRawData([ + 'searchTerm' => $searchTerm, + 'tags' => $tags, + 'startDate' => $startDate, + 'endDate' => $endDate, + 'orderBy' => $orderBy, + ]); + $adapter = new ShortUrlRepositoryAdapter($this->repo->reveal(), $params); + $orderBy = $params->orderBy(); + $dateRange = $params->dateRange(); $this->repo->findList(10, 5, $searchTerm, $tags, $orderBy, $dateRange)->shouldBeCalledOnce(); $adapter->getItems(5, 10); @@ -44,9 +53,17 @@ class ShortUrlRepositoryAdapterTest extends TestCase public function countFallsBackToCountList( ?string $searchTerm = null, array $tags = [], - ?DateRange $dateRange = null + ?string $startDate = null, + ?string $endDate = null ): void { - $adapter = new ShortUrlRepositoryAdapter($this->repo->reveal(), $searchTerm, $tags, null, $dateRange); + $params = ShortUrlsParams::fromRawData([ + 'searchTerm' => $searchTerm, + 'tags' => $tags, + 'startDate' => $startDate, + 'endDate' => $endDate, + ]); + $adapter = new ShortUrlRepositoryAdapter($this->repo->reveal(), $params); + $dateRange = $params->dateRange(); $this->repo->countList($searchTerm, $tags, $dateRange)->shouldBeCalledOnce(); $adapter->count(); @@ -58,12 +75,12 @@ class ShortUrlRepositoryAdapterTest extends TestCase yield ['search']; yield ['search', []]; yield ['search', ['foo', 'bar']]; - yield ['search', ['foo', 'bar'], null, 'order']; - yield ['search', ['foo', 'bar'], new DateRange(), 'order']; - yield ['search', ['foo', 'bar'], new DateRange(Chronos::now()), 'order']; - yield ['search', ['foo', 'bar'], new DateRange(null, Chronos::now()), 'order']; - yield ['search', ['foo', 'bar'], new DateRange(Chronos::now(), Chronos::now()), 'order']; - yield ['search', ['foo', 'bar'], new DateRange(Chronos::now())]; - yield [null, ['foo', 'bar'], new DateRange(Chronos::now(), Chronos::now())]; + 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 [null, ['foo', 'bar'], Chronos::now()->toAtomString()]; + yield [null, ['foo', 'bar'], Chronos::now()->toAtomString(), Chronos::now()->toAtomString()]; } } diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php index 719c69d5..69f0785d 100644 --- a/module/Core/test/Service/ShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrlServiceTest.php @@ -14,6 +14,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\Service\ShortUrlService; @@ -47,7 +48,7 @@ class ShortUrlServiceTest extends TestCase $repo->countList(Argument::cetera())->willReturn(count($list))->shouldBeCalledOnce(); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $list = $this->service->listShortUrls(); + $list = $this->service->listShortUrls(ShortUrlsParams::emptyInstance()); $this->assertEquals(4, $list->getCurrentItemCount()); } diff --git a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php index a50493db..5801eeec 100644 --- a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php +++ b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php @@ -4,14 +4,12 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Action\ShortUrl; -use Cake\Chronos\Chronos; -use InvalidArgumentException; use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\Paginator\Util\PaginatorUtilsTrait; -use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -36,38 +34,11 @@ class ListShortUrlsAction extends AbstractRestAction $this->domainConfig = $domainConfig; } - /** - * @throws InvalidArgumentException - */ public function handle(Request $request): Response { - $params = $this->queryToListParams($request->getQueryParams()); - $shortUrls = $this->shortUrlService->listShortUrls(...$params); + $shortUrls = $this->shortUrlService->listShortUrls(ShortUrlsParams::fromRawData($request->getQueryParams())); return new JsonResponse(['shortUrls' => $this->serializePaginator($shortUrls, new ShortUrlDataTransformer( $this->domainConfig, ))]); } - - /** - * @param array $query - * @return array - */ - private function queryToListParams(array $query): array - { - return [ - (int) ($query['page'] ?? 1), - $query['searchTerm'] ?? null, - $query['tags'] ?? [], - $query['orderBy'] ?? null, - $this->determineDateRangeFromQuery($query), - ]; - } - - private function determineDateRangeFromQuery(array $query): DateRange - { - return new DateRange( - isset($query['startDate']) ? Chronos::parse($query['startDate']) : null, - isset($query['endDate']) ? Chronos::parse($query['endDate']) : null, - ); - } } diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index f8d8c542..3fec6f7a 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -116,6 +116,13 @@ class ListShortUrlsTest extends ApiTestCase self::SHORT_URL_META, self::SHORT_URL_CUSTOM_DOMAIN, ]]; + yield [['orderBy' => ['shortCode' => 'DESC']], [ + self::SHORT_URL_CUSTOM_DOMAIN, + self::SHORT_URL_META, + self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, + self::SHORT_URL_CUSTOM_SLUG, + self::SHORT_URL_SHLINK, + ]]; yield [['startDate' => Chronos::parse('2018-12-01')->toAtomString()], [ self::SHORT_URL_META, self::SHORT_URL_CUSTOM_SLUG, diff --git a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php index 52cc62de..3d98c2fe 100644 --- a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php @@ -12,7 +12,7 @@ use Laminas\Paginator\Paginator; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Service\ShortUrlService; use Shlinkio\Shlink\Rest\Action\ShortUrl\ListShortUrlsAction; @@ -43,15 +43,17 @@ class ListShortUrlsActionTest extends TestCase ?string $expectedSearchTerm, array $expectedTags, ?string $expectedOrderBy, - DateRange $expectedDateRange + ?string $startDate = null, + ?string $endDate = null ): void { - $listShortUrls = $this->service->listShortUrls( - $expectedPage, - $expectedSearchTerm, - $expectedTags, - $expectedOrderBy, - $expectedDateRange, - )->willReturn(new Paginator(new ArrayAdapter())); + $listShortUrls = $this->service->listShortUrls(ShortUrlsParams::fromRawData([ + 'page' => $expectedPage, + 'searchTerm' => $expectedSearchTerm, + 'tags' => $expectedTags, + 'orderBy' => $expectedOrderBy, + 'startDate' => $startDate, + 'endDate' => $endDate, + ]))->willReturn(new Paginator(new ArrayAdapter())); /** @var JsonResponse $response */ $response = $this->action->handle((new ServerRequest())->withQueryParams($query)); @@ -66,25 +68,25 @@ class ListShortUrlsActionTest extends TestCase public function provideFilteringData(): iterable { - yield [[], 1, null, [], null, new DateRange()]; - yield [['page' => 10], 10, null, [], null, new DateRange()]; - yield [['page' => null], 1, null, [], null, new DateRange()]; - yield [['page' => '8'], 8, null, [], null, new DateRange()]; - yield [['searchTerm' => $searchTerm = 'foo'], 1, $searchTerm, [], null, new DateRange()]; - yield [['tags' => $tags = ['foo','bar']], 1, null, $tags, null, new DateRange()]; - yield [['orderBy' => $orderBy = 'something'], 1, null, [], $orderBy, new DateRange()]; + yield [[], 1, null, [], null]; + yield [['page' => 10], 10, null, [], null]; + yield [['page' => null], 1, null, [], null]; + 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 [[ 'page' => '2', 'orderBy' => $orderBy = 'something', 'tags' => $tags = ['one', 'two'], - ], 2, null, $tags, $orderBy, new DateRange()]; + ], 2, null, $tags, $orderBy]; yield [ ['startDate' => $date = Chronos::now()->toAtomString()], 1, null, [], null, - new DateRange(Chronos::parse($date)), + $date, ]; yield [ ['endDate' => $date = Chronos::now()->toAtomString()], @@ -92,7 +94,8 @@ class ListShortUrlsActionTest extends TestCase null, [], null, - new DateRange(null, Chronos::parse($date)), + null, + $date, ]; yield [ [ @@ -103,7 +106,8 @@ class ListShortUrlsActionTest extends TestCase null, [], null, - new DateRange(Chronos::parse($startDate), Chronos::parse($endDate)), + $startDate, + $endDate, ]; } }