From 60c0ca3ae5098924aa8fc9381b721c08801057c0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 16 Jan 2022 10:56:37 +0100 Subject: [PATCH] Changed VisitsCountFiltering and VisitsListFiltering so that they encapsulate an ApiKey instead of a Spec --- module/Core/src/Repository/VisitRepository.php | 12 +++++++----- .../Core/src/Repository/VisitRepositoryInterface.php | 1 + .../Adapter/ShortUrlVisitsPaginatorAdapter.php | 8 ++++---- .../Paginator/Adapter/TagVisitsPaginatorAdapter.php | 4 ++-- .../src/Visit/Persistence/VisitsCountFiltering.php | 8 ++++---- .../src/Visit/Persistence/VisitsListFiltering.php | 6 +++--- module/Core/src/Visit/VisitsStatsHelper.php | 9 +++++---- .../Core/test-db/Repository/VisitRepositoryTest.php | 8 ++++---- .../Adapter/VisitsForTagPaginatorAdapterTest.php | 2 +- .../Paginator/Adapter/VisitsPaginatorAdapterTest.php | 4 ++-- 10 files changed, 33 insertions(+), 29 deletions(-) diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index 4e83c03e..b8b4688c 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -104,10 +104,10 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo ): QueryBuilder { /** @var ShortUrlRepositoryInterface $shortUrlRepo */ $shortUrlRepo = $this->getEntityManager()->getRepository(ShortUrl::class); - $shortUrlId = $shortUrlRepo->findOne($identifier, $filtering->spec())?->getId() ?? '-1'; + $shortUrlId = $shortUrlRepo->findOne($identifier, $filtering->apiKey()?->spec())?->getId() ?? '-1'; // Parameters in this query need to be part of the query itself, as we need to use it a sub-query later - // Since they are not strictly provided by the caller, it's reasonably safe + // Since they are not provided by the caller, it's reasonably safe $qb = $this->getEntityManager()->createQueryBuilder(); $qb->from(Visit::class, 'v') ->where($qb->expr()->eq('v.shortUrl', $shortUrlId)); @@ -150,7 +150,7 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo } $this->applyDatesInline($qb, $filtering->dateRange()); - $this->applySpecification($qb, $filtering->spec(), 'v'); // FIXME This is actually binding arguments + $this->applySpecification($qb, $filtering->apiKey()?->spec(true), 'v'); return $qb; } @@ -175,11 +175,13 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo $qb = $this->createAllVisitsQueryBuilder($filtering); $qb->andWhere($qb->expr()->isNotNull('v.shortUrl')); - $this->applySpecification($qb, $filtering->spec()); + $this->applySpecification($qb, $filtering->apiKey()?->spec(true)); return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit(), $filtering->offset()); } + // TODO This should support counting in a date range or excluding bots + // TODO Rename to countNonOrphanVisits public function countVisits(?ApiKey $apiKey = null): int { return (int) $this->matchSingleScalarResult(new CountOfShortUrlVisits($apiKey)); @@ -188,7 +190,7 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo private function createAllVisitsQueryBuilder(VisitsListFiltering $filtering): QueryBuilder { // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later - // Since they are not strictly provided by the caller, it's reasonably safe + // Since they are not provided by the caller, it's reasonably safe $qb = $this->getEntityManager()->createQueryBuilder(); $qb->from(Visit::class, 'v'); diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php index a3c6497c..d541e20e 100644 --- a/module/Core/src/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Repository/VisitRepositoryInterface.php @@ -12,6 +12,7 @@ use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; use Shlinkio\Shlink\Rest\Entity\ApiKey; +// TODO Split into VisitsListsRepository and VisitsLocationRepository interface VisitRepositoryInterface extends ObjectRepository, EntitySpecificationRepositoryInterface { public const DEFAULT_BLOCK_SIZE = 10000; diff --git a/module/Core/src/Visit/Paginator/Adapter/ShortUrlVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/ShortUrlVisitsPaginatorAdapter.php index 0d33b867..2e47fbf8 100644 --- a/module/Core/src/Visit/Paginator/Adapter/ShortUrlVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/ShortUrlVisitsPaginatorAdapter.php @@ -4,13 +4,13 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Visit\Paginator\Adapter; -use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Paginator\Adapter\AbstractCacheableCountPaginatorAdapter; use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Rest\Entity\ApiKey; class ShortUrlVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { @@ -18,7 +18,7 @@ class ShortUrlVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdap private VisitRepositoryInterface $visitRepository, private ShortUrlIdentifier $identifier, private VisitsParams $params, - private ?Specification $spec, + private ?ApiKey $apiKey, ) { } @@ -29,7 +29,7 @@ class ShortUrlVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdap new VisitsListFiltering( $this->params->getDateRange(), $this->params->excludeBots(), - $this->spec, + $this->apiKey, $length, $offset, ), @@ -43,7 +43,7 @@ class ShortUrlVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdap new VisitsCountFiltering( $this->params->getDateRange(), $this->params->excludeBots(), - $this->spec, + $this->apiKey, ), ); } diff --git a/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php index f71c1df3..162b6cba 100644 --- a/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php @@ -28,7 +28,7 @@ class TagVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter new VisitsListFiltering( $this->params->getDateRange(), $this->params->excludeBots(), - $this->apiKey?->spec(true), + $this->apiKey, $length, $offset, ), @@ -42,7 +42,7 @@ class TagVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter new VisitsCountFiltering( $this->params->getDateRange(), $this->params->excludeBots(), - $this->apiKey?->spec(true), + $this->apiKey, ), ); } diff --git a/module/Core/src/Visit/Persistence/VisitsCountFiltering.php b/module/Core/src/Visit/Persistence/VisitsCountFiltering.php index bf459768..c0389ef6 100644 --- a/module/Core/src/Visit/Persistence/VisitsCountFiltering.php +++ b/module/Core/src/Visit/Persistence/VisitsCountFiltering.php @@ -4,15 +4,15 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Visit\Persistence; -use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Rest\Entity\ApiKey; class VisitsCountFiltering { public function __construct( private ?DateRange $dateRange = null, private bool $excludeBots = false, - private ?Specification $spec = null, + private ?ApiKey $apiKey = null, ) { } @@ -26,8 +26,8 @@ class VisitsCountFiltering return $this->excludeBots; } - public function spec(): ?Specification + public function apiKey(): ?ApiKey { - return $this->spec; + return $this->apiKey; } } diff --git a/module/Core/src/Visit/Persistence/VisitsListFiltering.php b/module/Core/src/Visit/Persistence/VisitsListFiltering.php index fb715182..b17964a6 100644 --- a/module/Core/src/Visit/Persistence/VisitsListFiltering.php +++ b/module/Core/src/Visit/Persistence/VisitsListFiltering.php @@ -4,19 +4,19 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Visit\Persistence; -use Happyr\DoctrineSpecification\Specification\Specification; use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Rest\Entity\ApiKey; final class VisitsListFiltering extends VisitsCountFiltering { public function __construct( ?DateRange $dateRange = null, bool $excludeBots = false, - ?Specification $spec = null, + ?ApiKey $apiKey = null, private ?int $limit = null, private ?int $offset = null, ) { - parent::__construct($dateRange, $excludeBots, $spec); + parent::__construct($dateRange, $excludeBots, $apiKey); } public function limit(): ?int diff --git a/module/Core/src/Visit/VisitsStatsHelper.php b/module/Core/src/Visit/VisitsStatsHelper.php index d0463d91..91f4d4fa 100644 --- a/module/Core/src/Visit/VisitsStatsHelper.php +++ b/module/Core/src/Visit/VisitsStatsHelper.php @@ -51,18 +51,19 @@ class VisitsStatsHelper implements VisitsStatsHelperInterface VisitsParams $params, ?ApiKey $apiKey = null, ): Paginator { - $spec = $apiKey?->spec(); - /** @var ShortUrlRepositoryInterface $repo */ $repo = $this->em->getRepository(ShortUrl::class); - if (! $repo->shortCodeIsInUse($identifier, $spec)) { + if (! $repo->shortCodeIsInUse($identifier, $apiKey?->spec())) { throw ShortUrlNotFoundException::fromNotFound($identifier); } /** @var VisitRepositoryInterface $repo */ $repo = $this->em->getRepository(Visit::class); - return $this->createPaginator(new ShortUrlVisitsPaginatorAdapter($repo, $identifier, $params, $spec), $params); + return $this->createPaginator( + new ShortUrlVisitsPaginatorAdapter($repo, $identifier, $params, $apiKey), + $params, + ); } /** diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index 0f90fc6a..390b1707 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -192,19 +192,19 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertNotEmpty($this->repo->findVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode1), - new VisitsListFiltering(null, false, $adminApiKey->spec()), + new VisitsListFiltering(null, false, $adminApiKey), )); self::assertNotEmpty($this->repo->findVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode2), - new VisitsListFiltering(null, false, $adminApiKey->spec()), + new VisitsListFiltering(null, false, $adminApiKey), )); self::assertEmpty($this->repo->findVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode1), - new VisitsListFiltering(null, false, $restrictedApiKey->spec()), + new VisitsListFiltering(null, false, $restrictedApiKey), )); self::assertNotEmpty($this->repo->findVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode2), - new VisitsListFiltering(null, false, $restrictedApiKey->spec()), + new VisitsListFiltering(null, false, $restrictedApiKey), )); } diff --git a/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php b/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php index cad1090b..442e7128 100644 --- a/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php +++ b/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php @@ -53,7 +53,7 @@ class VisitsForTagPaginatorAdapterTest extends TestCase $adapter = $this->createAdapter($apiKey); $countVisits = $this->repo->countVisitsByTag( 'foo', - new VisitsCountFiltering(DateRange::emptyInstance(), false, $apiKey->spec()), + new VisitsCountFiltering(DateRange::emptyInstance(), false, $apiKey), )->willReturn(3); for ($i = 0; $i < $count; $i++) { diff --git a/module/Core/test/Visit/Paginator/Adapter/VisitsPaginatorAdapterTest.php b/module/Core/test/Visit/Paginator/Adapter/VisitsPaginatorAdapterTest.php index bc9ca967..41b873df 100644 --- a/module/Core/test/Visit/Paginator/Adapter/VisitsPaginatorAdapterTest.php +++ b/module/Core/test/Visit/Paginator/Adapter/VisitsPaginatorAdapterTest.php @@ -54,7 +54,7 @@ class VisitsPaginatorAdapterTest extends TestCase $adapter = $this->createAdapter($apiKey); $countVisits = $this->repo->countVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain(''), - new VisitsCountFiltering(DateRange::emptyInstance(), false, $apiKey->spec()), + new VisitsCountFiltering(DateRange::emptyInstance(), false, $apiKey), )->willReturn(3); for ($i = 0; $i < $count; $i++) { @@ -70,7 +70,7 @@ class VisitsPaginatorAdapterTest extends TestCase $this->repo->reveal(), new ShortUrlIdentifier(''), VisitsParams::fromRawData([]), - $apiKey?->spec(), + $apiKey, ); } }