From 03a9697298c1b4ac101a09e38e846c6bfe71f695 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 25 Oct 2020 13:20:34 +0100 Subject: [PATCH] Created ImportedLinksProcessorTest --- module/Core/config/dependencies.config.php | 3 + .../src/Importer/ImportedLinksProcessor.php | 11 +- ...chIterator.php => DoctrineBatchHelper.php} | 18 +-- .../src/Util/DoctrineBatchHelperInterface.php | 10 ++ .../Importer/ImportedLinksProcessorTest.php | 145 ++++++++++++++++++ 5 files changed, 171 insertions(+), 16 deletions(-) rename module/Core/src/Util/{DoctrineBatchIterator.php => DoctrineBatchHelper.php} (63%) create mode 100644 module/Core/src/Util/DoctrineBatchHelperInterface.php create mode 100644 module/Core/test/Importer/ImportedLinksProcessorTest.php diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 2f0a6aa8..4d68101b 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -36,6 +36,7 @@ return [ Domain\DomainService::class => ConfigAbstractFactory::class, Util\UrlValidator::class => ConfigAbstractFactory::class, + Util\DoctrineBatchHelper::class => ConfigAbstractFactory::class, Action\RedirectAction::class => ConfigAbstractFactory::class, Action\PixelAction::class => ConfigAbstractFactory::class, @@ -87,6 +88,7 @@ return [ Domain\DomainService::class => ['em'], Util\UrlValidator::class => ['httpClient', Options\UrlShortenerOptions::class], + Util\DoctrineBatchHelper::class => ['em'], Action\RedirectAction::class => [ Service\ShortUrl\ShortUrlResolver::class, @@ -115,6 +117,7 @@ return [ 'em', Resolver\PersistenceDomainResolver::class, Service\ShortUrl\ShortCodeHelper::class, + Util\DoctrineBatchHelper::class, ], ], diff --git a/module/Core/src/Importer/ImportedLinksProcessor.php b/module/Core/src/Importer/ImportedLinksProcessor.php index b76ada9a..e072fbb8 100644 --- a/module/Core/src/Importer/ImportedLinksProcessor.php +++ b/module/Core/src/Importer/ImportedLinksProcessor.php @@ -9,7 +9,7 @@ use Shlinkio\Shlink\Core\Domain\Resolver\DomainResolverInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelperInterface; -use Shlinkio\Shlink\Core\Util\DoctrineBatchIterator; +use Shlinkio\Shlink\Core\Util\DoctrineBatchHelperInterface; use Shlinkio\Shlink\Core\Util\TagManagerTrait; use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; @@ -24,15 +24,18 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface private EntityManagerInterface $em; private DomainResolverInterface $domainResolver; private ShortCodeHelperInterface $shortCodeHelper; + private DoctrineBatchHelperInterface $batchHelper; public function __construct( EntityManagerInterface $em, DomainResolverInterface $domainResolver, - ShortCodeHelperInterface $shortCodeHelper + ShortCodeHelperInterface $shortCodeHelper, + DoctrineBatchHelperInterface $batchHelper ) { $this->em = $em; $this->domainResolver = $domainResolver; $this->shortCodeHelper = $shortCodeHelper; + $this->batchHelper = $batchHelper; } /** @@ -43,7 +46,7 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface /** @var ShortUrlRepositoryInterface $shortUrlRepo */ $shortUrlRepo = $this->em->getRepository(ShortUrl::class); $importShortCodes = $params['import_short_codes']; - $iterable = new DoctrineBatchIterator($shlinkUrls, $this->em, 100); + $iterable = $this->batchHelper->wrapIterable($shlinkUrls, 100); /** @var ImportedShlinkUrl $url */ foreach ($iterable as $url) { @@ -90,6 +93,6 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface return false; } - return $this->handleShortCodeUniqueness($url, $shortUrl, $io, false); + return $this->shortCodeHelper->ensureShortCodeUniqueness($shortUrl, false); } } diff --git a/module/Core/src/Util/DoctrineBatchIterator.php b/module/Core/src/Util/DoctrineBatchHelper.php similarity index 63% rename from module/Core/src/Util/DoctrineBatchIterator.php rename to module/Core/src/Util/DoctrineBatchHelper.php index 05311b09..207d2093 100644 --- a/module/Core/src/Util/DoctrineBatchIterator.php +++ b/module/Core/src/Util/DoctrineBatchHelper.php @@ -5,32 +5,26 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Util; use Doctrine\ORM\EntityManagerInterface; -use IteratorAggregate; use Throwable; /** * Inspired by ocramius/doctrine-batch-utils https://github.com/Ocramius/DoctrineBatchUtils */ -class DoctrineBatchIterator implements IteratorAggregate +class DoctrineBatchHelper implements DoctrineBatchHelperInterface { - private iterable $resultSet; private EntityManagerInterface $em; - private int $batchSize; - public function __construct(iterable $resultSet, EntityManagerInterface $em, int $batchSize) + public function __construct(EntityManagerInterface $em) { - $this->resultSet = $resultSet; $this->em = $em; - $this->batchSize = $batchSize; } /** * @throws Throwable */ - public function getIterator(): iterable + public function wrapIterable(iterable $resultSet, int $batchSize): iterable { $iteration = 0; - $resultSet = $this->resultSet; $this->em->beginTransaction(); @@ -38,7 +32,7 @@ class DoctrineBatchIterator implements IteratorAggregate foreach ($resultSet as $key => $value) { $iteration++; yield $key => $value; - $this->flushAndClearBatch($iteration); + $this->flushAndClearBatch($iteration, $batchSize); } } catch (Throwable $e) { $this->em->rollback(); @@ -50,9 +44,9 @@ class DoctrineBatchIterator implements IteratorAggregate $this->em->commit(); } - private function flushAndClearBatch(int $iteration): void + private function flushAndClearBatch(int $iteration, int $batchSize): void { - if ($iteration % $this->batchSize) { + if ($iteration % $batchSize) { return; } diff --git a/module/Core/src/Util/DoctrineBatchHelperInterface.php b/module/Core/src/Util/DoctrineBatchHelperInterface.php new file mode 100644 index 00000000..941561ed --- /dev/null +++ b/module/Core/src/Util/DoctrineBatchHelperInterface.php @@ -0,0 +1,10 @@ +em = $this->prophesize(EntityManagerInterface::class); + $this->repo = $this->prophesize(ShortUrlRepositoryInterface::class); + $this->em->getRepository(ShortUrl::class)->willReturn($this->repo->reveal()); + + $this->shortCodeHelper = $this->prophesize(ShortCodeHelperInterface::class); + $batchHelper = $this->prophesize(DoctrineBatchHelperInterface::class); + $batchHelper->wrapIterable(Argument::cetera())->willReturnArgument(0); + + $this->processor = new ImportedLinksProcessor( + $this->em->reveal(), + new SimpleDomainResolver(), + $this->shortCodeHelper->reveal(), + $batchHelper->reveal(), + ); + + $this->io = $this->prophesize(StyleInterface::class); + } + + /** @test */ + public function newUrlsWithNoErrorsAreAllPersisted(): void + { + $urls = [ + new ImportedShlinkUrl('', 'foo', [], Chronos::now(), null, 'foo'), + new ImportedShlinkUrl('', 'bar', [], Chronos::now(), null, 'bar'), + new ImportedShlinkUrl('', 'baz', [], Chronos::now(), null, 'baz'), + ]; + $expectedCalls = count($urls); + + $importedUrlExists = $this->repo->importedUrlExists(Argument::cetera())->willReturn(false); + $ensureUniqueness = $this->shortCodeHelper->ensureShortCodeUniqueness(Argument::cetera())->willReturn(true); + $persist = $this->em->persist(Argument::type(ShortUrl::class)); + + $this->processor->process($this->io->reveal(), $urls, ['import_short_codes' => true]); + + $importedUrlExists->shouldHaveBeenCalledTimes($expectedCalls); + $ensureUniqueness->shouldHaveBeenCalledTimes($expectedCalls); + $persist->shouldHaveBeenCalledTimes($expectedCalls); + $this->io->text(Argument::type('string'))->shouldHaveBeenCalledTimes($expectedCalls); + } + + /** @test */ + public function alreadyImportedUrlsAreSkipped(): void + { + $urls = [ + new ImportedShlinkUrl('', 'foo', [], Chronos::now(), null, 'foo'), + new ImportedShlinkUrl('', 'bar', [], Chronos::now(), null, 'bar'), + new ImportedShlinkUrl('', 'baz', [], Chronos::now(), null, 'baz'), + new ImportedShlinkUrl('', 'baz2', [], Chronos::now(), null, 'baz2'), + new ImportedShlinkUrl('', 'baz3', [], Chronos::now(), null, 'baz3'), + ]; + $contains = fn (string $needle) => fn (string $text) => str_contains($text, $needle); + + $importedUrlExists = $this->repo->importedUrlExists(Argument::cetera())->will(function (array $args): bool { + /** @var ImportedShlinkUrl $url */ + [$url] = $args; + + return contains(['foo', 'baz2', 'baz3'], $url->longUrl()); + }); + $ensureUniqueness = $this->shortCodeHelper->ensureShortCodeUniqueness(Argument::cetera())->willReturn(true); + $persist = $this->em->persist(Argument::type(ShortUrl::class)); + + $this->processor->process($this->io->reveal(), $urls, ['import_short_codes' => true]); + + $importedUrlExists->shouldHaveBeenCalledTimes(count($urls)); + $ensureUniqueness->shouldHaveBeenCalledTimes(2); + $persist->shouldHaveBeenCalledTimes(2); + $this->io->text(Argument::that($contains('Skipped')))->shouldHaveBeenCalledTimes(3); + $this->io->text(Argument::that($contains('Imported')))->shouldHaveBeenCalledTimes(2); + } + + /** @test */ + public function nonUniqueShortCodesAreAskedToUser(): void + { + $urls = [ + new ImportedShlinkUrl('', 'foo', [], Chronos::now(), null, 'foo'), + new ImportedShlinkUrl('', 'bar', [], Chronos::now(), null, 'bar'), + new ImportedShlinkUrl('', 'baz', [], Chronos::now(), null, 'baz'), + new ImportedShlinkUrl('', 'baz2', [], Chronos::now(), null, 'baz2'), + new ImportedShlinkUrl('', 'baz3', [], Chronos::now(), null, 'baz3'), + ]; + $contains = fn (string $needle) => fn (string $text) => str_contains($text, $needle); + + $importedUrlExists = $this->repo->importedUrlExists(Argument::cetera())->willReturn(false); + $failingEnsureUniqueness = $this->shortCodeHelper->ensureShortCodeUniqueness( + Argument::any(), + true, + )->willReturn(false); + $successEnsureUniqueness = $this->shortCodeHelper->ensureShortCodeUniqueness( + Argument::any(), + false, + )->willReturn(true); + $choice = $this->io->choice(Argument::cetera())->will(function (array $args) { + /** @var ImportedShlinkUrl $url */ + [$question] = $args; + + return some(['foo', 'baz2', 'baz3'], fn (string $item) => str_contains($question, $item)) ? 'Skip' : ''; + }); + $persist = $this->em->persist(Argument::type(ShortUrl::class)); + + $this->processor->process($this->io->reveal(), $urls, ['import_short_codes' => true]); + + $importedUrlExists->shouldHaveBeenCalledTimes(count($urls)); + $failingEnsureUniqueness->shouldHaveBeenCalledTimes(5); + $successEnsureUniqueness->shouldHaveBeenCalledTimes(2); + $choice->shouldHaveBeenCalledTimes(5); + $persist->shouldHaveBeenCalledTimes(2); + $this->io->text(Argument::that($contains('Skipped')))->shouldHaveBeenCalledTimes(3); + $this->io->text(Argument::that($contains('Imported')))->shouldHaveBeenCalledTimes(2); + } +}