Skip to content

Commit

Permalink
Support filtering orphan visits by type in VisitRepository
Browse files Browse the repository at this point in the history
  • Loading branch information
acelaya committed Feb 10, 2024
1 parent 17792a1 commit 46acf4d
Show file tree
Hide file tree
Showing 13 changed files with 154 additions and 61 deletions.
12 changes: 12 additions & 0 deletions module/Core/src/Visit/Model/OrphanVisitType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

declare(strict_types=1);

namespace Shlinkio\Shlink\Core\Visit\Model;

enum OrphanVisitType: string
{
case INVALID_SHORT_URL = 'invalid_short_url';
case BASE_URL = 'base_url';
case REGULAR_404 = 'regular_404';
}
6 changes: 3 additions & 3 deletions module/Core/src/Visit/Model/VisitType.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ enum VisitType: string
{
case VALID_SHORT_URL = 'valid_short_url';
case IMPORTED = 'imported';
case INVALID_SHORT_URL = 'invalid_short_url';
case BASE_URL = 'base_url';
case REGULAR_404 = 'regular_404';
case INVALID_SHORT_URL = OrphanVisitType::INVALID_SHORT_URL->value;
case BASE_URL = OrphanVisitType::BASE_URL->value;
case REGULAR_404 = OrphanVisitType::REGULAR_404->value;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

use Shlinkio\Shlink\Core\Paginator\Adapter\AbstractCacheableCountPaginatorAdapter;
use Shlinkio\Shlink\Core\Visit\Model\VisitsParams;
use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering;
use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface;
use Shlinkio\Shlink\Rest\Entity\ApiKey;

Expand All @@ -22,7 +22,7 @@ public function __construct(

protected function doCount(): int
{
return $this->repo->countOrphanVisits(new VisitsCountFiltering(
return $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(
dateRange: $this->params->dateRange,
excludeBots: $this->params->excludeBots,
apiKey: $this->apiKey,
Expand All @@ -31,7 +31,7 @@ protected function doCount(): int

public function getSlice(int $offset, int $length): iterable
{
return $this->repo->findOrphanVisits(new VisitsListFiltering(
return $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(
dateRange: $this->params->dateRange,
excludeBots: $this->params->excludeBots,
apiKey: $this->apiKey,
Expand Down
21 changes: 21 additions & 0 deletions module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

declare(strict_types=1);

namespace Shlinkio\Shlink\Core\Visit\Persistence;

use Shlinkio\Shlink\Common\Util\DateRange;
use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitType;
use Shlinkio\Shlink\Rest\Entity\ApiKey;

class OrphanVisitsCountFiltering extends VisitsCountFiltering
{
public function __construct(
?DateRange $dateRange = null,
bool $excludeBots = false,
?ApiKey $apiKey = null,
public readonly ?OrphanVisitType $type = null,
) {
parent::__construct($dateRange, $excludeBots, $apiKey);
}
}
23 changes: 23 additions & 0 deletions module/Core/src/Visit/Persistence/OrphanVisitsListFiltering.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);

namespace Shlinkio\Shlink\Core\Visit\Persistence;

use Shlinkio\Shlink\Common\Util\DateRange;
use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitType;
use Shlinkio\Shlink\Rest\Entity\ApiKey;

final class OrphanVisitsListFiltering extends OrphanVisitsCountFiltering
{
public function __construct(
?DateRange $dateRange = null,
bool $excludeBots = false,
?ApiKey $apiKey = null,
?OrphanVisitType $type = null,
public readonly ?int $limit = null,
public readonly ?int $offset = null,
) {
parent::__construct($dateRange, $excludeBots, $apiKey, $type);
}
}
5 changes: 0 additions & 5 deletions module/Core/src/Visit/Persistence/VisitsCountFiltering.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,4 @@ public function __construct(
public readonly ?ApiKey $apiKey = null,
) {
}

public static function withApiKey(?ApiKey $apiKey): self
{
return new self(apiKey: $apiKey);
}
}
15 changes: 12 additions & 3 deletions module/Core/src/Visit/Repository/VisitRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface;
use Shlinkio\Shlink\Core\Visit\Entity\Visit;
use Shlinkio\Shlink\Core\Visit\Entity\VisitLocation;
use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering;
use Shlinkio\Shlink\Core\Visit\Spec\CountOfNonOrphanVisits;
Expand Down Expand Up @@ -138,18 +140,25 @@ private function createVisitsByDomainQueryBuilder(string $domain, VisitsCountFil
return $qb;
}

public function findOrphanVisits(VisitsListFiltering $filtering): array
public function findOrphanVisits(OrphanVisitsListFiltering $filtering): array
{
if ($filtering->apiKey?->hasRole(Role::NO_ORPHAN_VISITS)) {
return [];
}

$qb = $this->createAllVisitsQueryBuilder($filtering);
$qb->andWhere($qb->expr()->isNull('v.shortUrl'));

// Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later
if ($filtering->type) {
$conn = $this->getEntityManager()->getConnection();
$qb->andWhere($qb->expr()->eq('v.type', $conn->quote($filtering->type->value)));
}

return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit, $filtering->offset);
}

public function countOrphanVisits(VisitsCountFiltering $filtering): int
public function countOrphanVisits(OrphanVisitsCountFiltering $filtering): int
{
if ($filtering->apiKey?->hasRole(Role::NO_ORPHAN_VISITS)) {
return 0;
Expand All @@ -176,7 +185,7 @@ public function countNonOrphanVisits(VisitsCountFiltering $filtering): int
return (int) $this->matchSingleScalarResult(new CountOfNonOrphanVisits($filtering));
}

private function createAllVisitsQueryBuilder(VisitsListFiltering $filtering): QueryBuilder
private function createAllVisitsQueryBuilder(VisitsListFiltering|OrphanVisitsListFiltering $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 provided by the caller, it's reasonably safe
Expand Down
6 changes: 4 additions & 2 deletions module/Core/src/Visit/Repository/VisitRepositoryInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepositoryInterface;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\Visit\Entity\Visit;
use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering;

Expand Down Expand Up @@ -37,9 +39,9 @@ public function countVisitsByDomain(string $domain, VisitsCountFiltering $filter
/**
* @return Visit[]
*/
public function findOrphanVisits(VisitsListFiltering $filtering): array;
public function findOrphanVisits(OrphanVisitsListFiltering $filtering): array;

public function countOrphanVisits(VisitsCountFiltering $filtering): int;
public function countOrphanVisits(OrphanVisitsCountFiltering $filtering): int;

/**
* @return Visit[]
Expand Down
8 changes: 6 additions & 2 deletions module/Core/src/Visit/Spec/CountOfOrphanVisits.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
use Happyr\DoctrineSpecification\Specification\BaseSpecification;
use Happyr\DoctrineSpecification\Specification\Specification;
use Shlinkio\Shlink\Core\Spec\InDateRange;
use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering;

class CountOfOrphanVisits extends BaseSpecification
{
public function __construct(private VisitsCountFiltering $filtering)
public function __construct(private readonly OrphanVisitsCountFiltering $filtering)
{
parent::__construct();
}
Expand All @@ -28,6 +28,10 @@ protected function getSpec(): Specification
$conditions[] = Spec::eq('potentialBot', false);
}

if ($this->filtering->type) {
$conditions[] = Spec::eq('type', $this->filtering->type->value);
}

return Spec::countOf(Spec::andX(...$conditions));
}
}
7 changes: 4 additions & 3 deletions module/Core/src/Visit/VisitsStatsHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\OrphanVisitsPaginatorAdapter;
use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\ShortUrlVisitsPaginatorAdapter;
use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\TagVisitsPaginatorAdapter;
use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering;
use Shlinkio\Shlink\Core\Visit\Repository\VisitRepository;
use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface;
Expand All @@ -42,13 +43,13 @@ public function getVisitsStats(?ApiKey $apiKey = null): VisitsStats
$visitsRepo = $this->em->getRepository(Visit::class);

return new VisitsStats(
nonOrphanVisitsTotal: $visitsRepo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($apiKey)),
orphanVisitsTotal: $visitsRepo->countOrphanVisits(VisitsCountFiltering::withApiKey($apiKey)),
nonOrphanVisitsTotal: $visitsRepo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey)),
orphanVisitsTotal: $visitsRepo->countOrphanVisits(new OrphanVisitsCountFiltering(apiKey: $apiKey)),
nonOrphanVisitsNonBots: $visitsRepo->countNonOrphanVisits(
new VisitsCountFiltering(excludeBots: true, apiKey: $apiKey),
),
orphanVisitsNonBots: $visitsRepo->countOrphanVisits(
new VisitsCountFiltering(excludeBots: true, apiKey: $apiKey),
new OrphanVisitsCountFiltering(excludeBots: true, apiKey: $apiKey),
),
);
}
Expand Down
80 changes: 52 additions & 28 deletions module/Core/test-db/Visit/Repository/VisitRepositoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter;
use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver;
use Shlinkio\Shlink\Core\Visit\Entity\Visit;
use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitType;
use Shlinkio\Shlink\Core\Visit\Model\Visitor;
use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering;
use Shlinkio\Shlink\Core\Visit\Repository\VisitRepository;
Expand Down Expand Up @@ -305,10 +308,12 @@ public function countVisitsReturnsExpectedResultBasedOnApiKey(): void
$this->getEntityManager()->flush();

self::assertEquals(4 + 5 + 7, $this->repo->countNonOrphanVisits(new VisitsCountFiltering()));
self::assertEquals(4, $this->repo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($apiKey1)));
self::assertEquals(5 + 7, $this->repo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($apiKey2)));
self::assertEquals(4 + 7, $this->repo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($domainApiKey)));
self::assertEquals(0, $this->repo->countOrphanVisits(VisitsCountFiltering::withApiKey($noOrphanVisitsApiKey)));
self::assertEquals(4, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey1)));
self::assertEquals(5 + 7, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey2)));
self::assertEquals(4 + 7, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $domainApiKey)));
self::assertEquals(0, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(
apiKey: $noOrphanVisitsApiKey,
)));
self::assertEquals(4, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::since(
Chronos::parse('2016-01-05')->startOfDay(),
))));
Expand All @@ -319,8 +324,8 @@ public function countVisitsReturnsExpectedResultBasedOnApiKey(): void
Chronos::parse('2016-01-07')->startOfDay(),
), false, $apiKey2)));
self::assertEquals(3 + 5, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(null, true, $apiKey2)));
self::assertEquals(4, $this->repo->countOrphanVisits(new VisitsCountFiltering()));
self::assertEquals(3, $this->repo->countOrphanVisits(new VisitsCountFiltering(null, true)));
self::assertEquals(4, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering()));
self::assertEquals(3, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(excludeBots: true)));
}

#[Test]
Expand Down Expand Up @@ -353,27 +358,36 @@ public function findOrphanVisitsReturnsExpectedResult(): void

$this->getEntityManager()->flush();

self::assertCount(0, $this->repo->findOrphanVisits(new VisitsListFiltering(apiKey: $noOrphanVisitsApiKey)));
self::assertCount(18, $this->repo->findOrphanVisits(new VisitsListFiltering()));
self::assertCount(15, $this->repo->findOrphanVisits(new VisitsListFiltering(null, true)));
self::assertCount(5, $this->repo->findOrphanVisits(new VisitsListFiltering(null, false, null, 5)));
self::assertCount(10, $this->repo->findOrphanVisits(new VisitsListFiltering(null, false, null, 15, 8)));
self::assertCount(9, $this->repo->findOrphanVisits(new VisitsListFiltering(
DateRange::since(Chronos::parse('2020-01-04')),
false,
null,
15,
self::assertCount(0, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(
apiKey: $noOrphanVisitsApiKey,
)));
self::assertCount(2, $this->repo->findOrphanVisits(new VisitsListFiltering(
DateRange::between(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')),
false,
null,
6,
4,
self::assertCount(18, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering()));
self::assertCount(15, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(excludeBots: true)));
self::assertCount(5, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(limit: 5)));
self::assertCount(10, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(limit: 15, offset: 8)));
self::assertCount(9, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(
dateRange: DateRange::since(Chronos::parse('2020-01-04')),
limit: 15,
)));
self::assertCount(2, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(
dateRange: DateRange::between(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')),
limit: 6,
offset: 4,
)));
self::assertCount(3, $this->repo->findOrphanVisits(new VisitsListFiltering(
self::assertCount(2, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(
dateRange: DateRange::between(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')),
type: OrphanVisitType::INVALID_SHORT_URL,
)));
self::assertCount(3, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(
DateRange::until(Chronos::parse('2020-01-01')),
)));
self::assertCount(6, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(
type: OrphanVisitType::REGULAR_404,
)));
self::assertCount(4, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(
type: OrphanVisitType::BASE_URL,
limit: 4,
)));
}

#[Test]
Expand All @@ -400,17 +414,27 @@ public function countOrphanVisitsReturnsExpectedResult(): void

$this->getEntityManager()->flush();

self::assertEquals(18, $this->repo->countOrphanVisits(new VisitsCountFiltering()));
self::assertEquals(18, $this->repo->countOrphanVisits(new VisitsCountFiltering(DateRange::allTime())));
self::assertEquals(18, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering()));
self::assertEquals(18, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(DateRange::allTime())));
self::assertEquals(9, $this->repo->countOrphanVisits(
new VisitsCountFiltering(DateRange::since(Chronos::parse('2020-01-04'))),
new OrphanVisitsCountFiltering(DateRange::since(Chronos::parse('2020-01-04'))),
));
self::assertEquals(6, $this->repo->countOrphanVisits(new VisitsCountFiltering(
self::assertEquals(6, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(
DateRange::between(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')),
)));
self::assertEquals(3, $this->repo->countOrphanVisits(
new VisitsCountFiltering(DateRange::until(Chronos::parse('2020-01-01'))),
new OrphanVisitsCountFiltering(DateRange::until(Chronos::parse('2020-01-01'))),
));
self::assertEquals(2, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(
dateRange: DateRange::between(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')),
type: OrphanVisitType::BASE_URL,
)));
self::assertEquals(6, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(
type: OrphanVisitType::INVALID_SHORT_URL,
)));
self::assertEquals(6, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(
type: OrphanVisitType::REGULAR_404,
)));
}

#[Test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
use Shlinkio\Shlink\Core\Visit\Model\Visitor;
use Shlinkio\Shlink\Core\Visit\Model\VisitsParams;
use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\OrphanVisitsPaginatorAdapter;
use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering;
use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering;
use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface;
use Shlinkio\Shlink\Rest\Entity\ApiKey;

Expand All @@ -38,7 +38,7 @@ public function countDelegatesToRepository(): void
{
$expectedCount = 5;
$this->repo->expects($this->once())->method('countOrphanVisits')->with(
new VisitsCountFiltering($this->params->dateRange, apiKey: $this->apiKey),
new OrphanVisitsCountFiltering($this->params->dateRange, apiKey: $this->apiKey),
)->willReturn($expectedCount);

$result = $this->adapter->getNbResults();
Expand All @@ -55,12 +55,12 @@ public function getSliceDelegatesToRepository(int $limit, int $offset): void
{
$visitor = Visitor::emptyInstance();
$list = [Visit::forRegularNotFound($visitor), Visit::forInvalidShortUrl($visitor)];
$this->repo->expects($this->once())->method('findOrphanVisits')->with(new VisitsListFiltering(
$this->params->dateRange,
$this->params->excludeBots,
$this->apiKey,
$limit,
$offset,
$this->repo->expects($this->once())->method('findOrphanVisits')->with(new OrphanVisitsListFiltering(
dateRange: $this->params->dateRange,
excludeBots: $this->params->excludeBots,
apiKey: $this->apiKey,
limit: $limit,
offset: $offset,
))->willReturn($list);

$result = $this->adapter->getSlice($offset, $limit);
Expand Down
Loading

0 comments on commit 46acf4d

Please sign in to comment.