diff --git a/.travis.yml b/.travis.yml index c9945281..d45f1d49 100644 --- a/.travis.yml +++ b/.travis.yml @@ -47,7 +47,7 @@ before_install: - yes | pecl install pdo_sqlsrv-5.9.0preview1 swoole-4.5.5 pcov install: - - composer self-update + - composer self-update --1 - composer install --no-interaction --prefer-dist $COMPOSER_FLAGS before_script: diff --git a/CHANGELOG.md b/CHANGELOG.md index ff32f795..b2afc50f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#832](https://github.com/shlinkio/shlink/issues/832) Added support to customize the port in which the docker image listens by using the `PORT` env var or the `port` config option. +* [#860](https://github.com/shlinkio/shlink/issues/860) Added support to import links from bit.ly. + + Run the command `short-urls:import bitly` and introduce requested information in order to import all your links. + + Other sources will be supported in future releases. + #### Changed * [#836](https://github.com/shlinkio/shlink/issues/836) Added support for the `-` notation while determining how to order the short URLs list, as in `?orderBy=shortCode-DESC`. This effectively deprecates the array notation (`?orderBy[shortCode]=DESC`), that will be removed in Shlink 3.0.0 @@ -45,6 +51,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#837](https://github.com/shlinkio/shlink/issues/837) Drastically improved performance when creating a new shortUrl and providing `findIfExists = true`. + ## 2.3.0 - 2020-08-09 #### Added diff --git a/composer.json b/composer.json index 99844320..c346e661 100644 --- a/composer.json +++ b/composer.json @@ -53,6 +53,7 @@ "shlinkio/shlink-common": "^3.2.0", "shlinkio/shlink-config": "^1.0", "shlinkio/shlink-event-dispatcher": "^1.4", + "shlinkio/shlink-importer": "^2.0.1", "shlinkio/shlink-installer": "^5.1.0", "shlinkio/shlink-ip-geolocation": "^1.5", "symfony/console": "^5.1", diff --git a/config/autoload/dependencies.global.php b/config/autoload/dependencies.global.php index 023b3c4e..dbc553f1 100644 --- a/config/autoload/dependencies.global.php +++ b/config/autoload/dependencies.global.php @@ -2,7 +2,9 @@ declare(strict_types=1); +use GuzzleHttp\Client; use Mezzio\Container; +use Psr\Http\Client\ClientInterface; return [ @@ -13,6 +15,10 @@ return [ ], ], + 'aliases' => [ + ClientInterface::class => Client::class, + ], + 'lazy_services' => [ 'proxies_target_dir' => 'data/proxies', 'proxies_namespace' => 'ShlinkProxy', diff --git a/config/config.php b/config/config.php index d7fd6a3b..ba0657fc 100644 --- a/config/config.php +++ b/config/config.php @@ -21,6 +21,7 @@ return (new ConfigAggregator\ConfigAggregator([ Diactoros\ConfigProvider::class, Common\ConfigProvider::class, Config\ConfigProvider::class, + Importer\ConfigProvider::class, IpGeolocation\ConfigProvider::class, EventDispatcher\ConfigProvider::class, Core\ConfigProvider::class, diff --git a/data/migrations/Version20201023090929.php b/data/migrations/Version20201023090929.php new file mode 100644 index 00000000..05d16c22 --- /dev/null +++ b/data/migrations/Version20201023090929.php @@ -0,0 +1,44 @@ +getTable('short_urls'); + $this->skipIf($shortUrls->hasColumn(self::IMPORT_SOURCE_COLUMN)); + + $shortUrls->addColumn(self::IMPORT_SOURCE_COLUMN, Types::STRING, [ + 'length' => 255, + 'notnull' => false, + ]); + $shortUrls->addColumn('import_original_short_code', Types::STRING, [ + 'length' => 255, + 'notnull' => false, + ]); + + $shortUrls->addUniqueIndex( + [self::IMPORT_SOURCE_COLUMN, 'import_original_short_code', 'domain_id'], + 'unique_imports', + ); + } + + public function down(Schema $schema): void + { + $shortUrls = $schema->getTable('short_urls'); + $this->skipIf(! $shortUrls->hasColumn(self::IMPORT_SOURCE_COLUMN)); + + $shortUrls->dropColumn(self::IMPORT_SOURCE_COLUMN); + $shortUrls->dropColumn('import_original_short_code'); + $shortUrls->dropIndex('unique_imports'); + } +} diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 5dcef9a2..4d68101b 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -10,6 +10,7 @@ use Psr\EventDispatcher\EventDispatcherInterface; use Shlinkio\Shlink\Core\Domain\Resolver; use Shlinkio\Shlink\Core\ErrorHandler; use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; +use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface; return [ @@ -31,9 +32,11 @@ return [ Tag\TagService::class => ConfigAbstractFactory::class, Service\ShortUrl\DeleteShortUrlService::class => ConfigAbstractFactory::class, Service\ShortUrl\ShortUrlResolver::class => ConfigAbstractFactory::class, + Service\ShortUrl\ShortCodeHelper::class => ConfigAbstractFactory::class, 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, @@ -42,6 +45,12 @@ return [ Resolver\PersistenceDomainResolver::class => ConfigAbstractFactory::class, Mercure\MercureUpdatesGenerator::class => ConfigAbstractFactory::class, + + Importer\ImportedLinksProcessor::class => ConfigAbstractFactory::class, + ], + + 'aliases' => [ + ImportedLinksProcessorInterface::class => Importer\ImportedLinksProcessor::class, ], ], @@ -54,7 +63,12 @@ return [ Options\NotFoundRedirectOptions::class => ['config.not_found_redirects'], Options\UrlShortenerOptions::class => ['config.url_shortener'], - Service\UrlShortener::class => [Util\UrlValidator::class, 'em', Resolver\PersistenceDomainResolver::class], + Service\UrlShortener::class => [ + Util\UrlValidator::class, + 'em', + Resolver\PersistenceDomainResolver::class, + Service\ShortUrl\ShortCodeHelper::class, + ], Service\VisitsTracker::class => [ 'em', EventDispatcherInterface::class, @@ -70,9 +84,11 @@ return [ Service\ShortUrl\ShortUrlResolver::class, ], Service\ShortUrl\ShortUrlResolver::class => ['em'], + Service\ShortUrl\ShortCodeHelper::class => ['em'], Domain\DomainService::class => ['em'], Util\UrlValidator::class => ['httpClient', Options\UrlShortenerOptions::class], + Util\DoctrineBatchHelper::class => ['em'], Action\RedirectAction::class => [ Service\ShortUrl\ShortUrlResolver::class, @@ -96,6 +112,13 @@ return [ Resolver\PersistenceDomainResolver::class => ['em'], Mercure\MercureUpdatesGenerator::class => ['config.url_shortener.domain'], + + Importer\ImportedLinksProcessor::class => [ + 'em', + Resolver\PersistenceDomainResolver::class, + Service\ShortUrl\ShortCodeHelper::class, + Util\DoctrineBatchHelper::class, + ], ], ]; diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php index 871ac113..4f9b3747 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php @@ -51,6 +51,16 @@ return static function (ClassMetadata $metadata, array $emConfig): void { ->nullable() ->build(); + $builder->createField('importSource', Types::STRING) + ->columnName('import_source') + ->nullable() + ->build(); + + $builder->createField('importOriginalShortCode', Types::STRING) + ->columnName('import_original_short_code') + ->nullable() + ->build(); + $builder->createOneToMany('visits', Entity\Visit::class) ->mappedBy('shortUrl') ->fetchExtraLazy() diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index ba10a44a..2d5cb6de 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -14,6 +14,8 @@ use Shlinkio\Shlink\Core\Domain\Resolver\SimpleDomainResolver; use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException; use Shlinkio\Shlink\Core\Model\ShortUrlEdit; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use function count; use function Shlinkio\Shlink\Core\generateRandomShortCode; @@ -33,6 +35,8 @@ class ShortUrl extends AbstractEntity private ?Domain $domain = null; private bool $customSlugWasProvided; private int $shortCodeLength; + private ?string $importSource = null; + private ?string $importOriginalShortCode = null; public function __construct( string $longUrl, @@ -54,6 +58,27 @@ class ShortUrl extends AbstractEntity $this->domain = ($domainResolver ?? new SimpleDomainResolver())->resolveDomain($meta->getDomain()); } + public static function fromImport( + ImportedShlinkUrl $url, + bool $importShortCode, + ?DomainResolverInterface $domainResolver = null + ): self { + $meta = [ + ShortUrlMetaInputFilter::DOMAIN => $url->domain(), + ShortUrlMetaInputFilter::VALIDATE_URL => false, + ]; + if ($importShortCode) { + $meta[ShortUrlMetaInputFilter::CUSTOM_SLUG] = $url->shortCode(); + } + + $instance = new self($url->longUrl(), ShortUrlMeta::fromRawData($meta), $domainResolver); + $instance->importSource = $url->source(); + $instance->importOriginalShortCode = $url->shortCode(); + $instance->dateCreated = Chronos::instance($url->createdAt()); + + return $instance; + } + public function getLongUrl(): string { return $this->longUrl; @@ -110,10 +135,10 @@ class ShortUrl extends AbstractEntity /** * @throws ShortCodeCannotBeRegeneratedException */ - public function regenerateShortCode(): self + public function regenerateShortCode(): void { - // 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(); } @@ -123,7 +148,6 @@ class ShortUrl extends AbstractEntity } $this->shortCode = generateRandomShortCode($this->shortCodeLength); - return $this; } public function getValidSince(): ?Chronos diff --git a/module/Core/src/Importer/ImportedLinksProcessor.php b/module/Core/src/Importer/ImportedLinksProcessor.php new file mode 100644 index 00000000..e072fbb8 --- /dev/null +++ b/module/Core/src/Importer/ImportedLinksProcessor.php @@ -0,0 +1,98 @@ +em = $em; + $this->domainResolver = $domainResolver; + $this->shortCodeHelper = $shortCodeHelper; + $this->batchHelper = $batchHelper; + } + + /** + * @param iterable|ImportedShlinkUrl[] $shlinkUrls + */ + public function process(StyleInterface $io, iterable $shlinkUrls, array $params): void + { + /** @var ShortUrlRepositoryInterface $shortUrlRepo */ + $shortUrlRepo = $this->em->getRepository(ShortUrl::class); + $importShortCodes = $params['import_short_codes']; + $iterable = $this->batchHelper->wrapIterable($shlinkUrls, 100); + + /** @var ImportedShlinkUrl $url */ + foreach ($iterable as $url) { + $longUrl = $url->longUrl(); + + // Skip already imported URLs + if ($shortUrlRepo->importedUrlExists($url)) { + $io->text(sprintf('%s: Skipped', $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; + } + + $this->em->persist($shortUrl); + $io->text(sprintf('%s: Imported', $longUrl)); + } + } + + private function handleShortCodeUniqueness( + ImportedShlinkUrl $url, + ShortUrl $shortUrl, + StyleInterface $io, + bool $importShortCodes + ): bool { + if ($this->shortCodeHelper->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: Skipped', $longUrl)); + return false; + } + + return $this->shortCodeHelper->ensureShortCodeUniqueness($shortUrl, false); + } +} diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index e2cf578f..27dac54b 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -11,6 +11,7 @@ use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use function array_column; use function array_key_exists; @@ -189,13 +190,7 @@ DQL; ->setParameter('slug', $slug) ->setMaxResults(1); - if ($domain !== null) { - $qb->join('s.domain', 'd') - ->andWhere($qb->expr()->eq('d.authority', ':authority')) - ->setParameter('authority', $domain); - } else { - $qb->andWhere($qb->expr()->isNull('s.domain')); - } + $this->whereDomainIs($qb, $domain); return $qb; } @@ -254,4 +249,32 @@ DQL; return $qb->getQuery()->getOneOrNullResult(); } + + public function importedUrlExists(ImportedShlinkUrl $url): bool + { + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->select('COUNT(DISTINCT s.id)') + ->from(ShortUrl::class, 's') + ->andWhere($qb->expr()->eq('s.importOriginalShortCode', ':shortCode')) + ->setParameter('shortCode', $url->shortCode()) + ->andWhere($qb->expr()->eq('s.importSource', ':importSource')) + ->setParameter('importSource', $url->source()) + ->setMaxResults(1); + + $this->whereDomainIs($qb, $url->domain()); + + $result = (int) $qb->getQuery()->getSingleScalarResult(); + return $result > 0; + } + + private function whereDomainIs(QueryBuilder $qb, ?string $domain): void + { + if ($domain !== null) { + $qb->join('s.domain', 'd') + ->andWhere($qb->expr()->eq('d.authority', ':authority')) + ->setParameter('authority', $domain); + } else { + $qb->andWhere($qb->expr()->isNull('s.domain')); + } + } } diff --git a/module/Core/src/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/Repository/ShortUrlRepositoryInterface.php index 65278a85..1d6f38a8 100644 --- a/module/Core/src/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/Repository/ShortUrlRepositoryInterface.php @@ -9,6 +9,7 @@ use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; interface ShortUrlRepositoryInterface extends ObjectRepository { @@ -30,4 +31,6 @@ interface ShortUrlRepositoryInterface extends ObjectRepository public function shortCodeIsInUse(string $slug, ?string $domain): bool; public function findOneMatching(string $url, array $tags, ShortUrlMeta $meta): ?ShortUrl; + + public function importedUrlExists(ImportedShlinkUrl $url): bool; } diff --git a/module/Core/src/Service/ShortUrl/ShortCodeHelper.php b/module/Core/src/Service/ShortUrl/ShortCodeHelper.php new file mode 100644 index 00000000..6e4e57ac --- /dev/null +++ b/module/Core/src/Service/ShortUrl/ShortCodeHelper.php @@ -0,0 +1,41 @@ +em = $em; + } + + public 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); + } +} diff --git a/module/Core/src/Service/ShortUrl/ShortCodeHelperInterface.php b/module/Core/src/Service/ShortUrl/ShortCodeHelperInterface.php new file mode 100644 index 00000000..af3f2aa5 --- /dev/null +++ b/module/Core/src/Service/ShortUrl/ShortCodeHelperInterface.php @@ -0,0 +1,12 @@ +urlValidator = $urlValidator; $this->em = $em; $this->domainResolver = $domainResolver; + $this->shortCodeHelper = $shortCodeHelper; } /** @@ -83,20 +86,16 @@ class UrlShortener implements UrlShortenerInterface private function verifyShortCodeUniqueness(ShortUrlMeta $meta, ShortUrl $shortUrlToBeCreated): void { - $shortCode = $shortUrlToBeCreated->getShortCode(); - $domain = $meta->getDomain(); + $couldBeMadeUnique = $this->shortCodeHelper->ensureShortCodeUniqueness( + $shortUrlToBeCreated, + $meta->hasCustomSlug(), + ); - /** @var ShortUrlRepository $repo */ - $repo = $this->em->getRepository(ShortUrl::class); - $otherShortUrlsExist = $repo->shortCodeIsInUse($shortCode, $domain); + if (! $couldBeMadeUnique) { + $domain = $shortUrlToBeCreated->getDomain(); + $domainAuthority = $domain !== null ? $domain->getAuthority() : null; - if ($otherShortUrlsExist && $meta->hasCustomSlug()) { - throw NonUniqueSlugException::fromSlug($shortCode, $domain); - } - - if ($otherShortUrlsExist) { - $shortUrlToBeCreated->regenerateShortCode(); - $this->verifyShortCodeUniqueness($meta, $shortUrlToBeCreated); + throw NonUniqueSlugException::fromSlug($shortUrlToBeCreated->getShortCode(), $domainAuthority); } } } diff --git a/module/Core/src/Util/DoctrineBatchHelper.php b/module/Core/src/Util/DoctrineBatchHelper.php new file mode 100644 index 00000000..207d2093 --- /dev/null +++ b/module/Core/src/Util/DoctrineBatchHelper.php @@ -0,0 +1,61 @@ +em = $em; + } + + /** + * @throws Throwable + */ + public function wrapIterable(iterable $resultSet, int $batchSize): iterable + { + $iteration = 0; + + $this->em->beginTransaction(); + + try { + foreach ($resultSet as $key => $value) { + $iteration++; + yield $key => $value; + $this->flushAndClearBatch($iteration, $batchSize); + } + } catch (Throwable $e) { + $this->em->rollback(); + + throw $e; + } + + $this->flushAndClearEntityManager(); + $this->em->commit(); + } + + private function flushAndClearBatch(int $iteration, int $batchSize): void + { + if ($iteration % $batchSize) { + return; + } + + $this->flushAndClearEntityManager(); + } + + private function flushAndClearEntityManager(): void + { + $this->em->flush(); + $this->em->clear(); + } +} 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 @@ +repo->findOneMatching('foo', $tags, ShortUrlMeta::fromRawData($meta)), ); } + + /** @test */ + public function importedShortUrlsAreSearchedAsExpected(): void + { + $buildImported = static fn (string $shortCode, ?String $domain = null) => + new ImportedShlinkUrl('', 'foo', [], Chronos::now(), $domain, $shortCode); + + $shortUrlWithoutDomain = ShortUrl::fromImport($buildImported('my-cool-slug'), true); + $this->getEntityManager()->persist($shortUrlWithoutDomain); + + $shortUrlWithDomain = ShortUrl::fromImport($buildImported('another-slug', 'doma.in'), true); + $this->getEntityManager()->persist($shortUrlWithDomain); + + $this->getEntityManager()->flush(); + + self::assertTrue($this->repo->importedUrlExists($buildImported('my-cool-slug'))); + self::assertTrue($this->repo->importedUrlExists($buildImported('another-slug', 'doma.in'))); + self::assertFalse($this->repo->importedUrlExists($buildImported('non-existing-slug'))); + self::assertFalse($this->repo->importedUrlExists($buildImported('non-existing-slug', 'doma.in'))); + self::assertFalse($this->repo->importedUrlExists($buildImported('my-cool-slug', 'doma.in'))); + self::assertFalse($this->repo->importedUrlExists($buildImported('another-slug'))); + } } diff --git a/module/Core/test/Entity/ShortUrlTest.php b/module/Core/test/Entity/ShortUrlTest.php index 054182ff..9f28c41b 100644 --- a/module/Core/test/Entity/ShortUrlTest.php +++ b/module/Core/test/Entity/ShortUrlTest.php @@ -4,11 +4,13 @@ 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; @@ -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 diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php new file mode 100644 index 00000000..7225071c --- /dev/null +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -0,0 +1,145 @@ +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); + } +} diff --git a/module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php b/module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php new file mode 100644 index 00000000..d77aecd5 --- /dev/null +++ b/module/Core/test/Service/ShortUrl/ShortCodeHelperTest.php @@ -0,0 +1,77 @@ +em = $this->prophesize(EntityManagerInterface::class); + $this->helper = new ShortCodeHelper($this->em->reveal()); + + $this->shortUrl = $this->prophesize(ShortUrl::class); + $this->shortUrl->getShortCode()->willReturn('abc123'); + } + + /** + * @test + * @dataProvider provideDomains + */ + public function shortCodeIsRegeneratedIfAlreadyInUse(?Domain $domain, ?string $expectedAuthority): void + { + $callIndex = 0; + $expectedCalls = 3; + $repo = $this->prophesize(ShortUrlRepository::class); + $shortCodeIsInUse = $repo->shortCodeIsInUse('abc123', $expectedAuthority)->will( + function () use (&$callIndex, $expectedCalls) { + $callIndex++; + return $callIndex < $expectedCalls; + }, + ); + $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + $this->shortUrl->getDomain()->willReturn($domain); + + $result = $this->helper->ensureShortCodeUniqueness($this->shortUrl->reveal(), false); + + self::assertTrue($result); + $this->shortUrl->regenerateShortCode()->shouldHaveBeenCalledTimes($expectedCalls - 1); + $getRepo->shouldBeCalledTimes($expectedCalls); + $shortCodeIsInUse->shouldBeCalledTimes($expectedCalls); + } + + public function provideDomains(): iterable + { + yield 'no domain' => [null, null]; + yield 'domain' => [new Domain($authority = 'doma.in'), $authority]; + } + + /** @test */ + public function inUseSlugReturnsError(): void + { + $repo = $this->prophesize(ShortUrlRepository::class); + $shortCodeIsInUse = $repo->shortCodeIsInUse('abc123', null)->willReturn(true); + $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + $this->shortUrl->getDomain()->willReturn(null); + + $result = $this->helper->ensureShortCodeUniqueness($this->shortUrl->reveal(), true); + + self::assertFalse($result); + $this->shortUrl->regenerateShortCode()->shouldNotHaveBeenCalled(); + $getRepo->shouldBeCalledOnce(); + $shortCodeIsInUse->shouldBeCalledOnce(); + } +} diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index ba7185bf..7945c32b 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -18,6 +18,7 @@ use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; +use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelperInterface; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Util\UrlValidatorInterface; @@ -26,6 +27,7 @@ class UrlShortenerTest extends TestCase private UrlShortener $urlShortener; private ObjectProphecy $em; private ObjectProphecy $urlValidator; + private ObjectProphecy $shortCodeHelper; public function setUp(): void { @@ -51,10 +53,14 @@ class UrlShortenerTest extends TestCase $repo->shortCodeIsInUse(Argument::cetera())->willReturn(false); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + $this->shortCodeHelper = $this->prophesize(ShortCodeHelperInterface::class); + $this->shortCodeHelper->ensureShortCodeUniqueness(Argument::cetera())->willReturn(true); + $this->urlShortener = new UrlShortener( $this->urlValidator->reveal(), $this->em->reveal(), new SimpleDomainResolver(), + $this->shortCodeHelper->reveal(), ); } @@ -71,29 +77,18 @@ class UrlShortenerTest extends TestCase } /** @test */ - public function shortCodeIsRegeneratedIfAlreadyInUse(): void + public function exceptionIsThrownWhenNonUniqueSlugIsProvided(): void { - $callIndex = 0; - $expectedCalls = 3; - $repo = $this->prophesize(ShortUrlRepository::class); - $shortCodeIsInUse = $repo->shortCodeIsInUse(Argument::cetera())->will( - function () use (&$callIndex, $expectedCalls) { - $callIndex++; - return $callIndex < $expectedCalls; - }, - ); - $repo->findBy(Argument::cetera())->willReturn([]); - $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + $ensureUniqueness = $this->shortCodeHelper->ensureShortCodeUniqueness(Argument::cetera())->willReturn(false); - $shortUrl = $this->urlShortener->urlToShortCode( + $ensureUniqueness->shouldBeCalledOnce(); + $this->expectException(NonUniqueSlugException::class); + + $this->urlShortener->urlToShortCode( 'http://foobar.com/12345/hello?foo=bar', [], - ShortUrlMeta::createEmpty(), + ShortUrlMeta::fromRawData(['customSlug' => 'custom-slug']), ); - - self::assertEquals('http://foobar.com/12345/hello?foo=bar', $shortUrl->getLongUrl()); - $getRepo->shouldBeCalledTimes($expectedCalls); - $shortCodeIsInUse->shouldBeCalledTimes($expectedCalls); } /** @test */ @@ -115,25 +110,6 @@ class UrlShortenerTest extends TestCase ); } - /** @test */ - public function exceptionIsThrownWhenNonUniqueSlugIsProvided(): void - { - $repo = $this->prophesize(ShortUrlRepository::class); - $shortCodeIsInUse = $repo->shortCodeIsInUse('custom-slug', null)->willReturn(true); - $repo->findBy(Argument::cetera())->willReturn([]); - $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - - $shortCodeIsInUse->shouldBeCalledOnce(); - $getRepo->shouldBeCalled(); - $this->expectException(NonUniqueSlugException::class); - - $this->urlShortener->urlToShortCode( - 'http://foobar.com/12345/hello?foo=bar', - [], - ShortUrlMeta::fromRawData(['customSlug' => 'custom-slug']), - ); - } - /** * @test * @dataProvider provideExistingShortUrls diff --git a/module/Core/test/Util/DoctrineBatchHelperTest.php b/module/Core/test/Util/DoctrineBatchHelperTest.php new file mode 100644 index 00000000..a89a653f --- /dev/null +++ b/module/Core/test/Util/DoctrineBatchHelperTest.php @@ -0,0 +1,70 @@ +em = $this->prophesize(EntityManagerInterface::class); + $this->helper = new DoctrineBatchHelper($this->em->reveal()); + } + + /** + * @test + * @dataProvider provideIterables + */ + public function entityManagerIsFlushedAndClearedTheExpectedAmountOfTimes( + array $iterable, + int $batchSize, + int $expectedCalls + ): void { + $wrappedIterable = $this->helper->wrapIterable($iterable, $batchSize); + + foreach ($wrappedIterable as $item) { + // Iterable needs to be iterated for the logic to be invoked + } + + $this->em->beginTransaction()->shouldHaveBeenCalledOnce(); + $this->em->commit()->shouldHaveBeenCalledOnce(); + $this->em->rollback()->shouldNotHaveBeenCalled(); + $this->em->flush()->shouldHaveBeenCalledTimes($expectedCalls); + $this->em->clear()->shouldHaveBeenCalledTimes($expectedCalls); + } + + public function provideIterables(): iterable + { + yield [[], 100, 1]; + yield [[1, 2, 3, 4, 5, 6, 7, 8, 9, 10], 3, 4]; + yield [[1, 2, 3, 4, 5, 6, 7, 8, 9, 10], 11, 1]; + } + + /** @test */ + public function transactionIsRolledBackWhenAnErrorOccurs(): void + { + $flush = $this->em->flush()->willThrow(RuntimeException::class); + + $wrappedIterable = $this->helper->wrapIterable([1, 2, 3], 1); + + self::expectException(RuntimeException::class); + $flush->shouldBeCalledOnce(); + $this->em->beginTransaction()->shouldBeCalledOnce(); + $this->em->commit()->shouldNotBeCalled(); + $this->em->rollback()->shouldBeCalledOnce(); + + foreach ($wrappedIterable as $item) { + // Iterable needs to be iterated for the logic to be invoked + } + } +}