diff --git a/module/Core/src/Model/Visitor.php b/module/Core/src/Model/Visitor.php index e9bdc36e..b73ed68a 100644 --- a/module/Core/src/Model/Visitor.php +++ b/module/Core/src/Model/Visitor.php @@ -53,6 +53,11 @@ final class Visitor return new self('', '', null, ''); } + public static function botInstance(): self + { + return new self('cf-facebook', '', null, ''); + } + public function getUserAgent(): string { return $this->userAgent; diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index 5d3bacbc..bc3f958b 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -114,6 +114,10 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo $qb->from(Visit::class, 'v') ->where($qb->expr()->eq('v.shortUrl', $shortUrlId)); + if ($filtering->excludeBots()) { + $qb->andWhere($qb->expr()->eq('v.potentialBot', 'false')); + } + // Apply date range filtering $this->applyDatesInline($qb, $filtering->dateRange()); @@ -144,6 +148,10 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo ->join('s.tags', 't') ->where($qb->expr()->eq('t.name', '\'' . $tag . '\'')); // This needs to be concatenated, not bound + if ($filtering->excludeBots()) { + $qb->andWhere($qb->expr()->eq('v.potentialBot', 'false')); + } + $this->applyDatesInline($qb, $filtering->dateRange()); $this->applySpecification($qb, $filtering->spec(), 'v'); @@ -158,6 +166,10 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo $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()); return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit(), $filtering->offset()); @@ -165,7 +177,7 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo public function countOrphanVisits(VisitsCountFiltering $filtering): int { - return (int) $this->matchSingleScalarResult(new CountOfOrphanVisits($filtering->dateRange())); + return (int) $this->matchSingleScalarResult(new CountOfOrphanVisits($filtering)); } public function countVisits(?ApiKey $apiKey = null): int diff --git a/module/Core/src/Visit/Spec/CountOfOrphanVisits.php b/module/Core/src/Visit/Spec/CountOfOrphanVisits.php index 97712944..b2cc9efd 100644 --- a/module/Core/src/Visit/Spec/CountOfOrphanVisits.php +++ b/module/Core/src/Visit/Spec/CountOfOrphanVisits.php @@ -7,24 +7,30 @@ namespace Shlinkio\Shlink\Core\Visit\Spec; use Happyr\DoctrineSpecification\Spec; use Happyr\DoctrineSpecification\Specification\BaseSpecification; use Happyr\DoctrineSpecification\Specification\Specification; -use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Spec\InDateRange; +use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; class CountOfOrphanVisits extends BaseSpecification { - private ?DateRange $dateRange; + private VisitsCountFiltering $filtering; - public function __construct(?DateRange $dateRange) + public function __construct(VisitsCountFiltering $filtering) { parent::__construct(); - $this->dateRange = $dateRange; + $this->filtering = $filtering; } protected function getSpec(): Specification { - return Spec::countOf(Spec::andX( + $conditions = [ Spec::isNull('shortUrl'), - new InDateRange($this->dateRange), - )); + new InDateRange($this->filtering->dateRange()), + ]; + + if ($this->filtering->excludeBots()) { + $conditions[] = Spec::eq('potentialBot', false); + } + + return Spec::countOf(Spec::andX(...$conditions)); } } diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index 02be1260..f77d7fd0 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -91,6 +91,7 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertCount(0, $this->repo->findVisitsByShortCode('invalid', null, new VisitsListFiltering())); self::assertCount(6, $this->repo->findVisitsByShortCode($shortCode, null, new VisitsListFiltering())); + self::assertCount(4, $this->repo->findVisitsByShortCode($shortCode, null, new VisitsListFiltering(null, true))); self::assertCount(3, $this->repo->findVisitsByShortCode($shortCode, $domain, new VisitsListFiltering())); self::assertCount(2, $this->repo->findVisitsByShortCode($shortCode, null, new VisitsListFiltering( DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), @@ -125,6 +126,10 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(0, $this->repo->countVisitsByShortCode('invalid', null, new VisitsCountFiltering())); self::assertEquals(6, $this->repo->countVisitsByShortCode($shortCode, null, new VisitsCountFiltering())); + self::assertEquals(4, $this->repo->countVisitsByShortCode($shortCode, null, new VisitsCountFiltering( + null, + true, + ))); self::assertEquals(3, $this->repo->countVisitsByShortCode($shortCode, $domain, new VisitsCountFiltering())); self::assertEquals(2, $this->repo->countVisitsByShortCode($shortCode, null, new VisitsCountFiltering( DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), @@ -154,6 +159,7 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertCount(0, $this->repo->findVisitsByTag('invalid', new VisitsListFiltering())); self::assertCount(18, $this->repo->findVisitsByTag($foo, new VisitsListFiltering())); + self::assertCount(12, $this->repo->findVisitsByTag($foo, new VisitsListFiltering(null, true))); self::assertCount(6, $this->repo->findVisitsByTag($foo, new VisitsListFiltering( DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ))); @@ -175,6 +181,7 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(0, $this->repo->countVisitsByTag('invalid', new VisitsCountFiltering())); self::assertEquals(12, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering())); + self::assertEquals(8, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering(null, true))); self::assertEquals(4, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering( DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ))); @@ -220,6 +227,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist(Visit::forBasePath(Visitor::emptyInstance())); $this->getEntityManager()->persist(Visit::forInvalidShortUrl(Visitor::emptyInstance())); $this->getEntityManager()->persist(Visit::forRegularNotFound(Visitor::emptyInstance())); + $this->getEntityManager()->persist(Visit::forRegularNotFound(Visitor::botInstance())); $this->getEntityManager()->flush(); @@ -227,7 +235,8 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(4, $this->repo->countVisits($apiKey1)); self::assertEquals(5 + 7, $this->repo->countVisits($apiKey2)); self::assertEquals(4 + 7, $this->repo->countVisits($domainApiKey)); - self::assertEquals(3, $this->repo->countOrphanVisits(new VisitsCountFiltering())); + self::assertEquals(4, $this->repo->countOrphanVisits(new VisitsCountFiltering())); + self::assertEquals(3, $this->repo->countOrphanVisits(new VisitsCountFiltering(null, true))); } /** @test */ @@ -237,9 +246,10 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 7); + $botsCount = 3; for ($i = 0; $i < 6; $i++) { $this->getEntityManager()->persist($this->setDateOnVisit( - Visit::forBasePath(Visitor::emptyInstance()), + Visit::forBasePath($botsCount < 1 ? Visitor::emptyInstance() : Visitor::botInstance()), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( @@ -250,11 +260,14 @@ class VisitRepositoryTest extends DatabaseTestCase Visit::forRegularNotFound(Visitor::emptyInstance()), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); + + $botsCount--; } $this->getEntityManager()->flush(); 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( @@ -338,13 +351,17 @@ class VisitRepositoryTest extends DatabaseTestCase return [$shortCode, $domain, $shortUrl]; } - private function createVisitsForShortUrl(ShortUrl $shortUrl, int $amount = 6): void + private function createVisitsForShortUrl(ShortUrl $shortUrl, int $amount = 6, int $botsAmount = 2): void { for ($i = 0; $i < $amount; $i++) { $visit = $this->setDateOnVisit( - Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()), + Visit::forValidShortUrl( + $shortUrl, + $botsAmount < 1 ? Visitor::emptyInstance() : Visitor::botInstance(), + ), Chronos::parse(sprintf('2016-01-0%s', $i + 1)), ); + $botsAmount--; $this->getEntityManager()->persist($visit); }