From 3ff9e101a88b16136c8bf2bd6161927c3983b0c8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 14 Jul 2020 13:00:56 +0200 Subject: [PATCH 1/5] Added support to print all short URLs at once from CLI --- .../Command/ShortUrl/ListShortUrlsCommand.php | 36 +++++++++++-------- module/Core/src/Model/ShortUrlsParams.php | 11 ++++++ .../Adapter/ShortUrlRepositoryAdapter.php | 2 -- module/Core/src/Service/ShortUrlService.php | 2 +- .../Validation/ShortUrlsParamsInputFilter.php | 20 ++++++++--- 5 files changed, 50 insertions(+), 21 deletions(-) diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 98f006d4..3c397cc1 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -11,7 +11,6 @@ use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Util\PaginatorUtilsTrait; 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; @@ -61,7 +60,7 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand 'page', 'p', InputOption::VALUE_REQUIRED, - sprintf('The first page to list (%s items per page)', ShortUrlRepositoryAdapter::ITEMS_PER_PAGE), + 'The first page to list (10 items per page unless "--all" is provided)', '1', ) ->addOption( @@ -82,7 +81,8 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand InputOption::VALUE_REQUIRED, 'The field from which we want to order by. Pass ASC or DESC separated by a comma', ) - ->addOption('showTags', null, InputOption::VALUE_NONE, 'Whether to display the tags or not'); + ->addOption('showTags', null, InputOption::VALUE_NONE, 'Whether to display the tags or not') + ->addOption('all', 'a', InputOption::VALUE_NONE, 'Disables pagination and just displays all existing URLs'); } protected function getStartDateDesc(): string @@ -104,24 +104,32 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand $tags = $input->getOption('tags'); $tags = ! empty($tags) ? explode(',', $tags) : []; $showTags = (bool) $input->getOption('showTags'); + $all = (bool) $input->getOption('all'); $startDate = $this->getDateOption($input, $output, 'startDate'); $endDate = $this->getDateOption($input, $output, 'endDate'); $orderBy = $this->processOrderBy($input); + $data = [ + 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, + ]; + + if ($all) { + $data[ShortUrlsParamsInputFilter::ITEMS_PER_PAGE] = -1; + } + do { - $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, - ])); + $result = $this->renderPage($output, $showTags, ShortUrlsParams::fromRawData($data)); $page++; - $continue = $this->isLastPage($result) - ? false - : $io->confirm(sprintf('Continue with page %s?', $page), false); + $continue = ! $this->isLastPage($result) && $io->confirm( + sprintf('Continue with page %s?', $page), + false, + ); } while ($continue); $io->newLine(); diff --git a/module/Core/src/Model/ShortUrlsParams.php b/module/Core/src/Model/ShortUrlsParams.php index cbd74bec..b27a6187 100644 --- a/module/Core/src/Model/ShortUrlsParams.php +++ b/module/Core/src/Model/ShortUrlsParams.php @@ -12,11 +12,14 @@ use function Shlinkio\Shlink\Core\parseDateField; final class ShortUrlsParams { + public const DEFAULT_ITEMS_PER_PAGE = 10; + private int $page; private ?string $searchTerm; private array $tags; private ShortUrlsOrdering $orderBy; private ?DateRange $dateRange; + private ?int $itemsPerPage = null; private function __construct() { @@ -56,6 +59,9 @@ final class ShortUrlsParams parseDateField($inputFilter->getValue(ShortUrlsParamsInputFilter::END_DATE)), ); $this->orderBy = ShortUrlsOrdering::fromRawData($query); + $this->itemsPerPage = (int) ( + $inputFilter->getValue(ShortUrlsParamsInputFilter::ITEMS_PER_PAGE) ?? self::DEFAULT_ITEMS_PER_PAGE + ); } public function page(): int @@ -63,6 +69,11 @@ final class ShortUrlsParams return $this->page; } + public function itemsPerPage(): int + { + return $this->itemsPerPage; + } + public function searchTerm(): ?string { return $this->searchTerm; diff --git a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php index a4cf3190..f395412c 100644 --- a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php +++ b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php @@ -10,8 +10,6 @@ use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; class ShortUrlRepositoryAdapter implements AdapterInterface { - public const ITEMS_PER_PAGE = 10; - private ShortUrlRepositoryInterface $repository; private ShortUrlsParams $params; diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index 5cdab93d..456c46ad 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -44,7 +44,7 @@ class ShortUrlService implements ShortUrlServiceInterface /** @var ShortUrlRepository $repo */ $repo = $this->em->getRepository(ShortUrl::class); $paginator = new Paginator(new ShortUrlRepositoryAdapter($repo, $params)); - $paginator->setItemCountPerPage(ShortUrlRepositoryAdapter::ITEMS_PER_PAGE) + $paginator->setItemCountPerPage($params->itemsPerPage()) ->setCurrentPageNumber($params->page()); return $paginator; diff --git a/module/Core/src/Validation/ShortUrlsParamsInputFilter.php b/module/Core/src/Validation/ShortUrlsParamsInputFilter.php index 20191fa1..e9a292d0 100644 --- a/module/Core/src/Validation/ShortUrlsParamsInputFilter.php +++ b/module/Core/src/Validation/ShortUrlsParamsInputFilter.php @@ -5,10 +5,13 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Validation; use Laminas\Filter; +use Laminas\InputFilter\Input; use Laminas\InputFilter\InputFilter; use Laminas\Validator; use Shlinkio\Shlink\Common\Validation; +use function is_numeric; + class ShortUrlsParamsInputFilter extends InputFilter { use Validation\InputFactoryTrait; @@ -18,6 +21,7 @@ class ShortUrlsParamsInputFilter extends InputFilter public const TAGS = 'tags'; public const START_DATE = 'startDate'; public const END_DATE = 'endDate'; + public const ITEMS_PER_PAGE = 'itemsPerPage'; public function __construct(array $data) { @@ -32,14 +36,22 @@ class ShortUrlsParamsInputFilter extends InputFilter $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); + $this->add($this->createNumericInput(self::PAGE, 1)); $tags = $this->createArrayInput(self::TAGS, false); $tags->getFilterChain()->attach(new Filter\StringToLower()) ->attach(new Filter\PregReplace(['pattern' => '/ /', 'replacement' => '-'])); $this->add($tags); + + $this->add($this->createNumericInput(self::ITEMS_PER_PAGE, -1)); + } + + private function createNumericInput(string $name, int $min): Input + { + $input = $this->createInput($name, false); + $input->getValidatorChain()->attach(new Validator\Callback(fn ($value) => is_numeric($value))) + ->attach(new Validator\GreaterThan(['min' => $min, 'inclusive' => true])); + + return $input; } } From 8e84b0e8ac4c0870114ac5165be58188f3dc2682 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 14 Jul 2020 13:14:53 +0200 Subject: [PATCH 2/5] Ensured page footer on list short URLs is not displayed when printing all URLs --- module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 3c397cc1..38e002ac 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -110,7 +110,6 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand $orderBy = $this->processOrderBy($input); $data = [ - ShortUrlsParamsInputFilter::PAGE => $page, ShortUrlsParamsInputFilter::SEARCH_TERM => $searchTerm, ShortUrlsParamsInputFilter::TAGS => $tags, ShortUrlsOrdering::ORDER_BY => $orderBy, @@ -123,7 +122,8 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand } do { - $result = $this->renderPage($output, $showTags, ShortUrlsParams::fromRawData($data)); + $data[ShortUrlsParamsInputFilter::PAGE] = $page; + $result = $this->renderPage($output, $showTags, ShortUrlsParams::fromRawData($data), $all); $page++; $continue = ! $this->isLastPage($result) && $io->confirm( @@ -138,7 +138,7 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand return ExitCodes::EXIT_SUCCESS; } - private function renderPage(OutputInterface $output, bool $showTags, ShortUrlsParams $params): Paginator + private function renderPage(OutputInterface $output, bool $showTags, ShortUrlsParams $params, bool $all): Paginator { $result = $this->shortUrlService->listShortUrls($params); @@ -159,7 +159,7 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand $rows[] = array_values(array_intersect_key($shortUrl, array_flip(self::COLUMNS_WHITELIST))); } - ShlinkTable::fromOutput($output)->render($headers, $rows, $this->formatCurrentPageMessage( + ShlinkTable::fromOutput($output)->render($headers, $rows, $all ? null : $this->formatCurrentPageMessage( $result, 'Page %s of %s', )); From 5f9b62967673f19ebb7914c5326c5581588f9e6d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 14 Jul 2020 13:28:38 +0200 Subject: [PATCH 3/5] Added test for short URLs with all items --- .../ShortUrl/ListShortUrlsCommandTest.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index 2e315581..d8bc0f60 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -192,4 +192,22 @@ class ListShortUrlsCommandTest extends TestCase yield [['--orderBy' => 'foo,ASC'], ['foo' => 'ASC']]; yield [['--orderBy' => 'bar,DESC'], ['bar' => 'DESC']]; } + + /** @test */ + public function requestingAllElementsWillSetItemsPerPage(): void + { + $listShortUrls = $this->shortUrlService->listShortUrls(ShortUrlsParams::fromRawData([ + 'page' => 1, + 'searchTerm' => null, + 'tags' => [], + 'startDate' => null, + 'endDate' => null, + 'orderBy' => null, + 'itemsPerPage' => -1, + ]))->willReturn(new Paginator(new ArrayAdapter())); + + $this->commandTester->execute(['--all' => true]); + + $listShortUrls->shouldHaveBeenCalledOnce(); + } } From 6be03109336bf9ea590ad60f6aaedccd94891f09 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 14 Jul 2020 15:31:18 +0200 Subject: [PATCH 4/5] Improved command flag description --- module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 38e002ac..38abbb4d 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -82,7 +82,13 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand 'The field from which we want to order by. Pass ASC or DESC separated by a comma', ) ->addOption('showTags', null, InputOption::VALUE_NONE, 'Whether to display the tags or not') - ->addOption('all', 'a', InputOption::VALUE_NONE, 'Disables pagination and just displays all existing URLs'); + ->addOption( + 'all', + 'a', + InputOption::VALUE_NONE, + 'Disables pagination and just displays all existing URLs. Caution! If the amount of short URLs is big,' + . ' this may end up failing due to memory usage.', + ); } protected function getStartDateDesc(): string From 007139e4ffb640e4ced33fadc37a0d37ae4e749f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 14 Jul 2020 15:37:21 +0200 Subject: [PATCH 5/5] Updated changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b83747d5..2ab803f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#734](https://github.com/shlinkio/shlink/issues/734) Added support to redirect to deeplinks and other links with schemas different from `http` and `https`. * [#709](https://github.com/shlinkio/shlink/issues/709) Added multi-architecture builds for the docker image. +* [#707](https://github.com/shlinkio/shlink/issues/707) Added `--all` flag to `short-urls:list` command, which will print all existing URLs in one go, with no pagination. + + It has one limitation, though. Because of the way the CLI tooling works, all rows in the table must be loaded in memory. If the amount of URLs is too high, the command may fail due to too much memory usage. + #### Changed * [#508](https://github.com/shlinkio/shlink/issues/508) Added mutation checks to database tests.