From 0e78deb8f2c4bda82c914f405f867a241c6f3cf7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 21 Feb 2024 10:12:40 +0100 Subject: [PATCH 1/6] Refactor ShortUrlInputFilter for creation and edition --- .../src/ShortUrl/Model/ShortUrlCreation.php | 40 +++--- .../src/ShortUrl/Model/ShortUrlEdition.php | 42 +++--- .../Model/Validation/CustomSlugFilter.php | 11 +- .../Model/Validation/ShortUrlInputFilter.php | 124 ++++++++++-------- 4 files changed, 115 insertions(+), 102 deletions(-) diff --git a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php index 1a1673ac..976973b2 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php @@ -18,39 +18,39 @@ use function Shlinkio\Shlink\Core\normalizeOptionalDate; use const Shlinkio\Shlink\DEFAULT_SHORT_CODES_LENGTH; -final class ShortUrlCreation implements TitleResolutionModelInterface +final readonly class ShortUrlCreation implements TitleResolutionModelInterface { /** * @param string[] $tags * @param DeviceLongUrlPair[] $deviceLongUrls */ 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, - public readonly ?string $customSlug = null, - public readonly ?int $maxVisits = null, - public readonly bool $findIfExists = false, - public readonly ?string $domain = null, - public readonly int $shortCodeLength = 5, - public readonly ?ApiKey $apiKey = null, - public readonly array $tags = [], - public readonly ?string $title = null, - public readonly bool $titleWasAutoResolved = false, - public readonly bool $crawlable = false, - public readonly bool $forwardQuery = true, + public string $longUrl, + public ShortUrlMode $shortUrlMode, + public array $deviceLongUrls = [], + public ?Chronos $validSince = null, + public ?Chronos $validUntil = null, + public ?string $customSlug = null, + public ?string $pathPrefix = null, + public ?int $maxVisits = null, + public bool $findIfExists = false, + public ?string $domain = null, + public int $shortCodeLength = 5, + public ?ApiKey $apiKey = null, + public array $tags = [], + public ?string $title = null, + public bool $titleWasAutoResolved = false, + public bool $crawlable = false, + public bool $forwardQuery = true, ) { } /** * @throws ValidationException */ - public static function fromRawData(array $data, ?UrlShortenerOptions $options = null): self + public static function fromRawData(array $data, UrlShortenerOptions $options = new UrlShortenerOptions()): self { - $options = $options ?? new UrlShortenerOptions(); - $inputFilter = ShortUrlInputFilter::withRequiredLongUrl($data, $options); + $inputFilter = ShortUrlInputFilter::forCreation($data, $options); if (! $inputFilter->isValid()) { throw ValidationException::fromInputFilter($inputFilter); } diff --git a/module/Core/src/ShortUrl/Model/ShortUrlEdition.php b/module/Core/src/ShortUrl/Model/ShortUrlEdition.php index 7a0000de..2502331a 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlEdition.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlEdition.php @@ -15,7 +15,7 @@ use function Shlinkio\Shlink\Core\getOptionalBoolFromInputFilter; use function Shlinkio\Shlink\Core\getOptionalIntFromInputFilter; use function Shlinkio\Shlink\Core\normalizeOptionalDate; -final class ShortUrlEdition implements TitleResolutionModelInterface +final readonly class ShortUrlEdition implements TitleResolutionModelInterface { /** * @param string[] $tags @@ -23,25 +23,25 @@ final class ShortUrlEdition implements TitleResolutionModelInterface * @param DeviceType[] $devicesToRemove */ private function __construct( - private readonly bool $longUrlPropWasProvided = false, - public readonly ?string $longUrl = null, - public readonly array $deviceLongUrls = [], - public readonly array $devicesToRemove = [], - private readonly bool $validSincePropWasProvided = false, - public readonly ?Chronos $validSince = null, - private readonly bool $validUntilPropWasProvided = false, - public readonly ?Chronos $validUntil = null, - private readonly bool $maxVisitsPropWasProvided = false, - public readonly ?int $maxVisits = null, - private readonly bool $tagsPropWasProvided = false, - public readonly array $tags = [], - private readonly bool $titlePropWasProvided = false, - public readonly ?string $title = null, - public readonly bool $titleWasAutoResolved = false, - private readonly bool $crawlablePropWasProvided = false, - public readonly bool $crawlable = false, - private readonly bool $forwardQueryPropWasProvided = false, - public readonly bool $forwardQuery = true, + private bool $longUrlPropWasProvided = false, + public ?string $longUrl = null, + public array $deviceLongUrls = [], + public array $devicesToRemove = [], + private bool $validSincePropWasProvided = false, + public ?Chronos $validSince = null, + private bool $validUntilPropWasProvided = false, + public ?Chronos $validUntil = null, + private bool $maxVisitsPropWasProvided = false, + public ?int $maxVisits = null, + private bool $tagsPropWasProvided = false, + public array $tags = [], + private bool $titlePropWasProvided = false, + public ?string $title = null, + public bool $titleWasAutoResolved = false, + private bool $crawlablePropWasProvided = false, + public bool $crawlable = false, + private bool $forwardQueryPropWasProvided = false, + public bool $forwardQuery = true, ) { } @@ -50,7 +50,7 @@ final class ShortUrlEdition implements TitleResolutionModelInterface */ public static function fromRawData(array $data): self { - $inputFilter = ShortUrlInputFilter::withNonRequiredLongUrl($data); + $inputFilter = ShortUrlInputFilter::forEdition($data); if (! $inputFilter->isValid()) { throw ValidationException::fromInputFilter($inputFilter); } diff --git a/module/Core/src/ShortUrl/Model/Validation/CustomSlugFilter.php b/module/Core/src/ShortUrl/Model/Validation/CustomSlugFilter.php index d7012bf1..2512fc44 100644 --- a/module/Core/src/ShortUrl/Model/Validation/CustomSlugFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/CustomSlugFilter.php @@ -12,9 +12,9 @@ use function str_replace; use function strtolower; use function trim; -class CustomSlugFilter implements FilterInterface +readonly class CustomSlugFilter implements FilterInterface { - public function __construct(private readonly UrlShortenerOptions $options) + public function __construct(private UrlShortenerOptions $options) { } @@ -25,9 +25,8 @@ class CustomSlugFilter implements FilterInterface } $value = $this->options->isLooseMode() ? strtolower($value) : $value; - return (match ($this->options->multiSegmentSlugsEnabled) { - true => trim(str_replace(' ', '-', $value), '/'), - false => str_replace([' ', '/'], '-', $value), - }); + return $this->options->multiSegmentSlugsEnabled + ? trim(str_replace(' ', '-', $value), '/') + : str_replace([' ', '/'], '-', $value); } } diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php index 325b400f..ad3a6df7 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php @@ -19,68 +19,52 @@ use function substr; use const Shlinkio\Shlink\LOOSE_URI_MATCHER; use const Shlinkio\Shlink\MIN_SHORT_CODES_LENGTH; -/** - * @todo Pass forCreation/forEdition, instead of withRequiredLongUrl/withNonRequiredLongUrl. - * Make it also dynamically add the relevant fields - */ class ShortUrlInputFilter extends InputFilter { use Validation\InputFactoryTrait; - public const VALID_SINCE = 'validSince'; - public const VALID_UNTIL = 'validUntil'; + // Fields for creation only + public const SHORT_CODE_LENGTH = 'shortCodeLength'; public const CUSTOM_SLUG = 'customSlug'; - public const MAX_VISITS = 'maxVisits'; + public const PATH_PREFIX = 'pathPrefix'; public const FIND_IF_EXISTS = 'findIfExists'; public const DOMAIN = 'domain'; - public const SHORT_CODE_LENGTH = 'shortCodeLength'; + + // Fields for creation and edition public const LONG_URL = 'longUrl'; public const DEVICE_LONG_URLS = 'deviceLongUrls'; - public const API_KEY = 'apiKey'; - public const TAGS = 'tags'; + public const VALID_SINCE = 'validSince'; + public const VALID_UNTIL = 'validUntil'; + public const MAX_VISITS = 'maxVisits'; public const TITLE = 'title'; + public const TAGS = 'tags'; public const CRAWLABLE = 'crawlable'; public const FORWARD_QUERY = 'forwardQuery'; + public const API_KEY = 'apiKey'; - private function __construct(array $data, bool $requireLongUrl, UrlShortenerOptions $options) + public static function forCreation(array $data, UrlShortenerOptions $options): self { - $this->initialize($requireLongUrl, $options); - $this->setData($data); + $instance = new self(); + $instance->initializeForCreation($options); + $instance->setData($data); + + return $instance; } - public static function withRequiredLongUrl(array $data, UrlShortenerOptions $options): self + public static function forEdition(array $data): self { - return new self($data, true, $options); + $instance = new self(); + $instance->initializeForEdition(); + $instance->setData($data); + + return $instance; } - public static function withNonRequiredLongUrl(array $data): self + private function initializeForCreation(UrlShortenerOptions $options): void { - return new self($data, false, new UrlShortenerOptions()); - } - - private function initialize(bool $requireLongUrl, UrlShortenerOptions $options): void - { - $longUrlInput = $this->createInput(self::LONG_URL, $requireLongUrl); - $longUrlInput->getValidatorChain()->merge($this->longUrlValidators()); - $this->add($longUrlInput); - - $deviceLongUrlsInput = $this->createInput(self::DEVICE_LONG_URLS, false); - $deviceLongUrlsInput->getValidatorChain()->attach( - new DeviceLongUrlsValidator($this->longUrlValidators(allowNull: ! $requireLongUrl)), - ); - $this->add($deviceLongUrlsInput); - - $validSince = $this->createInput(self::VALID_SINCE, false); - $validSince->getValidatorChain()->attach(new Validator\Date(['format' => DateTimeInterface::ATOM])); - $this->add($validSince); - - $validUntil = $this->createInput(self::VALID_UNTIL, false); - $validUntil->getValidatorChain()->attach(new Validator\Date(['format' => DateTimeInterface::ATOM])); - $this->add($validUntil); - // The only way to enforce the NotEmpty validator to be evaluated when the key is present with an empty value - // is with setContinueIfEmpty - $customSlug = $this->createInput(self::CUSTOM_SLUG, false)->setContinueIfEmpty(true); + // is with setContinueIfEmpty(true) + $customSlug = $this->createInput(self::CUSTOM_SLUG, required: false)->setContinueIfEmpty(true); $customSlug->getFilterChain()->attach(new CustomSlugFilter($options)); $customSlug->getValidatorChain() ->attach(new Validator\NotEmpty([ @@ -90,32 +74,62 @@ class ShortUrlInputFilter extends InputFilter ->attach(CustomSlugValidator::forUrlShortenerOptions($options)); $this->add($customSlug); - $this->add($this->createNumericInput(self::MAX_VISITS, false)); - $this->add($this->createNumericInput(self::SHORT_CODE_LENGTH, false, MIN_SHORT_CODES_LENGTH)); + // The path prefix is subject to the same filtering and validation logic as the custom slug, which takes into + // consideration if multi-segment slugs are enabled or not. + // The only difference is that empty values are allowed here. + $pathPrefix = $this->createInput(self::PATH_PREFIX, required: false); + $pathPrefix->getFilterChain()->attach(new CustomSlugFilter($options)); + $pathPrefix->getValidatorChain()->attach(CustomSlugValidator::forUrlShortenerOptions($options)); + $this->add($pathPrefix); - $this->add($this->createBooleanInput(self::FIND_IF_EXISTS, false)); + $this->add($this->createNumericInput(self::SHORT_CODE_LENGTH, required: false, min: MIN_SHORT_CODES_LENGTH)); + $this->add($this->createBooleanInput(self::FIND_IF_EXISTS, required: false)); - // This cannot be defined as a boolean inputs, because they can actually have 3 values: true, false and null. - // Defining them as boolean will make null fall back to false, which is not the desired behavior. - $this->add($this->createInput(self::FORWARD_QUERY, false)); - - $domain = $this->createInput(self::DOMAIN, false); + $domain = $this->createInput(self::DOMAIN, required: false); $domain->getValidatorChain()->attach(new Validation\HostAndPortValidator()); $this->add($domain); - $apiKeyInput = $this->createInput(self::API_KEY, false); - $apiKeyInput->getValidatorChain()->attach(new Validator\IsInstanceOf(['className' => ApiKey::class])); - $this->add($apiKeyInput); + $this->initializeForEdition(requireLongUrl: true); + } - $this->add($this->createTagsInput(self::TAGS, false)); + private function initializeForEdition(bool $requireLongUrl = false): void + { + $longUrlInput = $this->createInput(self::LONG_URL, $requireLongUrl); + $longUrlInput->getValidatorChain()->merge($this->longUrlValidators()); + $this->add($longUrlInput); - $title = $this->createInput(self::TITLE, false); + $deviceLongUrlsInput = $this->createInput(self::DEVICE_LONG_URLS, required: false); + $deviceLongUrlsInput->getValidatorChain()->attach( + new DeviceLongUrlsValidator($this->longUrlValidators(allowNull: ! $requireLongUrl)), + ); + $this->add($deviceLongUrlsInput); + + $validSince = $this->createInput(self::VALID_SINCE, required: false); + $validSince->getValidatorChain()->attach(new Validator\Date(['format' => DateTimeInterface::ATOM])); + $this->add($validSince); + + $validUntil = $this->createInput(self::VALID_UNTIL, required: false); + $validUntil->getValidatorChain()->attach(new Validator\Date(['format' => DateTimeInterface::ATOM])); + $this->add($validUntil); + + $this->add($this->createNumericInput(self::MAX_VISITS, required: false)); + + $title = $this->createInput(self::TITLE, required: false); $title->getFilterChain()->attach(new Filter\Callback( static fn (?string $value) => $value === null ? $value : substr($value, 0, 512), )); $this->add($title); - $this->add($this->createBooleanInput(self::CRAWLABLE, false)); + $this->add($this->createTagsInput(self::TAGS, required: false)); + $this->add($this->createBooleanInput(self::CRAWLABLE, required: false)); + + // This cannot be defined as a boolean inputs, because it can actually have 3 values: true, false and null. + // Defining them as boolean will make null fall back to false, which is not the desired behavior. + $this->add($this->createInput(self::FORWARD_QUERY, required: false)); + + $apiKeyInput = $this->createInput(self::API_KEY, required: false); + $apiKeyInput->getValidatorChain()->attach(new Validator\IsInstanceOf(['className' => ApiKey::class])); + $this->add($apiKeyInput); } private function longUrlValidators(bool $allowNull = false): Validator\ValidatorChain From 467dbdd183df9920387b581041d77ad3b058b6fe Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 21 Feb 2024 17:57:45 +0100 Subject: [PATCH 2/6] Update to latest shlink-common --- composer.json | 2 +- config/autoload/dependencies.global.php | 3 ++ .../Validation/DomainRedirectsInputFilter.php | 15 ++++---- .../Model/Validation/ShortUrlInputFilter.php | 37 +++++++++---------- .../Validation/ShortUrlsParamsInputFilter.php | 24 ++++++------ 5 files changed, 40 insertions(+), 41 deletions(-) diff --git a/composer.json b/composer.json index 15e7ab89..76a98041 100644 --- a/composer.json +++ b/composer.json @@ -42,7 +42,7 @@ "pugx/shortid-php": "^1.1", "ramsey/uuid": "^4.7", "shlinkio/doctrine-specification": "^2.1.1", - "shlinkio/shlink-common": "dev-main#b9a6bd5 as 6.0", + "shlinkio/shlink-common": "dev-main#3e5bf59 as 6.0", "shlinkio/shlink-config": "dev-main#a43b380 as 3.0", "shlinkio/shlink-event-dispatcher": "dev-main#aa9023c as 4.0", "shlinkio/shlink-importer": "dev-main#65a9a30 as 5.3", diff --git a/config/autoload/dependencies.global.php b/config/autoload/dependencies.global.php index a0014ef6..469171ca 100644 --- a/config/autoload/dependencies.global.php +++ b/config/autoload/dependencies.global.php @@ -4,6 +4,7 @@ declare(strict_types=1); use GuzzleHttp\Client; use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory; +use Laminas\ServiceManager\Factory\InvokableFactory; use Mezzio\Application; use Mezzio\Container; use Psr\Http\Client\ClientInterface; @@ -12,12 +13,14 @@ use Psr\Http\Message\StreamFactoryInterface; use Psr\Http\Message\UploadedFileFactoryInterface; use Spiral\RoadRunner\Http\PSR7Worker; use Spiral\RoadRunner\WorkerInterface; +use Symfony\Component\Filesystem\Filesystem; return [ 'dependencies' => [ 'factories' => [ PSR7Worker::class => ConfigAbstractFactory::class, + Filesystem::class => InvokableFactory::class, ], 'delegators' => [ diff --git a/module/Core/src/Domain/Validation/DomainRedirectsInputFilter.php b/module/Core/src/Domain/Validation/DomainRedirectsInputFilter.php index de627c1c..48035c6c 100644 --- a/module/Core/src/Domain/Validation/DomainRedirectsInputFilter.php +++ b/module/Core/src/Domain/Validation/DomainRedirectsInputFilter.php @@ -5,12 +5,11 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Domain\Validation; use Laminas\InputFilter\InputFilter; -use Shlinkio\Shlink\Common\Validation; +use Shlinkio\Shlink\Common\Validation\HostAndPortValidator; +use Shlinkio\Shlink\Common\Validation\InputFactory; class DomainRedirectsInputFilter extends InputFilter { - use Validation\InputFactoryTrait; - public const DOMAIN = 'domain'; public const BASE_URL_REDIRECT = 'baseUrlRedirect'; public const REGULAR_404_REDIRECT = 'regular404Redirect'; @@ -32,12 +31,12 @@ class DomainRedirectsInputFilter extends InputFilter private function initializeInputs(): void { - $domain = $this->createInput(self::DOMAIN); - $domain->getValidatorChain()->attach(new Validation\HostAndPortValidator()); + $domain = InputFactory::basic(self::DOMAIN, required: true); + $domain->getValidatorChain()->attach(new HostAndPortValidator()); $this->add($domain); - $this->add($this->createInput(self::BASE_URL_REDIRECT, false)); - $this->add($this->createInput(self::REGULAR_404_REDIRECT, false)); - $this->add($this->createInput(self::INVALID_SHORT_URL_REDIRECT, false)); + $this->add(InputFactory::basic(self::BASE_URL_REDIRECT)); + $this->add(InputFactory::basic(self::REGULAR_404_REDIRECT)); + $this->add(InputFactory::basic(self::INVALID_SHORT_URL_REDIRECT)); } } diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php index ad3a6df7..287ea746 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php @@ -8,7 +8,8 @@ use DateTimeInterface; use Laminas\Filter; use Laminas\InputFilter\InputFilter; use Laminas\Validator; -use Shlinkio\Shlink\Common\Validation; +use Shlinkio\Shlink\Common\Validation\HostAndPortValidator; +use Shlinkio\Shlink\Common\Validation\InputFactory; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -21,8 +22,6 @@ use const Shlinkio\Shlink\MIN_SHORT_CODES_LENGTH; class ShortUrlInputFilter extends InputFilter { - use Validation\InputFactoryTrait; - // Fields for creation only public const SHORT_CODE_LENGTH = 'shortCodeLength'; public const CUSTOM_SLUG = 'customSlug'; @@ -64,7 +63,7 @@ class ShortUrlInputFilter extends InputFilter { // The only way to enforce the NotEmpty validator to be evaluated when the key is present with an empty value // is with setContinueIfEmpty(true) - $customSlug = $this->createInput(self::CUSTOM_SLUG, required: false)->setContinueIfEmpty(true); + $customSlug = InputFactory::basic(self::CUSTOM_SLUG)->setContinueIfEmpty(true); $customSlug->getFilterChain()->attach(new CustomSlugFilter($options)); $customSlug->getValidatorChain() ->attach(new Validator\NotEmpty([ @@ -77,16 +76,16 @@ class ShortUrlInputFilter extends InputFilter // The path prefix is subject to the same filtering and validation logic as the custom slug, which takes into // consideration if multi-segment slugs are enabled or not. // The only difference is that empty values are allowed here. - $pathPrefix = $this->createInput(self::PATH_PREFIX, required: false); + $pathPrefix = InputFactory::basic(self::PATH_PREFIX); $pathPrefix->getFilterChain()->attach(new CustomSlugFilter($options)); $pathPrefix->getValidatorChain()->attach(CustomSlugValidator::forUrlShortenerOptions($options)); $this->add($pathPrefix); - $this->add($this->createNumericInput(self::SHORT_CODE_LENGTH, required: false, min: MIN_SHORT_CODES_LENGTH)); - $this->add($this->createBooleanInput(self::FIND_IF_EXISTS, required: false)); + $this->add(InputFactory::numeric(self::SHORT_CODE_LENGTH, min: MIN_SHORT_CODES_LENGTH)); + $this->add(InputFactory::boolean(self::FIND_IF_EXISTS)); - $domain = $this->createInput(self::DOMAIN, required: false); - $domain->getValidatorChain()->attach(new Validation\HostAndPortValidator()); + $domain = InputFactory::basic(self::DOMAIN); + $domain->getValidatorChain()->attach(new HostAndPortValidator()); $this->add($domain); $this->initializeForEdition(requireLongUrl: true); @@ -94,40 +93,40 @@ class ShortUrlInputFilter extends InputFilter private function initializeForEdition(bool $requireLongUrl = false): void { - $longUrlInput = $this->createInput(self::LONG_URL, $requireLongUrl); + $longUrlInput = InputFactory::basic(self::LONG_URL, required: $requireLongUrl); $longUrlInput->getValidatorChain()->merge($this->longUrlValidators()); $this->add($longUrlInput); - $deviceLongUrlsInput = $this->createInput(self::DEVICE_LONG_URLS, required: false); + $deviceLongUrlsInput = InputFactory::basic(self::DEVICE_LONG_URLS); $deviceLongUrlsInput->getValidatorChain()->attach( new DeviceLongUrlsValidator($this->longUrlValidators(allowNull: ! $requireLongUrl)), ); $this->add($deviceLongUrlsInput); - $validSince = $this->createInput(self::VALID_SINCE, required: false); + $validSince = InputFactory::basic(self::VALID_SINCE); $validSince->getValidatorChain()->attach(new Validator\Date(['format' => DateTimeInterface::ATOM])); $this->add($validSince); - $validUntil = $this->createInput(self::VALID_UNTIL, required: false); + $validUntil = InputFactory::basic(self::VALID_UNTIL); $validUntil->getValidatorChain()->attach(new Validator\Date(['format' => DateTimeInterface::ATOM])); $this->add($validUntil); - $this->add($this->createNumericInput(self::MAX_VISITS, required: false)); + $this->add(InputFactory::numeric(self::MAX_VISITS)); - $title = $this->createInput(self::TITLE, required: false); + $title = InputFactory::basic(self::TITLE); $title->getFilterChain()->attach(new Filter\Callback( static fn (?string $value) => $value === null ? $value : substr($value, 0, 512), )); $this->add($title); - $this->add($this->createTagsInput(self::TAGS, required: false)); - $this->add($this->createBooleanInput(self::CRAWLABLE, required: false)); + $this->add(InputFactory::tags(self::TAGS)); + $this->add(InputFactory::boolean(self::CRAWLABLE)); // This cannot be defined as a boolean inputs, because it can actually have 3 values: true, false and null. // Defining them as boolean will make null fall back to false, which is not the desired behavior. - $this->add($this->createInput(self::FORWARD_QUERY, required: false)); + $this->add(InputFactory::basic(self::FORWARD_QUERY)); - $apiKeyInput = $this->createInput(self::API_KEY, required: false); + $apiKeyInput = InputFactory::basic(self::API_KEY); $apiKeyInput->getValidatorChain()->attach(new Validator\IsInstanceOf(['className' => ApiKey::class])); $this->add($apiKeyInput); } diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php index d7cda41e..f4f7c338 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php @@ -7,7 +7,7 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Model\Validation; use Laminas\InputFilter\InputFilter; use Laminas\Validator\InArray; use Shlinkio\Shlink\Common\Paginator\Paginator; -use Shlinkio\Shlink\Common\Validation; +use Shlinkio\Shlink\Common\Validation\InputFactory; use Shlinkio\Shlink\Core\ShortUrl\Model\OrderableField; use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; @@ -15,8 +15,6 @@ use function Shlinkio\Shlink\Core\enumValues; class ShortUrlsParamsInputFilter extends InputFilter { - use Validation\InputFactoryTrait; - public const PAGE = 'page'; public const SEARCH_TERM = 'searchTerm'; public const TAGS = 'tags'; @@ -36,26 +34,26 @@ class ShortUrlsParamsInputFilter extends InputFilter private function initialize(): void { - $this->add($this->createDateInput(self::START_DATE, false)); - $this->add($this->createDateInput(self::END_DATE, false)); + $this->add(InputFactory::date(self::START_DATE)); + $this->add(InputFactory::date(self::END_DATE)); - $this->add($this->createInput(self::SEARCH_TERM, false)); + $this->add(InputFactory::basic(self::SEARCH_TERM)); - $this->add($this->createNumericInput(self::PAGE, false)); - $this->add($this->createNumericInput(self::ITEMS_PER_PAGE, false, Paginator::ALL_ITEMS)); + $this->add(InputFactory::numeric(self::PAGE)); + $this->add(InputFactory::numeric(self::ITEMS_PER_PAGE, Paginator::ALL_ITEMS)); - $this->add($this->createTagsInput(self::TAGS, false)); + $this->add(InputFactory::tags(self::TAGS)); - $tagsMode = $this->createInput(self::TAGS_MODE, false); + $tagsMode = InputFactory::basic(self::TAGS_MODE); $tagsMode->getValidatorChain()->attach(new InArray([ 'haystack' => enumValues(TagsMode::class), 'strict' => InArray::COMPARE_STRICT, ])); $this->add($tagsMode); - $this->add($this->createOrderByInput(self::ORDER_BY, enumValues(OrderableField::class))); + $this->add(InputFactory::orderBy(self::ORDER_BY, enumValues(OrderableField::class))); - $this->add($this->createBooleanInput(self::EXCLUDE_MAX_VISITS_REACHED, false)); - $this->add($this->createBooleanInput(self::EXCLUDE_PAST_VALID_UNTIL, false)); + $this->add(InputFactory::boolean(self::EXCLUDE_MAX_VISITS_REACHED)); + $this->add(InputFactory::boolean(self::EXCLUDE_PAST_VALID_UNTIL)); } } From f30c74b9877dfd2a1aad44a347773496f85631b5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 21 Feb 2024 18:06:06 +0100 Subject: [PATCH 3/6] Prepend path prefix to generated short code or custom slug --- docs/swagger/paths/v1_short-urls.json | 5 +++++ module/Core/src/ShortUrl/Entity/ShortUrl.php | 8 +++++--- module/Core/src/ShortUrl/Model/ShortUrlCreation.php | 2 ++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index 08e08b67..c9bbe68a 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -303,6 +303,10 @@ "description": "A unique custom slug to be used instead of the generated short code", "type": "string" }, + "pathPrefix": { + "description": "A prefix that will be prepended to provided custom slug or auto-generated short code", + "type": "string" + }, "findIfExists": { "description": "Will force existing matching URL to be returned if found, instead of creating a new one", "type": "boolean" @@ -382,6 +386,7 @@ "validSince", "validUntil", "customSlug", + "pathPrefix", "maxVisits", "findIfExists", "domain" diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index ee2c5920..411e7bb1 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -33,6 +33,7 @@ use function Shlinkio\Shlink\Core\enumValues; use function Shlinkio\Shlink\Core\generateRandomShortCode; use function Shlinkio\Shlink\Core\normalizeDate; use function Shlinkio\Shlink\Core\normalizeOptionalDate; +use function sprintf; class ShortUrl extends AbstractEntity { @@ -100,9 +101,10 @@ class ShortUrl extends AbstractEntity $instance->maxVisits = $creation->maxVisits; $instance->customSlugWasProvided = $creation->hasCustomSlug(); $instance->shortCodeLength = $creation->shortCodeLength; - $instance->shortCode = $creation->customSlug ?? generateRandomShortCode( - $instance->shortCodeLength, - $creation->shortUrlMode, + $instance->shortCode = sprintf( + '%s%s', + $creation->pathPrefix ?? '', + $creation->customSlug ?? generateRandomShortCode($instance->shortCodeLength, $creation->shortUrlMode), ); $instance->domain = $relationResolver->resolveDomain($creation->domain); $instance->authorApiKey = $creation->apiKey; diff --git a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php index 976973b2..4d22a8be 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php @@ -66,6 +66,7 @@ final readonly class ShortUrlCreation implements TitleResolutionModelInterface validSince: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)), validUntil: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_UNTIL)), customSlug: $inputFilter->getValue(ShortUrlInputFilter::CUSTOM_SLUG), + pathPrefix: $inputFilter->getValue(ShortUrlInputFilter::PATH_PREFIX), maxVisits: getOptionalIntFromInputFilter($inputFilter, ShortUrlInputFilter::MAX_VISITS), findIfExists: $inputFilter->getValue(ShortUrlInputFilter::FIND_IF_EXISTS) ?? false, domain: getNonEmptyOptionalValueFromInputFilter($inputFilter, ShortUrlInputFilter::DOMAIN), @@ -90,6 +91,7 @@ final readonly class ShortUrlCreation implements TitleResolutionModelInterface validSince: $this->validSince, validUntil: $this->validUntil, customSlug: $this->customSlug, + pathPrefix: $this->pathPrefix, maxVisits: $this->maxVisits, findIfExists: $this->findIfExists, domain: $this->domain, From ff963a9df48c0acecc13a9a0a2d7c22ba9ac7363 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 21 Feb 2024 19:14:30 +0100 Subject: [PATCH 4/6] Add API test for short URL path prefix --- .../Rest/test-api/Action/CreateShortUrlTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/module/Rest/test-api/Action/CreateShortUrlTest.php b/module/Rest/test-api/Action/CreateShortUrlTest.php index f612f628..96dc5e7b 100644 --- a/module/Rest/test-api/Action/CreateShortUrlTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlTest.php @@ -8,6 +8,7 @@ use Cake\Chronos\Chronos; use GuzzleHttp\RequestOptions; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\TestWith; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; use function array_map; @@ -339,6 +340,21 @@ class CreateShortUrlTest extends ApiTestCase self::assertNull($payload['title']); } + #[Test] + #[TestWith([null])] + #[TestWith(['my-custom-slug'])] + public function prefixCanBeSet(?string $customSlug): void + { + [$statusCode, $payload] = $this->createShortUrl([ + 'longUrl' => 'https://github.com/shlinkio/shlink/issues/1557', + 'pathPrefix' => 'foo/b ar-baz', + 'customSlug' => $customSlug, + ]); + + self::assertEquals(self::STATUS_OK, $statusCode); + self::assertStringStartsWith('foo-b--ar-baz', $payload['shortCode']); + } + /** * @return array{int, array} */ From f08951a9b99a735f53af35806a2c678157eb1e5f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 21 Feb 2024 19:24:30 +0100 Subject: [PATCH 5/6] Add unit test for short URL path prefix --- .../test/ShortUrl/Entity/ShortUrlTest.php | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php index 0a898399..eb89df5c 100644 --- a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php +++ b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php @@ -7,6 +7,7 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl\Entity; use Cake\Chronos\Chronos; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException; use Shlinkio\Shlink\Core\Model\DeviceType; @@ -91,6 +92,27 @@ class ShortUrlTest extends TestCase yield from array_map(fn (int $value) => [$value, $value], range(4, 10)); } + #[Test] + #[TestWith([null, '', 5])] + #[TestWith(['foo bar/', 'foo-bar-', 13])] + public function shortCodesHaveExpectedPrefix( + ?string $pathPrefix, + string $expectedPrefix, + int $expectedShortCodeLength, + ): void { + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ + 'longUrl' => 'https://longUrl', + ShortUrlInputFilter::SHORT_CODE_LENGTH => 5, + ShortUrlInputFilter::PATH_PREFIX => $pathPrefix, + ])); + $shortCode = $shortUrl->getShortCode(); + + if (strlen($expectedPrefix) > 0) { + self::assertStringStartsWith($expectedPrefix, $shortCode); + } + self::assertEquals($expectedShortCodeLength, strlen($shortCode)); + } + #[Test] public function deviceLongUrlsAreUpdated(): void { From 7673232793b6899acda7602095416f72dd105551 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 21 Feb 2024 19:38:11 +0100 Subject: [PATCH 6/6] Add --path-prefix to short URL creation --- CHANGELOG.md | 5 +++++ .../CLI/src/Command/ShortUrl/CreateShortUrlCommand.php | 10 ++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39797631..9f51f641 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this This is supported both by the `GET /visits/orphan` API endpoint via `type=...` query param, and by the `visit:orphan` CLI command, via `--type` flag. * [#1904](https://github.com/shlinkio/shlink/issues/1904) Allow to customize QR codes foreground color, background color and logo. +* [#1884](https://github.com/shlinkio/shlink/issues/1884) Allow a path prefix to be provided during short URL creation. + + This can be useful to let Shlink generate partially random URLs, but with a known prefix. + + Path prefixes are validated and filtered taking multi-segment slugs into consideration, which means slashes are replaced with dashes as long as multi-segment slugs are disabled. ### Changed * [#1935](https://github.com/shlinkio/shlink/issues/1935) Replace dependency on abandoned `php-middleware/request-id` with userland simple middleware. diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index 118ad201..4b6a088d 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -70,6 +70,12 @@ class CreateShortUrlCommand extends Command InputOption::VALUE_REQUIRED, 'If provided, this slug will be used instead of generating a short code', ) + ->addOption( + 'path-prefix', + 'p', + InputOption::VALUE_REQUIRED, + 'Prefix to prepend before the generated short code or provided custom slug', + ) ->addOption( 'max-visits', 'm', @@ -138,7 +144,6 @@ class CreateShortUrlCommand extends Command $explodeWithComma = static fn (string $tag) => explode(',', $tag); $tags = array_unique(flatten(array_map($explodeWithComma, $input->getOption('tags')))); - $customSlug = $input->getOption('custom-slug'); $maxVisits = $input->getOption('max-visits'); $shortCodeLength = $input->getOption('short-code-length') ?? $this->options->defaultShortCodesLength; @@ -147,8 +152,9 @@ class CreateShortUrlCommand extends Command ShortUrlInputFilter::LONG_URL => $longUrl, ShortUrlInputFilter::VALID_SINCE => $input->getOption('valid-since'), ShortUrlInputFilter::VALID_UNTIL => $input->getOption('valid-until'), - ShortUrlInputFilter::CUSTOM_SLUG => $customSlug, ShortUrlInputFilter::MAX_VISITS => $maxVisits !== null ? (int) $maxVisits : null, + ShortUrlInputFilter::CUSTOM_SLUG => $input->getOption('custom-slug'), + ShortUrlInputFilter::PATH_PREFIX => $input->getOption('path-prefix'), ShortUrlInputFilter::FIND_IF_EXISTS => $input->getOption('find-if-exists'), ShortUrlInputFilter::DOMAIN => $input->getOption('domain'), ShortUrlInputFilter::SHORT_CODE_LENGTH => $shortCodeLength,