From 292937b9623117315addb03e487bdcd9fb747183 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 22 Feb 2019 19:31:03 +0100 Subject: [PATCH] Updated VisitRepository::findUnlocatedVisits methods so that it paginates the amount of elements loaded in memory --- .../Adapter/PaginableQueryAdapter.php | 34 +++++++++++++++++ .../src/Paginator/ImplicitLoopPaginator.php | 38 +++++++++++++++++++ .../Core/src/Repository/VisitRepository.php | 21 ++++++++-- .../Repository/VisitRepositoryInterface.php | 7 +++- module/Core/src/Service/VisitService.php | 2 +- .../Repository/VisitRepositoryTest.php | 18 +++++++-- module/Core/test/Service/VisitServiceTest.php | 6 +-- 7 files changed, 115 insertions(+), 11 deletions(-) create mode 100644 module/Common/src/Paginator/Adapter/PaginableQueryAdapter.php create mode 100644 module/Common/src/Paginator/ImplicitLoopPaginator.php diff --git a/module/Common/src/Paginator/Adapter/PaginableQueryAdapter.php b/module/Common/src/Paginator/Adapter/PaginableQueryAdapter.php new file mode 100644 index 00000000..d6de1ada --- /dev/null +++ b/module/Common/src/Paginator/Adapter/PaginableQueryAdapter.php @@ -0,0 +1,34 @@ +query = $query; + $this->totalItems = $totalItems; + } + + public function getItems($offset, $itemCountPerPage): iterable + { + return $this->query + ->setMaxResults($itemCountPerPage) + ->setFirstResult($offset) + ->iterate(); + } + + public function count(): int + { + return $this->totalItems; + } +} diff --git a/module/Common/src/Paginator/ImplicitLoopPaginator.php b/module/Common/src/Paginator/ImplicitLoopPaginator.php new file mode 100644 index 00000000..9019f248 --- /dev/null +++ b/module/Common/src/Paginator/ImplicitLoopPaginator.php @@ -0,0 +1,38 @@ +paginator = $paginator; + $this->valueParser = $valueParser ?? function ($value) { + return $value; + }; + } + + public function getIterator(): iterable + { + $totalPages = $this->paginator->count(); + $processedPages = 0; + + do { + $processedPages++; + $this->paginator->setCurrentPageNumber($processedPages); + + foreach ($this->paginator as $key => $value) { + yield $key => ($this->valueParser)($value); + } + } while ($processedPages < $totalPages); + } +} diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index 781df089..dfa5498f 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -5,17 +5,32 @@ namespace Shlinkio\Shlink\Core\Repository; use Doctrine\ORM\EntityRepository; use Doctrine\ORM\QueryBuilder; +use Shlinkio\Shlink\Common\Paginator\Adapter\PaginableQueryAdapter; +use Shlinkio\Shlink\Common\Paginator\ImplicitLoopPaginator; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Visit; +use Zend\Paginator\Paginator; +use function array_shift; class VisitRepository extends EntityRepository implements VisitRepositoryInterface { - public function findUnlocatedVisits(): iterable + /** + * @return iterable|Visit[] + */ + public function findUnlocatedVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable { - $dql = 'SELECT v FROM Shlinkio\Shlink\Core\Entity\Visit AS v WHERE v.visitLocation IS NULL'; + $count = $this->count(['visitLocation' => null]); + $dql = <<getEntityManager()->createQuery($dql); - return $query->iterate(); + $paginator = new Paginator(new PaginableQueryAdapter($query, $count)); + $paginator->setItemCountPerPage($blockSize); + + return new ImplicitLoopPaginator($paginator, function (array $value) { + return array_shift($value); + }); } /** diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php index 4de9024e..2307946a 100644 --- a/module/Core/src/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Repository/VisitRepositoryInterface.php @@ -9,7 +9,12 @@ use Shlinkio\Shlink\Core\Entity\Visit; interface VisitRepositoryInterface extends ObjectRepository { - public function findUnlocatedVisits(): iterable; + public const DEFAULT_BLOCK_SIZE = 10000; + + /** + * @return iterable|Visit[] + */ + public function findUnlocatedVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable; /** * @return Visit[] diff --git a/module/Core/src/Service/VisitService.php b/module/Core/src/Service/VisitService.php index 705dab59..78c0eddd 100644 --- a/module/Core/src/Service/VisitService.php +++ b/module/Core/src/Service/VisitService.php @@ -26,7 +26,7 @@ class VisitService implements VisitServiceInterface $repo = $this->em->getRepository(Visit::class); $results = $repo->findUnlocatedVisits(); - foreach ($results as [$visit]) { + foreach ($results as $visit) { try { /** @var Location $location */ $location = $geolocateVisit($visit); diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index db797920..64702fcd 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -12,6 +12,8 @@ use Shlinkio\Shlink\Core\Entity\VisitLocation; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\VisitRepository; use ShlinkioTest\Shlink\Common\DbTest\DatabaseTestCase; +use function Functional\map; +use function range; use function sprintf; class VisitRepositoryTest extends DatabaseTestCase @@ -30,8 +32,11 @@ class VisitRepositoryTest extends DatabaseTestCase $this->repo = $this->getEntityManager()->getRepository(Visit::class); } - /** @test */ - public function findUnlocatedVisitsReturnsProperVisits(): void + /** + * @test + * @dataProvider provideBlockSize + */ + public function findUnlocatedVisitsReturnsProperVisits(int $blockSize): void { $shortUrl = new ShortUrl(''); $this->getEntityManager()->persist($shortUrl); @@ -50,7 +55,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); $resultsCount = 0; - $results = $this->repo->findUnlocatedVisits(); + $results = $this->repo->findUnlocatedVisits($blockSize); foreach ($results as $value) { $resultsCount++; } @@ -58,6 +63,13 @@ class VisitRepositoryTest extends DatabaseTestCase $this->assertEquals(3, $resultsCount); } + public function provideBlockSize(): iterable + { + return map(range(1, 5), function (int $value) { + return [$value]; + }); + } + /** @test */ public function findVisitsByShortCodeReturnsProperData(): void { diff --git a/module/Core/test/Service/VisitServiceTest.php b/module/Core/test/Service/VisitServiceTest.php index 618e4781..4f249b23 100644 --- a/module/Core/test/Service/VisitServiceTest.php +++ b/module/Core/test/Service/VisitServiceTest.php @@ -36,8 +36,8 @@ class VisitServiceTest extends TestCase public function locateVisitsIteratesAndLocatesUnlocatedVisits(): void { $unlocatedVisits = [ - [new Visit(new ShortUrl('foo'), Visitor::emptyInstance())], - [new Visit(new ShortUrl('bar'), Visitor::emptyInstance())], + new Visit(new ShortUrl('foo'), Visitor::emptyInstance()), + new Visit(new ShortUrl('bar'), Visitor::emptyInstance()), ]; $repo = $this->prophesize(VisitRepository::class); @@ -71,7 +71,7 @@ class VisitServiceTest extends TestCase public function visitsWhichCannotBeLocatedAreIgnored() { $unlocatedVisits = [ - [new Visit(new ShortUrl('foo'), Visitor::emptyInstance())], + new Visit(new ShortUrl('foo'), Visitor::emptyInstance()), ]; $repo = $this->prophesize(VisitRepository::class);