Add ShortUrlVisitsCountTrackerTest

module/Core/src/Visit/Entity/ShortUrlVisitsCount.php

@@ -12,8 +12,8 @@ class ShortUrlVisitsCount extends AbstractEntity
     public function __construct(
         private readonly ShortUrl $shortUrl,
         private readonly bool $potentialBot = false,
-        private readonly int $slotId = 1,
+        public readonly int $slotId = 1,
-        private readonly string $count = '1',
+        public readonly string $count = '1',
     ) {
     }
 }

module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php

@@ -136,8 +136,9 @@ final class ShortUrlVisitsCountTracker
             $qb->forUpdate();
         }
 
-        $resultSet = $qb->executeQuery()->fetchOne();
+        $visitsCountId = $qb->executeQuery()->fetchOne();
-        $writeQb = ! $resultSet
+
+        $writeQb = ! $visitsCountId
             ? $conn->createQueryBuilder()
                 ->insert('short_url_visits_counts')
                 ->values([
@@ -145,18 +146,15 @@ final class ShortUrlVisitsCountTracker
                     'potential_bot' => ':potential_bot',
                     'slot_id' => ':slot_id',
                 ])
-            : $conn->createQueryBuilder()
+                ->setParameter('short_url_id', $shortUrlId)
-                   ->update('short_url_visits_counts')
-                   ->set('count', 'count + 1')
-                   ->where($qb->expr()->and(
-                       $qb->expr()->eq('short_url_id', ':short_url_id'),
-                       $qb->expr()->eq('potential_bot', ':potential_bot'),
-                       $qb->expr()->eq('slot_id', ':slot_id'),
-                   ));
-
-        $writeQb->setParameter('short_url_id', $shortUrlId)
                 ->setParameter('potential_bot', $potentialBot ? '1' : '0')
                 ->setParameter('slot_id', $slotId)
-                ->executeStatement();
+            : $conn->createQueryBuilder()
+                ->update('short_url_visits_counts')
+                ->set('count', 'count + 1')
+                ->where($qb->expr()->eq('id', ':visits_count_id'))
+                ->setParameter('visits_count_id', $visitsCountId);
+
+        $writeQb->executeStatement();
     }
 }

module/Core/test-db/Visit/Listener/ShortUrlVisitsCountTrackerTest.php

@@ -0,0 +1,76 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioDbTest\Shlink\Core\Visit\Listener;
+
+use Doctrine\ORM\EntityRepository;
+use PHPUnit\Framework\Attributes\Test;
+use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\Visit\Entity\ShortUrlVisitsCount;
+use Shlinkio\Shlink\Core\Visit\Entity\Visit;
+use Shlinkio\Shlink\Core\Visit\Model\Visitor;
+use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase;
+
+use function array_filter;
+use function array_values;
+
+class ShortUrlVisitsCountTrackerTest extends DatabaseTestCase
+{
+    private EntityRepository $repo;
+
+    protected function setUp(): void
+    {
+        $this->repo = $this->getEntityManager()->getRepository(ShortUrlVisitsCount::class);
+    }
+
+    #[Test]
+    public function createsNewEntriesWhenNoneExist(): void
+    {
+        $shortUrl = ShortUrl::createFake();
+        $this->getEntityManager()->persist($shortUrl);
+
+        $visit = Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance());
+        $this->getEntityManager()->persist($visit);
+        $this->getEntityManager()->flush();
+
+        /** @var ShortUrlVisitsCount[] $result */
+        $result = $this->repo->findBy(['shortUrl' => $shortUrl]);
+
+        self::assertCount(1, $result);
+        self::assertEquals('1', $result[0]->count);
+        self::assertGreaterThanOrEqual(0, $result[0]->slotId);
+        self::assertLessThan(100, $result[0]->slotId);
+    }
+
+    #[Test]
+    public function editsExistingEntriesWhenAlreadyExist(): void
+    {
+        $shortUrl = ShortUrl::createFake();
+        $this->getEntityManager()->persist($shortUrl);
+
+        for ($i = 0; $i < 100; $i++) {
+            $this->getEntityManager()->persist(new ShortUrlVisitsCount($shortUrl, slotId: $i));
+        }
+        $this->getEntityManager()->flush();
+
+        $visit = Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance());
+        $this->getEntityManager()->persist($visit);
+        $this->getEntityManager()->flush();
+
+        // Clear entity manager to force it to get fresh data from the database
+        // This is needed because the tracker inserts natively, bypassing the entity manager
+        $this->getEntityManager()->clear();
+
+        /** @var ShortUrlVisitsCount[] $result */
+        $result = $this->repo->findBy(['shortUrl' => $shortUrl]);
+        $itemsWithCountBiggerThanOnce = array_values(array_filter(
+            $result,
+            static fn (ShortUrlVisitsCount $item) => ((int) $item->count) > 1,
+        ));
+
+        self::assertCount(100, $result);
+        self::assertCount(1, $itemsWithCountBiggerThanOnce);
+        self::assertEquals('2', $itemsWithCountBiggerThanOnce[0]->count);
+    }
+}

phpunit-db.xml

@@ -20,6 +20,7 @@
             <directory>./module/*/src/Spec</directory>
             <directory>./module/*/src/**/Spec</directory>
             <directory>./module/*/src/**/**/Spec</directory>
+            <file>./module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php</file>
         </include>
     </source>
 </phpunit>

phpunit.xml.dist

@@ -30,6 +30,7 @@
             <directory>./module/Core/src/Spec</directory>
             <directory>./module/Core/src/**/Spec</directory>
             <directory>./module/Core/src/**/**/Spec</directory>
+            <file>./module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php</file>
         </exclude>
     </source>
 </phpunit>