From 14314ef9392c1685f448332c98c0762a1df65d0a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 6 Oct 2022 19:49:32 +0200 Subject: [PATCH 1/4] Updated shlink deps --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 475ccbd4..277c6ba5 100644 --- a/composer.json +++ b/composer.json @@ -45,8 +45,8 @@ "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.0", "ramsey/uuid": "^4.3", - "shlinkio/shlink-common": "^5.1", - "shlinkio/shlink-config": "^2.1", + "shlinkio/shlink-common": "dev-main#7515008 as 5.2", + "shlinkio/shlink-config": "dev-main#bcd8222 as 2.2", "shlinkio/shlink-event-dispatcher": "^2.6", "shlinkio/shlink-importer": "^4.0", "shlinkio/shlink-installer": "^8.2", From 27b680e0cdfda223d386c4a314554cf4727dc5a6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 6 Oct 2022 21:01:11 +0200 Subject: [PATCH 2/4] Created CLI test for short URLs list --- .../test-cli/Command/ListShortUrlsTest.php | 66 +++++++++++++++++++ module/Rest/src/ApiKey/Role.php | 2 - 2 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 module/CLI/test-cli/Command/ListShortUrlsTest.php diff --git a/module/CLI/test-cli/Command/ListShortUrlsTest.php b/module/CLI/test-cli/Command/ListShortUrlsTest.php new file mode 100644 index 00000000..faa47a2f --- /dev/null +++ b/module/CLI/test-cli/Command/ListShortUrlsTest.php @@ -0,0 +1,66 @@ +exec([ListShortUrlsCommand::NAME, ...$flags], ['no']); + self::assertStringContainsString($expectedOutput, $output); + } + + public function provideFlagsAndOutput(): iterable + { + // phpcs:disable Generic.Files.LineLength + yield 'no flags' => [[], << [['--start-date=2019-01'], << [['-e 2018-12-01'], << [['-s 2018-06-20', '--end-date=2019-01-01T00:00:20+00:00'], << Date: Thu, 6 Oct 2022 21:29:27 +0200 Subject: [PATCH 3/4] Moved logic to reuse command options to option classes instead of base abstract command classes --- .../Command/Domain/GetDomainVisitsCommand.php | 2 +- .../ShortUrl/GetShortUrlVisitsCommand.php | 2 +- .../Command/ShortUrl/ListShortUrlsCommand.php | 31 ++++----- .../src/Command/Tag/GetTagVisitsCommand.php | 2 +- .../Util/AbstractWithDateRangeCommand.php | 69 ------------------- .../Visit/AbstractVisitsListCommand.php | 26 +++---- .../Visit/GetNonOrphanVisitsCommand.php | 2 +- .../Command/Visit/GetOrphanVisitsCommand.php | 2 +- module/CLI/src/Option/DateOption.php | 51 ++++++++++++++ module/CLI/src/Option/EndDateOption.php | 30 ++++++++ module/CLI/src/Option/StartDateOption.php | 30 ++++++++ 11 files changed, 141 insertions(+), 106 deletions(-) delete mode 100644 module/CLI/src/Command/Util/AbstractWithDateRangeCommand.php create mode 100644 module/CLI/src/Option/DateOption.php create mode 100644 module/CLI/src/Option/EndDateOption.php create mode 100644 module/CLI/src/Option/StartDateOption.php diff --git a/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php b/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php index 676a2141..8d2eb8c9 100644 --- a/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php +++ b/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php @@ -25,7 +25,7 @@ class GetDomainVisitsCommand extends AbstractVisitsListCommand parent::__construct($visitsHelper); } - protected function doConfigure(): void + protected function configure(): void { $this ->setName(self::NAME) diff --git a/module/CLI/src/Command/ShortUrl/GetShortUrlVisitsCommand.php b/module/CLI/src/Command/ShortUrl/GetShortUrlVisitsCommand.php index 7f81e4da..a6a4f31d 100644 --- a/module/CLI/src/Command/ShortUrl/GetShortUrlVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/GetShortUrlVisitsCommand.php @@ -20,7 +20,7 @@ class GetShortUrlVisitsCommand extends AbstractVisitsListCommand { public const NAME = 'short-url:visits'; - protected function doConfigure(): void + protected function configure(): void { $this ->setName(self::NAME) diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 0889bb03..11443abc 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -4,7 +4,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; -use Shlinkio\Shlink\CLI\Command\Util\AbstractWithDateRangeCommand; +use Shlinkio\Shlink\CLI\Option\EndDateOption; +use Shlinkio\Shlink\CLI\Option\StartDateOption; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Paginator; @@ -15,6 +16,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlsParamsInputFilter; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlServiceInterface; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; @@ -27,20 +29,25 @@ use function Functional\map; use function implode; use function sprintf; -class ListShortUrlsCommand extends AbstractWithDateRangeCommand +class ListShortUrlsCommand extends Command { use PagerfantaUtilsTrait; public const NAME = 'short-url:list'; + private readonly StartDateOption $startDateOption; + private readonly EndDateOption $endDateOption; + public function __construct( - private ShortUrlServiceInterface $shortUrlService, - private DataTransformerInterface $transformer, + private readonly ShortUrlServiceInterface $shortUrlService, + private readonly DataTransformerInterface $transformer, ) { parent::__construct(); + $this->startDateOption = new StartDateOption($this, 'short URLs'); + $this->endDateOption = new EndDateOption($this, 'short URLs'); } - protected function doConfigure(): void + protected function configure(): void { $this ->setName(self::NAME) @@ -104,16 +111,6 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand ); } - protected function getStartDateDesc(string $optionName): string - { - return sprintf('Allows to filter short URLs, returning only those created after "%s".', $optionName); - } - - protected function getEndDateDesc(string $optionName): string - { - return sprintf('Allows to filter short URLs, returning only those created before "%s".', $optionName); - } - protected function execute(InputInterface $input, OutputInterface $output): ?int { $io = new SymfonyStyle($input, $output); @@ -124,8 +121,8 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand $tagsMode = $input->getOption('including-all-tags') === true ? TagsMode::ALL->value : TagsMode::ANY->value; $tags = ! empty($tags) ? explode(',', $tags) : []; $all = $input->getOption('all'); - $startDate = $this->getStartDateOption($input, $output); - $endDate = $this->getEndDateOption($input, $output); + $startDate = $this->startDateOption->get($input, $output); + $endDate = $this->endDateOption->get($input, $output); $orderBy = $this->processOrderBy($input); $columnsMap = $this->resolveColumnsMap($input); diff --git a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php index 842c9b45..290a172a 100644 --- a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php +++ b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php @@ -25,7 +25,7 @@ class GetTagVisitsCommand extends AbstractVisitsListCommand parent::__construct($visitsHelper); } - protected function doConfigure(): void + protected function configure(): void { $this ->setName(self::NAME) diff --git a/module/CLI/src/Command/Util/AbstractWithDateRangeCommand.php b/module/CLI/src/Command/Util/AbstractWithDateRangeCommand.php deleted file mode 100644 index c3e3c407..00000000 --- a/module/CLI/src/Command/Util/AbstractWithDateRangeCommand.php +++ /dev/null @@ -1,69 +0,0 @@ -doConfigure(); - $this - ->addOption(self::START_DATE, 's', InputOption::VALUE_REQUIRED, $this->getStartDateDesc(self::START_DATE)) - ->addOption(self::END_DATE, 'e', InputOption::VALUE_REQUIRED, $this->getEndDateDesc(self::END_DATE)); - } - - protected function getStartDateOption(InputInterface $input, OutputInterface $output): ?Chronos - { - return $this->getDateOption($input, $output, self::START_DATE); - } - - protected function getEndDateOption(InputInterface $input, OutputInterface $output): ?Chronos - { - return $this->getDateOption($input, $output, self::END_DATE); - } - - private function getDateOption(InputInterface $input, OutputInterface $output, string $key): ?Chronos - { - $value = $input->getOption($key); - if (empty($value) || ! is_string($value)) { - return null; - } - - try { - return Chronos::parse($value); - } catch (Throwable $e) { - $output->writeln(sprintf( - '> Ignored provided "%s" since its value "%s" is not a valid date. <', - $key, - $value, - )); - - if ($output->isVeryVerbose()) { - $this->getApplication()?->renderThrowable($e, $output); - } - - return null; - } - } - - abstract protected function doConfigure(): void; - - abstract protected function getStartDateDesc(string $optionName): string; - - abstract protected function getEndDateDesc(string $optionName): string; -} diff --git a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php index 37a875c6..402d5ba4 100644 --- a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php +++ b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php @@ -4,13 +4,15 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; -use Shlinkio\Shlink\CLI\Command\Util\AbstractWithDateRangeCommand; +use Shlinkio\Shlink\CLI\Option\EndDateOption; +use Shlinkio\Shlink\CLI\Option\StartDateOption; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -19,29 +21,23 @@ use function Functional\map; use function Functional\select_keys; use function Shlinkio\Shlink\Common\buildDateRange; use function Shlinkio\Shlink\Core\camelCaseToHumanFriendly; -use function sprintf; -abstract class AbstractVisitsListCommand extends AbstractWithDateRangeCommand +abstract class AbstractVisitsListCommand extends Command { + private readonly StartDateOption $startDateOption; + private readonly EndDateOption $endDateOption; + public function __construct(protected readonly VisitsStatsHelperInterface $visitsHelper) { parent::__construct(); - } - - final protected function getStartDateDesc(string $optionName): string - { - return sprintf('Allows to filter visits, returning only those older than "%s".', $optionName); - } - - final protected function getEndDateDesc(string $optionName): string - { - return sprintf('Allows to filter visits, returning only those newer than "%s".', $optionName); + $this->startDateOption = new StartDateOption($this, 'visits'); + $this->endDateOption = new EndDateOption($this, 'visits'); } final protected function execute(InputInterface $input, OutputInterface $output): ?int { - $startDate = $this->getStartDateOption($input, $output); - $endDate = $this->getEndDateOption($input, $output); + $startDate = $this->startDateOption->get($input, $output); + $endDate = $this->endDateOption->get($input, $output); $paginator = $this->getVisitsPaginator($input, buildDateRange($startDate, $endDate)); [$rows, $headers] = $this->resolveRowsAndHeaders($paginator); diff --git a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php index 0b4a4612..0dd32f3e 100644 --- a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php @@ -23,7 +23,7 @@ class GetNonOrphanVisitsCommand extends AbstractVisitsListCommand parent::__construct($visitsHelper); } - protected function doConfigure(): void + protected function configure(): void { $this ->setName(self::NAME) diff --git a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php index c2d353af..618a35cd 100644 --- a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php @@ -14,7 +14,7 @@ class GetOrphanVisitsCommand extends AbstractVisitsListCommand { public const NAME = 'visit:orphan'; - protected function doConfigure(): void + protected function configure(): void { $this ->setName(self::NAME) diff --git a/module/CLI/src/Option/DateOption.php b/module/CLI/src/Option/DateOption.php new file mode 100644 index 00000000..a863696f --- /dev/null +++ b/module/CLI/src/Option/DateOption.php @@ -0,0 +1,51 @@ +addOption($name, $shortcut, InputOption::VALUE_REQUIRED, $description); + } + + public function get(InputInterface $input, OutputInterface $output): ?Chronos + { + $value = $input->getOption($this->name); + if (empty($value) || ! is_string($value)) { + return null; + } + + try { + return Chronos::parse($value); + } catch (Throwable $e) { + $output->writeln(sprintf( + '> Ignored provided "%s" since its value "%s" is not a valid date. <', + $this->name, + $value, + )); + + if ($output->isVeryVerbose()) { + $this->command->getApplication()?->renderThrowable($e, $output); + } + + return null; + } + } +} diff --git a/module/CLI/src/Option/EndDateOption.php b/module/CLI/src/Option/EndDateOption.php new file mode 100644 index 00000000..72421981 --- /dev/null +++ b/module/CLI/src/Option/EndDateOption.php @@ -0,0 +1,30 @@ +dateOption = new DateOption($command, 'end-date', 'e', sprintf( + 'Allows to filter %s, returning only those newer than provided date.', + $descriptionHint, + )); + } + + public function get(InputInterface $input, OutputInterface $output): ?Chronos + { + return $this->dateOption->get($input, $output); + } +} diff --git a/module/CLI/src/Option/StartDateOption.php b/module/CLI/src/Option/StartDateOption.php new file mode 100644 index 00000000..2da5aaee --- /dev/null +++ b/module/CLI/src/Option/StartDateOption.php @@ -0,0 +1,30 @@ +dateOption = new DateOption($command, 'start-date', 's', sprintf( + 'Allows to filter %s, returning only those older than provided date.', + $descriptionHint, + )); + } + + public function get(InputInterface $input, OutputInterface $output): ?Chronos + { + return $this->dateOption->get($input, $output); + } +} From 14bf3a134b927ab1e67516ec61f2ce87a48336b0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 6 Oct 2022 21:30:23 +0200 Subject: [PATCH 4/4] Updated changelog --- CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d4838a2..816b5e16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ 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 +* [#1563](https://github.com/shlinkio/shlink/issues/1563) Moved logic to reuse command options to option classes instead of base abstract command classes. + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* *Nothing* + + ## [3.3.1] - 2022-09-30 ### Added * *Nothing*