diff --git a/module/Core/src/Repository/TagRepository.php b/module/Core/src/Repository/TagRepository.php index 0182908c..1aa35603 100644 --- a/module/Core/src/Repository/TagRepository.php +++ b/module/Core/src/Repository/TagRepository.php @@ -60,9 +60,7 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito } $apiKey = $filtering?->apiKey(); - if ($apiKey !== null) { - $this->applySpecification($subQb, $apiKey->spec(false, 'shortUrls'), 't'); - } + $this->applySpecification($subQb, $apiKey?->spec(false, 'shortUrls'), 't'); $subQuery = $subQb->getQuery(); $subQuerySql = $subQuery->getSQL(); diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index d4faa836..4e83c03e 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -104,8 +104,7 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo ): QueryBuilder { /** @var ShortUrlRepositoryInterface $shortUrlRepo */ $shortUrlRepo = $this->getEntityManager()->getRepository(ShortUrl::class); - $shortUrl = $shortUrlRepo->findOne($identifier, $filtering->spec()); - $shortUrlId = $shortUrl?->getId() ?? '-1'; + $shortUrlId = $shortUrlRepo->findOne($identifier, $filtering->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 @@ -139,8 +138,7 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo private function createVisitsByTagQueryBuilder(string $tag, VisitsCountFiltering $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 + // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later. $qb = $this->getEntityManager()->createQueryBuilder(); $qb->from(Visit::class, 'v') ->join('v.shortUrl', 's') @@ -152,25 +150,15 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo } $this->applyDatesInline($qb, $filtering->dateRange()); - $this->applySpecification($qb, $filtering->spec(), 'v'); + $this->applySpecification($qb, $filtering->spec(), 'v'); // FIXME This is actually binding arguments return $qb; } public function findOrphanVisits(VisitsListFiltering $filtering): array { - // 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 - $qb = $this->getEntityManager()->createQueryBuilder(); - $qb->from(Visit::class, 'v') - ->where($qb->expr()->isNull('v.shortUrl')); - - if ($filtering->excludeBots()) { - $qb->andWhere($qb->expr()->eq('v.potentialBot', 'false')); - } - - $this->applyDatesInline($qb, $filtering->dateRange()); - + $qb = $this->createAllVisitsQueryBuilder($filtering); + $qb->andWhere($qb->expr()->isNull('v.shortUrl')); return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit(), $filtering->offset()); } @@ -179,11 +167,40 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return (int) $this->matchSingleScalarResult(new CountOfOrphanVisits($filtering)); } + /** + * @return Visit[] + */ + public function findNonOrphanVisits(VisitsListFiltering $filtering): array + { + $qb = $this->createAllVisitsQueryBuilder($filtering); + $qb->andWhere($qb->expr()->isNotNull('v.shortUrl')); + + $this->applySpecification($qb, $filtering->spec()); + + return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit(), $filtering->offset()); + } + public function countVisits(?ApiKey $apiKey = null): int { return (int) $this->matchSingleScalarResult(new CountOfShortUrlVisits($apiKey)); } + 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 + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->from(Visit::class, 'v'); + + if ($filtering->excludeBots()) { + $qb->andWhere($qb->expr()->eq('v.potentialBot', 'false')); + } + + $this->applyDatesInline($qb, $filtering->dateRange()); + + return $qb; + } + private function applyDatesInline(QueryBuilder $qb, ?DateRange $dateRange): void { $conn = $this->getEntityManager()->getConnection(); diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php index 29360ddd..a3c6497c 100644 --- a/module/Core/src/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Repository/VisitRepositoryInterface.php @@ -52,10 +52,10 @@ interface VisitRepositoryInterface extends ObjectRepository, EntitySpecification public function countOrphanVisits(VisitsCountFiltering $filtering): int; -// /** -// * @return Visit[] -// */ -// public function findExistingVisits(VisitsListFiltering $filtering): array; + /** + * @return Visit[] + */ + public function findNonOrphanVisits(VisitsListFiltering $filtering): array; public function countVisits(?ApiKey $apiKey = null): int; } diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index 475cf374..0f90fc6a 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -29,6 +29,9 @@ use function Functional\map; use function is_string; use function range; use function sprintf; +use function str_pad; + +use const STR_PAD_LEFT; class VisitRepositoryTest extends DatabaseTestCase { @@ -388,6 +391,49 @@ class VisitRepositoryTest extends DatabaseTestCase )); } + /** @test */ + public function findNonOrphanVisitsReturnsExpectedResult(): void + { + $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['longUrl' => '1'])); + $this->getEntityManager()->persist($shortUrl); + $this->createVisitsForShortUrl($shortUrl, 7); + + $shortUrl2 = ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['longUrl' => '2'])); + $this->getEntityManager()->persist($shortUrl2); + $this->createVisitsForShortUrl($shortUrl2, 4); + + $shortUrl3 = ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['longUrl' => '3'])); + $this->getEntityManager()->persist($shortUrl3); + $this->createVisitsForShortUrl($shortUrl3, 10); + + $this->getEntityManager()->flush(); + + self::assertCount(21, $this->repo->findNonOrphanVisits(new VisitsListFiltering())); + self::assertCount(21, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::emptyInstance()))); + self::assertCount(7, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::withStartDate( + Chronos::parse('2016-01-05')->endOfDay(), + )))); + self::assertCount(12, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::withEndDate( + Chronos::parse('2016-01-04')->endOfDay(), + )))); + self::assertCount(6, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::withStartAndEndDate( + Chronos::parse('2016-01-03')->startOfDay(), + Chronos::parse('2016-01-04')->endOfDay(), + )))); + self::assertCount(13, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::withStartAndEndDate( + Chronos::parse('2016-01-03')->startOfDay(), + Chronos::parse('2016-01-08')->endOfDay(), + )))); + self::assertCount(3, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::withStartAndEndDate( + Chronos::parse('2016-01-03')->startOfDay(), + Chronos::parse('2016-01-08')->endOfDay(), + ), false, null, 10, 10))); + self::assertCount(15, $this->repo->findNonOrphanVisits(new VisitsListFiltering(null, true))); + self::assertCount(10, $this->repo->findNonOrphanVisits(new VisitsListFiltering(null, false, null, 10))); + self::assertCount(1, $this->repo->findNonOrphanVisits(new VisitsListFiltering(null, false, null, 10, 20))); + self::assertCount(5, $this->repo->findNonOrphanVisits(new VisitsListFiltering(null, false, null, 5, 5))); + } + /** * @return array{string, string, ShortUrl} */ @@ -429,7 +475,7 @@ class VisitRepositoryTest extends DatabaseTestCase $shortUrl, $botsAmount < 1 ? Visitor::emptyInstance() : Visitor::botInstance(), ), - Chronos::parse(sprintf('2016-01-0%s', $i + 1)), + Chronos::parse(sprintf('2016-01-%s', str_pad((string) ($i + 1), 2, '0', STR_PAD_LEFT)))->startOfDay(), ); $botsAmount--;