From 839ca318218f2446ec86c66944adfc8a2789f6fe Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 16 Dec 2019 21:46:27 +0100 Subject: [PATCH] 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;