diff --git a/module/Core/src/Exception/NonUniqueSlugException.php b/module/Core/src/Exception/NonUniqueSlugException.php index 8887f961..3b8b0b15 100644 --- a/module/Core/src/Exception/NonUniqueSlugException.php +++ b/module/Core/src/Exception/NonUniqueSlugException.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Exception; use Fig\Http\Message\StatusCodeInterface; use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use function sprintf; @@ -34,4 +35,9 @@ class NonUniqueSlugException extends InvalidArgumentException implements Problem return $e; } + + public static function fromImport(ImportedShlinkUrl $importedUrl): self + { + return self::fromSlug($importedUrl->shortCode(), $importedUrl->domain()); + } } diff --git a/module/Core/src/Importer/ImportedLinksProcessor.php b/module/Core/src/Importer/ImportedLinksProcessor.php index 25393055..e700e8a8 100644 --- a/module/Core/src/Importer/ImportedLinksProcessor.php +++ b/module/Core/src/Importer/ImportedLinksProcessor.php @@ -4,16 +4,16 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Importer; -use Cake\Chronos\Chronos; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Entity\Visit; +use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelperInterface; use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface; use Shlinkio\Shlink\Core\Util\DoctrineBatchHelperInterface; use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; +use Shlinkio\Shlink\Importer\Sources\ImportSources; use Symfony\Component\Console\Style\StyleInterface; use function sprintf; @@ -45,7 +45,8 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface public function process(StyleInterface $io, iterable $shlinkUrls, array $params): void { $importShortCodes = $params['import_short_codes']; - $iterable = $this->batchHelper->wrapIterable($shlinkUrls, 100); + $source = $params['source']; + $iterable = $this->batchHelper->wrapIterable($shlinkUrls, $source === ImportSources::SHLINK ? 10 : 100); /** @var ImportedShlinkUrl $importedUrl */ foreach ($iterable as $importedUrl) { @@ -59,53 +60,37 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface return $action === 'Skip'; }; - [$shortUrl, $isNew] = $this->getOrCreateShortUrl($importedUrl, $importShortCodes, $skipOnShortCodeConflict); - $longUrl = $importedUrl->longUrl(); - if ($shortUrl === null) { + + try { + $shortUrlImporting = $this->resolveShortUrl($importedUrl, $importShortCodes, $skipOnShortCodeConflict); + } catch (NonUniqueSlugException $e) { $io->text(sprintf('%s: Error', $longUrl)); continue; } - $importedVisits = $this->importVisits($importedUrl, $shortUrl); - - if ($importedVisits === 0) { - $io->text( - $isNew - ? sprintf('%s: Imported', $longUrl) - : sprintf('%s: Skipped', $longUrl), - ); - } else { - $io->text( - $isNew - ? sprintf('%s: Imported with %s visits', $longUrl, $importedVisits) - : sprintf( - '%s: Skipped. Imported %s visits', - $longUrl, - $importedVisits, - ), - ); - } + $resultMessage = $shortUrlImporting->importVisits($importedUrl->visits(), $this->em); + $io->text(sprintf('%s: %s', $longUrl, $resultMessage)); } } - private function getOrCreateShortUrl( + private function resolveShortUrl( ImportedShlinkUrl $importedUrl, bool $importShortCodes, callable $skipOnShortCodeConflict - ): array { + ): ShortUrlImporting { $alreadyImportedShortUrl = $this->shortUrlRepo->findOneByImportedUrl($importedUrl); if ($alreadyImportedShortUrl !== null) { - return [$alreadyImportedShortUrl, false]; + return ShortUrlImporting::fromExistingShortUrl($alreadyImportedShortUrl); } $shortUrl = ShortUrl::fromImport($importedUrl, $importShortCodes, $this->relationResolver); if (! $this->handleShortCodeUniqueness($shortUrl, $importShortCodes, $skipOnShortCodeConflict)) { - return [null, false]; + throw NonUniqueSlugException::fromImport($importedUrl); } $this->em->persist($shortUrl); - return [$shortUrl, true]; + return ShortUrlImporting::fromNewShortUrl($shortUrl); } private function handleShortCodeUniqueness( @@ -123,25 +108,4 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface return $this->shortCodeHelper->ensureShortCodeUniqueness($shortUrl, false); } - - private function importVisits(ImportedShlinkUrl $importedUrl, ShortUrl $shortUrl): int - { - $mostRecentImportedDate = $shortUrl->mostRecentImportedVisitDate(); - - $importedVisits = 0; - foreach ($importedUrl->visits() as $importedVisit) { - // Skip visits which are older than the most recent already imported visit's date - if ( - $mostRecentImportedDate !== null - && $mostRecentImportedDate->gte(Chronos::instance($importedVisit->date())) - ) { - continue; - } - - $this->em->persist(Visit::fromImport($shortUrl, $importedVisit)); - $importedVisits++; - } - - return $importedVisits; - } } diff --git a/module/Core/src/Importer/ShortUrlImporting.php b/module/Core/src/Importer/ShortUrlImporting.php new file mode 100644 index 00000000..b5ae4651 --- /dev/null +++ b/module/Core/src/Importer/ShortUrlImporting.php @@ -0,0 +1,65 @@ +shortUrl = $shortUrl; + $this->isNew = $isNew; + } + + public static function fromExistingShortUrl(ShortUrl $shortUrl): self + { + return new self($shortUrl, false); + } + + public static function fromNewShortUrl(ShortUrl $shortUrl): self + { + return new self($shortUrl, true); + } + + /** + * @param iterable|ImportedShlinkVisit[] $visits + */ + public function importVisits(iterable $visits, EntityManagerInterface $em): string + { + $mostRecentImportedDate = $this->shortUrl->mostRecentImportedVisitDate(); + + $importedVisits = 0; + foreach ($visits as $importedVisit) { + // Skip visits which are older than the most recent already imported visit's date + if ( + $mostRecentImportedDate !== null + && $mostRecentImportedDate->gte(Chronos::instance($importedVisit->date())) + ) { + continue; + } + + $em->persist(Visit::fromImport($this->shortUrl, $importedVisit)); + $importedVisits++; + } + + if ($importedVisits === 0) { + return $this->isNew ? 'Imported' : 'Skipped'; + } + + return $this->isNew + ? sprintf('Imported with %s visits', $importedVisits) + : sprintf('Skipped. Imported %s visits', $importedVisits); + } +} diff --git a/module/Core/src/Service/ShortUrl/ShortCodeHelper.php b/module/Core/src/Service/ShortUrl/ShortCodeHelper.php index 6e4e57ac..3df4c016 100644 --- a/module/Core/src/Service/ShortUrl/ShortCodeHelper.php +++ b/module/Core/src/Service/ShortUrl/ShortCodeHelper.php @@ -8,7 +8,7 @@ use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; -class ShortCodeHelper implements ShortCodeHelperInterface +class ShortCodeHelper implements ShortCodeHelperInterface // TODO Rename to ShortCodeUniquenessHelper { private EntityManagerInterface $em; diff --git a/module/Core/src/Service/ShortUrl/ShortCodeHelperInterface.php b/module/Core/src/Service/ShortUrl/ShortCodeHelperInterface.php index af3f2aa5..a020a30c 100644 --- a/module/Core/src/Service/ShortUrl/ShortCodeHelperInterface.php +++ b/module/Core/src/Service/ShortUrl/ShortCodeHelperInterface.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\Core\Service\ShortUrl; use Shlinkio\Shlink\Core\Entity\ShortUrl; -interface ShortCodeHelperInterface +interface ShortCodeHelperInterface // TODO Rename to ShortCodeUniquenessHelperInterface { public function ensureShortCodeUniqueness(ShortUrl $shortUrlToBeCreated, bool $hasCustomSlug): bool; } diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index 170e21b7..d17c5720 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -20,6 +20,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Resolver\SimpleShortUrlRelationResolver; use Shlinkio\Shlink\Core\Util\DoctrineBatchHelperInterface; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisit; +use Shlinkio\Shlink\Importer\Sources\ImportSources; use Symfony\Component\Console\Style\StyleInterface; use function count; @@ -31,6 +32,8 @@ class ImportedLinksProcessorTest extends TestCase { use ProphecyTrait; + private const PARAMS = ['import_short_codes' => true, 'source' => ImportSources::BITLY]; + private ImportedLinksProcessor $processor; private ObjectProphecy $em; private ObjectProphecy $shortCodeHelper; @@ -71,7 +74,7 @@ class ImportedLinksProcessorTest extends TestCase $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]); + $this->processor->process($this->io->reveal(), $urls, self::PARAMS); $importedUrlExists->shouldHaveBeenCalledTimes($expectedCalls); $ensureUniqueness->shouldHaveBeenCalledTimes($expectedCalls); @@ -101,7 +104,7 @@ class ImportedLinksProcessorTest extends TestCase $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]); + $this->processor->process($this->io->reveal(), $urls, self::PARAMS); $importedUrlExists->shouldHaveBeenCalledTimes(count($urls)); $ensureUniqueness->shouldHaveBeenCalledTimes(2); @@ -138,7 +141,7 @@ class ImportedLinksProcessorTest extends TestCase }); $persist = $this->em->persist(Argument::type(ShortUrl::class)); - $this->processor->process($this->io->reveal(), $urls, ['import_short_codes' => true]); + $this->processor->process($this->io->reveal(), $urls, self::PARAMS); $importedUrlExists->shouldHaveBeenCalledTimes(count($urls)); $failingEnsureUniqueness->shouldHaveBeenCalledTimes(5); @@ -164,7 +167,7 @@ class ImportedLinksProcessorTest extends TestCase $persistUrl = $this->em->persist(Argument::type(ShortUrl::class)); $persistVisits = $this->em->persist(Argument::type(Visit::class)); - $this->processor->process($this->io->reveal(), [$importedUrl], ['import_short_codes' => true]); + $this->processor->process($this->io->reveal(), [$importedUrl], self::PARAMS); $findExisting->shouldHaveBeenCalledOnce(); $ensureUniqueness->shouldHaveBeenCalledTimes($foundShortUrl === null ? 1 : 0);