diff --git a/composer.json b/composer.json index 29068f06..ec9a028d 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,7 @@ "predis/predis": "^1.1", "pugx/shortid-php": "^0.7", "ramsey/uuid": "^3.9", - "shlinkio/shlink-common": "dev-main#b889f5d as 3.5", + "shlinkio/shlink-common": "dev-main#62d4b84 as 3.5", "shlinkio/shlink-config": "^1.0", "shlinkio/shlink-event-dispatcher": "^2.0", "shlinkio/shlink-importer": "^2.2", diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index 3bcee8e1..efcf5d8f 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -28,15 +28,10 @@ class Visit extends AbstractEntity implements JsonSerializable private ?ShortUrl $shortUrl; private ?VisitLocation $visitLocation = null; - public function __construct( - ?ShortUrl $shortUrl, - Visitor $visitor, - bool $anonymize = true, - ?Chronos $date = null, - string $type = self::TYPE_VALID_SHORT_URL - ) { + private function __construct(?ShortUrl $shortUrl, Visitor $visitor, string $type, bool $anonymize = true) + { $this->shortUrl = $shortUrl; - $this->date = $date ?? Chronos::now(); + $this->date = Chronos::now(); $this->userAgent = $visitor->getUserAgent(); $this->referer = $visitor->getReferer(); $this->remoteAddr = $this->processAddress($anonymize, $visitor->getRemoteAddress()); @@ -60,22 +55,22 @@ class Visit extends AbstractEntity implements JsonSerializable public static function forValidShortUrl(ShortUrl $shortUrl, Visitor $visitor, bool $anonymize = true): self { - return new self($shortUrl, $visitor, $anonymize); + return new self($shortUrl, $visitor, self::TYPE_VALID_SHORT_URL, $anonymize); } public static function forBasePath(Visitor $visitor, bool $anonymize = true): self { - return new self(null, $visitor, $anonymize, null, self::TYPE_BASE_URL); + return new self(null, $visitor, self::TYPE_BASE_URL, $anonymize); } public static function forInvalidShortUrl(Visitor $visitor, bool $anonymize = true): self { - return new self(null, $visitor, $anonymize, null, self::TYPE_INVALID_SHORT_URL); + return new self(null, $visitor, self::TYPE_INVALID_SHORT_URL, $anonymize); } public static function forRegularNotFound(Visitor $visitor, bool $anonymize = true): self { - return new self(null, $visitor, $anonymize, null, self::TYPE_REGULAR_404); + return new self(null, $visitor, self::TYPE_REGULAR_404, $anonymize); } public function getRemoteAddr(): ?string diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index 082b17b8..b869093e 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -168,6 +168,29 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return $qb; } + public function findOrphanVisits(?DateRange $dateRange = null, ?int $limit = null, ?int $offset = null): 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')); + + $this->applyDatesInline($qb, $dateRange); + + return $this->resolveVisitsWithNativeQuery($qb, $limit, $offset); + } + + public function countOrphanVisits(?DateRange $dateRange = null): int + { + return (int) $this->matchSingleScalarResult(new CountOfOrphanVisits($dateRange)); + } + + public function countVisits(?ApiKey $apiKey = null): int + { + return (int) $this->matchSingleScalarResult(new CountOfShortUrlVisits($apiKey)); + } + private function applyDatesInline(QueryBuilder $qb, ?DateRange $dateRange): void { if ($dateRange !== null && $dateRange->getStartDate() !== null) { @@ -208,14 +231,4 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return $query->getResult(); } - - public function countVisits(?ApiKey $apiKey = null): int - { - return (int) $this->matchSingleScalarResult(new CountOfShortUrlVisits($apiKey)); - } - - public function countOrphanVisits(): int - { - return (int) $this->matchSingleScalarResult(new CountOfOrphanVisits()); - } } diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php index 0d637f75..3ecf0bca 100644 --- a/module/Core/src/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Repository/VisitRepositoryInterface.php @@ -62,7 +62,12 @@ interface VisitRepositoryInterface extends ObjectRepository, EntitySpecification public function countVisitsByTag(string $tag, ?DateRange $dateRange = null, ?Specification $spec = null): int; - public function countVisits(?ApiKey $apiKey = null): int; + /** + * @return Visit[] + */ + public function findOrphanVisits(?DateRange $dateRange = null, ?int $limit = null, ?int $offset = null): array; - public function countOrphanVisits(): int; + public function countOrphanVisits(?DateRange $dateRange = null): int; + + public function countVisits(?ApiKey $apiKey = null): int; } diff --git a/module/Core/src/Spec/InDateRange.php b/module/Core/src/Spec/InDateRange.php new file mode 100644 index 00000000..44944aed --- /dev/null +++ b/module/Core/src/Spec/InDateRange.php @@ -0,0 +1,38 @@ +dateRange = $dateRange; + $this->field = $field; + } + + protected function getSpec(): Specification + { + $criteria = []; + + if ($this->dateRange !== null && $this->dateRange->getStartDate() !== null) { + $criteria[] = Spec::gte($this->field, $this->dateRange->getStartDate()->toDateTimeString()); + } + + if ($this->dateRange !== null && $this->dateRange->getEndDate() !== null) { + $criteria[] = Spec::lte($this->field, $this->dateRange->getEndDate()->toDateTimeString()); + } + + return Spec::andX(...$criteria); + } +} diff --git a/module/Core/src/Visit/Spec/CountOfOrphanVisits.php b/module/Core/src/Visit/Spec/CountOfOrphanVisits.php index 7e15f330..fb8ee3bd 100644 --- a/module/Core/src/Visit/Spec/CountOfOrphanVisits.php +++ b/module/Core/src/Visit/Spec/CountOfOrphanVisits.php @@ -7,11 +7,24 @@ namespace Shlinkio\Shlink\Core\Visit\Spec; use Happyr\DoctrineSpecification\BaseSpecification; use Happyr\DoctrineSpecification\Spec; use Happyr\DoctrineSpecification\Specification\Specification; +use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Spec\InDateRange; class CountOfOrphanVisits extends BaseSpecification { + private ?DateRange $dateRange; + + public function __construct(?DateRange $dateRange) + { + parent::__construct(); + $this->dateRange = $dateRange; + } + protected function getSpec(): Specification { - return Spec::countOf(Spec::isNull('shortUrl')); + return Spec::countOf(Spec::andX( + Spec::isNull('shortUrl'), + new InDateRange($this->dateRange), + )); } } diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index 00b558d4..b6c23699 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Repository; use Cake\Chronos\Chronos; +use ReflectionObject; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; @@ -214,6 +215,75 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(3, $this->repo->countOrphanVisits()); } + /** @test */ + public function findOrphanVisitsReturnsExpectedResult(): void + { + $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['longUrl' => ''])); + $this->getEntityManager()->persist($shortUrl); + $this->createVisitsForShortUrl($shortUrl, 7); + + for ($i = 0; $i < 6; $i++) { + $this->getEntityManager()->persist($this->setDateOnVisit( + Visit::forBasePath(Visitor::emptyInstance()), + Chronos::parse(sprintf('2020-01-0%s', $i + 1)), + )); + $this->getEntityManager()->persist($this->setDateOnVisit( + Visit::forInvalidShortUrl(Visitor::emptyInstance()), + Chronos::parse(sprintf('2020-01-0%s', $i + 1)), + )); + $this->getEntityManager()->persist($this->setDateOnVisit( + Visit::forRegularNotFound(Visitor::emptyInstance()), + Chronos::parse(sprintf('2020-01-0%s', $i + 1)), + )); + } + + $this->getEntityManager()->flush(); + + self::assertCount(18, $this->repo->findOrphanVisits()); + self::assertCount(5, $this->repo->findOrphanVisits(null, 5)); + self::assertCount(10, $this->repo->findOrphanVisits(null, 15, 8)); + self::assertCount(9, $this->repo->findOrphanVisits(DateRange::withStartDate(Chronos::parse('2020-01-04')), 15)); + self::assertCount(2, $this->repo->findOrphanVisits( + DateRange::withStartAndEndDate(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')), + 6, + 4, + )); + self::assertCount(3, $this->repo->findOrphanVisits(DateRange::withEndDate(Chronos::parse('2020-01-01')))); + } + + /** @test */ + public function countOrphanVisitsReturnsExpectedResult(): void + { + $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['longUrl' => ''])); + $this->getEntityManager()->persist($shortUrl); + $this->createVisitsForShortUrl($shortUrl, 7); + + for ($i = 0; $i < 6; $i++) { + $this->getEntityManager()->persist($this->setDateOnVisit( + Visit::forBasePath(Visitor::emptyInstance()), + Chronos::parse(sprintf('2020-01-0%s', $i + 1)), + )); + $this->getEntityManager()->persist($this->setDateOnVisit( + Visit::forInvalidShortUrl(Visitor::emptyInstance()), + Chronos::parse(sprintf('2020-01-0%s', $i + 1)), + )); + $this->getEntityManager()->persist($this->setDateOnVisit( + Visit::forRegularNotFound(Visitor::emptyInstance()), + Chronos::parse(sprintf('2020-01-0%s', $i + 1)), + )); + } + + $this->getEntityManager()->flush(); + + self::assertEquals(18, $this->repo->countOrphanVisits()); + self::assertEquals(18, $this->repo->countOrphanVisits(DateRange::emptyInstance())); + self::assertEquals(9, $this->repo->countOrphanVisits(DateRange::withStartDate(Chronos::parse('2020-01-04')))); + self::assertEquals(6, $this->repo->countOrphanVisits( + DateRange::withStartAndEndDate(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')), + )); + self::assertEquals(3, $this->repo->countOrphanVisits(DateRange::withEndDate(Chronos::parse('2020-01-01')))); + } + private function createShortUrlsAndVisits(bool $withDomain = true, array $tags = []): array { $shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData([ @@ -243,13 +313,22 @@ class VisitRepositoryTest extends DatabaseTestCase private function createVisitsForShortUrl(ShortUrl $shortUrl, int $amount = 6): void { for ($i = 0; $i < $amount; $i++) { - $visit = new Visit( - $shortUrl, - Visitor::emptyInstance(), - true, + $visit = $this->setDateOnVisit( + Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()), Chronos::parse(sprintf('2016-01-0%s', $i + 1)), ); + $this->getEntityManager()->persist($visit); } } + + private function setDateOnVisit(Visit $visit, Chronos $date): Visit + { + $ref = new ReflectionObject($visit); + $dateProp = $ref->getProperty('date'); + $dateProp->setAccessible(true); + $dateProp->setValue($visit, $date); + + return $visit; + } }