diff --git a/config/autoload/entity-manager.global.php b/config/autoload/entity-manager.global.php index 84233915..c3aa6632 100644 --- a/config/autoload/entity-manager.global.php +++ b/config/autoload/entity-manager.global.php @@ -5,6 +5,7 @@ declare(strict_types=1); use Doctrine\ORM\Events; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; use Shlinkio\Shlink\Core\Config\EnvVars; +use Shlinkio\Shlink\Core\Visit\Listener\OrphanVisitsCountTracker; use Shlinkio\Shlink\Core\Visit\Listener\ShortUrlVisitsCountTracker; use function Shlinkio\Shlink\Core\ArrayUtils\contains; @@ -63,8 +64,8 @@ return (static function (): array { 'load_mappings_using_functional_style' => true, 'default_repository_classname' => EntitySpecificationRepository::class, 'listeners' => [ - Events::onFlush => [ShortUrlVisitsCountTracker::class], - Events::postFlush => [ShortUrlVisitsCountTracker::class], + Events::onFlush => [ShortUrlVisitsCountTracker::class, OrphanVisitsCountTracker::class], + Events::postFlush => [ShortUrlVisitsCountTracker::class, OrphanVisitsCountTracker::class], ], ], 'connection' => $resolveConnection(), diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 951c7b52..5437555b 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -77,6 +77,7 @@ return [ Visit\Entity\Visit::class, ], Visit\Listener\ShortUrlVisitsCountTracker::class => InvokableFactory::class, + Visit\Listener\OrphanVisitsCountTracker::class => InvokableFactory::class, Util\DoctrineBatchHelper::class => ConfigAbstractFactory::class, Util\RedirectResponseHelper::class => ConfigAbstractFactory::class, diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.OrphanVisitsCount.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.OrphanVisitsCount.php new file mode 100644 index 00000000..dbdf9e9a --- /dev/null +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Visit.Entity.OrphanVisitsCount.php @@ -0,0 +1,41 @@ +setTable(determineTableName('orphan_visits_counts', $emConfig)) + ->setCustomRepositoryClass(Visit\Repository\OrphanVisitsCountRepository::class); + + $builder->createField('id', Types::BIGINT) + ->columnName('id') + ->makePrimaryKey() + ->generatedValue('IDENTITY') + ->option('unsigned', true) + ->build(); + + $builder->createField('potentialBot', Types::BOOLEAN) + ->columnName('potential_bot') + ->option('default', false) + ->build(); + + $builder->createField('count', Types::BIGINT) + ->columnName('count') + ->option('unsigned', true) + ->option('default', 1) + ->build(); + + $builder->createField('slotId', Types::INTEGER) + ->columnName('slot_id') + ->option('unsigned', true) + ->build(); + + $builder->addUniqueConstraint(['potential_bot', 'slot_id'], 'UQ_slot'); +}; diff --git a/module/Core/src/Visit/Entity/OrphanVisitsCount.php b/module/Core/src/Visit/Entity/OrphanVisitsCount.php new file mode 100644 index 00000000..8199b174 --- /dev/null +++ b/module/Core/src/Visit/Entity/OrphanVisitsCount.php @@ -0,0 +1,17 @@ +entitiesToBeCreated = $args->getObjectManager()->getUnitOfWork()->getScheduledEntityInsertions(); + } + + /** + * @throws Exception + */ + public function postFlush(PostFlushEventArgs $args): void + { + $em = $args->getObjectManager(); + $entitiesToBeCreated = $this->entitiesToBeCreated; + + // Reset tracked entities until next flush operation + $this->entitiesToBeCreated = []; + + foreach ($entitiesToBeCreated as $entity) { + $this->trackVisitCount($em, $entity); + } + } + + /** + * @throws Exception + */ + private function trackVisitCount(EntityManagerInterface $em, object $entity): void + { + // This is not an orphan visit + if (! $entity instanceof Visit || ! $entity->isOrphan()) { + return; + } + $visit = $entity; + + $isBot = $visit->potentialBot; + $conn = $em->getConnection(); + $platformClass = $conn->getDatabasePlatform(); + + match ($platformClass::class) { + PostgreSQLPlatform::class => $this->incrementForPostgres($conn, $isBot), + SQLitePlatform::class, SQLServerPlatform::class => $this->incrementForOthers($conn, $isBot), + default => $this->incrementForMySQL($conn, $isBot), + }; + } + + /** + * @throws Exception + */ + private function incrementForMySQL(Connection $conn, bool $potentialBot): void + { + $this->incrementWithPreparedStatement($conn, $potentialBot, <<incrementWithPreparedStatement($conn, $potentialBot, <<prepare($query); + $statement->bindValue('potential_bot', $potentialBot ? 1 : 0); + $statement->executeStatement(); + } + + /** + * @throws Exception + */ + private function incrementForOthers(Connection $conn, bool $potentialBot): void + { + $slotId = rand(1, 100); + + // For engines without a specific UPSERT syntax, do a regular locked select followed by an insert or update + $qb = $conn->createQueryBuilder(); + $qb->select('id') + ->from('orphan_visits_counts') + ->where($qb->expr()->and( + $qb->expr()->eq('potential_bot', ':potential_bot'), + $qb->expr()->eq('slot_id', ':slot_id'), + )) + ->setParameter('potential_bot', $potentialBot ? '1' : '0') + ->setParameter('slot_id', $slotId) + ->setMaxResults(1); + + if ($conn->getDatabasePlatform()::class === SQLServerPlatform::class) { + $qb->forUpdate(); + } + + $visitsCountId = $qb->executeQuery()->fetchOne(); + + $writeQb = ! $visitsCountId + ? $conn->createQueryBuilder() + ->insert('orphan_visits_counts') + ->values([ + 'potential_bot' => ':potential_bot', + 'slot_id' => ':slot_id', + ]) + ->setParameter('potential_bot', $potentialBot ? '1' : '0') + ->setParameter('slot_id', $slotId) + : $conn->createQueryBuilder() + ->update('orphan_visits_counts') + ->set('count', 'count + 1') + ->where($qb->expr()->eq('id', ':visits_count_id')) + ->setParameter('visits_count_id', $visitsCountId); + + $writeQb->executeStatement(); + } +} diff --git a/module/Core/src/Visit/Repository/OrphanVisitsCountRepository.php b/module/Core/src/Visit/Repository/OrphanVisitsCountRepository.php new file mode 100644 index 00000000..d5e7670c --- /dev/null +++ b/module/Core/src/Visit/Repository/OrphanVisitsCountRepository.php @@ -0,0 +1,31 @@ +apiKey?->hasRole(Role::NO_ORPHAN_VISITS)) { + return 0; + } + + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->select('COALESCE(SUM(vc.count), 0)') + ->from(OrphanVisitsCount::class, 'vc'); + + if ($filtering->excludeBots) { + $qb->andWhere($qb->expr()->eq('vc.potentialBot', ':potentialBot')) + ->setParameter('potentialBot', false); + } + + return (int) $qb->getQuery()->getSingleScalarResult(); + } +} diff --git a/module/Core/src/Visit/Repository/OrphanVisitsCountRepositoryInterface.php b/module/Core/src/Visit/Repository/OrphanVisitsCountRepositoryInterface.php new file mode 100644 index 00000000..adbbe076 --- /dev/null +++ b/module/Core/src/Visit/Repository/OrphanVisitsCountRepositoryInterface.php @@ -0,0 +1,12 @@ +em->getRepository(Visit::class); + /** @var OrphanVisitsCountRepository $orphanVisitsCountRepo */ + $orphanVisitsCountRepo = $this->em->getRepository(OrphanVisitsCount::class); /** @var ShortUrlVisitsCountRepository $visitsCountRepo */ $visitsCountRepo = $this->em->getRepository(ShortUrlVisitsCount::class); return new VisitsStats( nonOrphanVisitsTotal: $visitsCountRepo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey)), - orphanVisitsTotal: $visitsRepo->countOrphanVisits(new OrphanVisitsCountFiltering(apiKey: $apiKey)), + orphanVisitsTotal: $orphanVisitsCountRepo->countOrphanVisits( + new OrphanVisitsCountFiltering(apiKey: $apiKey), + ), nonOrphanVisitsNonBots: $visitsCountRepo->countNonOrphanVisits( new VisitsCountFiltering(excludeBots: true, apiKey: $apiKey), ), - orphanVisitsNonBots: $visitsRepo->countOrphanVisits( + orphanVisitsNonBots: $orphanVisitsCountRepo->countOrphanVisits( new OrphanVisitsCountFiltering(excludeBots: true, apiKey: $apiKey), ), ); diff --git a/module/Core/test-db/Visit/Listener/OrphanVisitsCountTrackerTest.php b/module/Core/test-db/Visit/Listener/OrphanVisitsCountTrackerTest.php new file mode 100644 index 00000000..20302c75 --- /dev/null +++ b/module/Core/test-db/Visit/Listener/OrphanVisitsCountTrackerTest.php @@ -0,0 +1,69 @@ +repo = $this->getEntityManager()->getRepository(OrphanVisitsCount::class); + } + + #[Test] + public function createsNewEntriesWhenNoneExist(): void + { + $visit = Visit::forBasePath(Visitor::emptyInstance()); + $this->getEntityManager()->persist($visit); + $this->getEntityManager()->flush(); + + /** @var OrphanVisitsCount[] $result */ + $result = $this->repo->findAll(); + + self::assertCount(1, $result); + self::assertEquals('1', $result[0]->count); + self::assertGreaterThanOrEqual(0, $result[0]->slotId); + self::assertLessThanOrEqual(100, $result[0]->slotId); + } + + #[Test] + public function editsExistingEntriesWhenAlreadyExist(): void + { + for ($i = 0; $i <= 100; $i++) { + $this->getEntityManager()->persist(new OrphanVisitsCount(slotId: $i)); + } + $this->getEntityManager()->flush(); + + $visit = Visit::forRegularNotFound(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 OrphanVisitsCount[] $result */ + $result = $this->repo->findAll(); + $itemsWithCountBiggerThanOnce = array_values(array_filter( + $result, + static fn (OrphanVisitsCount $item) => ((int) $item->count) > 1, + )); + + self::assertCount(101, $result); + self::assertCount(1, $itemsWithCountBiggerThanOnce); + self::assertEquals('2', $itemsWithCountBiggerThanOnce[0]->count); + } +} diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index d14e7078..1506dd1a 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -14,6 +14,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; +use Shlinkio\Shlink\Core\Visit\Entity\OrphanVisitsCount; use Shlinkio\Shlink\Core\Visit\Entity\ShortUrlVisitsCount; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitType; @@ -22,6 +23,7 @@ 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\OrphanVisitsCountRepository; use Shlinkio\Shlink\Core\Visit\Repository\ShortUrlVisitsCountRepository; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepository; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; @@ -39,14 +41,18 @@ class VisitRepositoryTest extends DatabaseTestCase { private VisitRepository $repo; private ShortUrlVisitsCountRepository $countRepo; + private OrphanVisitsCountRepository $orphanCountRepo; private PersistenceShortUrlRelationResolver $relationResolver; protected function setUp(): void { $this->repo = $this->getEntityManager()->getRepository(Visit::class); - // Testing the ShortUrlVisitsCountRepository in this very same test, helps checking the fact that results should + + // Testing the visits count repositories in this very same test, helps checking the fact that results should // match what VisitRepository returns $this->countRepo = $this->getEntityManager()->getRepository(ShortUrlVisitsCount::class); + $this->orphanCountRepo = $this->getEntityManager()->getRepository(OrphanVisitsCount::class); + $this->relationResolver = new PersistenceShortUrlRelationResolver($this->getEntityManager()); } @@ -326,6 +332,9 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(0, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering( apiKey: $noOrphanVisitsApiKey, ))); + self::assertEquals(0, $this->orphanCountRepo->countOrphanVisits(new OrphanVisitsCountFiltering( + apiKey: $noOrphanVisitsApiKey, + ))); self::assertEquals(4, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::since( Chronos::parse('2016-01-05')->startOfDay(), )))); @@ -342,7 +351,11 @@ class VisitRepositoryTest extends DatabaseTestCase new VisitsCountFiltering(excludeBots: true, apiKey: $apiKey2), )); self::assertEquals(4, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering())); + self::assertEquals(4, $this->orphanCountRepo->countOrphanVisits(new OrphanVisitsCountFiltering())); self::assertEquals(3, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(excludeBots: true))); + self::assertEquals(3, $this->orphanCountRepo->countOrphanVisits( + new OrphanVisitsCountFiltering(excludeBots: true), + )); } #[Test] @@ -432,6 +445,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); self::assertEquals(18, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering())); + self::assertEquals(18, $this->orphanCountRepo->countOrphanVisits(new OrphanVisitsCountFiltering())); self::assertEquals(18, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(DateRange::allTime()))); self::assertEquals(9, $this->repo->countOrphanVisits( new OrphanVisitsCountFiltering(DateRange::since(Chronos::parse('2020-01-04'))), diff --git a/module/Core/test/Visit/VisitsStatsHelperTest.php b/module/Core/test/Visit/VisitsStatsHelperTest.php index e109852a..c1aa0747 100644 --- a/module/Core/test/Visit/VisitsStatsHelperTest.php +++ b/module/Core/test/Visit/VisitsStatsHelperTest.php @@ -22,6 +22,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository; use Shlinkio\Shlink\Core\Tag\Entity\Tag; use Shlinkio\Shlink\Core\Tag\Repository\TagRepository; +use Shlinkio\Shlink\Core\Visit\Entity\OrphanVisitsCount; use Shlinkio\Shlink\Core\Visit\Entity\ShortUrlVisitsCount; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; @@ -32,6 +33,7 @@ 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\OrphanVisitsCountRepository; use Shlinkio\Shlink\Core\Visit\Repository\ShortUrlVisitsCountRepository; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepository; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelper; @@ -68,13 +70,13 @@ class VisitsStatsHelperTest extends TestCase }, ); - $visitsRepo = $this->createMock(VisitRepository::class); - $visitsRepo->expects($this->exactly(2))->method('countOrphanVisits')->with( + $orphanVisitsCountRepo = $this->createMock(OrphanVisitsCountRepository::class); + $orphanVisitsCountRepo->expects($this->exactly(2))->method('countOrphanVisits')->with( $this->isInstanceOf(VisitsCountFiltering::class), )->willReturn($expectedCount); $this->em->expects($this->exactly(2))->method('getRepository')->willReturnMap([ - [Visit::class, $visitsRepo], + [OrphanVisitsCount::class, $orphanVisitsCountRepo], [ShortUrlVisitsCount::class, $visitsCountRepo], ]); diff --git a/phpunit-db.xml b/phpunit-db.xml index 3c5ffb64..17e748b8 100644 --- a/phpunit-db.xml +++ b/phpunit-db.xml @@ -20,7 +20,7 @@ ./module/*/src/Spec ./module/*/src/**/Spec ./module/*/src/**/**/Spec - ./module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php + ./module/Core/src/Visit/Listener/*.php diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 4364c82c..30f2286d 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -30,7 +30,7 @@ ./module/Core/src/Spec ./module/Core/src/**/Spec ./module/Core/src/**/**/Spec - ./module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php + ./module/Core/src/Visit/Listener/*.php