Encapsulated logic to get rid of nested ifs

This commit is contained in:
Alejandro Celaya 2021-04-18 17:07:56 +02:00
parent b277f431c2
commit e9a5284dde
6 changed files with 95 additions and 57 deletions

View file

@ -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());
}
}

View file

@ -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: <fg=red>Error</>', $longUrl));
continue;
}
$importedVisits = $this->importVisits($importedUrl, $shortUrl);
if ($importedVisits === 0) {
$io->text(
$isNew
? sprintf('%s: <info>Imported</info>', $longUrl)
: sprintf('%s: <comment>Skipped</comment>', $longUrl),
);
} else {
$io->text(
$isNew
? sprintf('%s: <info>Imported</info> with <info>%s</info> visits', $longUrl, $importedVisits)
: sprintf(
'%s: <comment>Skipped</comment>. Imported <info>%s</info> 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;
}
}

View file

@ -0,0 +1,65 @@
<?php
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\Importer\Model\ImportedShlinkVisit;
use function sprintf;
final class ShortUrlImporting
{
private ShortUrl $shortUrl;
private bool $isNew;
private function __construct(ShortUrl $shortUrl, bool $isNew)
{
$this->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 ? '<info>Imported</info>' : '<comment>Skipped</comment>';
}
return $this->isNew
? sprintf('<info>Imported</info> with <info>%s</info> visits', $importedVisits)
: sprintf('<comment>Skipped</comment>. Imported <info>%s</info> visits', $importedVisits);
}
}

View file

@ -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;

View file

@ -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;
}

View file

@ -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);