From 55e7f7ccb069fd32319bf605580ffe9cb7f54fb3 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Mon, 8 Feb 2021 22:00:07 +0100
Subject: [PATCH] Improved VisitRepository tests

---
 .../Core/src/Repository/VisitRepository.php   | 14 +++++----
 .../Repository/VisitRepositoryInterface.php   |  2 ++
 .../src/Visit/Spec/CountOfOrphanVisits.php    | 17 +++++++++++
 .../src/Visit/Spec/CountOfShortUrlVisits.php  | 30 +++++++++++++++++++
 .../Repository/VisitRepositoryTest.php        |  8 ++++-
 5 files changed, 64 insertions(+), 7 deletions(-)
 create mode 100644 module/Core/src/Visit/Spec/CountOfOrphanVisits.php
 create mode 100644 module/Core/src/Visit/Spec/CountOfShortUrlVisits.php

diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php
index 61bc50bb..082b17b8 100644
--- a/module/Core/src/Repository/VisitRepository.php
+++ b/module/Core/src/Repository/VisitRepository.php
@@ -7,13 +7,13 @@ namespace Shlinkio\Shlink\Core\Repository;
 use Doctrine\ORM\Query\ResultSetMappingBuilder;
 use Doctrine\ORM\QueryBuilder;
 use Happyr\DoctrineSpecification\EntitySpecificationRepository;
-use Happyr\DoctrineSpecification\Spec;
 use Happyr\DoctrineSpecification\Specification\Specification;
 use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Entity\VisitLocation;
-use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin;
+use Shlinkio\Shlink\Core\Visit\Spec\CountOfOrphanVisits;
+use Shlinkio\Shlink\Core\Visit\Spec\CountOfShortUrlVisits;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
 use const PHP_INT_MAX;
@@ -211,9 +211,11 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo
 
     public function countVisits(?ApiKey $apiKey = null): int
     {
-        return (int) $this->matchSingleScalarResult(Spec::countOf(Spec::andX(
-            Spec::isNotNull('shortUrl'),
-            new WithApiKeySpecsEnsuringJoin($apiKey, 'shortUrl'),
-        )));
+        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 526645df..0d637f75 100644
--- a/module/Core/src/Repository/VisitRepositoryInterface.php
+++ b/module/Core/src/Repository/VisitRepositoryInterface.php
@@ -63,4 +63,6 @@ interface VisitRepositoryInterface extends ObjectRepository, EntitySpecification
     public function countVisitsByTag(string $tag, ?DateRange $dateRange = null, ?Specification $spec = null): int;
 
     public function countVisits(?ApiKey $apiKey = null): int;
+
+    public function countOrphanVisits(): int;
 }
diff --git a/module/Core/src/Visit/Spec/CountOfOrphanVisits.php b/module/Core/src/Visit/Spec/CountOfOrphanVisits.php
new file mode 100644
index 00000000..7e15f330
--- /dev/null
+++ b/module/Core/src/Visit/Spec/CountOfOrphanVisits.php
@@ -0,0 +1,17 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Visit\Spec;
+
+use Happyr\DoctrineSpecification\BaseSpecification;
+use Happyr\DoctrineSpecification\Spec;
+use Happyr\DoctrineSpecification\Specification\Specification;
+
+class CountOfOrphanVisits extends BaseSpecification
+{
+    protected function getSpec(): Specification
+    {
+        return Spec::countOf(Spec::isNull('shortUrl'));
+    }
+}
diff --git a/module/Core/src/Visit/Spec/CountOfShortUrlVisits.php b/module/Core/src/Visit/Spec/CountOfShortUrlVisits.php
new file mode 100644
index 00000000..6a125ee9
--- /dev/null
+++ b/module/Core/src/Visit/Spec/CountOfShortUrlVisits.php
@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Visit\Spec;
+
+use Happyr\DoctrineSpecification\BaseSpecification;
+use Happyr\DoctrineSpecification\Spec;
+use Happyr\DoctrineSpecification\Specification\Specification;
+use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin;
+use Shlinkio\Shlink\Rest\Entity\ApiKey;
+
+class CountOfShortUrlVisits extends BaseSpecification
+{
+    private ?ApiKey $apiKey;
+
+    public function __construct(?ApiKey $apiKey)
+    {
+        parent::__construct();
+        $this->apiKey = $apiKey;
+    }
+
+    protected function getSpec(): Specification
+    {
+        return Spec::countOf(Spec::andX(
+            Spec::isNotNull('shortUrl'),
+            new WithApiKeySpecsEnsuringJoin($this->apiKey, 'shortUrl'),
+        ));
+    }
+}
diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php
index 5681ee26..00b558d4 100644
--- a/module/Core/test-db/Repository/VisitRepositoryTest.php
+++ b/module/Core/test-db/Repository/VisitRepositoryTest.php
@@ -168,7 +168,7 @@ class VisitRepositoryTest extends DatabaseTestCase
     }
 
     /** @test */
-    public function countReturnsExpectedResultBasedOnApiKey(): void
+    public function countVisitsReturnsExpectedResultBasedOnApiKey(): void
     {
         $domain = new Domain('foo.com');
         $this->getEntityManager()->persist($domain);
@@ -200,12 +200,18 @@ class VisitRepositoryTest extends DatabaseTestCase
         $domainApiKey = ApiKey::withRoles(RoleDefinition::forDomain($domain));
         $this->getEntityManager()->persist($domainApiKey);
 
+        // Visits not linked to any short URL
+        $this->getEntityManager()->persist(Visit::forBasePath(Visitor::emptyInstance()));
+        $this->getEntityManager()->persist(Visit::forInvalidShortUrl(Visitor::emptyInstance()));
+        $this->getEntityManager()->persist(Visit::forRegularNotFound(Visitor::emptyInstance()));
+
         $this->getEntityManager()->flush();
 
         self::assertEquals(4 + 5 + 7, $this->repo->countVisits());
         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());
     }
 
     private function createShortUrlsAndVisits(bool $withDomain = true, array $tags = []): array