From 60ef98b836145fb87a689104120f582082e04ef2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 14 Dec 2022 14:38:22 +0100 Subject: [PATCH] Extracted method to find crawlable short codes to its own query object --- module/Core/config/dependencies.config.php | 4 ++ module/Core/src/Crawling/CrawlingHelper.php | 10 ++-- .../Repository/CrawlableShortCodesQuery.php | 38 ++++++++++++++ .../CrawlableShortCodesQueryInterface.php | 13 +++++ .../Repository/ShortUrlRepository.php | 24 --------- .../ShortUrlRepositoryInterface.php | 2 - .../CrawlableShortCodesQueryTest.php | 50 +++++++++++++++++++ .../Repository/ShortUrlRepositoryTest.php | 33 ------------ .../Core/test/Crawling/CrawlingHelperTest.php | 20 +++----- 9 files changed, 114 insertions(+), 80 deletions(-) create mode 100644 module/Core/src/ShortUrl/Repository/CrawlableShortCodesQuery.php create mode 100644 module/Core/src/ShortUrl/Repository/CrawlableShortCodesQueryInterface.php create mode 100644 module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 0a501566..48fe3cb2 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -50,6 +50,10 @@ return [ EntityRepositoryFactory::class, ShortUrl\Entity\ShortUrl::class, ], + ShortUrl\Repository\CrawlableShortCodesQuery::class => [ + EntityRepositoryFactory::class, + ShortUrl\Entity\ShortUrl::class, + ], Tag\TagService::class => ConfigAbstractFactory::class, diff --git a/module/Core/src/Crawling/CrawlingHelper.php b/module/Core/src/Crawling/CrawlingHelper.php index 2c38fabd..958cb96e 100644 --- a/module/Core/src/Crawling/CrawlingHelper.php +++ b/module/Core/src/Crawling/CrawlingHelper.php @@ -4,20 +4,16 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Crawling; -use Doctrine\ORM\EntityManagerInterface; -use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; -use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface; +use Shlinkio\Shlink\Core\ShortUrl\Repository\CrawlableShortCodesQueryInterface; class CrawlingHelper implements CrawlingHelperInterface { - public function __construct(private EntityManagerInterface $em) + public function __construct(private readonly CrawlableShortCodesQueryInterface $query) { } public function listCrawlableShortCodes(): iterable { - /** @var ShortUrlRepositoryInterface $repo */ - $repo = $this->em->getRepository(ShortUrl::class); - yield from $repo->findCrawlableShortCodes(); + yield from ($this->query)(); } } diff --git a/module/Core/src/ShortUrl/Repository/CrawlableShortCodesQuery.php b/module/Core/src/ShortUrl/Repository/CrawlableShortCodesQuery.php new file mode 100644 index 00000000..7b3821d8 --- /dev/null +++ b/module/Core/src/ShortUrl/Repository/CrawlableShortCodesQuery.php @@ -0,0 +1,38 @@ + + */ + public function __invoke(): iterable + { + $blockSize = 1000; + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->select('DISTINCT s.shortCode') + ->from(ShortUrl::class, 's') + ->where($qb->expr()->eq('s.crawlable', ':crawlable')) + ->setParameter('crawlable', true) + ->setMaxResults($blockSize); + + $page = 0; + do { + $qbClone = (clone $qb)->setFirstResult($blockSize * $page); + $iterator = $qbClone->getQuery()->toIterable(); + $resultsFound = false; + $page++; + + foreach ($iterator as ['shortCode' => $shortCode]) { + $resultsFound = true; + yield $shortCode; + } + } while ($resultsFound); + } +} diff --git a/module/Core/src/ShortUrl/Repository/CrawlableShortCodesQueryInterface.php b/module/Core/src/ShortUrl/Repository/CrawlableShortCodesQueryInterface.php new file mode 100644 index 00000000..9e8211e5 --- /dev/null +++ b/module/Core/src/ShortUrl/Repository/CrawlableShortCodesQueryInterface.php @@ -0,0 +1,13 @@ + + */ + public function __invoke(): iterable; +} diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php index 216c4579..5e95f777 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php @@ -190,28 +190,4 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU $qb->andWhere($qb->expr()->isNull('s.domain')); } } - - public function findCrawlableShortCodes(): iterable - { - $blockSize = 1000; - $qb = $this->getEntityManager()->createQueryBuilder(); - $qb->select('DISTINCT s.shortCode') - ->from(ShortUrl::class, 's') - ->where($qb->expr()->eq('s.crawlable', ':crawlable')) - ->setParameter('crawlable', true) - ->setMaxResults($blockSize); - - $page = 0; - do { - $qbClone = (clone $qb)->setFirstResult($blockSize * $page); - $iterator = $qbClone->getQuery()->toIterable(); - $resultsFound = false; - $page++; - - foreach ($iterator as ['shortCode' => $shortCode]) { - $resultsFound = true; - yield $shortCode; - } - } while ($resultsFound); - } } diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php index ad5e3a5d..cc574ac5 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php @@ -25,6 +25,4 @@ interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificat public function findOneMatching(ShortUrlCreation $meta): ?ShortUrl; public function findOneByImportedUrl(ImportedShlinkUrl $url): ?ShortUrl; - - public function findCrawlableShortCodes(): iterable; } diff --git a/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php b/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php new file mode 100644 index 00000000..04c670fa --- /dev/null +++ b/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php @@ -0,0 +1,50 @@ +getEntityManager(); + $this->query = new CrawlableShortCodesQuery($em, $em->getClassMetadata(ShortUrl::class)); + } + + /** @test */ + public function invokingQueryReturnsExpectedResult(): void + { + $createShortUrl = fn (bool $crawlable) => ShortUrl::create( + ShortUrlCreation::fromRawData(['crawlable' => $crawlable, 'longUrl' => 'foo.com']), + ); + + $shortUrl1 = $createShortUrl(true); + $this->getEntityManager()->persist($shortUrl1); + $shortUrl2 = $createShortUrl(false); + $this->getEntityManager()->persist($shortUrl2); + $shortUrl3 = $createShortUrl(true); + $this->getEntityManager()->persist($shortUrl3); + $shortUrl4 = $createShortUrl(true); + $this->getEntityManager()->persist($shortUrl4); + $shortUrl5 = $createShortUrl(false); + $this->getEntityManager()->persist($shortUrl5); + $this->getEntityManager()->flush(); + + $results = [...($this->query)()]; + + self::assertCount(3, $results); + self::assertContains($shortUrl1->getShortCode(), $results); + self::assertContains($shortUrl3->getShortCode(), $results); + self::assertContains($shortUrl4->getShortCode(), $results); + self::assertNotContains($shortUrl2->getShortCode(), $results); + self::assertNotContains($shortUrl5->getShortCode(), $results); + } +} diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php index a477eff8..c842bcb4 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php @@ -391,37 +391,4 @@ class ShortUrlRepositoryTest extends DatabaseTestCase self::assertNull($this->repo->findOneByImportedUrl($buildImported('my-cool-slug', 'doma.in'))); self::assertNull($this->repo->findOneByImportedUrl($buildImported('another-slug'))); } - - /** @test */ - public function findCrawlableShortCodesReturnsExpectedResult(): void - { - $createShortUrl = fn (bool $crawlable) => ShortUrl::create( - ShortUrlCreation::fromRawData(['crawlable' => $crawlable, 'longUrl' => 'foo.com']), - ); - - $shortUrl1 = $createShortUrl(true); - $this->getEntityManager()->persist($shortUrl1); - $shortUrl2 = $createShortUrl(false); - $this->getEntityManager()->persist($shortUrl2); - $shortUrl3 = $createShortUrl(true); - $this->getEntityManager()->persist($shortUrl3); - $shortUrl4 = $createShortUrl(true); - $this->getEntityManager()->persist($shortUrl4); - $shortUrl5 = $createShortUrl(false); - $this->getEntityManager()->persist($shortUrl5); - $this->getEntityManager()->flush(); - - $iterable = $this->repo->findCrawlableShortCodes(); - $results = []; - foreach ($iterable as $shortCode) { - $results[] = $shortCode; - } - - self::assertCount(3, $results); - self::assertContains($shortUrl1->getShortCode(), $results); - self::assertContains($shortUrl3->getShortCode(), $results); - self::assertContains($shortUrl4->getShortCode(), $results); - self::assertNotContains($shortUrl2->getShortCode(), $results); - self::assertNotContains($shortUrl5->getShortCode(), $results); - } } diff --git a/module/Core/test/Crawling/CrawlingHelperTest.php b/module/Core/test/Crawling/CrawlingHelperTest.php index 1843d35c..295b7ec3 100644 --- a/module/Core/test/Crawling/CrawlingHelperTest.php +++ b/module/Core/test/Crawling/CrawlingHelperTest.php @@ -4,34 +4,26 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Crawling; -use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Crawling\CrawlingHelper; -use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; -use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface; +use Shlinkio\Shlink\Core\ShortUrl\Repository\CrawlableShortCodesQueryInterface; class CrawlingHelperTest extends TestCase { private CrawlingHelper $helper; - private MockObject & EntityManagerInterface $em; + private MockObject & CrawlableShortCodesQueryInterface $query; protected function setUp(): void { - $this->em = $this->createMock(EntityManagerInterface::class); - $this->helper = new CrawlingHelper($this->em); + $this->query = $this->createMock(CrawlableShortCodesQueryInterface::class); + $this->helper = new CrawlingHelper($this->query); } /** @test */ public function listCrawlableShortCodesDelegatesIntoRepository(): void { - $repo = $this->createMock(ShortUrlRepositoryInterface::class); - $repo->expects($this->once())->method('findCrawlableShortCodes')->willReturn([]); - $this->em->expects($this->once())->method('getRepository')->with(ShortUrl::class)->willReturn($repo); - - $result = $this->helper->listCrawlableShortCodes(); - foreach ($result as $shortCode) { - // $result is a generator and therefore, it needs to be iterated - } + $this->query->expects($this->once())->method('__invoke')->willReturn([]); + [...$this->helper->listCrawlableShortCodes()]; } }