From 1183d651843cd6ed0657b5c320f1c5cc8fdebe25 Mon Sep 17 00:00:00 2001 From: Alejandro Medina Date: Sat, 14 Dec 2019 11:58:08 -0300 Subject: [PATCH 01/18] Add date range filter to short url repository interface --- module/Core/src/Repository/ShortUrlRepositoryInterface.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/module/Core/src/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/Repository/ShortUrlRepositoryInterface.php index da5cef61..23c555b3 100644 --- a/module/Core/src/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/Repository/ShortUrlRepositoryInterface.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Repository; use Doctrine\Common\Persistence\ObjectRepository; +use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; interface ShortUrlRepositoryInterface extends ObjectRepository @@ -19,7 +20,8 @@ interface ShortUrlRepositoryInterface extends ObjectRepository ?int $offset = null, ?string $searchTerm = null, array $tags = [], - $orderBy = null + $orderBy = null, + ?DateRange $dateRange = null ): array; /** From 5928f28699a72005bfc67b2147188e0604458c7f Mon Sep 17 00:00:00 2001 From: Alejandro Medina Date: Sat, 14 Dec 2019 11:58:52 -0300 Subject: [PATCH 02/18] Add date range filter to short url repository --- .../Core/src/Repository/ShortUrlRepository.php | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 47f8f985..d8f71b27 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Repository; use Cake\Chronos\Chronos; use Doctrine\ORM\EntityRepository; use Doctrine\ORM\QueryBuilder; +use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use function array_column; @@ -27,7 +28,8 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI ?int $offset = null, ?string $searchTerm = null, array $tags = [], - $orderBy = null + $orderBy = null, + ?DateRange $dateRange = null ): array { $qb = $this->createListQueryBuilder($searchTerm, $tags); $qb->select('DISTINCT s'); @@ -40,6 +42,18 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI $qb->setFirstResult($offset); } + // Date filters + if ($dateRange !== null) { + if ($dateRange->getStartDate() !== null) { + $qb->andWhere($qb->expr()->gte('s.dateCreated', ':startDate')); + $qb->setParameter('startDate', $dateRange->getStartDate()); + } + if ($dateRange->getEndDate() !== null) { + $qb->andWhere($qb->expr()->lte('s.dateCreated', ':endDate')); + $qb->setParameter('endDate', $dateRange->getEndDate()); + } + } + // In case the ordering has been specified, the query could be more complex. Process it if ($orderBy !== null) { return $this->processOrderByForList($qb, $orderBy); From 661efcb51ffa6dfecd20c4aa2421043a0cc594d1 Mon Sep 17 00:00:00 2001 From: Alejandro Medina Date: Sat, 14 Dec 2019 12:01:56 -0300 Subject: [PATCH 03/18] Add date range filter to short url repository adapter --- .../Paginator/Adapter/ShortUrlRepositoryAdapter.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php index 80ffb1e8..90ae55f1 100644 --- a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php +++ b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Paginator\Adapter; +use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; use Zend\Paginator\Adapter\AdapterInterface; @@ -22,17 +23,21 @@ class ShortUrlRepositoryAdapter implements AdapterInterface private $orderBy; /** @var array */ private $tags; + /** @var DateRange|null */ + private $dateRange; public function __construct( ShortUrlRepositoryInterface $repository, $searchTerm = null, array $tags = [], - $orderBy = null + $orderBy = null, + ?DateRange $dateRange = null ) { $this->repository = $repository; $this->searchTerm = $searchTerm !== null ? trim(strip_tags($searchTerm)) : null; $this->orderBy = $orderBy; $this->tags = $tags; + $this->dateRange = $dateRange; } /** @@ -49,7 +54,8 @@ class ShortUrlRepositoryAdapter implements AdapterInterface $offset, $this->searchTerm, $this->tags, - $this->orderBy + $this->orderBy, + $this->dateRange, ); } From f9ba322547cb2a0177b8a402d181e07e2873a349 Mon Sep 17 00:00:00 2001 From: Alejandro Medina Date: Sat, 14 Dec 2019 13:55:03 -0300 Subject: [PATCH 04/18] Add date range filter to list urls endpoint parameters --- .../Rest/src/Action/ShortUrl/ListShortUrlsAction.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php index 157fbe06..b448b507 100644 --- a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php +++ b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php @@ -4,11 +4,13 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Action\ShortUrl; +use Cake\Chronos\Chronos; use InvalidArgumentException; 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\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -56,11 +58,19 @@ class ListShortUrlsAction extends AbstractRestAction */ private function queryToListParams(array $query): array { + $dateRange = null; + $dateStart = isset($query['dateStart']) ? Chronos::parse($query['dateStart']) : null; + $dateEnd = isset($query['dateEnd']) ? Chronos::parse($query['dateEnd']) : null; + if ($dateStart != null || $dateEnd != null) { + $dateRange = new DateRange($dateStart, $dateEnd); + } + return [ (int) ($query['page'] ?? 1), $query['searchTerm'] ?? null, $query['tags'] ?? [], $query['orderBy'] ?? null, + $dateRange, ]; } } From 27008505e53d410c770982ac3212a3a69b4e3e1e Mon Sep 17 00:00:00 2001 From: Alejandro Medina Date: Sat, 14 Dec 2019 15:03:39 -0300 Subject: [PATCH 05/18] Add date range filter to short url service interface --- module/Core/src/Service/ShortUrlServiceInterface.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/module/Core/src/Service/ShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrlServiceInterface.php index cb2c7dca..7c105c59 100644 --- a/module/Core/src/Service/ShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrlServiceInterface.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service; +use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; @@ -12,11 +13,16 @@ use Zend\Paginator\Paginator; interface ShortUrlServiceInterface { /** + * @param int $page + * @param string|null $searchQuery * @param string[] $tags * @param array|string|null $orderBy * @return ShortUrl[]|Paginator + * @param DateRange|null $dateRange + * + * @return ShortUrl[]|Paginator */ - public function listShortUrls(int $page = 1, ?string $searchQuery = null, array $tags = [], $orderBy = null); + public function listShortUrls(int $page = 1, ?string $searchQuery = null, array $tags = [], $orderBy = null, ?DateRange $dateRange = null); /** * @param string[] $tags From f17c46bbed495e349ec5c702cd96013b8492a0a0 Mon Sep 17 00:00:00 2001 From: Alejandro Medina Date: Sat, 14 Dec 2019 15:10:09 -0300 Subject: [PATCH 06/18] Add date range filter to short url service --- module/Core/src/Service/ShortUrlService.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index 7719cafe..e82b3741 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service; use Doctrine\ORM; +use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; @@ -28,15 +29,19 @@ class ShortUrlService implements ShortUrlServiceInterface } /** + * @param int $page + * @param string|null $searchQuery * @param string[] $tags * @param array|string|null $orderBy + * @param DateRange|null $dateRange + * * @return ShortUrl[]|Paginator */ - public function listShortUrls(int $page = 1, ?string $searchQuery = null, array $tags = [], $orderBy = null) + public function listShortUrls(int $page = 1, ?string $searchQuery = null, array $tags = [], $orderBy = null, ?DateRange $dateRange = null) { /** @var ShortUrlRepository $repo */ $repo = $this->em->getRepository(ShortUrl::class); - $paginator = new Paginator(new ShortUrlRepositoryAdapter($repo, $searchQuery, $tags, $orderBy)); + $paginator = new Paginator(new ShortUrlRepositoryAdapter($repo, $searchQuery, $tags, $orderBy, $dateRange)); $paginator->setItemCountPerPage(ShortUrlRepositoryAdapter::ITEMS_PER_PAGE) ->setCurrentPageNumber($page); From a28e7987e6af276ea1721a8231191fca3f11138e Mon Sep 17 00:00:00 2001 From: Alejandro Medina Date: Sat, 14 Dec 2019 18:32:58 -0300 Subject: [PATCH 07/18] fixup! Add date range filter to list urls endpoint parameters --- module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php index b448b507..5cb8aa90 100644 --- a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php +++ b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php @@ -59,10 +59,10 @@ class ListShortUrlsAction extends AbstractRestAction private function queryToListParams(array $query): array { $dateRange = null; - $dateStart = isset($query['dateStart']) ? Chronos::parse($query['dateStart']) : null; - $dateEnd = isset($query['dateEnd']) ? Chronos::parse($query['dateEnd']) : null; - if ($dateStart != null || $dateEnd != null) { - $dateRange = new DateRange($dateStart, $dateEnd); + $startDate = isset($query['startDate']) ? Chronos::parse($query['startDate']) : null; + $endDate = isset($query['endDate']) ? Chronos::parse($query['endDate']) : null; + if ($startDate != null || $endDate != null) { + $dateRange = new DateRange($startDate, $endDate); } return [ From d7ffcd903da1fc31c2bfec542a500d3d9fed5540 Mon Sep 17 00:00:00 2001 From: Alejandro Medina Date: Sat, 14 Dec 2019 18:42:02 -0300 Subject: [PATCH 08/18] Add date filter fields to short urls documentation --- docs/swagger/paths/v1_short-urls.json | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index d0aebfec..46708e22 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -54,6 +54,24 @@ "visits" ] } + }, + { + "name": "startDate", + "in": "query", + "description": "The date (in ISO-8601 format) from which we want to get short URLs.", + "required": false, + "schema": { + "type": "string" + } + }, + { + "name": "endDate", + "in": "query", + "description": "The date (in ISO-8601 format) until which we want to get short URLs.", + "required": false, + "schema": { + "type": "string" + } } ], "security": [ From 99fd5f937ed3d4b75abf33662b8a0288d83060c8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 16 Dec 2019 15:16:18 +0100 Subject: [PATCH 09/18] Fixed existing tests and coding styles --- module/Core/src/Service/ShortUrlService.php | 12 +++++++----- module/Core/src/Service/ShortUrlServiceInterface.php | 12 +++++++----- .../Adapter/ShortUrlRepositoryAdapterTest.php | 2 +- .../Rest/src/Action/ShortUrl/ListShortUrlsAction.php | 2 +- .../test/Action/ShortUrl/ListShortUrlsActionTest.php | 3 ++- 5 files changed, 18 insertions(+), 13 deletions(-) diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index e82b3741..15a3b432 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -29,16 +29,18 @@ class ShortUrlService implements ShortUrlServiceInterface } /** - * @param int $page - * @param string|null $searchQuery * @param string[] $tags * @param array|string|null $orderBy - * @param DateRange|null $dateRange * * @return ShortUrl[]|Paginator */ - public function listShortUrls(int $page = 1, ?string $searchQuery = null, array $tags = [], $orderBy = null, ?DateRange $dateRange = null) - { + public function listShortUrls( + int $page = 1, + ?string $searchQuery = null, + array $tags = [], + $orderBy = null, + ?DateRange $dateRange = null + ) { /** @var ShortUrlRepository $repo */ $repo = $this->em->getRepository(ShortUrl::class); $paginator = new Paginator(new ShortUrlRepositoryAdapter($repo, $searchQuery, $tags, $orderBy, $dateRange)); diff --git a/module/Core/src/Service/ShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrlServiceInterface.php index 7c105c59..6e3fe199 100644 --- a/module/Core/src/Service/ShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrlServiceInterface.php @@ -13,16 +13,18 @@ use Zend\Paginator\Paginator; interface ShortUrlServiceInterface { /** - * @param int $page - * @param string|null $searchQuery * @param string[] $tags * @param array|string|null $orderBy - * @return ShortUrl[]|Paginator - * @param DateRange|null $dateRange * * @return ShortUrl[]|Paginator */ - public function listShortUrls(int $page = 1, ?string $searchQuery = null, array $tags = [], $orderBy = null, ?DateRange $dateRange = null); + public function listShortUrls( + int $page = 1, + ?string $searchQuery = null, + array $tags = [], + $orderBy = null, + ?DateRange $dateRange = null + ); /** * @param string[] $tags diff --git a/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php b/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php index a7229147..391fdad6 100644 --- a/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php @@ -25,7 +25,7 @@ class ShortUrlRepositoryAdapterTest extends TestCase /** @test */ public function getItemsFallbacksToFindList(): void { - $this->repo->findList(10, 5, 'search', ['foo', 'bar'], 'order')->shouldBeCalledOnce(); + $this->repo->findList(10, 5, 'search', ['foo', 'bar'], 'order', null)->shouldBeCalledOnce(); $this->adapter->getItems(5, 10); } diff --git a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php index 5cb8aa90..24c44628 100644 --- a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php +++ b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php @@ -61,7 +61,7 @@ class ListShortUrlsAction extends AbstractRestAction $dateRange = null; $startDate = isset($query['startDate']) ? Chronos::parse($query['startDate']) : null; $endDate = isset($query['endDate']) ? Chronos::parse($query['endDate']) : null; - if ($startDate != null || $endDate != null) { + if ($startDate !== null || $endDate !== null) { $dateRange = new DateRange($startDate, $endDate); } diff --git a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php index 4197aba8..cfe9a683 100644 --- a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php @@ -49,7 +49,8 @@ class ListShortUrlsActionTest extends TestCase $expectedPage, $expectedSearchTerm, $expectedTags, - $expectedOrderBy + $expectedOrderBy, + null )->willReturn(new Paginator(new ArrayAdapter())); /** @var JsonResponse $response */ From 03a92e555601bcf2bc40e2ce6a13b99c554c13dd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 16 Dec 2019 15:22:03 +0100 Subject: [PATCH 10/18] Fixed trailing method comma which is not compatible with PHP 7.2 --- module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php index 90ae55f1..f275c757 100644 --- a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php +++ b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php @@ -55,7 +55,7 @@ class ShortUrlRepositoryAdapter implements AdapterInterface $this->searchTerm, $this->tags, $this->orderBy, - $this->dateRange, + $this->dateRange ); } From 839ca318218f2446ec86c66944adfc8a2789f6fe Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 16 Dec 2019 21:46:27 +0100 Subject: [PATCH 11/18] Ensured date range filtering is also passed to the count function on ShortUrlRepository --- .../Adapter/ShortUrlRepositoryAdapter.php | 2 +- .../src/Repository/ShortUrlRepository.php | 34 ++++++------ .../ShortUrlRepositoryInterface.php | 9 +--- .../src/Repository/TagRepositoryInterface.php | 2 +- .../Repository/VisitRepositoryInterface.php | 2 +- .../PersistenceDomainResolverTest.php | 2 +- .../Adapter/ShortUrlRepositoryAdapterTest.php | 52 ++++++++++++++----- .../Rest/test-api/Fixtures/ApiKeyFixture.php | 2 +- .../test-api/Fixtures/ShortUrlsFixture.php | 2 +- module/Rest/test-api/Fixtures/TagsFixture.php | 2 +- .../Rest/test-api/Fixtures/VisitsFixture.php | 2 +- 11 files changed, 68 insertions(+), 43 deletions(-) diff --git a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php index f275c757..de382d17 100644 --- a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php +++ b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php @@ -70,6 +70,6 @@ class ShortUrlRepositoryAdapter implements AdapterInterface */ public function count(): int { - return $this->repository->countList($this->searchTerm, $this->tags); + return $this->repository->countList($this->searchTerm, $this->tags, $this->dateRange); } } diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index d8f71b27..40635f83 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -31,7 +31,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI $orderBy = null, ?DateRange $dateRange = null ): array { - $qb = $this->createListQueryBuilder($searchTerm, $tags); + $qb = $this->createListQueryBuilder($searchTerm, $tags, $dateRange); $qb->select('DISTINCT s'); // Set limit and offset @@ -42,18 +42,6 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI $qb->setFirstResult($offset); } - // Date filters - if ($dateRange !== null) { - if ($dateRange->getStartDate() !== null) { - $qb->andWhere($qb->expr()->gte('s.dateCreated', ':startDate')); - $qb->setParameter('startDate', $dateRange->getStartDate()); - } - if ($dateRange->getEndDate() !== null) { - $qb->andWhere($qb->expr()->lte('s.dateCreated', ':endDate')); - $qb->setParameter('endDate', $dateRange->getEndDate()); - } - } - // In case the ordering has been specified, the query could be more complex. Process it if ($orderBy !== null) { return $this->processOrderByForList($qb, $orderBy); @@ -91,7 +79,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI return $qb->getQuery()->getResult(); } - public function countList(?string $searchTerm = null, array $tags = []): int + public function countList(?string $searchTerm = null, array $tags = [], ?DateRange $dateRange = null): int { $qb = $this->createListQueryBuilder($searchTerm, $tags); $qb->select('COUNT(DISTINCT s)'); @@ -99,12 +87,26 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI return (int) $qb->getQuery()->getSingleScalarResult(); } - private function createListQueryBuilder(?string $searchTerm = null, array $tags = []): QueryBuilder - { + private function createListQueryBuilder( + ?string $searchTerm = null, + array $tags = [], + ?DateRange $dateRange = null + ): QueryBuilder { $qb = $this->getEntityManager()->createQueryBuilder(); $qb->from(ShortUrl::class, 's'); $qb->where('1=1'); + if ($dateRange !== null) { + if ($dateRange->getStartDate() !== null) { + $qb->andWhere($qb->expr()->gte('s.dateCreated', ':startDate')); + $qb->setParameter('startDate', $dateRange->getStartDate()); + } + if ($dateRange->getEndDate() !== null) { + $qb->andWhere($qb->expr()->lte('s.dateCreated', ':endDate')); + $qb->setParameter('endDate', $dateRange->getEndDate()); + } + } + // Apply search term to every searchable field if not empty if (! empty($searchTerm)) { // Left join with tags only if no tags were provided. In case of tags, an inner join will be done later diff --git a/module/Core/src/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/Repository/ShortUrlRepositoryInterface.php index 23c555b3..8695021a 100644 --- a/module/Core/src/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/Repository/ShortUrlRepositoryInterface.php @@ -4,15 +4,13 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Repository; -use Doctrine\Common\Persistence\ObjectRepository; +use Doctrine\Persistence\ObjectRepository; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; interface ShortUrlRepositoryInterface extends ObjectRepository { /** - * Gets a list of elements using provided filtering data - * * @param string|array|null $orderBy */ public function findList( @@ -24,10 +22,7 @@ interface ShortUrlRepositoryInterface extends ObjectRepository ?DateRange $dateRange = null ): array; - /** - * Counts the number of elements in a list using provided filtering data - */ - public function countList(?string $searchTerm = null, array $tags = []): int; + public function countList(?string $searchTerm = null, array $tags = [], ?DateRange $dateRange = null): int; public function findOneByShortCode(string $shortCode, ?string $domain = null): ?ShortUrl; diff --git a/module/Core/src/Repository/TagRepositoryInterface.php b/module/Core/src/Repository/TagRepositoryInterface.php index 182df847..e253f7a4 100644 --- a/module/Core/src/Repository/TagRepositoryInterface.php +++ b/module/Core/src/Repository/TagRepositoryInterface.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Repository; -use Doctrine\Common\Persistence\ObjectRepository; +use Doctrine\Persistence\ObjectRepository; interface TagRepositoryInterface extends ObjectRepository { diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php index a0bbfe99..e70c989e 100644 --- a/module/Core/src/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Repository/VisitRepositoryInterface.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Repository; -use Doctrine\Common\Persistence\ObjectRepository; +use Doctrine\Persistence\ObjectRepository; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Visit; diff --git a/module/Core/test/Domain/Resolver/PersistenceDomainResolverTest.php b/module/Core/test/Domain/Resolver/PersistenceDomainResolverTest.php index be0640b6..4ba796ab 100644 --- a/module/Core/test/Domain/Resolver/PersistenceDomainResolverTest.php +++ b/module/Core/test/Domain/Resolver/PersistenceDomainResolverTest.php @@ -4,8 +4,8 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Domain\Resolver; -use Doctrine\Common\Persistence\ObjectRepository; use Doctrine\ORM\EntityManagerInterface; +use Doctrine\Persistence\ObjectRepository; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Domain\Resolver\PersistenceDomainResolver; diff --git a/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php b/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php index 391fdad6..8bf69faf 100644 --- a/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php @@ -4,35 +4,63 @@ declare(strict_types=1); 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\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; class ShortUrlRepositoryAdapterTest extends TestCase { - /** @var ShortUrlRepositoryAdapter */ - private $adapter; /** @var ObjectProphecy */ private $repo; public function setUp(): void { $this->repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $this->adapter = new ShortUrlRepositoryAdapter($this->repo->reveal(), 'search', ['foo', 'bar'], 'order'); } - /** @test */ - public function getItemsFallbacksToFindList(): void - { - $this->repo->findList(10, 5, 'search', ['foo', 'bar'], 'order', null)->shouldBeCalledOnce(); - $this->adapter->getItems(5, 10); + /** + * @test + * @dataProvider provideFilteringArgs + */ + public function getItemsFallsBackToFindList( + $searchTerm = null, + array $tags = [], + ?DateRange $dateRange = null, + $orderBy = null + ): void { + $adapter = new ShortUrlRepositoryAdapter($this->repo->reveal(), $searchTerm, $tags, $orderBy, $dateRange); + + $this->repo->findList(10, 5, $searchTerm, $tags, $orderBy, $dateRange)->shouldBeCalledOnce(); + $adapter->getItems(5, 10); } - /** @test */ - public function countFallbacksToCountList(): void + /** + * @test + * @dataProvider provideFilteringArgs + */ + public function countFallsBackToCountList($searchTerm = null, array $tags = [], ?DateRange $dateRange = null): void { - $this->repo->countList('search', ['foo', 'bar'])->shouldBeCalledOnce(); - $this->adapter->count(); + $adapter = new ShortUrlRepositoryAdapter($this->repo->reveal(), $searchTerm, $tags, null, $dateRange); + + $this->repo->countList($searchTerm, $tags, $dateRange)->shouldBeCalledOnce(); + $adapter->count(); + } + + public function provideFilteringArgs(): iterable + { + yield []; + 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())]; } } diff --git a/module/Rest/test-api/Fixtures/ApiKeyFixture.php b/module/Rest/test-api/Fixtures/ApiKeyFixture.php index 2bc26187..971054fd 100644 --- a/module/Rest/test-api/Fixtures/ApiKeyFixture.php +++ b/module/Rest/test-api/Fixtures/ApiKeyFixture.php @@ -6,7 +6,7 @@ namespace ShlinkioApiTest\Shlink\Rest\Fixtures; use Cake\Chronos\Chronos; use Doctrine\Common\DataFixtures\FixtureInterface; -use Doctrine\Common\Persistence\ObjectManager; +use Doctrine\Persistence\ObjectManager; use ReflectionObject; use Shlinkio\Shlink\Rest\Entity\ApiKey; diff --git a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php index 253b0032..3282e575 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php @@ -6,7 +6,7 @@ namespace ShlinkioApiTest\Shlink\Rest\Fixtures; use Cake\Chronos\Chronos; use Doctrine\Common\DataFixtures\AbstractFixture; -use Doctrine\Common\Persistence\ObjectManager; +use Doctrine\Persistence\ObjectManager; use ReflectionObject; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; diff --git a/module/Rest/test-api/Fixtures/TagsFixture.php b/module/Rest/test-api/Fixtures/TagsFixture.php index f498796b..5bd10ca7 100644 --- a/module/Rest/test-api/Fixtures/TagsFixture.php +++ b/module/Rest/test-api/Fixtures/TagsFixture.php @@ -7,7 +7,7 @@ namespace ShlinkioApiTest\Shlink\Rest\Fixtures; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\DataFixtures\AbstractFixture; use Doctrine\Common\DataFixtures\DependentFixtureInterface; -use Doctrine\Common\Persistence\ObjectManager; +use Doctrine\Persistence\ObjectManager; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Tag; diff --git a/module/Rest/test-api/Fixtures/VisitsFixture.php b/module/Rest/test-api/Fixtures/VisitsFixture.php index 9c1594a4..2c85c1a1 100644 --- a/module/Rest/test-api/Fixtures/VisitsFixture.php +++ b/module/Rest/test-api/Fixtures/VisitsFixture.php @@ -6,7 +6,7 @@ namespace ShlinkioApiTest\Shlink\Rest\Fixtures; use Doctrine\Common\DataFixtures\AbstractFixture; use Doctrine\Common\DataFixtures\DependentFixtureInterface; -use Doctrine\Common\Persistence\ObjectManager; +use Doctrine\Persistence\ObjectManager; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\Visitor; From 8ad8b08aa402ed0f75b06b16f4987d726010df56 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 16 Dec 2019 21:55:48 +0100 Subject: [PATCH 12/18] Improved ListShortUrlsActionTest covering different scenarios in which date ranges are provided --- .../Action/ShortUrl/ListShortUrlsAction.php | 17 ++++--- .../ShortUrl/ListShortUrlsActionTest.php | 50 +++++++++++++++---- 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php index 24c44628..87e7930b 100644 --- a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php +++ b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php @@ -58,19 +58,20 @@ class ListShortUrlsAction extends AbstractRestAction */ private function queryToListParams(array $query): array { - $dateRange = null; - $startDate = isset($query['startDate']) ? Chronos::parse($query['startDate']) : null; - $endDate = isset($query['endDate']) ? Chronos::parse($query['endDate']) : null; - if ($startDate !== null || $endDate !== null) { - $dateRange = new DateRange($startDate, $endDate); - } - return [ (int) ($query['page'] ?? 1), $query['searchTerm'] ?? null, $query['tags'] ?? [], $query['orderBy'] ?? null, - $dateRange, + $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/Action/ShortUrl/ListShortUrlsActionTest.php b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php index cfe9a683..1dfdc258 100644 --- a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php @@ -4,9 +4,11 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Action\ShortUrl; +use Cake\Chronos\Chronos; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Psr\Log\LoggerInterface; +use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Service\ShortUrlService; use Shlinkio\Shlink\Rest\Action\ShortUrl\ListShortUrlsAction; use Zend\Diactoros\Response\JsonResponse; @@ -43,14 +45,15 @@ class ListShortUrlsActionTest extends TestCase int $expectedPage, ?string $expectedSearchTerm, array $expectedTags, - ?string $expectedOrderBy + ?string $expectedOrderBy, + DateRange $expectedDateRange ): void { $listShortUrls = $this->service->listShortUrls( $expectedPage, $expectedSearchTerm, $expectedTags, $expectedOrderBy, - null + $expectedDateRange )->willReturn(new Paginator(new ArrayAdapter())); /** @var JsonResponse $response */ @@ -66,17 +69,44 @@ class ListShortUrlsActionTest extends TestCase public function provideFilteringData(): iterable { - 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 [[], 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 [[ 'page' => '2', 'orderBy' => $orderBy = 'something', 'tags' => $tags = ['one', 'two'], - ], 2, null, $tags, $orderBy]; + ], 2, null, $tags, $orderBy, new DateRange()]; + yield [ + ['startDate' => $date = Chronos::now()->toAtomString()], + 1, + null, + [], + null, + new DateRange(Chronos::parse($date)), + ]; + yield [ + ['endDate' => $date = Chronos::now()->toAtomString()], + 1, + null, + [], + null, + new DateRange(null, Chronos::parse($date)), + ]; + yield [ + [ + 'startDate' => $startDate = Chronos::now()->subDays(10)->toAtomString(), + 'endDate' => $endDate = Chronos::now()->toAtomString(), + ], + 1, + null, + [], + null, + new DateRange(Chronos::parse($startDate), Chronos::parse($endDate)), + ]; } } From 35eeaf4282697851ccd8bc4bc3ccc1748ab4f0eb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 16 Dec 2019 22:13:11 +0100 Subject: [PATCH 13/18] Improved repository tests covering fetching and counting filtered short URL lists --- .../src/Repository/ShortUrlRepository.php | 28 ++++++++----------- .../Repository/ShortUrlRepositoryTest.php | 23 +++++++++++++-- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 40635f83..1c26c122 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -81,7 +81,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI public function countList(?string $searchTerm = null, array $tags = [], ?DateRange $dateRange = null): int { - $qb = $this->createListQueryBuilder($searchTerm, $tags); + $qb = $this->createListQueryBuilder($searchTerm, $tags, $dateRange); $qb->select('COUNT(DISTINCT s)'); return (int) $qb->getQuery()->getSingleScalarResult(); @@ -96,15 +96,13 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI $qb->from(ShortUrl::class, 's'); $qb->where('1=1'); - if ($dateRange !== null) { - if ($dateRange->getStartDate() !== null) { - $qb->andWhere($qb->expr()->gte('s.dateCreated', ':startDate')); - $qb->setParameter('startDate', $dateRange->getStartDate()); - } - if ($dateRange->getEndDate() !== null) { - $qb->andWhere($qb->expr()->lte('s.dateCreated', ':endDate')); - $qb->setParameter('endDate', $dateRange->getEndDate()); - } + if ($dateRange !== null && $dateRange->getStartDate() !== null) { + $qb->andWhere($qb->expr()->gte('s.dateCreated', ':startDate')); + $qb->setParameter('startDate', $dateRange->getStartDate()); + } + if ($dateRange !== null && $dateRange->getEndDate() !== null) { + $qb->andWhere($qb->expr()->lte('s.dateCreated', ':endDate')); + $qb->setParameter('endDate', $dateRange->getEndDate()); } // Apply search term to every searchable field if not empty @@ -114,14 +112,12 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI $qb->leftJoin('s.tags', 't'); } - $conditions = [ + // Apply search conditions + $qb->andWhere($qb->expr()->orX( $qb->expr()->like('s.longUrl', ':searchPattern'), $qb->expr()->like('s.shortCode', ':searchPattern'), - $qb->expr()->like('t.name', ':searchPattern'), - ]; - - // Unpack and apply search conditions - $qb->andWhere($qb->expr()->orX(...$conditions)); + $qb->expr()->like('t.name', ':searchPattern') + )); $qb->setParameter('searchPattern', '%' . $searchTerm . '%'); } diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index f3913ea2..2006623a 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -6,6 +6,8 @@ namespace ShlinkioTest\Shlink\Core\Repository; use Cake\Chronos\Chronos; use Doctrine\Common\Collections\ArrayCollection; +use ReflectionObject; +use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Tag; @@ -108,7 +110,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase } /** @test */ - public function findListProperlyFiltersByTagAndSearchTerm(): void + public function findListProperlyFiltersResult(): void { $tag = new Tag('bar'); $this->getEntityManager()->persist($tag); @@ -124,12 +126,17 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($bar); $foo2 = new ShortUrl('foo_2'); + $ref = new ReflectionObject($foo2); + $dateProp = $ref->getProperty('dateCreated'); + $dateProp->setAccessible(true); + $dateProp->setValue($foo2, Chronos::now()->subDays(5)); $this->getEntityManager()->persist($foo2); $this->getEntityManager()->flush(); $result = $this->repo->findList(null, null, 'foo', ['bar']); $this->assertCount(1, $result); + $this->assertEquals(1, $this->repo->countList('foo', ['bar'])); $this->assertSame($foo, $result[0]); $result = $this->repo->findList(); @@ -141,12 +148,22 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $result = $this->repo->findList(2, 1); $this->assertCount(2, $result); - $result = $this->repo->findList(2, 2); - $this->assertCount(1, $result); + $this->assertCount(1, $this->repo->findList(2, 2)); $result = $this->repo->findList(null, null, null, [], ['visits' => 'DESC']); $this->assertCount(3, $result); $this->assertSame($bar, $result[0]); + + $result = $this->repo->findList(null, null, null, [], null, new DateRange(null, Chronos::now()->subDays(2))); + $this->assertCount(1, $result); + $this->assertEquals(1, $this->repo->countList(null, [], new DateRange(null, Chronos::now()->subDays(2)))); + $this->assertSame($foo2, $result[0]); + + $this->assertCount( + 2, + $this->repo->findList(null, null, null, [], null, new DateRange(Chronos::now()->subDays(2))) + ); + $this->assertEquals(2, $this->repo->countList(null, [], new DateRange(Chronos::now()->subDays(2)))); } /** @test */ From 8142801f1f9f9f9512f3eac55f9c20c21a775c7a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 16 Dec 2019 23:03:32 +0100 Subject: [PATCH 14/18] Updated ListShortUrlsAction api test so that it covers filtering use cases --- .../src/Repository/ShortUrlRepository.php | 19 +- .../test-api/Action/ListShortUrlsTest.php | 233 +++++++++++------- .../test-api/Fixtures/ShortUrlsFixture.php | 9 +- 3 files changed, 158 insertions(+), 103 deletions(-) diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 1c26c122..ac7b5f50 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -54,15 +54,9 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI private function processOrderByForList(QueryBuilder $qb, $orderBy): array { - // Map public field names to column names - $fieldNameMap = [ - 'originalUrl' => 'longUrl', - 'longUrl' => 'longUrl', - 'shortCode' => 'shortCode', - 'dateCreated' => 'dateCreated', - ]; - $fieldName = is_array($orderBy) ? key($orderBy) : $orderBy; - $order = is_array($orderBy) ? $orderBy[$fieldName] : 'ASC'; + $isArray = is_array($orderBy); + $fieldName = $isArray ? key($orderBy) : $orderBy; + $order = $isArray ? $orderBy[$fieldName] : 'ASC'; if (contains(['visits', 'visitsCount', 'visitCount'], $fieldName)) { $qb->addSelect('COUNT(DISTINCT v) AS totalVisits') @@ -73,6 +67,13 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI return array_column($qb->getQuery()->getResult(), 0); } + // Map public field names to column names + $fieldNameMap = [ + 'originalUrl' => 'longUrl', + 'longUrl' => 'longUrl', + 'shortCode' => 'shortCode', + 'dateCreated' => 'dateCreated', + ]; if (array_key_exists($fieldName, $fieldNameMap)) { $qb->orderBy('s.' . $fieldNameMap[$fieldName], $order); } diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index d2171b3a..eaa13a0a 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -4,107 +4,160 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Action; +use Cake\Chronos\Chronos; +use GuzzleHttp\RequestOptions; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; +use function count; + class ListShortUrlsTest extends ApiTestCase { - /** @test */ - public function shortUrlsAreProperlyListed(): void + private const SHORT_URL_SHLINK = [ + 'shortCode' => 'abc123', + 'shortUrl' => 'http://doma.in/abc123', + 'longUrl' => 'https://shlink.io', + 'dateCreated' => '2018-05-01T00:00:00+00:00', + 'visitsCount' => 3, + 'tags' => ['foo'], + 'meta' => [ + 'validSince' => null, + 'validUntil' => null, + 'maxVisits' => null, + ], + 'originalUrl' => 'https://shlink.io', + ]; + private const SHORT_URL_CUSTOM_SLUG_AND_DOMAIN = [ + 'shortCode' => 'custom-with-domain', + 'shortUrl' => 'http://some-domain.com/custom-with-domain', + 'longUrl' => 'https://google.com', + 'dateCreated' => '2018-10-20T00:00:00+00:00', + 'visitsCount' => 0, + 'tags' => [], + 'meta' => [ + 'validSince' => null, + 'validUntil' => null, + 'maxVisits' => null, + ], + 'originalUrl' => 'https://google.com', + ]; + private const SHORT_URL_META = [ + 'shortCode' => 'def456', + 'shortUrl' => 'http://doma.in/def456', + 'longUrl' => + 'https://blog.alejandrocelaya.com/2017/12/09' + . '/acmailer-7-0-the-most-important-release-in-a-long-time/', + 'dateCreated' => '2019-01-01T00:00:00+00:00', + 'visitsCount' => 2, + 'tags' => ['bar', 'foo'], + 'meta' => [ + 'validSince' => '2020-05-01T00:00:00+00:00', + 'validUntil' => null, + 'maxVisits' => null, + ], + 'originalUrl' => + 'https://blog.alejandrocelaya.com/2017/12/09' + . '/acmailer-7-0-the-most-important-release-in-a-long-time/', + ]; + private const SHORT_URL_CUSTOM_SLUG = [ + 'shortCode' => 'custom', + 'shortUrl' => 'http://doma.in/custom', + 'longUrl' => 'https://shlink.io', + 'dateCreated' => '2019-01-01T00:00:00+00:00', + 'visitsCount' => 0, + 'tags' => [], + 'meta' => [ + 'validSince' => null, + 'validUntil' => null, + 'maxVisits' => 2, + ], + 'originalUrl' => 'https://shlink.io', + ]; + private const SHORT_URL_CUSTOM_DOMAIN = [ + 'shortCode' => 'ghi789', + 'shortUrl' => 'http://example.com/ghi789', + 'longUrl' => + 'https://blog.alejandrocelaya.com/2019/04/27' + . '/considerations-to-properly-use-open-source-software-projects/', + 'dateCreated' => '2019-01-01T00:00:00+00:00', + 'visitsCount' => 0, + 'tags' => [], + 'meta' => [ + 'validSince' => null, + 'validUntil' => null, + 'maxVisits' => null, + ], + 'originalUrl' => + 'https://blog.alejandrocelaya.com/2019/04/27' + . '/considerations-to-properly-use-open-source-software-projects/', + ]; + + /** + * @test + * @dataProvider provideFilteredLists + */ + public function shortUrlsAreProperlyListed(array $query, array $expectedShortUrls): void { - $resp = $this->callApiWithKey(self::METHOD_GET, '/short-urls'); + $resp = $this->callApiWithKey(self::METHOD_GET, '/short-urls', [RequestOptions::QUERY => $query]); $respPayload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_OK, $resp->getStatusCode()); $this->assertEquals([ 'shortUrls' => [ - 'data' => [ - [ - 'shortCode' => 'abc123', - 'shortUrl' => 'http://doma.in/abc123', - 'longUrl' => 'https://shlink.io', - 'dateCreated' => '2019-01-01T00:00:00+00:00', - 'visitsCount' => 3, - 'tags' => ['foo'], - 'meta' => [ - 'validSince' => null, - 'validUntil' => null, - 'maxVisits' => null, - ], - 'originalUrl' => 'https://shlink.io', - ], - [ - 'shortCode' => 'def456', - 'shortUrl' => 'http://doma.in/def456', - 'longUrl' => - 'https://blog.alejandrocelaya.com/2017/12/09' - . '/acmailer-7-0-the-most-important-release-in-a-long-time/', - 'dateCreated' => '2019-01-01T00:00:00+00:00', - 'visitsCount' => 2, - 'tags' => ['bar', 'foo'], - 'meta' => [ - 'validSince' => '2020-05-01T00:00:00+00:00', - 'validUntil' => null, - 'maxVisits' => null, - ], - 'originalUrl' => - 'https://blog.alejandrocelaya.com/2017/12/09' - . '/acmailer-7-0-the-most-important-release-in-a-long-time/', - ], - [ - 'shortCode' => 'custom', - 'shortUrl' => 'http://doma.in/custom', - 'longUrl' => 'https://shlink.io', - 'dateCreated' => '2019-01-01T00:00:00+00:00', - 'visitsCount' => 0, - 'tags' => [], - 'meta' => [ - 'validSince' => null, - 'validUntil' => null, - 'maxVisits' => 2, - ], - 'originalUrl' => 'https://shlink.io', - ], - [ - 'shortCode' => 'ghi789', - 'shortUrl' => 'http://example.com/ghi789', - 'longUrl' => - 'https://blog.alejandrocelaya.com/2019/04/27' - . '/considerations-to-properly-use-open-source-software-projects/', - 'dateCreated' => '2019-01-01T00:00:00+00:00', - 'visitsCount' => 0, - 'tags' => [], - 'meta' => [ - 'validSince' => null, - 'validUntil' => null, - 'maxVisits' => null, - ], - 'originalUrl' => - 'https://blog.alejandrocelaya.com/2019/04/27' - . '/considerations-to-properly-use-open-source-software-projects/', - ], - [ - 'shortCode' => 'custom-with-domain', - 'shortUrl' => 'http://some-domain.com/custom-with-domain', - 'longUrl' => 'https://google.com', - 'dateCreated' => '2019-01-01T00:00:00+00:00', - 'visitsCount' => 0, - 'tags' => [], - 'meta' => [ - 'validSince' => null, - 'validUntil' => null, - 'maxVisits' => null, - ], - 'originalUrl' => 'https://google.com', - ], - ], - 'pagination' => [ - 'currentPage' => 1, - 'pagesCount' => 1, - 'itemsPerPage' => 10, - 'itemsInCurrentPage' => 5, - 'totalItems' => 5, - ], + 'data' => $expectedShortUrls, + 'pagination' => $this->buildPagination(count($expectedShortUrls)), ], ], $respPayload); } + + public function provideFilteredLists(): iterable + { + yield [[], [ + self::SHORT_URL_SHLINK, + self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, + self::SHORT_URL_META, + self::SHORT_URL_CUSTOM_SLUG, + self::SHORT_URL_CUSTOM_DOMAIN, + ]]; + yield [['orderBy' => 'shortCode'], [ + self::SHORT_URL_SHLINK, + self::SHORT_URL_CUSTOM_SLUG, + self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, + self::SHORT_URL_META, + self::SHORT_URL_CUSTOM_DOMAIN, + ]]; + yield [['startDate' => Chronos::parse('2018-12-01')->toAtomString()], [ + self::SHORT_URL_META, + self::SHORT_URL_CUSTOM_SLUG, + self::SHORT_URL_CUSTOM_DOMAIN, + ]]; + yield [['endDate' => Chronos::parse('2018-12-01')->toAtomString()], [ + self::SHORT_URL_SHLINK, + self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, + ]]; + yield [['tags' => ['foo']], [ + self::SHORT_URL_SHLINK, + self::SHORT_URL_META, + ]]; + yield [['tags' => ['bar']], [ + self::SHORT_URL_META, + ]]; + yield [['tags' => ['foo'], 'endDate' => Chronos::parse('2018-12-01')->toAtomString()], [ + self::SHORT_URL_SHLINK, + ]]; + yield [['searchTerm' => 'alejandro'], [ + self::SHORT_URL_META, + self::SHORT_URL_CUSTOM_DOMAIN, + ]]; + } + + private function buildPagination(int $itemsCount): array + { + return [ + 'currentPage' => 1, + 'pagesCount' => 1, + 'itemsPerPage' => 10, + 'itemsInCurrentPage' => $itemsCount, + 'totalItems' => $itemsCount, + ]; + } } diff --git a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php index 3282e575..62ec00a9 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php @@ -21,7 +21,8 @@ class ShortUrlsFixture extends AbstractFixture public function load(ObjectManager $manager): void { $abcShortUrl = $this->setShortUrlDate( - new ShortUrl('https://shlink.io', ShortUrlMeta::createFromRawData(['customSlug' => 'abc123'])) + new ShortUrl('https://shlink.io', ShortUrlMeta::createFromRawData(['customSlug' => 'abc123'])), + Chronos::parse('2018-05-01') ); $manager->persist($abcShortUrl); @@ -46,7 +47,7 @@ class ShortUrlsFixture extends AbstractFixture $withDomainAndSlugShortUrl = $this->setShortUrlDate(new ShortUrl( 'https://google.com', ShortUrlMeta::createFromRawData(['domain' => 'some-domain.com', 'customSlug' => 'custom-with-domain']) - )); + ), Chronos::parse('2018-10-20')); $manager->persist($withDomainAndSlugShortUrl); $manager->flush(); @@ -55,12 +56,12 @@ class ShortUrlsFixture extends AbstractFixture $this->addReference('def456_short_url', $defShortUrl); } - private function setShortUrlDate(ShortUrl $shortUrl): ShortUrl + private function setShortUrlDate(ShortUrl $shortUrl, ?Chronos $date = null): ShortUrl { $ref = new ReflectionObject($shortUrl); $dateProp = $ref->getProperty('dateCreated'); $dateProp->setAccessible(true); - $dateProp->setValue($shortUrl, Chronos::create(2019, 1, 1, 0, 0, 0)); + $dateProp->setValue($shortUrl, $date ?? Chronos::parse('2019-01-01')); return $shortUrl; } From 56165791310121c5ffdefd5b279ad37dc79cfad8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 17 Dec 2019 09:59:54 +0100 Subject: [PATCH 15/18] Added startDate and endDate params to ListShortUrlsCommand --- .../src/Command/Api/GenerateKeyCommand.php | 2 +- .../src/Command/ShortUrl/GetVisitsCommand.php | 51 +++++----- .../Command/ShortUrl/ListShortUrlsCommand.php | 47 ++++++++-- .../Util/AbstractWithDateRangeCommand.php | 54 +++++++++++ .../Command/ShortUrl/GetVisitsCommandTest.php | 27 +++++- .../ShortUrl/ListShortUrlsCommandTest.php | 92 +++++++++++++++---- 6 files changed, 215 insertions(+), 58 deletions(-) create mode 100644 module/CLI/src/Command/Util/AbstractWithDateRangeCommand.php diff --git a/module/CLI/src/Command/Api/GenerateKeyCommand.php b/module/CLI/src/Command/Api/GenerateKeyCommand.php index f35fa012..bbe86a51 100644 --- a/module/CLI/src/Command/Api/GenerateKeyCommand.php +++ b/module/CLI/src/Command/Api/GenerateKeyCommand.php @@ -36,7 +36,7 @@ class GenerateKeyCommand extends Command ->addOption( 'expirationDate', 'e', - InputOption::VALUE_OPTIONAL, + InputOption::VALUE_REQUIRED, 'The date in which the API key should expire. Use any valid PHP format.' ); } diff --git a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php index 7873fba6..416c1bfb 100644 --- a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php @@ -5,24 +5,25 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Cake\Chronos\Chronos; +use Shlinkio\Shlink\CLI\Command\Util\AbstractWithDateRangeCommand; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; -use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; -use Zend\Stdlib\ArrayUtils; +use Throwable; -use function array_map; +use function Functional\map; use function Functional\select_keys; +use function sprintf; -class GetVisitsCommand extends Command +class GetVisitsCommand extends AbstractWithDateRangeCommand { public const NAME = 'short-url:visits'; private const ALIASES = ['shortcode:visits', 'short-code:visits']; @@ -36,25 +37,23 @@ class GetVisitsCommand extends Command parent::__construct(); } - protected function configure(): void + protected function doConfigure(): void { $this ->setName(self::NAME) ->setAliases(self::ALIASES) ->setDescription('Returns the detailed visits information for provided short code') - ->addArgument('shortCode', InputArgument::REQUIRED, 'The short code which visits we want to get') - ->addOption( - 'startDate', - 's', - InputOption::VALUE_OPTIONAL, - 'Allows to filter visits, returning only those older than start date' - ) - ->addOption( - 'endDate', - 'e', - InputOption::VALUE_OPTIONAL, - 'Allows to filter visits, returning only those newer than end date' - ); + ->addArgument('shortCode', InputArgument::REQUIRED, 'The short code which visits we want to get'); + } + + protected function getStartDateDesc(): string + { + return 'Allows to filter visits, returning only those older than start date'; + } + + protected function getEndDateDesc(): string + { + return 'Allows to filter visits, returning only those newer than end date'; } protected function interact(InputInterface $input, OutputInterface $output): void @@ -74,24 +73,18 @@ class GetVisitsCommand extends Command protected function execute(InputInterface $input, OutputInterface $output): ?int { $shortCode = $input->getArgument('shortCode'); - $startDate = $this->getDateOption($input, 'startDate'); - $endDate = $this->getDateOption($input, 'endDate'); + $startDate = $this->getDateOption($input, $output, 'startDate'); + $endDate = $this->getDateOption($input, $output, 'endDate'); $paginator = $this->visitsTracker->info($shortCode, new VisitsParams(new DateRange($startDate, $endDate))); - $visits = ArrayUtils::iteratorToArray($paginator->getCurrentItems()); - $rows = array_map(function (Visit $visit) { + $rows = map($paginator->getCurrentItems(), function (Visit $visit) { $rowData = $visit->jsonSerialize(); $rowData['country'] = $visit->getVisitLocation()->getCountryName(); return select_keys($rowData, ['referer', 'date', 'userAgent', 'country']); - }, $visits); + }); ShlinkTable::fromOutput($output)->render(['Referer', 'Date', 'User agent', 'Country'], $rows); + return ExitCodes::EXIT_SUCCESS; } - - private function getDateOption(InputInterface $input, $key) - { - $value = $input->getOption($key); - return ! empty($value) ? Chronos::parse($value) : $value; - } } diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 3d9b528d..5871cc76 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -4,14 +4,16 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; +use Cake\Chronos\Chronos; +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\Rest\DataTransformerInterface; +use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; -use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; @@ -26,7 +28,7 @@ use function explode; use function implode; use function sprintf; -class ListShortUrlsCommand extends Command +class ListShortUrlsCommand extends AbstractWithDateRangeCommand { use PaginatorUtilsTrait; @@ -53,7 +55,7 @@ class ListShortUrlsCommand extends Command $this->domainConfig = $domainConfig; } - protected function configure(): void + protected function doConfigure(): void { $this ->setName(self::NAME) @@ -68,7 +70,7 @@ class ListShortUrlsCommand extends Command ) ->addOption( 'searchTerm', - 's', + 'st', InputOption::VALUE_REQUIRED, 'A query used to filter results by searching for it on the longUrl and shortCode fields' ) @@ -87,6 +89,16 @@ class ListShortUrlsCommand extends Command ->addOption('showTags', null, InputOption::VALUE_NONE, 'Whether to display the tags or not'); } + protected function getStartDateDesc(): string + { + return 'Allows to filter short URLs, returning only those created after "startDate"'; + } + + protected function getEndDateDesc(): string + { + return 'Allows to filter short URLs, returning only those created before "endDate"'; + } + protected function execute(InputInterface $input, OutputInterface $output): ?int { $io = new SymfonyStyle($input, $output); @@ -95,10 +107,23 @@ class ListShortUrlsCommand extends Command $tags = $input->getOption('tags'); $tags = ! empty($tags) ? explode(',', $tags) : []; $showTags = (bool) $input->getOption('showTags'); + $startDate = $this->getDateOption($input, $output, 'startDate'); + $endDate = $this->getDateOption($input, $output, 'endDate'); + $transformer = new ShortUrlDataTransformer($this->domainConfig); do { - $result = $this->renderPage($input, $output, $page, $searchTerm, $tags, $showTags, $transformer); + $result = $this->renderPage( + $input, + $output, + $page, + $searchTerm, + $tags, + $showTags, + $startDate, + $endDate, + $transformer + ); $page++; $continue = $this->isLastPage($result) @@ -108,6 +133,7 @@ class ListShortUrlsCommand extends Command $io->newLine(); $io->success('Short URLs properly listed'); + return ExitCodes::EXIT_SUCCESS; } @@ -118,9 +144,17 @@ class ListShortUrlsCommand extends Command ?string $searchTerm, array $tags, bool $showTags, + ?Chronos $startDate, + ?Chronos $endDate, DataTransformerInterface $transformer ): Paginator { - $result = $this->shortUrlService->listShortUrls($page, $searchTerm, $tags, $this->processOrderBy($input)); + $result = $this->shortUrlService->listShortUrls( + $page, + $searchTerm, + $tags, + $this->processOrderBy($input), + new DateRange($startDate, $endDate) + ); $headers = ['Short code', 'Short URL', 'Long URL', 'Date created', 'Visits count']; if ($showTags) { @@ -143,6 +177,7 @@ class ListShortUrlsCommand extends Command $result, 'Page %s of %s' )); + return $result; } diff --git a/module/CLI/src/Command/Util/AbstractWithDateRangeCommand.php b/module/CLI/src/Command/Util/AbstractWithDateRangeCommand.php new file mode 100644 index 00000000..c6b10be6 --- /dev/null +++ b/module/CLI/src/Command/Util/AbstractWithDateRangeCommand.php @@ -0,0 +1,54 @@ +doConfigure(); + $this + ->addOption('startDate', 's', InputOption::VALUE_REQUIRED, $this->getStartDateDesc()) + ->addOption('endDate', 'e', InputOption::VALUE_REQUIRED, $this->getEndDateDesc()); + } + + protected function getDateOption(InputInterface $input, OutputInterface $output, string $key): ?Chronos + { + $value = $input->getOption($key); + if (empty($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; + abstract protected function getEndDateDesc(): string; +} diff --git a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php index a61dd7d4..e2ea29d1 100644 --- a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php @@ -22,6 +22,8 @@ use Symfony\Component\Console\Tester\CommandTester; use Zend\Paginator\Adapter\ArrayAdapter; use Zend\Paginator\Paginator; +use function sprintf; + class GetVisitsCommandTest extends TestCase { /** @var CommandTester */ @@ -39,7 +41,7 @@ class GetVisitsCommandTest extends TestCase } /** @test */ - public function noDateFlagsTriesToListWithoutDateRange() + public function noDateFlagsTriesToListWithoutDateRange(): void { $shortCode = 'abc123'; $this->visitsTracker->info($shortCode, new VisitsParams(new DateRange(null, null)))->willReturn( @@ -50,7 +52,7 @@ class GetVisitsCommandTest extends TestCase } /** @test */ - public function providingDateFlagsTheListGetsFiltered() + public function providingDateFlagsTheListGetsFiltered(): void { $shortCode = 'abc123'; $startDate = '2016-01-01'; @@ -69,6 +71,27 @@ class GetVisitsCommandTest extends TestCase ]); } + /** @test */ + public function providingInvalidDatesPrintsWarning(): void + { + $shortCode = 'abc123'; + $startDate = 'foo'; + $info = $this->visitsTracker->info($shortCode, new VisitsParams(new DateRange())) + ->willReturn(new Paginator(new ArrayAdapter([]))); + + $this->commandTester->execute([ + 'shortCode' => $shortCode, + '--startDate' => $startDate, + ]); + $output = $this->commandTester->getDisplay(); + + $info->shouldHaveBeenCalledOnce(); + $this->assertStringContainsString( + sprintf('Ignored provided "startDate" since its value "%s" is not a valid date', $startDate), + $output + ); + } + /** @test */ public function outputIsProperlyGenerated(): void { diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index 6ad96ed3..71babc47 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -4,10 +4,12 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\CLI\Command\ShortUrl; +use Cake\Chronos\Chronos; 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\Service\ShortUrlServiceInterface; use Symfony\Component\Console\Application; @@ -15,6 +17,8 @@ use Symfony\Component\Console\Tester\CommandTester; use Zend\Paginator\Adapter\ArrayAdapter; use Zend\Paginator\Paginator; +use function explode; + class ListShortUrlsCommandTest extends TestCase { /** @var CommandTester */ @@ -32,17 +36,7 @@ class ListShortUrlsCommandTest extends TestCase } /** @test */ - public function noInputCallsListJustOnce() - { - $this->shortUrlService->listShortUrls(1, null, [], null)->willReturn(new Paginator(new ArrayAdapter())) - ->shouldBeCalledOnce(); - - $this->commandTester->setInputs(['n']); - $this->commandTester->execute([]); - } - - /** @test */ - public function loadingMorePagesCallsListMoreTimes() + public function loadingMorePagesCallsListMoreTimes(): void { // The paginator will return more than one page $data = []; @@ -64,7 +58,7 @@ class ListShortUrlsCommandTest extends TestCase } /** @test */ - public function havingMorePagesButAnsweringNoCallsListJustOnce() + public function havingMorePagesButAnsweringNoCallsListJustOnce(): void { // The paginator will return more than one page $data = []; @@ -72,8 +66,9 @@ class ListShortUrlsCommandTest extends TestCase $data[] = new ShortUrl('url_' . $i); } - $this->shortUrlService->listShortUrls(1, null, [], null)->willReturn(new Paginator(new ArrayAdapter($data))) - ->shouldBeCalledOnce(); + $this->shortUrlService->listShortUrls(1, null, [], null, new DateRange()) + ->willReturn(new Paginator(new ArrayAdapter($data))) + ->shouldBeCalledOnce(); $this->commandTester->setInputs(['n']); $this->commandTester->execute([]); @@ -89,25 +84,82 @@ class ListShortUrlsCommandTest extends TestCase } /** @test */ - public function passingPageWillMakeListStartOnThatPage() + public function passingPageWillMakeListStartOnThatPage(): void { $page = 5; - $this->shortUrlService->listShortUrls($page, null, [], null)->willReturn(new Paginator(new ArrayAdapter())) - ->shouldBeCalledOnce(); + $this->shortUrlService->listShortUrls($page, null, [], null, new DateRange()) + ->willReturn(new Paginator(new ArrayAdapter())) + ->shouldBeCalledOnce(); $this->commandTester->setInputs(['y']); $this->commandTester->execute(['--page' => $page]); } /** @test */ - public function ifTagsFlagIsProvidedTagsColumnIsIncluded() + public function ifTagsFlagIsProvidedTagsColumnIsIncluded(): void { - $this->shortUrlService->listShortUrls(1, null, [], null)->willReturn(new Paginator(new ArrayAdapter())) - ->shouldBeCalledOnce(); + $this->shortUrlService->listShortUrls(1, null, [], null, new DateRange()) + ->willReturn(new Paginator(new ArrayAdapter())) + ->shouldBeCalledOnce(); $this->commandTester->setInputs(['y']); $this->commandTester->execute(['--showTags' => true]); $output = $this->commandTester->getDisplay(); $this->assertStringContainsString('Tags', $output); } + + /** + * @test + * @dataProvider provideArgs + */ + public function serviceIsInvokedWithProvidedArgs( + array $commandArgs, + ?int $page, + ?string $searchTerm, + array $tags, + ?DateRange $dateRange + ): void { + $listShortUrls = $this->shortUrlService->listShortUrls($page, $searchTerm, $tags, null, $dateRange) + ->willReturn(new Paginator(new ArrayAdapter())); + + $this->commandTester->setInputs(['n']); + $this->commandTester->execute($commandArgs); + + $listShortUrls->shouldHaveBeenCalledOnce(); + } + + 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 [ + ['--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)), + ]; + yield [ + ['--endDate' => $endDate = '2020-05-23'], + 1, + null, + [], + new DateRange(null, Chronos::parse($endDate)), + ]; + yield [ + ['--startDate' => $startDate = '2019-01-01', '--endDate' => $endDate = '2020-05-23'], + 1, + null, + [], + new DateRange(Chronos::parse($startDate), Chronos::parse($endDate)), + ]; + } } From 4b113e578125459f172bcfa3fd700f08be245790 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 17 Dec 2019 10:06:18 +0100 Subject: [PATCH 16/18] Added tests covering how orderBy is parsed on ListShortUrlsCommand --- .../Command/ShortUrl/ListShortUrlsCommand.php | 10 +++++--- .../ShortUrl/ListShortUrlsCommandTest.php | 23 +++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 5871cc76..52918afa 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -109,12 +109,12 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand $showTags = (bool) $input->getOption('showTags'); $startDate = $this->getDateOption($input, $output, 'startDate'); $endDate = $this->getDateOption($input, $output, 'endDate'); + $orderBy = $this->processOrderBy($input); $transformer = new ShortUrlDataTransformer($this->domainConfig); do { $result = $this->renderPage( - $input, $output, $page, $searchTerm, @@ -122,6 +122,7 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand $showTags, $startDate, $endDate, + $orderBy, $transformer ); $page++; @@ -138,7 +139,6 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand } private function renderPage( - InputInterface $input, OutputInterface $output, int $page, ?string $searchTerm, @@ -146,13 +146,14 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand bool $showTags, ?Chronos $startDate, ?Chronos $endDate, + $orderBy, DataTransformerInterface $transformer ): Paginator { $result = $this->shortUrlService->listShortUrls( $page, $searchTerm, $tags, - $this->processOrderBy($input), + $orderBy, new DateRange($startDate, $endDate) ); @@ -181,6 +182,9 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand return $result; } + /** + * @return array|string|null + */ private function processOrderBy(InputInterface $input) { $orderBy = $input->getOption('orderBy'); diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index 71babc47..0bf3bfab 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -162,4 +162,27 @@ class ListShortUrlsCommandTest extends TestCase new DateRange(Chronos::parse($startDate), Chronos::parse($endDate)), ]; } + + /** + * @test + * @dataProvider provideOrderBy + */ + public function orderByIsProperlyComputed(array $commandArgs, $expectedOrderBy): void + { + $listShortUrls = $this->shortUrlService->listShortUrls(1, null, [], $expectedOrderBy, new DateRange()) + ->willReturn(new Paginator(new ArrayAdapter())); + + $this->commandTester->setInputs(['n']); + $this->commandTester->execute($commandArgs); + + $listShortUrls->shouldHaveBeenCalledOnce(); + } + + public function provideOrderBy(): iterable + { + yield [[], null]; + yield [['--orderBy' => 'foo'], 'foo']; + yield [['--orderBy' => 'foo,ASC'], ['foo' => 'ASC']]; + yield [['--orderBy' => 'bar,DESC'], ['bar' => 'DESC']]; + } } From f7d09bf1732d4a8d214c382e1947a97949c4d380 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 17 Dec 2019 10:11:12 +0100 Subject: [PATCH 17/18] Slight refactoring on ListSHortUrlsCommand --- .../src/Command/ShortUrl/GetVisitsCommand.php | 4 --- .../Command/ShortUrl/ListShortUrlsCommand.php | 27 +++++-------------- 2 files changed, 7 insertions(+), 24 deletions(-) diff --git a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php index 416c1bfb..7a51ac0b 100644 --- a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; -use Cake\Chronos\Chronos; use Shlinkio\Shlink\CLI\Command\Util\AbstractWithDateRangeCommand; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; @@ -14,14 +13,11 @@ use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; -use Throwable; use function Functional\map; use function Functional\select_keys; -use function sprintf; class GetVisitsCommand extends AbstractWithDateRangeCommand { diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 52918afa..01080189 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -9,7 +9,6 @@ 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\Rest\DataTransformerInterface; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; @@ -45,14 +44,14 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand /** @var ShortUrlServiceInterface */ private $shortUrlService; - /** @var array */ - private $domainConfig; + /** @var ShortUrlDataTransformer */ + private $transformer; public function __construct(ShortUrlServiceInterface $shortUrlService, array $domainConfig) { parent::__construct(); $this->shortUrlService = $shortUrlService; - $this->domainConfig = $domainConfig; + $this->transformer = new ShortUrlDataTransformer($domainConfig); } protected function doConfigure(): void @@ -102,6 +101,7 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand protected function execute(InputInterface $input, OutputInterface $output): ?int { $io = new SymfonyStyle($input, $output); + $page = (int) $input->getOption('page'); $searchTerm = $input->getOption('searchTerm'); $tags = $input->getOption('tags'); @@ -111,20 +111,8 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand $endDate = $this->getDateOption($input, $output, 'endDate'); $orderBy = $this->processOrderBy($input); - $transformer = new ShortUrlDataTransformer($this->domainConfig); - do { - $result = $this->renderPage( - $output, - $page, - $searchTerm, - $tags, - $showTags, - $startDate, - $endDate, - $orderBy, - $transformer - ); + $result = $this->renderPage($output, $page, $searchTerm, $tags, $showTags, $startDate, $endDate, $orderBy); $page++; $continue = $this->isLastPage($result) @@ -146,8 +134,7 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand bool $showTags, ?Chronos $startDate, ?Chronos $endDate, - $orderBy, - DataTransformerInterface $transformer + $orderBy ): Paginator { $result = $this->shortUrlService->listShortUrls( $page, @@ -164,7 +151,7 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand $rows = []; foreach ($result as $row) { - $shortUrl = $transformer->transform($row); + $shortUrl = $this->transformer->transform($row); if ($showTags) { $shortUrl['tags'] = implode(', ', $shortUrl['tags']); } else { From 524914fd35a5e3719213018534a3196de3ef26da Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 17 Dec 2019 10:14:18 +0100 Subject: [PATCH 18/18] Updated changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6fc5f19..27e08896 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this > After Shlink v2 is released, both API versions will behave like API v2. +* [#575](https://github.com/shlinkio/shlink/issues/575) Added support to filter short URL lists by date ranges. + + * The `GET /short-urls` endpoint now accepts the `startDate` and `endDate` query params. + * The `short-urls:list` command now allows `--startDate` and `--endDate` flags to be optionally provided. + #### Changed * [#492](https://github.com/shlinkio/shlink/issues/492) Updated to monolog 2, together with other dependencies, like Symfony 5 and infection-php.