diff --git a/CHANGELOG.md b/CHANGELOG.md index a5066055..ba4d3eb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this This option will also be available on shlink-installer 1.3.0, so the installer will ask for it. It can also be provided for the docker image as the `BASE_PATH` env var. +* [#479](https://github.com/shlinkio/shlink/issues/479) Added preliminary support for multiple domains. + + Endpoints and commands which create short URLs support providing the `domain` now (via query param or CLI flag). If not provided, the short URLs will still be "attached" to the default domain. + + Custom slugs can be created on multiple domains, allowing to share links like `https://doma.in/my-compaign` and `https://example.com/my-campaign`, under the same shlink instance. + + When resolving a short URL to redirect end users, the following rules are applied: + + * If the domain used for the request plus the short code/slug are found, the user is redirected to that long URL and the visit is tracked. + * If the domain is not known but the short code/slug is defined for default domain, the user is redirected there and the visit is tracked. + * In any other case, no redirection happens and no visit is tracked (if a fall back redirection is configured for not-found URLs, it will still happen). + #### Changed * [#486](https://github.com/shlinkio/shlink/issues/486) Updated to [shlink-installer](https://github.com/shlinkio/shlink-installer) v2, which supports asking for base path in which shlink is served. diff --git a/config/autoload/ip-address.global.php b/config/autoload/client-detection.global.php similarity index 100% rename from config/autoload/ip-address.global.php rename to config/autoload/client-detection.global.php diff --git a/data/migrations/Version20190930165521.php b/data/migrations/Version20190930165521.php new file mode 100644 index 00000000..65b83aae --- /dev/null +++ b/data/migrations/Version20190930165521.php @@ -0,0 +1,54 @@ +getTable('short_urls'); + if ($shortUrls->hasColumn('domain_id')) { + return; + } + + $domains = $schema->createTable('domains'); + $domains->addColumn('id', Type::BIGINT, [ + 'unsigned' => true, + 'autoincrement' => true, + 'notnull' => true, + ]); + $domains->addColumn('authority', Type::STRING, [ + 'length' => 512, + 'notnull' => true, + ]); + $domains->addUniqueIndex(['authority']); + $domains->setPrimaryKey(['id']); + + $shortUrls->addColumn('domain_id', Type::BIGINT, [ + 'unsigned' => true, + 'notnull' => false, + ]); + $shortUrls->addForeignKeyConstraint('domains', ['domain_id'], ['id'], [ + 'onDelete' => 'RESTRICT', + 'onUpdate' => 'RESTRICT', + ]); + } + + /** + * @throws SchemaException + */ + public function down(Schema $schema): void + { + $schema->getTable('short_urls')->dropColumn('domain_id'); + $schema->dropTable('domains'); + } +} diff --git a/data/migrations/Version20191001201532.php b/data/migrations/Version20191001201532.php new file mode 100644 index 00000000..ff4aebbf --- /dev/null +++ b/data/migrations/Version20191001201532.php @@ -0,0 +1,48 @@ +getTable('short_urls'); + if ($shortUrls->hasIndex('unique_short_code_plus_domain')) { + return; + } + + /** @var Index|null $shortCodesIndex */ + $shortCodesIndex = array_reduce($shortUrls->getIndexes(), function (?Index $found, Index $current) { + [$column] = $current->getColumns(); + return $column === 'short_code' ? $current : $found; + }); + if ($shortCodesIndex === null) { + return; + } + + $shortUrls->dropIndex($shortCodesIndex->getName()); + $shortUrls->addUniqueIndex(['short_code', 'domain_id'], 'unique_short_code_plus_domain'); + } + + /** + * @throws SchemaException + */ + public function down(Schema $schema): void + { + $shortUrls = $schema->getTable('short_urls'); + + $shortUrls->dropIndex('unique_short_code_plus_domain'); + $shortUrls->addUniqueIndex(['short_code']); + } +} diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index 2254a732..f8fa1ec8 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -216,6 +216,10 @@ "findIfExists": { "description": "Will force existing matching URL to be returned if found, instead of creating a new one", "type": "boolean" + }, + "domain": { + "description": "The domain to which the short URL will be attached", + "type": "string" } } } diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}.json b/docs/swagger/paths/v1_short-urls_{shortCode}.json index 41d1499c..bbb7145c 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}.json @@ -15,6 +15,15 @@ "schema": { "type": "string" } + }, + { + "name": "domain", + "in": "query", + "description": "The domain in which the short code should be searched for. Will fall back to default domain if not found.", + "required": false, + "schema": { + "type": "string" + } } ], "security": [ diff --git a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php index 5977094f..c13e205e 100644 --- a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php @@ -9,7 +9,6 @@ use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; -use Shlinkio\Shlink\Core\Util\ShortUrlBuilderTrait; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -26,8 +25,6 @@ use function sprintf; class GenerateShortUrlCommand extends Command { - use ShortUrlBuilderTrait; - public const NAME = 'short-url:generate'; private const ALIASES = ['shortcode:generate', 'short-code:generate']; @@ -87,6 +84,12 @@ class GenerateShortUrlCommand extends Command 'f', InputOption::VALUE_NONE, 'This will force existing matching URL to be returned if found, instead of creating a new one.' + ) + ->addOption( + 'domain', + 'd', + InputOption::VALUE_REQUIRED, + 'The domain to which this short URL will be attached.' ); } @@ -119,7 +122,7 @@ class GenerateShortUrlCommand extends Command $maxVisits = $input->getOption('maxVisits'); try { - $shortCode = $this->urlShortener->urlToShortCode( + $shortUrl = $this->urlShortener->urlToShortCode( new Uri($longUrl), $tags, ShortUrlMeta::createFromParams( @@ -127,14 +130,14 @@ class GenerateShortUrlCommand extends Command $this->getOptionalDate($input, 'validUntil'), $customSlug, $maxVisits !== null ? (int) $maxVisits : null, - $input->getOption('findIfExists') + $input->getOption('findIfExists'), + $input->getOption('domain') ) - )->getShortCode(); - $shortUrl = $this->buildShortUrl($this->domainConfig, $shortCode); + ); $io->writeln([ sprintf('Processed long URL: %s', $longUrl), - sprintf('Generated short URL: %s', $shortUrl), + sprintf('Generated short URL: %s', $shortUrl->toString($this->domainConfig)), ]); return ExitCodes::EXIT_SUCCESS; } catch (InvalidUrlException $e) { diff --git a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php index 48fae0a0..13370c02 100644 --- a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php @@ -10,6 +10,7 @@ use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; @@ -35,7 +36,8 @@ class ResolveUrlCommand extends Command ->setName(self::NAME) ->setAliases(self::ALIASES) ->setDescription('Returns the long URL behind a short code') - ->addArgument('shortCode', InputArgument::REQUIRED, 'The short code to parse'); + ->addArgument('shortCode', InputArgument::REQUIRED, 'The short code to parse') + ->addOption('domain', 'd', InputOption::VALUE_REQUIRED, 'The domain to which the short URL is attached.'); } protected function interact(InputInterface $input, OutputInterface $output): void @@ -56,9 +58,10 @@ class ResolveUrlCommand extends Command { $io = new SymfonyStyle($input, $output); $shortCode = $input->getArgument('shortCode'); + $domain = $input->getOption('domain'); try { - $url = $this->urlShortener->shortCodeToUrl($shortCode); + $url = $this->urlShortener->shortCodeToUrl($shortCode, $domain); $output->writeln(sprintf('Long URL: %s', $url->getLongUrl())); return ExitCodes::EXIT_SUCCESS; } catch (InvalidShortCodeException $e) { diff --git a/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php index 064bfe78..6cda9528 100644 --- a/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php @@ -9,8 +9,10 @@ use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\UriInterface; use Shlinkio\Shlink\CLI\Command\ShortUrl\GenerateShortUrlCommand; +use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; +use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Service\UrlShortener; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -35,7 +37,7 @@ class GenerateShortUrlCommandTest extends TestCase } /** @test */ - public function properShortCodeIsCreatedIfLongUrlIsCorrect() + public function properShortCodeIsCreatedIfLongUrlIsCorrect(): void { $urlToShortCode = $this->urlShortener->urlToShortCode(Argument::cetera())->willReturn( (new ShortUrl(''))->setShortCode('abc123') @@ -47,26 +49,41 @@ class GenerateShortUrlCommandTest extends TestCase ]); $output = $this->commandTester->getDisplay(); + $this->assertEquals(ExitCodes::EXIT_SUCCESS, $this->commandTester->getStatusCode()); $this->assertStringContainsString('http://foo.com/abc123', $output); $urlToShortCode->shouldHaveBeenCalledOnce(); } /** @test */ - public function exceptionWhileParsingLongUrlOutputsError() + public function exceptionWhileParsingLongUrlOutputsError(): void { $this->urlShortener->urlToShortCode(Argument::cetera())->willThrow(new InvalidUrlException()) ->shouldBeCalledOnce(); $this->commandTester->execute(['longUrl' => 'http://domain.com/invalid']); $output = $this->commandTester->getDisplay(); - $this->assertStringContainsString( - 'Provided URL "http://domain.com/invalid" is invalid.', - $output - ); + + $this->assertEquals(ExitCodes::EXIT_FAILURE, $this->commandTester->getStatusCode()); + $this->assertStringContainsString('Provided URL "http://domain.com/invalid" is invalid.', $output); } /** @test */ - public function properlyProcessesProvidedTags() + public function providingNonUniqueSlugOutputsError(): void + { + $urlToShortCode = $this->urlShortener->urlToShortCode(Argument::cetera())->willThrow( + NonUniqueSlugException::class + ); + + $this->commandTester->execute(['longUrl' => 'http://domain.com/invalid', '--customSlug' => 'my-slug']); + $output = $this->commandTester->getDisplay(); + + $this->assertEquals(ExitCodes::EXIT_FAILURE, $this->commandTester->getStatusCode()); + $this->assertStringContainsString('Provided slug "my-slug" is already in use', $output); + $urlToShortCode->shouldHaveBeenCalledOnce(); + } + + /** @test */ + public function properlyProcessesProvidedTags(): void { $urlToShortCode = $this->urlShortener->urlToShortCode( Argument::type(UriInterface::class), @@ -83,6 +100,7 @@ class GenerateShortUrlCommandTest extends TestCase ]); $output = $this->commandTester->getDisplay(); + $this->assertEquals(ExitCodes::EXIT_SUCCESS, $this->commandTester->getStatusCode()); $this->assertStringContainsString('http://foo.com/abc123', $output); $urlToShortCode->shouldHaveBeenCalledOnce(); } diff --git a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php index a4ee7f45..ce145165 100644 --- a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php @@ -33,13 +33,13 @@ class ResolveUrlCommandTest extends TestCase } /** @test */ - public function correctShortCodeResolvesUrl() + public function correctShortCodeResolvesUrl(): void { $shortCode = 'abc123'; $expectedUrl = 'http://domain.com/foo/bar'; $shortUrl = new ShortUrl($expectedUrl); - $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl) - ->shouldBeCalledOnce(); + $this->urlShortener->shortCodeToUrl($shortCode, null)->willReturn($shortUrl) + ->shouldBeCalledOnce(); $this->commandTester->execute(['shortCode' => $shortCode]); $output = $this->commandTester->getDisplay(); @@ -47,11 +47,11 @@ class ResolveUrlCommandTest extends TestCase } /** @test */ - public function incorrectShortCodeOutputsErrorMessage() + public function incorrectShortCodeOutputsErrorMessage(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class) - ->shouldBeCalledOnce(); + $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(EntityDoesNotExistException::class) + ->shouldBeCalledOnce(); $this->commandTester->execute(['shortCode' => $shortCode]); $output = $this->commandTester->getDisplay(); @@ -59,11 +59,11 @@ class ResolveUrlCommandTest extends TestCase } /** @test */ - public function wrongShortCodeFormatOutputsErrorMessage() + public function wrongShortCodeFormatOutputsErrorMessage(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(new InvalidShortCodeException()) - ->shouldBeCalledOnce(); + $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(new InvalidShortCodeException()) + ->shouldBeCalledOnce(); $this->commandTester->execute(['shortCode' => $shortCode]); $output = $this->commandTester->getDisplay(); diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Domain.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Domain.php new file mode 100644 index 00000000..d2e52541 --- /dev/null +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Domain.php @@ -0,0 +1,24 @@ +setTable('domains'); + +$builder->createField('id', Type::BIGINT) + ->columnName('id') + ->makePrimaryKey() + ->generatedValue('IDENTITY') + ->option('unsigned', true) + ->build(); + +$builder->createField('authority', Type::STRING) + ->unique() + ->build(); 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 94398df9..47bc9d10 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 @@ -28,7 +28,6 @@ $builder->createField('longUrl', Type::STRING) $builder->createField('shortCode', Type::STRING) ->columnName('short_code') - ->unique() ->length(255) ->build(); @@ -61,3 +60,10 @@ $builder->createManyToMany('tags', Entity\Tag::class) ->addInverseJoinColumn('tag_id', 'id', true, false, 'CASCADE') ->addJoinColumn('short_url_id', 'id', true, false, 'CASCADE') ->build(); + +$builder->createManyToOne('domain', Entity\Domain::class) + ->addJoinColumn('domain_id', 'id', true, false, 'RESTRICT') + ->cascadePersist() + ->build(); + +$builder->addUniqueConstraint(['short_code', 'domain_id'], 'unique_short_code_plus_domain'); diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 6bd3995a..db23c286 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -53,11 +53,12 @@ abstract class AbstractTrackingAction implements MiddlewareInterface public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { $shortCode = $request->getAttribute('shortCode', ''); + $domain = $request->getUri()->getAuthority(); $query = $request->getQueryParams(); $disableTrackParam = $this->appOptions->getDisableTrackParam(); try { - $url = $this->urlShortener->shortCodeToUrl($shortCode); + $url = $this->urlShortener->shortCodeToUrl($shortCode, $domain); // Track visit to this short code if ($disableTrackParam === null || ! array_key_exists($disableTrackParam, $query)) { diff --git a/module/Core/src/Domain/Resolver/DomainResolverInterface.php b/module/Core/src/Domain/Resolver/DomainResolverInterface.php new file mode 100644 index 00000000..af23aa37 --- /dev/null +++ b/module/Core/src/Domain/Resolver/DomainResolverInterface.php @@ -0,0 +1,11 @@ +em = $em; + } + + public function resolveDomain(?string $domain): ?Domain + { + if ($domain === null) { + return null; + } + + /** @var Domain|null $existingDomain */ + $existingDomain = $this->em->getRepository(Domain::class)->findOneBy(['authority' => $domain]); + return $existingDomain ?? new Domain($domain); + } +} diff --git a/module/Core/src/Domain/Resolver/SimpleDomainResolver.php b/module/Core/src/Domain/Resolver/SimpleDomainResolver.php new file mode 100644 index 00000000..924ca17f --- /dev/null +++ b/module/Core/src/Domain/Resolver/SimpleDomainResolver.php @@ -0,0 +1,14 @@ +authority = $authority; + } + + public function getAuthority(): string + { + return $this->authority; + } +} diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index 0f5cf088..709499df 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -7,9 +7,15 @@ use Cake\Chronos\Chronos; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; use Shlinkio\Shlink\Common\Entity\AbstractEntity; +use Shlinkio\Shlink\Core\Domain\Resolver\DomainResolverInterface; +use Shlinkio\Shlink\Core\Domain\Resolver\SimpleDomainResolver; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Zend\Diactoros\Uri; +use function array_reduce; use function count; +use function Functional\contains; +use function Functional\invoke; class ShortUrl extends AbstractEntity { @@ -29,9 +35,14 @@ class ShortUrl extends AbstractEntity private $validUntil; /** @var integer|null */ private $maxVisits; + /** @var Domain|null */ + private $domain; - public function __construct(string $longUrl, ?ShortUrlMeta $meta = null) - { + public function __construct( + string $longUrl, + ?ShortUrlMeta $meta = null, + ?DomainResolverInterface $domainResolver = null + ) { $meta = $meta ?? ShortUrlMeta::createEmpty(); $this->longUrl = $longUrl; @@ -42,6 +53,7 @@ class ShortUrl extends AbstractEntity $this->validUntil = $meta->getValidUntil(); $this->maxVisits = $meta->getMaxVisits(); $this->shortCode = $meta->getCustomSlug() ?? ''; // TODO logic to calculate short code should be passed somehow + $this->domain = ($domainResolver ?? new SimpleDomainResolver())->resolveDomain($meta->getDomain()); } public function getLongUrl(): string @@ -131,4 +143,47 @@ class ShortUrl extends AbstractEntity { return $this->maxVisits !== null && $this->getVisitsCount() >= $this->maxVisits; } + + public function toString(array $domainConfig): string + { + return (string) (new Uri())->withPath($this->shortCode) + ->withScheme($domainConfig['schema'] ?? 'http') + ->withHost($this->resolveDomain($domainConfig['hostname'] ?? '')); + } + + private function resolveDomain(string $fallback = ''): string + { + if ($this->domain === null) { + return $fallback; + } + + return $this->domain->getAuthority(); + } + + public function matchesCriteria(ShortUrlMeta $meta, array $tags): bool + { + if ($meta->hasMaxVisits() && $meta->getMaxVisits() !== $this->maxVisits) { + return false; + } + if ($meta->hasDomain() && $meta->getDomain() !== $this->resolveDomain()) { + return false; + } + if ($meta->hasValidSince() && ! $meta->getValidSince()->eq($this->validSince)) { + return false; + } + if ($meta->hasValidUntil() && ! $meta->getValidUntil()->eq($this->validUntil)) { + return false; + } + + $shortUrlTags = invoke($this->getTags(), '__toString'); + $hasAllTags = count($shortUrlTags) === count($tags) && array_reduce( + $tags, + function (bool $hasAllTags, string $tag) use ($shortUrlTags) { + return $hasAllTags && contains($shortUrlTags, $tag); + }, + true + ); + + return $hasAllTags; + } } diff --git a/module/Core/src/Exception/NonUniqueSlugException.php b/module/Core/src/Exception/NonUniqueSlugException.php index 09e22b9b..2b975808 100644 --- a/module/Core/src/Exception/NonUniqueSlugException.php +++ b/module/Core/src/Exception/NonUniqueSlugException.php @@ -7,8 +7,13 @@ use function sprintf; class NonUniqueSlugException extends InvalidArgumentException { - public static function fromSlug(string $slug): self + public static function fromSlug(string $slug, ?string $domain): self { - return new self(sprintf('Provided slug "%s" is not unique.', $slug)); + $suffix = ''; + if ($domain !== null) { + $suffix = sprintf(' for domain "%s"', $domain); + } + + return new self(sprintf('Provided slug "%s" is not unique%s.', $slug, $suffix)); } } diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index 2eb6ebca..b6d00834 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -19,6 +19,8 @@ final class ShortUrlMeta private $maxVisits; /** @var bool|null */ private $findIfExists; + /** @var string|null */ + private $domain; // Force named constructors private function __construct() @@ -47,6 +49,7 @@ final class ShortUrlMeta * @param string|null $customSlug * @param int|null $maxVisits * @param bool|null $findIfExists + * @param string|null $domain * @throws ValidationException */ public static function createFromParams( @@ -54,7 +57,8 @@ final class ShortUrlMeta $validUntil = null, $customSlug = null, $maxVisits = null, - $findIfExists = null + $findIfExists = null, + $domain = null ): self { // We do not type hint the arguments because that will be done by the validation process and we would get a // type error if any of them do not match @@ -65,6 +69,7 @@ final class ShortUrlMeta ShortUrlMetaInputFilter::CUSTOM_SLUG => $customSlug, ShortUrlMetaInputFilter::MAX_VISITS => $maxVisits, ShortUrlMetaInputFilter::FIND_IF_EXISTS => $findIfExists, + ShortUrlMetaInputFilter::DOMAIN => $domain, ]); return $instance; } @@ -86,6 +91,7 @@ final class ShortUrlMeta $this->maxVisits = $inputFilter->getValue(ShortUrlMetaInputFilter::MAX_VISITS); $this->maxVisits = $this->maxVisits !== null ? (int) $this->maxVisits : null; $this->findIfExists = $inputFilter->getValue(ShortUrlMetaInputFilter::FIND_IF_EXISTS); + $this->domain = $inputFilter->getValue(ShortUrlMetaInputFilter::DOMAIN); } /** @@ -144,4 +150,14 @@ final class ShortUrlMeta { return (bool) $this->findIfExists; } + + public function hasDomain(): bool + { + return $this->domain !== null; + } + + public function getDomain(): ?string + { + return $this->domain; + } } diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index b1d22b08..d11f95d2 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -117,14 +117,22 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI return $qb; } - public function findOneByShortCode(string $shortCode): ?ShortUrl + public function findOneByShortCode(string $shortCode, ?string $domain = null): ?ShortUrl { + // When ordering DESC, Postgres puts nulls at the beginning while the rest of supported DB engines put them at + // the bottom + $dbPlatform = $this->getEntityManager()->getConnection()->getDatabasePlatform()->getName(); + $ordering = $dbPlatform === 'postgresql' ? 'ASC' : 'DESC'; + $dql= <<= :now OR s.validUntil IS NULL) + AND (s.domain IS NULL OR d.authority = :domain) + ORDER BY s.domain {$ordering} DQL; $query = $this->getEntityManager()->createQuery($dql); @@ -132,10 +140,38 @@ DQL; ->setParameters([ 'shortCode' => $shortCode, 'now' => Chronos::now(), + 'domain' => $domain, ]); - /** @var ShortUrl|null $result */ - $result = $query->getOneOrNullResult(); - return $result === null || $result->maxVisitsReached() ? null : $result; + // Since we ordered by domain, we will have first the URL matching provided domain, followed by the one + // with no domain (if any), so it is safe to fetch 1 max result and we will get: + // * The short URL matching both the short code and the domain, or + // * The short URL matching the short code but without any domain, or + // * No short URL at all + + /** @var ShortUrl|null $shortUrl */ + $shortUrl = $query->getOneOrNullResult(); + return $shortUrl !== null && ! $shortUrl->maxVisitsReached() ? $shortUrl : null; + } + + public function slugIsInUse(string $slug, ?string $domain = null): bool + { + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->select('COUNT(DISTINCT s.id)') + ->from(ShortUrl::class, 's') + ->where($qb->expr()->isNotNull('s.shortCode')) + ->andWhere($qb->expr()->eq('s.shortCode', ':slug')) + ->setParameter('slug', $slug); + + 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')); + } + + $result = (int) $qb->getQuery()->getSingleScalarResult(); + return $result > 0; } } diff --git a/module/Core/src/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/Repository/ShortUrlRepositoryInterface.php index fcec73e4..22b6bd73 100644 --- a/module/Core/src/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/Repository/ShortUrlRepositoryInterface.php @@ -26,5 +26,7 @@ interface ShortUrlRepositoryInterface extends ObjectRepository */ public function countList(?string $searchTerm = null, array $tags = []): int; - public function findOneByShortCode(string $shortCode): ?ShortUrl; + public function findOneByShortCode(string $shortCode, ?string $domain = null): ?ShortUrl; + + public function slugIsInUse(string $slug, ?string $domain): bool; } diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index fe041900..0d0c8b27 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -9,6 +9,7 @@ use GuzzleHttp\ClientInterface; use GuzzleHttp\Exception\GuzzleException; use GuzzleHttp\RequestOptions; use Psr\Http\Message\UriInterface; +use Shlinkio\Shlink\Core\Domain\Resolver\PersistenceDomainResolver; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; @@ -22,11 +23,8 @@ use Shlinkio\Shlink\Core\Util\TagManagerTrait; use Throwable; use function array_reduce; -use function count; use function floor; use function fmod; -use function Functional\contains; -use function Functional\invoke; use function preg_match; use function strlen; @@ -77,7 +75,7 @@ class UrlShortener implements UrlShortenerInterface $this->em->beginTransaction(); // First, create the short URL with an empty short code - $shortUrl = new ShortUrl($url, $meta); + $shortUrl = new ShortUrl($url, $meta, new PersistenceDomainResolver($this->em)); $this->em->persist($shortUrl); $this->em->flush(); @@ -120,30 +118,11 @@ class UrlShortener implements UrlShortenerInterface // Iterate short URLs until one that matches is found, or return null otherwise return array_reduce($shortUrls, function (?ShortUrl $found, ShortUrl $shortUrl) use ($tags, $meta) { - if ($found) { + if ($found !== null) { return $found; } - if ($meta->hasMaxVisits() && $meta->getMaxVisits() !== $shortUrl->getMaxVisits()) { - return null; - } - if ($meta->hasValidSince() && ! $meta->getValidSince()->eq($shortUrl->getValidSince())) { - return null; - } - if ($meta->hasValidUntil() && ! $meta->getValidUntil()->eq($shortUrl->getValidUntil())) { - return null; - } - - $shortUrlTags = invoke($shortUrl->getTags(), '__toString'); - $hasAllTags = count($shortUrlTags) === count($tags) && array_reduce( - $tags, - function (bool $hasAllTags, string $tag) use ($shortUrlTags) { - return $hasAllTags && contains($shortUrlTags, $tag); - }, - true - ); - - return $hasAllTags ? $shortUrl : null; + return $shortUrl->matchesCriteria($meta, $tags) ? $shortUrl : null; }); } @@ -165,12 +144,13 @@ class UrlShortener implements UrlShortenerInterface } $customSlug = $meta->getCustomSlug(); + $domain = $meta->getDomain(); /** @var ShortUrlRepository $repo */ $repo = $this->em->getRepository(ShortUrl::class); - $shortUrlsCount = $repo->count(['shortCode' => $customSlug]); + $shortUrlsCount = $repo->slugIsInUse($customSlug, $domain); if ($shortUrlsCount > 0) { - throw NonUniqueSlugException::fromSlug($customSlug); + throw NonUniqueSlugException::fromSlug($customSlug, $domain); } } @@ -195,7 +175,7 @@ class UrlShortener implements UrlShortenerInterface * @throws InvalidShortCodeException * @throws EntityDoesNotExistException */ - public function shortCodeToUrl(string $shortCode): ShortUrl + public function shortCodeToUrl(string $shortCode, ?string $domain = null): ShortUrl { $chars = $this->options->getChars(); @@ -206,7 +186,7 @@ class UrlShortener implements UrlShortenerInterface /** @var ShortUrlRepository $shortUrlRepo */ $shortUrlRepo = $this->em->getRepository(ShortUrl::class); - $shortUrl = $shortUrlRepo->findOneByShortCode($shortCode); + $shortUrl = $shortUrlRepo->findOneByShortCode($shortCode, $domain); if ($shortUrl === null) { throw EntityDoesNotExistException::createFromEntityAndConditions(ShortUrl::class, [ 'shortCode' => $shortCode, diff --git a/module/Core/src/Service/UrlShortenerInterface.php b/module/Core/src/Service/UrlShortenerInterface.php index 314b6de2..4a200b4b 100644 --- a/module/Core/src/Service/UrlShortenerInterface.php +++ b/module/Core/src/Service/UrlShortenerInterface.php @@ -26,5 +26,5 @@ interface UrlShortenerInterface * @throws InvalidShortCodeException * @throws EntityDoesNotExistException */ - public function shortCodeToUrl(string $shortCode): ShortUrl; + public function shortCodeToUrl(string $shortCode, ?string $domain = null): ShortUrl; } diff --git a/module/Core/src/Transformer/ShortUrlDataTransformer.php b/module/Core/src/Transformer/ShortUrlDataTransformer.php index 6bd2bfa4..ddb8b9d6 100644 --- a/module/Core/src/Transformer/ShortUrlDataTransformer.php +++ b/module/Core/src/Transformer/ShortUrlDataTransformer.php @@ -5,15 +5,12 @@ namespace Shlinkio\Shlink\Core\Transformer; use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Util\ShortUrlBuilderTrait; use function Functional\invoke; use function Functional\invoke_if; class ShortUrlDataTransformer implements DataTransformerInterface { - use ShortUrlBuilderTrait; - /** @var array */ private $domainConfig; @@ -23,21 +20,20 @@ class ShortUrlDataTransformer implements DataTransformerInterface } /** - * @param ShortUrl $value + * @param ShortUrl $shortUrl */ - public function transform($value): array + public function transform($shortUrl): array { - $longUrl = $value->getLongUrl(); - $shortCode = $value->getShortCode(); + $longUrl = $shortUrl->getLongUrl(); return [ - 'shortCode' => $shortCode, - 'shortUrl' => $this->buildShortUrl($this->domainConfig, $shortCode), + 'shortCode' => $shortUrl->getShortCode(), + 'shortUrl' => $shortUrl->toString($this->domainConfig), 'longUrl' => $longUrl, - 'dateCreated' => $value->getDateCreated()->toAtomString(), - 'visitsCount' => $value->getVisitsCount(), - 'tags' => invoke($value->getTags(), '__toString'), - 'meta' => $this->buildMeta($value), + 'dateCreated' => $shortUrl->getDateCreated()->toAtomString(), + 'visitsCount' => $shortUrl->getVisitsCount(), + 'tags' => invoke($shortUrl->getTags(), '__toString'), + 'meta' => $this->buildMeta($shortUrl), // Deprecated 'originalUrl' => $longUrl, diff --git a/module/Core/src/Util/ShortUrlBuilderTrait.php b/module/Core/src/Util/ShortUrlBuilderTrait.php deleted file mode 100644 index fe627b0f..00000000 --- a/module/Core/src/Util/ShortUrlBuilderTrait.php +++ /dev/null @@ -1,16 +0,0 @@ -withPath($shortCode) - ->withScheme($domainConfig['schema'] ?? 'http') - ->withHost($domainConfig['hostname'] ?? ''); - } -} diff --git a/module/Core/src/Util/TagManagerTrait.php b/module/Core/src/Util/TagManagerTrait.php index 854a98b6..af6a8658 100644 --- a/module/Core/src/Util/TagManagerTrait.php +++ b/module/Core/src/Util/TagManagerTrait.php @@ -18,7 +18,7 @@ trait TagManagerTrait * @param string[] $tags * @return Collections\Collection|Tag[] */ - private function tagNamesToEntities(EntityManagerInterface $em, array $tags) + private function tagNamesToEntities(EntityManagerInterface $em, array $tags): Collections\Collection { $entities = []; foreach ($tags as $tagName) { diff --git a/module/Core/src/Validation/ShortUrlMetaInputFilter.php b/module/Core/src/Validation/ShortUrlMetaInputFilter.php index fad63f6c..afeefafb 100644 --- a/module/Core/src/Validation/ShortUrlMetaInputFilter.php +++ b/module/Core/src/Validation/ShortUrlMetaInputFilter.php @@ -17,6 +17,7 @@ class ShortUrlMetaInputFilter extends InputFilter public const CUSTOM_SLUG = 'customSlug'; public const MAX_VISITS = 'maxVisits'; public const FIND_IF_EXISTS = 'findIfExists'; + public const DOMAIN = 'domain'; public function __construct(?array $data = null) { @@ -46,5 +47,11 @@ class ShortUrlMetaInputFilter extends InputFilter $this->add($maxVisits); $this->add($this->createBooleanInput(self::FIND_IF_EXISTS, false)); + + $domain = $this->createInput(self::DOMAIN, false); + $domain->getValidatorChain()->attach(new Validator\Hostname([ + 'allow' => Validator\Hostname::ALLOW_DNS | Validator\Hostname::ALLOW_LOCAL, + ])); + $this->add($domain); } } diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index 2a243ce3..b1fac238 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -5,6 +5,7 @@ namespace ShlinkioTest\Shlink\Core\Repository; use Cake\Chronos\Chronos; use Doctrine\Common\Collections\ArrayCollection; +use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Entity\Visit; @@ -21,6 +22,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase Tag::class, Visit::class, ShortUrl::class, + Domain::class, ]; /** @var ShortUrlRepository */ @@ -32,37 +34,64 @@ class ShortUrlRepositoryTest extends DatabaseTestCase } /** @test */ - public function findOneByShortCodeReturnsProperData() + public function findOneByShortCodeReturnsProperData(): void { - $foo = new ShortUrl('foo'); - $foo->setShortCode('foo'); - $this->getEntityManager()->persist($foo); + $regularOne = new ShortUrl('foo'); + $regularOne->setShortCode('foo'); + $this->getEntityManager()->persist($regularOne); - $bar = new ShortUrl('bar', ShortUrlMeta::createFromParams(Chronos::now()->addMonth())); - $bar->setShortCode('bar_very_long_text'); - $this->getEntityManager()->persist($bar); + $notYetValid = new ShortUrl('bar', ShortUrlMeta::createFromParams(Chronos::now()->addMonth())); + $notYetValid->setShortCode('bar_very_long_text'); + $this->getEntityManager()->persist($notYetValid); - $baz = new ShortUrl('baz', ShortUrlMeta::createFromRawData(['maxVisits' => 3])); + $expired = new ShortUrl('expired', ShortUrlMeta::createFromParams(null, Chronos::now()->subMonth())); + $expired->setShortCode('expired'); + $this->getEntityManager()->persist($expired); + + $allVisitsComplete = new ShortUrl('baz', ShortUrlMeta::createFromRawData(['maxVisits' => 3])); $visits = []; for ($i = 0; $i < 3; $i++) { - $visit = new Visit($baz, Visitor::emptyInstance()); + $visit = new Visit($allVisitsComplete, Visitor::emptyInstance()); $this->getEntityManager()->persist($visit); $visits[] = $visit; } - $baz->setShortCode('baz') - ->setVisits(new ArrayCollection($visits)); - $this->getEntityManager()->persist($baz); + $allVisitsComplete->setShortCode('baz') + ->setVisits(new ArrayCollection($visits)); + $this->getEntityManager()->persist($allVisitsComplete); + + $withDomain = new ShortUrl('foo', ShortUrlMeta::createFromRawData(['domain' => 'example.com'])); + $withDomain->setShortCode('domain-short-code'); + $this->getEntityManager()->persist($withDomain); + + $withDomainDuplicatingRegular = new ShortUrl('foo_with_domain', ShortUrlMeta::createFromRawData([ + 'domain' => 'doma.in', + ])); + $withDomainDuplicatingRegular->setShortCode('foo'); + $this->getEntityManager()->persist($withDomainDuplicatingRegular); $this->getEntityManager()->flush(); - $this->assertSame($foo, $this->repo->findOneByShortCode($foo->getShortCode())); + $this->assertSame($regularOne, $this->repo->findOneByShortCode($regularOne->getShortCode())); + $this->assertSame($regularOne, $this->repo->findOneByShortCode($withDomainDuplicatingRegular->getShortCode())); + $this->assertSame($withDomain, $this->repo->findOneByShortCode($withDomain->getShortCode(), 'example.com')); + $this->assertSame( + $withDomainDuplicatingRegular, + $this->repo->findOneByShortCode($withDomainDuplicatingRegular->getShortCode(), 'doma.in') + ); + $this->assertSame( + $regularOne, + $this->repo->findOneByShortCode($withDomainDuplicatingRegular->getShortCode(), 'other-domain.com') + ); $this->assertNull($this->repo->findOneByShortCode('invalid')); - $this->assertNull($this->repo->findOneByShortCode($bar->getShortCode())); - $this->assertNull($this->repo->findOneByShortCode($baz->getShortCode())); + $this->assertNull($this->repo->findOneByShortCode($withDomain->getShortCode())); + $this->assertNull($this->repo->findOneByShortCode($withDomain->getShortCode(), 'other-domain.com')); + $this->assertNull($this->repo->findOneByShortCode($notYetValid->getShortCode())); + $this->assertNull($this->repo->findOneByShortCode($expired->getShortCode())); + $this->assertNull($this->repo->findOneByShortCode($allVisitsComplete->getShortCode())); } /** @test */ - public function countListReturnsProperNumberOfResults() + public function countListReturnsProperNumberOfResults(): void { $count = 5; for ($i = 0; $i < $count; $i++) { @@ -76,7 +105,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase } /** @test */ - public function findListProperlyFiltersByTagAndSearchTerm() + public function findListProperlyFiltersByTagAndSearchTerm(): void { $tag = new Tag('bar'); $this->getEntityManager()->persist($tag); @@ -121,7 +150,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase } /** @test */ - public function findListProperlyMapsFieldNamesToColumnNamesWhenOrdering() + public function findListProperlyMapsFieldNamesToColumnNamesWhenOrdering(): void { $urls = ['a', 'z', 'c', 'b']; foreach ($urls as $url) { @@ -140,4 +169,26 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->assertEquals('c', $result[2]->getLongUrl()); $this->assertEquals('z', $result[3]->getLongUrl()); } + + /** @test */ + public function slugIsInUseLooksForShortUrlInProperSetOfTables(): void + { + $shortUrlWithoutDomain = (new ShortUrl('foo'))->setShortCode('my-cool-slug'); + $this->getEntityManager()->persist($shortUrlWithoutDomain); + + $shortUrlWithDomain = (new ShortUrl( + 'foo', + ShortUrlMeta::createFromRawData(['domain' => 'doma.in']) + ))->setShortCode('another-slug'); + $this->getEntityManager()->persist($shortUrlWithDomain); + + $this->getEntityManager()->flush(); + + $this->assertTrue($this->repo->slugIsInUse('my-cool-slug')); + $this->assertFalse($this->repo->slugIsInUse('my-cool-slug', 'doma.in')); + $this->assertFalse($this->repo->slugIsInUse('slug-not-in-use')); + $this->assertFalse($this->repo->slugIsInUse('another-slug')); + $this->assertFalse($this->repo->slugIsInUse('another-slug', 'example.com')); + $this->assertTrue($this->repo->slugIsInUse('another-slug', 'doma.in')); + } } diff --git a/module/Core/test/Action/PixelActionTest.php b/module/Core/test/Action/PixelActionTest.php index 9fce8f0e..8e2ecc7f 100644 --- a/module/Core/test/Action/PixelActionTest.php +++ b/module/Core/test/Action/PixelActionTest.php @@ -41,7 +41,7 @@ class PixelActionTest extends TestCase public function imageIsReturned(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willReturn( + $this->urlShortener->shortCodeToUrl($shortCode, '')->willReturn( new ShortUrl('http://domain.com/foo/bar') )->shouldBeCalledOnce(); $this->visitTracker->track(Argument::cetera())->shouldBeCalledOnce(); diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index da2199e4..e0477b02 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -47,8 +47,8 @@ class RedirectActionTest extends TestCase $shortCode = 'abc123'; $expectedUrl = 'http://domain.com/foo/bar'; $shortUrl = new ShortUrl($expectedUrl); - $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl) - ->shouldBeCalledOnce(); + $this->urlShortener->shortCodeToUrl($shortCode, '')->willReturn($shortUrl) + ->shouldBeCalledOnce(); $this->visitTracker->track(Argument::cetera())->shouldBeCalledOnce(); $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); @@ -64,8 +64,8 @@ class RedirectActionTest extends TestCase public function nextMiddlewareIsInvokedIfLongUrlIsNotFound(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class) - ->shouldBeCalledOnce(); + $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(EntityDoesNotExistException::class) + ->shouldBeCalledOnce(); $this->visitTracker->track(Argument::cetera())->shouldNotBeCalled(); $handler = $this->prophesize(RequestHandlerInterface::class); @@ -81,7 +81,7 @@ class RedirectActionTest extends TestCase public function redirectToCustomUrlIsReturnedIfConfiguredSoAndShortUrlIsNotFound(): void { $shortCode = 'abc123'; - $shortCodeToUrl = $this->urlShortener->shortCodeToUrl($shortCode)->willThrow( + $shortCodeToUrl = $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow( EntityDoesNotExistException::class ); @@ -106,8 +106,8 @@ class RedirectActionTest extends TestCase $shortCode = 'abc123'; $expectedUrl = 'http://domain.com/foo/bar'; $shortUrl = new ShortUrl($expectedUrl); - $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl) - ->shouldBeCalledOnce(); + $this->urlShortener->shortCodeToUrl($shortCode, '')->willReturn($shortUrl) + ->shouldBeCalledOnce(); $this->visitTracker->track(Argument::cetera())->shouldNotBeCalled(); $request = (new ServerRequest())->withAttribute('shortCode', $shortCode) diff --git a/module/Core/test/Domain/Resolver/PersistenceDomainResolverTest.php b/module/Core/test/Domain/Resolver/PersistenceDomainResolverTest.php new file mode 100644 index 00000000..aeb72873 --- /dev/null +++ b/module/Core/test/Domain/Resolver/PersistenceDomainResolverTest.php @@ -0,0 +1,63 @@ +em = $this->prophesize(EntityManagerInterface::class); + $this->domainResolver = new PersistenceDomainResolver($this->em->reveal()); + } + + /** @test */ + public function returnsEmptyWhenNoDomainIsProvided(): void + { + $getRepository = $this->em->getRepository(Domain::class); + + $this->assertNull($this->domainResolver->resolveDomain(null)); + $getRepository->shouldNotHaveBeenCalled(); + } + + /** + * @test + * @dataProvider provideFoundDomains + */ + public function findsOrCreatesDomainWhenValueIsProvided(?Domain $foundDomain, string $authority): void + { + $repo = $this->prophesize(ObjectRepository::class); + $findDomain = $repo->findOneBy(['authority' => $authority])->willReturn($foundDomain); + $getRepository = $this->em->getRepository(Domain::class)->willReturn($repo->reveal()); + + $result = $this->domainResolver->resolveDomain($authority); + + if ($foundDomain !== null) { + $this->assertSame($result, $foundDomain); + } + $this->assertInstanceOf(Domain::class, $result); + $this->assertEquals($authority, $result->getAuthority()); + $findDomain->shouldHaveBeenCalledOnce(); + $getRepository->shouldHaveBeenCalledOnce(); + } + + public function provideFoundDomains(): iterable + { + $authority = 'doma.in'; + + yield 'without found domain' => [null, $authority]; + yield 'with found domain' => [new Domain($authority), $authority]; + } +} diff --git a/module/Core/test/Domain/Resolver/SimpleDomainResolverTest.php b/module/Core/test/Domain/Resolver/SimpleDomainResolverTest.php new file mode 100644 index 00000000..5e175c92 --- /dev/null +++ b/module/Core/test/Domain/Resolver/SimpleDomainResolverTest.php @@ -0,0 +1,41 @@ +domainResolver = new SimpleDomainResolver(); + } + + /** + * @test + * @dataProvider provideDomains + */ + public function resolvesExpectedDomain(?string $domain): void + { + $result = $this->domainResolver->resolveDomain($domain); + + if ($domain === null) { + $this->assertNull($result); + } else { + $this->assertInstanceOf(Domain::class, $result); + $this->assertEquals($domain, $result->getAuthority()); + } + } + + public function provideDomains(): iterable + { + yield 'with empty domain' => [null]; + yield 'with non-empty domain' => ['domain.com']; + } +} diff --git a/module/Core/test/Exception/NonUniqueSlugExceptionTest.php b/module/Core/test/Exception/NonUniqueSlugExceptionTest.php new file mode 100644 index 00000000..aa491b5d --- /dev/null +++ b/module/Core/test/Exception/NonUniqueSlugExceptionTest.php @@ -0,0 +1,34 @@ +assertEquals($expectedMessage, $e->getMessage()); + } + + public function provideMessages(): iterable + { + yield 'without domain' => [ + 'Provided slug "foo" is not unique.', + 'foo', + null, + ]; + yield 'with domain' => [ + 'Provided slug "baz" is not unique for domain "bar".', + 'baz', + 'bar', + ]; + } +} diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 50ea91e1..77db0bae 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -55,7 +55,7 @@ class UrlShortenerTest extends TestCase $shortUrl->setId('10'); }); $repo = $this->prophesize(ShortUrlRepository::class); - $repo->count(Argument::any())->willReturn(0); + $repo->slugIsInUse(Argument::cetera())->willReturn(false); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $this->setUrlShortener(false); @@ -122,11 +122,11 @@ class UrlShortenerTest extends TestCase public function exceptionIsThrownWhenNonUniqueSlugIsProvided(): void { $repo = $this->prophesize(ShortUrlRepository::class); - $countBySlug = $repo->count(['shortCode' => 'custom-slug'])->willReturn(1); + $slugIsInUse = $repo->slugIsInUse('custom-slug', null)->willReturn(true); $repo->findBy(Argument::cetera())->willReturn([]); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $countBySlug->shouldBeCalledOnce(); + $slugIsInUse->shouldBeCalledOnce(); $getRepo->shouldBeCalled(); $this->expectException(NonUniqueSlugException::class); @@ -247,7 +247,7 @@ class UrlShortenerTest extends TestCase $shortUrl->setShortCode($shortCode); $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $repo->findOneByShortCode($shortCode)->willReturn($shortUrl); + $repo->findOneByShortCode($shortCode, null)->willReturn($shortUrl); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $url = $this->urlShortener->shortCodeToUrl($shortCode); diff --git a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php index ff79818b..17d7eab1 100644 --- a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php @@ -36,7 +36,8 @@ class CreateShortUrlAction extends AbstractCreateShortUrlAction $this->getOptionalDate($postData, 'validUntil'), $postData['customSlug'] ?? null, $postData['maxVisits'] ?? null, - $postData['findIfExists'] ?? null + $postData['findIfExists'] ?? null, + $postData['domain'] ?? null ) ); } diff --git a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php index 13fc1258..24288da1 100644 --- a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php @@ -45,10 +45,11 @@ class ResolveShortUrlAction extends AbstractRestAction public function handle(Request $request): Response { $shortCode = $request->getAttribute('shortCode'); + $domain = $request->getQueryParams()['domain'] ?? null; $transformer = new ShortUrlDataTransformer($this->domainConfig); try { - $url = $this->urlShortener->shortCodeToUrl($shortCode); + $url = $this->urlShortener->shortCodeToUrl($shortCode, $domain); return new JsonResponse($transformer->transform($url)); } catch (InvalidShortCodeException $e) { $this->logger->warning('Provided short code with invalid format. {e}', ['e' => $e]); diff --git a/module/Rest/test-api/Action/CreateShortUrlActionTest.php b/module/Rest/test-api/Action/CreateShortUrlActionTest.php index 099eb9c4..1970cd7a 100644 --- a/module/Rest/test-api/Action/CreateShortUrlActionTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlActionTest.php @@ -5,6 +5,7 @@ namespace ShlinkioApiTest\Shlink\Rest\Action; use Cake\Chronos\Chronos; use GuzzleHttp\RequestOptions; +use Shlinkio\Shlink\Rest\Util\RestUtils; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; use function Functional\map; @@ -33,6 +34,18 @@ class CreateShortUrlActionTest extends ApiTestCase $this->assertEquals('my-cool-slug', $payload['shortCode']); } + /** + * @test + * @dataProvider provideConflictingSlugs + */ + public function failsToCreateShortUrlWithDuplicatedSlug(string $slug, ?string $domain): void + { + [$statusCode, $payload] = $this->createShortUrl(['customSlug' => $slug, 'domain' => $domain]); + + $this->assertEquals(self::STATUS_BAD_REQUEST, $statusCode); + $this->assertEquals(RestUtils::INVALID_SLUG_ERROR, $payload['error']); + } + /** @test */ public function createsNewShortUrlWithTags(): void { @@ -126,22 +139,32 @@ class CreateShortUrlActionTest extends ApiTestCase ]]; } - /** @test */ - public function returnsErrorWhenRequestingReturnExistingButCustomSlugIsInUse(): void + /** + * @test + * @dataProvider provideConflictingSlugs + */ + public function returnsErrorWhenRequestingReturnExistingButCustomSlugIsInUse(string $slug, ?string $domain): void { $longUrl = 'https://www.alejandrocelaya.com'; [$firstStatusCode] = $this->createShortUrl(['longUrl' => $longUrl]); [$secondStatusCode] = $this->createShortUrl([ 'longUrl' => $longUrl, - 'customSlug' => 'custom', + 'customSlug' => $slug, 'findIfExists' => true, + 'domain' => $domain, ]); $this->assertEquals(self::STATUS_OK, $firstStatusCode); $this->assertEquals(self::STATUS_BAD_REQUEST, $secondStatusCode); } + public function provideConflictingSlugs(): iterable + { + yield 'without domain' => ['custom', null]; + yield 'with domain' => ['custom-with-domain', 'some-domain.com']; + } + /** @test */ public function createsNewShortUrlIfRequestedToFindButThereIsNoMatch(): void { diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index 8f7db414..02d3b404 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -63,13 +63,45 @@ class ListShortUrlsTest extends ApiTestCase ], 'originalUrl' => 'https://shlink.io', ], + [ + 'shortCode' => 'ghi789', + 'shortUrl' => 'http://example.com/ghi789', + 'longUrl' => + 'https://blog.alejandrocelaya.com/2019/04/27' + . '/considerations-to-properly-use-open-source-software-projects/', + 'dateCreated' => '2019-01-01T00:00:00+00:00', + 'visitsCount' => 0, + 'tags' => [], + 'meta' => [ + 'validSince' => null, + 'validUntil' => null, + 'maxVisits' => null, + ], + 'originalUrl' => + 'https://blog.alejandrocelaya.com/2019/04/27' + . '/considerations-to-properly-use-open-source-software-projects/', + ], + [ + 'shortCode' => 'custom-with-domain', + 'shortUrl' => 'http://some-domain.com/custom-with-domain', + 'longUrl' => 'https://google.com', + 'dateCreated' => '2019-01-01T00:00:00+00:00', + 'visitsCount' => 0, + 'tags' => [], + 'meta' => [ + 'validSince' => null, + 'validUntil' => null, + 'maxVisits' => null, + ], + 'originalUrl' => 'https://google.com', + ], ], 'pagination' => [ 'currentPage' => 1, 'pagesCount' => 1, 'itemsPerPage' => 10, - 'itemsInCurrentPage' => 3, - 'totalItems' => 3, + 'itemsInCurrentPage' => 5, + 'totalItems' => 5, ], ], ], $respPayload); diff --git a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php index 51bdaaad..5e7d98ba 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php @@ -34,6 +34,18 @@ class ShortUrlsFixture extends AbstractFixture )); $manager->persist($customShortUrl); + $withDomainShortUrl = $this->setShortUrlDate(new ShortUrl( + 'https://blog.alejandrocelaya.com/2019/04/27/considerations-to-properly-use-open-source-software-projects/', + ShortUrlMeta::createFromRawData(['domain' => 'example.com']) + ))->setShortCode('ghi789'); + $manager->persist($withDomainShortUrl); + + $withDomainAndSlugShortUrl = $this->setShortUrlDate(new ShortUrl( + 'https://google.com', + ShortUrlMeta::createFromRawData(['domain' => 'some-domain.com']) + ))->setShortCode('custom-with-domain'); + $manager->persist($withDomainAndSlugShortUrl); + $manager->flush(); $this->addReference('abc123_short_url', $abcShortUrl); diff --git a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php index d743ffa0..f2c2745b 100644 --- a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php @@ -36,14 +36,14 @@ class CreateShortUrlActionTest extends TestCase } /** @test */ - public function missingLongUrlParamReturnsError() + public function missingLongUrlParamReturnsError(): void { $response = $this->action->handle(new ServerRequest()); $this->assertEquals(400, $response->getStatusCode()); } /** @test */ - public function properShortcodeConversionReturnsData() + public function properShortcodeConversionReturnsData(): void { $this->urlShortener->urlToShortCode(Argument::type(Uri::class), Argument::type('array'), Argument::cetera()) ->willReturn( @@ -60,7 +60,7 @@ class CreateShortUrlActionTest extends TestCase } /** @test */ - public function anInvalidUrlReturnsError() + public function anInvalidUrlReturnsError(): void { $this->urlShortener->urlToShortCode(Argument::type(Uri::class), Argument::type('array'), Argument::cetera()) ->willThrow(InvalidUrlException::class) @@ -75,7 +75,7 @@ class CreateShortUrlActionTest extends TestCase } /** @test */ - public function nonUniqueSlugReturnsError() + public function nonUniqueSlugReturnsError(): void { $this->urlShortener->urlToShortCode( Argument::type(Uri::class), @@ -94,7 +94,7 @@ class CreateShortUrlActionTest extends TestCase } /** @test */ - public function aGenericExceptionWillReturnError() + public function aGenericExceptionWillReturnError(): void { $this->urlShortener->urlToShortCode(Argument::type(Uri::class), Argument::type('array'), Argument::cetera()) ->willThrow(Exception::class) diff --git a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php index fc8f5d01..84fbf14e 100644 --- a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php @@ -30,11 +30,11 @@ class ResolveShortUrlActionTest extends TestCase } /** @test */ - public function incorrectShortCodeReturnsError() + public function incorrectShortCodeReturnsError(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(EntityDoesNotExistException::class) - ->shouldBeCalledOnce(); + $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(EntityDoesNotExistException::class) + ->shouldBeCalledOnce(); $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); $response = $this->action->handle($request); @@ -43,10 +43,10 @@ class ResolveShortUrlActionTest extends TestCase } /** @test */ - public function correctShortCodeReturnsSuccess() + public function correctShortCodeReturnsSuccess(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willReturn( + $this->urlShortener->shortCodeToUrl($shortCode, null)->willReturn( new ShortUrl('http://domain.com/foo/bar') )->shouldBeCalledOnce(); @@ -57,11 +57,11 @@ class ResolveShortUrlActionTest extends TestCase } /** @test */ - public function invalidShortCodeExceptionReturnsError() + public function invalidShortCodeExceptionReturnsError(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(InvalidShortCodeException::class) - ->shouldBeCalledOnce(); + $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(InvalidShortCodeException::class) + ->shouldBeCalledOnce(); $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); $response = $this->action->handle($request); @@ -70,11 +70,11 @@ class ResolveShortUrlActionTest extends TestCase } /** @test */ - public function unexpectedExceptionWillReturnError() + public function unexpectedExceptionWillReturnError(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willThrow(Exception::class) - ->shouldBeCalledOnce(); + $this->urlShortener->shortCodeToUrl($shortCode, null)->willThrow(Exception::class) + ->shouldBeCalledOnce(); $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); $response = $this->action->handle($request);