From 05acd4ae88da0a1c4502b26b0ffd40df8a0752ab Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 25 Jan 2023 20:33:07 +0100 Subject: [PATCH] Add two modes for short URLs --- config/autoload/url-shortener.global.php | 4 +++ .../ShortUrl/CreateShortUrlCommand.php | 2 +- module/Core/config/dependencies.config.php | 2 +- module/Core/functions/functions.php | 7 ++++-- module/Core/src/Config/EnvVars.php | 1 + .../Core/src/Options/UrlShortenerOptions.php | 3 +++ module/Core/src/ShortUrl/Entity/ShortUrl.php | 10 +++++--- .../Helper/ShortCodeUniquenessHelper.php | 9 ++++--- .../src/ShortUrl/Model/ShortUrlCreation.php | 5 +++- .../Core/src/ShortUrl/Model/ShortUrlMode.php | 9 +++++++ .../Model/Validation/ShortUrlInputFilter.php | 1 - .../test/ShortUrl/Entity/ShortUrlTest.php | 25 +++++++++++++++++-- .../Helper/ShortCodeUniquenessHelperTest.php | 3 ++- .../Action/ShortUrl/CreateShortUrlAction.php | 2 +- .../SingleStepCreateShortUrlAction.php | 2 +- 15 files changed, 68 insertions(+), 17 deletions(-) create mode 100644 module/Core/src/ShortUrl/Model/ShortUrlMode.php diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index ec3c1409..2816577d 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -3,6 +3,7 @@ declare(strict_types=1); use Shlinkio\Shlink\Core\Config\EnvVars; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; use const Shlinkio\Shlink\DEFAULT_SHORT_CODES_LENGTH; use const Shlinkio\Shlink\MIN_SHORT_CODES_LENGTH; @@ -12,6 +13,8 @@ return (static function (): array { (int) EnvVars::DEFAULT_SHORT_CODES_LENGTH->loadFromEnv(DEFAULT_SHORT_CODES_LENGTH), MIN_SHORT_CODES_LENGTH, ); + $modeFromEnv = EnvVars::SHORT_URL_MODE->loadFromEnv(ShortUrlMode::STRICT->value); + $mode = ShortUrlMode::tryFrom($modeFromEnv) ?? ShortUrlMode::STRICT; return [ @@ -25,6 +28,7 @@ return (static function (): array { 'append_extra_path' => (bool) EnvVars::REDIRECT_APPEND_EXTRA_PATH->loadFromEnv(false), 'multi_segment_slugs_enabled' => (bool) EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->loadFromEnv(false), 'trailing_slash_enabled' => (bool) EnvVars::SHORT_URL_TRAILING_SLASH->loadFromEnv(false), + 'mode' => $mode, ], ]; diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index 71ab5fa7..46a1cc8f 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -176,7 +176,7 @@ class CreateShortUrlCommand extends Command ShortUrlInputFilter::CRAWLABLE => $input->getOption('crawlable'), ShortUrlInputFilter::FORWARD_QUERY => !$input->getOption('no-forward-query'), EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->value => $this->options->multiSegmentSlugsEnabled, - ])); + ], $this->options->mode)); $io->writeln([ sprintf('Processed long URL: %s', $longUrl), diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 48fe3cb2..40985f42 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -137,7 +137,7 @@ return [ ShortUrl\ShortUrlResolver::class, ], ShortUrl\ShortUrlResolver::class => ['em'], - ShortUrl\Helper\ShortCodeUniquenessHelper::class => ['em'], + ShortUrl\Helper\ShortCodeUniquenessHelper::class => ['em', Options\UrlShortenerOptions::class], Domain\DomainService::class => ['em', 'config.url_shortener.domain.hostname'], Util\UrlValidator::class => ['httpClient', Options\UrlShortenerOptions::class], diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 574d604c..b6acbb35 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -15,6 +15,7 @@ use Laminas\Filter\Word\CamelCaseToUnderscore; use Laminas\InputFilter\InputFilter; use PUGX\Shortid\Factory as ShortIdFactory; use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; use function date_default_timezone_get; use function Functional\map; @@ -27,14 +28,16 @@ use function str_repeat; use function strtolower; use function ucfirst; -function generateRandomShortCode(int $length): string +function generateRandomShortCode(int $length, ShortUrlMode $mode = ShortUrlMode::STRICT): string { static $shortIdFactory; if ($shortIdFactory === null) { $shortIdFactory = new ShortIdFactory(); } - $alphabet = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'; + $alphabet = $mode === ShortUrlMode::STRICT + ? '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ' + : '0123456789abcdefghijklmnopqrstuvwxyz'; return $shortIdFactory->generate($length, $alphabet)->serialize(); } diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 75454ecc..44919415 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -43,6 +43,7 @@ enum EnvVars: string case REDIRECT_CACHE_LIFETIME = 'REDIRECT_CACHE_LIFETIME'; case BASE_PATH = 'BASE_PATH'; case SHORT_URL_TRAILING_SLASH = 'SHORT_URL_TRAILING_SLASH'; + case SHORT_URL_MODE = 'SHORT_URL_MODE'; case PORT = 'PORT'; case TASK_WORKER_NUM = 'TASK_WORKER_NUM'; case WEB_WORKER_NUM = 'WEB_WORKER_NUM'; diff --git a/module/Core/src/Options/UrlShortenerOptions.php b/module/Core/src/Options/UrlShortenerOptions.php index 9aacc085..827988fa 100644 --- a/module/Core/src/Options/UrlShortenerOptions.php +++ b/module/Core/src/Options/UrlShortenerOptions.php @@ -4,6 +4,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Options; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; + use const Shlinkio\Shlink\DEFAULT_SHORT_CODES_LENGTH; final class UrlShortenerOptions @@ -16,6 +18,7 @@ final class UrlShortenerOptions public readonly bool $appendExtraPath = false, public readonly bool $multiSegmentSlugsEnabled = false, public readonly bool $trailingSlashEnabled = false, + public readonly ShortUrlMode $mode = ShortUrlMode::STRICT, ) { } } diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index d0e9cba4..e3d6544c 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -16,6 +16,7 @@ use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\ShortUrl\Model\DeviceLongUrlPair; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlEdition; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface; use Shlinkio\Shlink\Core\ShortUrl\Resolver\SimpleShortUrlRelationResolver; @@ -95,7 +96,10 @@ class ShortUrl extends AbstractEntity $instance->maxVisits = $creation->maxVisits; $instance->customSlugWasProvided = $creation->hasCustomSlug(); $instance->shortCodeLength = $creation->shortCodeLength; - $instance->shortCode = $creation->customSlug ?? generateRandomShortCode($instance->shortCodeLength); + $instance->shortCode = $creation->customSlug ?? generateRandomShortCode( + $instance->shortCodeLength, + $creation->shortUrlMode, + ); $instance->domain = $relationResolver->resolveDomain($creation->domain); $instance->authorApiKey = $creation->apiKey; $instance->title = $creation->title; @@ -292,7 +296,7 @@ class ShortUrl extends AbstractEntity /** * @throws ShortCodeCannotBeRegeneratedException */ - public function regenerateShortCode(): void + public function regenerateShortCode(ShortUrlMode $mode): void { // In ShortUrls where a custom slug was provided, throw error, unless it is an imported one if ($this->customSlugWasProvided && $this->importSource === null) { @@ -304,7 +308,7 @@ class ShortUrl extends AbstractEntity throw ShortCodeCannotBeRegeneratedException::forShortUrlAlreadyPersisted(); } - $this->shortCode = generateRandomShortCode($this->shortCodeLength); + $this->shortCode = generateRandomShortCode($this->shortCodeLength, $mode); } public function isEnabled(): bool diff --git a/module/Core/src/ShortUrl/Helper/ShortCodeUniquenessHelper.php b/module/Core/src/ShortUrl/Helper/ShortCodeUniquenessHelper.php index 1f16f037..b428019e 100644 --- a/module/Core/src/ShortUrl/Helper/ShortCodeUniquenessHelper.php +++ b/module/Core/src/ShortUrl/Helper/ShortCodeUniquenessHelper.php @@ -5,14 +5,17 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Helper; use Doctrine\ORM\EntityManagerInterface; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository; class ShortCodeUniquenessHelper implements ShortCodeUniquenessHelperInterface { - public function __construct(private readonly EntityManagerInterface $em) - { + public function __construct( + private readonly EntityManagerInterface $em, + private readonly UrlShortenerOptions $options, + ) { } public function ensureShortCodeUniqueness(ShortUrl $shortUrlToBeCreated, bool $hasCustomSlug): bool @@ -29,7 +32,7 @@ class ShortCodeUniquenessHelper implements ShortCodeUniquenessHelperInterface return false; } - $shortUrlToBeCreated->regenerateShortCode(); + $shortUrlToBeCreated->regenerateShortCode($this->options->mode); return $this->ensureShortCodeUniqueness($shortUrlToBeCreated, $hasCustomSlug); } } diff --git a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php index c29817b6..d0d9b279 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php @@ -25,6 +25,7 @@ final class ShortUrlCreation implements TitleResolutionModelInterface */ private function __construct( public readonly string $longUrl, + public readonly ShortUrlMode $shortUrlMode, public readonly array $deviceLongUrls = [], public readonly ?Chronos $validSince = null, public readonly ?Chronos $validUntil = null, @@ -47,7 +48,7 @@ final class ShortUrlCreation implements TitleResolutionModelInterface /** * @throws ValidationException */ - public static function fromRawData(array $data): self + public static function fromRawData(array $data, ShortUrlMode $mode = ShortUrlMode::STRICT): self { $inputFilter = ShortUrlInputFilter::withRequiredLongUrl($data); if (! $inputFilter->isValid()) { @@ -60,6 +61,7 @@ final class ShortUrlCreation implements TitleResolutionModelInterface return new self( longUrl: $inputFilter->getValue(ShortUrlInputFilter::LONG_URL), + shortUrlMode: $mode, deviceLongUrls: $deviceLongUrls, validSince: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)), validUntil: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_UNTIL)), @@ -84,6 +86,7 @@ final class ShortUrlCreation implements TitleResolutionModelInterface { return new self( longUrl: $this->longUrl, + shortUrlMode: $this->shortUrlMode, deviceLongUrls: $this->deviceLongUrls, validSince: $this->validSince, validUntil: $this->validUntil, diff --git a/module/Core/src/ShortUrl/Model/ShortUrlMode.php b/module/Core/src/ShortUrl/Model/ShortUrlMode.php new file mode 100644 index 00000000..41698e18 --- /dev/null +++ b/module/Core/src/ShortUrl/Model/ShortUrlMode.php @@ -0,0 +1,9 @@ +initialize($requireLongUrl, $data[EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->value] ?? false); $this->setData($data); } diff --git a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php index 2d950d5f..be1747fd 100644 --- a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php +++ b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php @@ -11,13 +11,16 @@ use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlEdition; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Importer\Sources\ImportSource; +use function Functional\every; use function Functional\map; use function range; use function strlen; +use function strtolower; use const Shlinkio\Shlink\DEFAULT_SHORT_CODES_LENGTH; @@ -34,7 +37,7 @@ class ShortUrlTest extends TestCase $this->expectException(ShortCodeCannotBeRegeneratedException::class); $this->expectExceptionMessage($expectedMessage); - $shortUrl->regenerateShortCode(); + $shortUrl->regenerateShortCode(ShortUrlMode::STRICT); } public function provideInvalidShortUrls(): iterable @@ -58,7 +61,7 @@ class ShortUrlTest extends TestCase ): void { $firstShortCode = $shortUrl->getShortCode(); - $shortUrl->regenerateShortCode(); + $shortUrl->regenerateShortCode(ShortUrlMode::STRICT); $secondShortCode = $shortUrl->getShortCode(); self::assertNotEquals($firstShortCode, $secondShortCode); @@ -133,4 +136,22 @@ class ShortUrlTest extends TestCase DeviceType::DESKTOP->value => 'desktop', ], $shortUrl->deviceLongUrls()); } + + /** @test */ + public function generatesLowercaseOnlyShortCodesInLooselyMode(): void + { + $range = range(1, 1000); // Use a "big" number to reduce false negatives + $allFor = static fn (ShortUrlMode $mode): bool => every($range, static function () use ($mode): bool { + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData( + [ShortUrlInputFilter::LONG_URL => 'foo'], + $mode, + )); + $shortCode = $shortUrl->getShortCode(); + + return $shortCode === strtolower($shortCode); + }); + + self::assertTrue($allFor(ShortUrlMode::LOOSELY)); + self::assertFalse($allFor(ShortUrlMode::STRICT)); + } } diff --git a/module/Core/test/ShortUrl/Helper/ShortCodeUniquenessHelperTest.php b/module/Core/test/ShortUrl/Helper/ShortCodeUniquenessHelperTest.php index 5df79fc5..ae0d9363 100644 --- a/module/Core/test/ShortUrl/Helper/ShortCodeUniquenessHelperTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortCodeUniquenessHelperTest.php @@ -8,6 +8,7 @@ use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Domain\Entity\Domain; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortCodeUniquenessHelper; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; @@ -22,7 +23,7 @@ class ShortCodeUniquenessHelperTest extends TestCase protected function setUp(): void { $this->em = $this->createMock(EntityManagerInterface::class); - $this->helper = new ShortCodeUniquenessHelper($this->em); + $this->helper = new ShortCodeUniquenessHelper($this->em, new UrlShortenerOptions()); $this->shortUrl = $this->createMock(ShortUrl::class); $this->shortUrl->method('getShortCode')->willReturn('abc123'); diff --git a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php index e60414b2..c7b6b33f 100644 --- a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php @@ -25,6 +25,6 @@ class CreateShortUrlAction extends AbstractCreateShortUrlAction $payload[ShortUrlInputFilter::API_KEY] = AuthenticationMiddleware::apiKeyFromRequest($request); $payload[EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->value] = $this->urlShortenerOptions->multiSegmentSlugsEnabled; - return ShortUrlCreation::fromRawData($payload); + return ShortUrlCreation::fromRawData($payload, $this->urlShortenerOptions->mode); } } diff --git a/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php index 89989dda..b32e8a5d 100644 --- a/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/SingleStepCreateShortUrlAction.php @@ -25,6 +25,6 @@ class SingleStepCreateShortUrlAction extends AbstractCreateShortUrlAction ShortUrlInputFilter::API_KEY => $apiKey, // This will usually be null, unless this API key enforces one specific domain ShortUrlInputFilter::DOMAIN => $request->getAttribute(ShortUrlInputFilter::DOMAIN), - ]); + ], $this->urlShortenerOptions->mode); } }