Ensured uniqueness on imported short URLs short code

This commit is contained in:
Alejandro Celaya 2020-10-25 10:26:11 +01:00
parent 2256f6a9e7
commit b1a073b1ab
3 changed files with 74 additions and 10 deletions

View file

@ -135,8 +135,8 @@ class ShortUrl extends AbstractEntity
*/
public function regenerateShortCode(): self
{
// In ShortUrls where a custom slug was provided, do nothing
if ($this->customSlugWasProvided) {
// In ShortUrls where a custom slug was provided, throw error, unless it is an imported one
if ($this->customSlugWasProvided && $this->importSource === null) {
throw ShortCodeCannotBeRegeneratedException::forShortUrlWithCustomSlug();
}

View file

@ -7,12 +7,14 @@ namespace Shlinkio\Shlink\Core\Importer;
use Doctrine\ORM\EntityManagerInterface;
use Shlinkio\Shlink\Core\Domain\Resolver\DomainResolverInterface;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface;
use Shlinkio\Shlink\Core\Util\DoctrineBatchIterator;
use Shlinkio\Shlink\Core\Util\TagManagerTrait;
use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface;
use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl;
use Symfony\Component\Console\Style\StyleInterface;
use function sprintf;
class ImportedLinksProcessor implements ImportedLinksProcessorInterface
@ -40,21 +42,71 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface
/** @var ImportedShlinkUrl $url */
foreach ($iterable as $url) {
$longUrl = $url->longUrl();
// Skip already imported URLs
if ($shortUrlRepo->importedUrlExists($url, $importShortCodes)) {
$io->text(sprintf('%s: <comment>Skipped</comment>', $url->longUrl()));
$io->text(sprintf('%s: <comment>Skipped</comment>', $longUrl));
continue;
}
$shortUrl = ShortUrl::fromImport($url, $importShortCodes, $this->domainResolver);
$shortUrl->setTags($this->tagNamesToEntities($this->em, $url->tags()));
if (! $this->handleShortcodeUniqueness($url, $shortUrl, $io, $importShortCodes)) {
continue;
}
// TODO Handle errors while creating short URLs, to avoid making the whole process fail
// * Duplicated short code
$this->em->persist($shortUrl);
$io->text(sprintf('%s: <info>Imported</info>', $url->longUrl()));
$io->text(sprintf('%s: <info>Imported</info>', $longUrl));
}
}
private function handleShortcodeUniqueness(
ImportedShlinkUrl $url,
ShortUrl $shortUrl,
StyleInterface $io,
bool $importShortCodes
): bool {
if ($this->ensureShortCodeUniqueness($shortUrl, $importShortCodes)) {
return true;
}
$longUrl = $url->longUrl();
$action = $io->choice(sprintf(
'Failed to import URL "%s" because its short-code "%s" is already in use. Do you want to generate a new '
. 'one or skip it?',
$longUrl,
$url->shortCode(),
), ['Generate new short-code', 'Skip'], 1);
if ($action === 'Skip') {
$io->text(sprintf('%s: <comment>Skipped</comment>', $longUrl));
return false;
}
return $this->handleShortcodeUniqueness($url, $shortUrl, $io, false);
}
private function ensureShortCodeUniqueness(ShortUrl $shortUrlToBeCreated, bool $hasCustomSlug): bool
{
$shortCode = $shortUrlToBeCreated->getShortCode();
$domain = $shortUrlToBeCreated->getDomain();
$domainAuthority = $domain !== null ? $domain->getAuthority() : null;
/** @var ShortUrlRepository $repo */
$repo = $this->em->getRepository(ShortUrl::class);
$otherShortUrlsExist = $repo->shortCodeIsInUse($shortCode, $domainAuthority);
if (! $otherShortUrlsExist) {
return true;
}
if ($hasCustomSlug) {
return false;
}
$shortUrlToBeCreated->regenerateShortCode();
return $this->ensureShortCodeUniqueness($shortUrlToBeCreated, $hasCustomSlug);
}
}

View file

@ -4,12 +4,14 @@ declare(strict_types=1);
namespace ShlinkioTest\Shlink\Core\Entity;
use Cake\Chronos\Chronos;
use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException;
use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl;
use function Functional\map;
use function range;
use function strlen;
@ -44,10 +46,12 @@ class ShortUrlTest extends TestCase
];
}
/** @test */
public function regenerateShortCodeProperlyChangesTheValueOnValidShortUrls(): void
/**
* @test
* @dataProvider provideValidShortUrls
*/
public function regenerateShortCodeProperlyChangesTheValueOnValidShortUrls(ShortUrl $shortUrl): void
{
$shortUrl = new ShortUrl('');
$firstShortCode = $shortUrl->getShortCode();
$shortUrl->regenerateShortCode();
@ -56,6 +60,14 @@ class ShortUrlTest extends TestCase
self::assertNotEquals($firstShortCode, $secondShortCode);
}
public function provideValidShortUrls(): iterable
{
yield 'no custom slug' => [new ShortUrl('')];
yield 'imported with custom slug' => [
ShortUrl::fromImport(new ImportedShlinkUrl('', '', [], Chronos::now(), null, 'custom-slug'), true),
];
}
/**
* @test
* @dataProvider provideLengths