From 3e26f1113d5ca301bc553fd79a0dacb70b6e8b77 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 14 Jan 2023 16:50:42 +0100 Subject: [PATCH] Extract device long URL validation to its own validation class --- .../src/ShortUrl/Entity/DeviceLongUrl.php | 11 +++- .../Helper/ShortUrlTitleResolutionHelper.php | 9 ++- ...ShortUrlTitleResolutionHelperInterface.php | 3 + .../Helper/TitleResolutionModelInterface.php | 2 +- .../src/ShortUrl/Model/ShortUrlCreation.php | 2 +- .../src/ShortUrl/Model/ShortUrlEdition.php | 2 +- .../Validation/DeviceLongUrlsValidator.php | 57 +++++++++++++++++++ .../Model/Validation/ShortUrlInputFilter.php | 34 ++--------- .../PersistenceShortUrlRelationResolver.php | 2 +- .../ShortUrlRelationResolverInterface.php | 2 +- .../SimpleShortUrlRelationResolver.php | 2 +- module/Core/src/ShortUrl/ShortUrlService.php | 1 - module/Core/src/ShortUrl/UrlShortener.php | 13 ++--- .../src/ShortUrl/UrlShortenerInterface.php | 2 +- module/Rest/src/Entity/ApiKey.php | 2 +- 15 files changed, 96 insertions(+), 48 deletions(-) create mode 100644 module/Core/src/ShortUrl/Model/Validation/DeviceLongUrlsValidator.php diff --git a/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php b/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php index b1dc1086..b3db666d 100644 --- a/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php +++ b/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php @@ -6,16 +6,23 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Entity; use Shlinkio\Shlink\Common\Entity\AbstractEntity; use Shlinkio\Shlink\Core\Model\DeviceType; +use Shlinkio\Shlink\Core\ShortUrl\Model\DeviceLongUrlPair; class DeviceLongUrl extends AbstractEntity { - public function __construct( - public readonly ShortUrl $shortUrl, + private ShortUrl $shortUrl; // @phpstan-ignore-line + + private function __construct( public readonly DeviceType $deviceType, private string $longUrl, ) { } + public static function fromPair(DeviceLongUrlPair $pair): self + { + return new self($pair->deviceType, $pair->longUrl); + } + public function longUrl(): string { return $this->longUrl; diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php index 00eecc61..a4920cdd 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php @@ -4,14 +4,21 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Helper; +use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Util\UrlValidatorInterface; class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionHelperInterface { - public function __construct(private UrlValidatorInterface $urlValidator) + public function __construct(private readonly UrlValidatorInterface $urlValidator) { } + /** + * @template T of TitleResolutionModelInterface + * @param T $data + * @return T + * @throws InvalidUrlException + */ public function processTitleAndValidateUrl(TitleResolutionModelInterface $data): TitleResolutionModelInterface { if ($data->hasTitle()) { diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelperInterface.php b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelperInterface.php index 50022746..6989140a 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelperInterface.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelperInterface.php @@ -9,6 +9,9 @@ use Shlinkio\Shlink\Core\Exception\InvalidUrlException; interface ShortUrlTitleResolutionHelperInterface { /** + * @template T of TitleResolutionModelInterface + * @param T $data + * @return T * @throws InvalidUrlException */ public function processTitleAndValidateUrl(TitleResolutionModelInterface $data): TitleResolutionModelInterface; diff --git a/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php b/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php index 8af28706..1c834331 100644 --- a/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php +++ b/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php @@ -12,5 +12,5 @@ interface TitleResolutionModelInterface public function doValidateUrl(): bool; - public function withResolvedTitle(string $title): self; + public function withResolvedTitle(string $title): static; } diff --git a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php index d63482ec..f2e156f4 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php @@ -85,7 +85,7 @@ final class ShortUrlCreation implements TitleResolutionModelInterface ); } - public function withResolvedTitle(string $title): self + public function withResolvedTitle(string $title): static { return new self( longUrl: $this->longUrl, diff --git a/module/Core/src/ShortUrl/Model/ShortUrlEdition.php b/module/Core/src/ShortUrl/Model/ShortUrlEdition.php index 32451d2e..fb0f9bb0 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlEdition.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlEdition.php @@ -76,7 +76,7 @@ final class ShortUrlEdition implements TitleResolutionModelInterface ); } - public function withResolvedTitle(string $title): self + public function withResolvedTitle(string $title): static { return new self( longUrlPropWasProvided: $this->longUrlPropWasProvided, diff --git a/module/Core/src/ShortUrl/Model/Validation/DeviceLongUrlsValidator.php b/module/Core/src/ShortUrl/Model/Validation/DeviceLongUrlsValidator.php new file mode 100644 index 00000000..1e9d9824 --- /dev/null +++ b/module/Core/src/ShortUrl/Model/Validation/DeviceLongUrlsValidator.php @@ -0,0 +1,57 @@ + 'Provided value is not an array.', + self::INVALID_DEVICE => 'You have provided at least one invalid device identifier.', + self::INVALID_LONG_URL => 'At least one of the long URLs are invalid.', + ]; + + public function __construct(private readonly ValidatorChain $longUrlValidators) + { + parent::__construct(); + } + + public function isValid(mixed $value): bool + { + if (! is_array($value)) { + $this->error(self::NOT_ARRAY); + return false; + } + + $validValues = enumValues(DeviceType::class); + $keys = array_keys($value); + if (! every($keys, static fn ($key) => contains($validValues, $key))) { + $this->error(self::INVALID_DEVICE); + return false; + } + + $longUrls = array_values($value); + $result = every($longUrls, $this->longUrlValidators->isValid(...)); + if (! $result) { + $this->error(self::INVALID_LONG_URL); + } + + return $result; + } +} diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php index 72708250..7b01841b 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php @@ -10,16 +10,9 @@ use Laminas\InputFilter\InputFilter; use Laminas\Validator; use Shlinkio\Shlink\Common\Validation; use Shlinkio\Shlink\Core\Config\EnvVars; -use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Rest\Entity\ApiKey; -use function array_keys; -use function array_values; -use function Functional\contains; -use function Functional\every; -use function is_array; use function is_string; -use function Shlinkio\Shlink\Core\enumValues; use function str_replace; use function substr; use function trim; @@ -64,37 +57,20 @@ class ShortUrlInputFilter extends InputFilter private function initialize(bool $requireLongUrl, bool $multiSegmentEnabled): void { - $notEmptyValidator = new Validator\NotEmpty([ + $longUrlInput = $this->createInput(self::LONG_URL, $requireLongUrl); + $longUrlInput->getValidatorChain()->attach(new Validator\NotEmpty([ Validator\NotEmpty::OBJECT, Validator\NotEmpty::SPACE, Validator\NotEmpty::NULL, Validator\NotEmpty::EMPTY_ARRAY, Validator\NotEmpty::BOOLEAN, Validator\NotEmpty::STRING, - ]); - - $longUrlInput = $this->createInput(self::LONG_URL, $requireLongUrl); - $longUrlInput->getValidatorChain()->attach($notEmptyValidator); + ])); $this->add($longUrlInput); $deviceLongUrlsInput = $this->createInput(self::DEVICE_LONG_URLS, false); - $deviceLongUrlsInput->getValidatorChain()->attach( // TODO Extract callback to own validator - new Validator\Callback(function (mixed $value) use ($notEmptyValidator): bool { - if (! is_array($value)) { - // TODO Set proper error: Not array - return false; - } - - $validValues = enumValues(DeviceType::class); - $keys = array_keys($value); - if (! every($keys, static fn ($key) => contains($validValues, $key))) { - // TODO Set proper error: Provided invalid device type - return false; - } - - $longUrls = array_values($value); - return every($longUrls, $notEmptyValidator->isValid(...)); - }), + $deviceLongUrlsInput->getValidatorChain()->attach( + new DeviceLongUrlsValidator($longUrlInput->getValidatorChain()), ); $this->add($deviceLongUrlsInput); diff --git a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php index 971ef932..db6721d5 100644 --- a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php @@ -49,7 +49,7 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt /** * @param string[] $tags - * @return Collection|Tag[] + * @return Collection */ public function resolveTags(array $tags): Collections\Collection { diff --git a/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php b/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php index a71f2ccc..b5228214 100644 --- a/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php +++ b/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php @@ -14,7 +14,7 @@ interface ShortUrlRelationResolverInterface /** * @param string[] $tags - * @return Collection|Tag[] + * @return Collection */ public function resolveTags(array $tags): Collection; } diff --git a/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php index f25ff8a1..2048aba9 100644 --- a/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php @@ -20,7 +20,7 @@ class SimpleShortUrlRelationResolver implements ShortUrlRelationResolverInterfac /** * @param string[] $tags - * @return Collection|Tag[] + * @return Collection */ public function resolveTags(array $tags): Collections\Collection { diff --git a/module/Core/src/ShortUrl/ShortUrlService.php b/module/Core/src/ShortUrl/ShortUrlService.php index 163989d8..95561fc5 100644 --- a/module/Core/src/ShortUrl/ShortUrlService.php +++ b/module/Core/src/ShortUrl/ShortUrlService.php @@ -34,7 +34,6 @@ class ShortUrlService implements ShortUrlServiceInterface ?ApiKey $apiKey = null, ): ShortUrl { if ($shortUrlEdit->longUrlWasProvided()) { - /** @var ShortUrlEdition $shortUrlEdit */ $shortUrlEdit = $this->titleResolutionHelper->processTitleAndValidateUrl($shortUrlEdit); } diff --git a/module/Core/src/ShortUrl/UrlShortener.php b/module/Core/src/ShortUrl/UrlShortener.php index 4236189c..27e96262 100644 --- a/module/Core/src/ShortUrl/UrlShortener.php +++ b/module/Core/src/ShortUrl/UrlShortener.php @@ -31,22 +31,21 @@ class UrlShortener implements UrlShortenerInterface * @throws NonUniqueSlugException * @throws InvalidUrlException */ - public function shorten(ShortUrlCreation $meta): ShortUrl + public function shorten(ShortUrlCreation $creation): ShortUrl { // First, check if a short URL exists for all provided params - $existingShortUrl = $this->findExistingShortUrlIfExists($meta); + $existingShortUrl = $this->findExistingShortUrlIfExists($creation); if ($existingShortUrl !== null) { return $existingShortUrl; } - /** @var \Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation $meta */ - $meta = $this->titleResolutionHelper->processTitleAndValidateUrl($meta); + $creation = $this->titleResolutionHelper->processTitleAndValidateUrl($creation); /** @var ShortUrl $newShortUrl */ - $newShortUrl = $this->em->wrapInTransaction(function () use ($meta) { - $shortUrl = ShortUrl::create($meta, $this->relationResolver); + $newShortUrl = $this->em->wrapInTransaction(function () use ($creation): ShortUrl { + $shortUrl = ShortUrl::create($creation, $this->relationResolver); - $this->verifyShortCodeUniqueness($meta, $shortUrl); + $this->verifyShortCodeUniqueness($creation, $shortUrl); $this->em->persist($shortUrl); return $shortUrl; diff --git a/module/Core/src/ShortUrl/UrlShortenerInterface.php b/module/Core/src/ShortUrl/UrlShortenerInterface.php index c15b7ebf..d7ae45f0 100644 --- a/module/Core/src/ShortUrl/UrlShortenerInterface.php +++ b/module/Core/src/ShortUrl/UrlShortenerInterface.php @@ -15,5 +15,5 @@ interface UrlShortenerInterface * @throws NonUniqueSlugException * @throws InvalidUrlException */ - public function shorten(ShortUrlCreation $meta): ShortUrl; + public function shorten(ShortUrlCreation $creation): ShortUrl; } diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index beb9e0f9..297cdb45 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -21,7 +21,7 @@ class ApiKey extends AbstractEntity private string $key; private ?Chronos $expirationDate = null; private bool $enabled; - /** @var Collection|ApiKeyRole[] */ + /** @var Collection */ private Collection $roles; private ?string $name = null;