From 822652cac3daff23c6710efaa90f474ef0d7542a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 14 Jan 2023 15:44:12 +0100 Subject: [PATCH] Allow providing device long URLs during short URL edition --- module/Core/src/ShortUrl/Entity/ShortUrl.php | 19 +- .../src/ShortUrl/Model/DeviceLongUrlPair.php | 35 ++++ .../src/ShortUrl/Model/ShortUrlCreation.php | 43 +++-- .../src/ShortUrl/Model/ShortUrlEdition.php | 167 +++++++----------- .../Model/Validation/ShortUrlInputFilter.php | 2 +- .../test/ShortUrl/ShortUrlServiceTest.php | 8 +- 6 files changed, 140 insertions(+), 134 deletions(-) create mode 100644 module/Core/src/ShortUrl/Model/DeviceLongUrlPair.php diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index 6c49e1c3..51bd09ea 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -55,6 +55,9 @@ class ShortUrl extends AbstractEntity { } + /** + * @deprecated This should not be allowed + */ public static function createEmpty(): self { return self::create(ShortUrlCreation::createEmpty()); @@ -226,34 +229,34 @@ class ShortUrl extends AbstractEntity ?ShortUrlRelationResolverInterface $relationResolver = null, ): void { if ($shortUrlEdit->validSinceWasProvided()) { - $this->validSince = $shortUrlEdit->validSince(); + $this->validSince = $shortUrlEdit->validSince; } if ($shortUrlEdit->validUntilWasProvided()) { - $this->validUntil = $shortUrlEdit->validUntil(); + $this->validUntil = $shortUrlEdit->validUntil; } if ($shortUrlEdit->maxVisitsWasProvided()) { - $this->maxVisits = $shortUrlEdit->maxVisits(); + $this->maxVisits = $shortUrlEdit->maxVisits; } if ($shortUrlEdit->longUrlWasProvided()) { - $this->longUrl = $shortUrlEdit->longUrl() ?? $this->longUrl; + $this->longUrl = $shortUrlEdit->longUrl ?? $this->longUrl; } if ($shortUrlEdit->tagsWereProvided()) { $relationResolver = $relationResolver ?? new SimpleShortUrlRelationResolver(); - $this->tags = $relationResolver->resolveTags($shortUrlEdit->tags()); + $this->tags = $relationResolver->resolveTags($shortUrlEdit->tags); } if ($shortUrlEdit->crawlableWasProvided()) { - $this->crawlable = $shortUrlEdit->crawlable(); + $this->crawlable = $shortUrlEdit->crawlable; } if ( $this->title === null || $shortUrlEdit->titleWasProvided() || ($this->titleWasAutoResolved && $shortUrlEdit->titleWasAutoResolved()) ) { - $this->title = $shortUrlEdit->title(); + $this->title = $shortUrlEdit->title; $this->titleWasAutoResolved = $shortUrlEdit->titleWasAutoResolved(); } if ($shortUrlEdit->forwardQueryWasProvided()) { - $this->forwardQuery = $shortUrlEdit->forwardQuery(); + $this->forwardQuery = $shortUrlEdit->forwardQuery; } } diff --git a/module/Core/src/ShortUrl/Model/DeviceLongUrlPair.php b/module/Core/src/ShortUrl/Model/DeviceLongUrlPair.php new file mode 100644 index 00000000..6d0234ec --- /dev/null +++ b/module/Core/src/ShortUrl/Model/DeviceLongUrlPair.php @@ -0,0 +1,35 @@ + $map + * @return self[] + */ + public static function fromMapToList(array $map): array + { + return array_values(map( + $map, + fn (string $longUrl, string $deviceType) => self::fromRawTypeAndLongUrl($deviceType, $longUrl), + )); + } +} diff --git a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php index e2af5cf1..d63482ec 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php @@ -6,17 +6,14 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Model; use Cake\Chronos\Chronos; use Shlinkio\Shlink\Core\Exception\ValidationException; -use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\ShortUrl\Helper\TitleResolutionModelInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; use Shlinkio\Shlink\Rest\Entity\ApiKey; -use function Functional\map; use function Shlinkio\Shlink\Core\getNonEmptyOptionalValueFromInputFilter; use function Shlinkio\Shlink\Core\getOptionalBoolFromInputFilter; use function Shlinkio\Shlink\Core\getOptionalIntFromInputFilter; use function Shlinkio\Shlink\Core\normalizeOptionalDate; -use function trim; use const Shlinkio\Shlink\DEFAULT_SHORT_CODES_LENGTH; @@ -24,7 +21,7 @@ final class ShortUrlCreation implements TitleResolutionModelInterface { /** * @param string[] $tags - * @param array{DeviceType, string}[] $deviceLongUrls + * @param DeviceLongUrlPair[] $deviceLongUrls */ private function __construct( public readonly string $longUrl, @@ -46,6 +43,9 @@ final class ShortUrlCreation implements TitleResolutionModelInterface ) { } + /** + * @deprecated This should not be allowed + */ public static function createEmpty(): self { return new self(''); @@ -63,9 +63,8 @@ final class ShortUrlCreation implements TitleResolutionModelInterface return new self( longUrl: $inputFilter->getValue(ShortUrlInputFilter::LONG_URL), - deviceLongUrls: map( + deviceLongUrls: DeviceLongUrlPair::fromMapToList( $inputFilter->getValue(ShortUrlInputFilter::DEVICE_LONG_URLS) ?? [], - static fn (string $longUrl, string $deviceType) => [DeviceType::from($deviceType), trim($longUrl)], ), validSince: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)), validUntil: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_UNTIL)), @@ -89,22 +88,22 @@ final class ShortUrlCreation implements TitleResolutionModelInterface public function withResolvedTitle(string $title): self { return new self( - $this->longUrl, - $this->deviceLongUrls, - $this->validSince, - $this->validUntil, - $this->customSlug, - $this->maxVisits, - $this->findIfExists, - $this->domain, - $this->shortCodeLength, - $this->validateUrl, - $this->apiKey, - $this->tags, - $title, - true, - $this->crawlable, - $this->forwardQuery, + longUrl: $this->longUrl, + deviceLongUrls: $this->deviceLongUrls, + validSince: $this->validSince, + validUntil: $this->validUntil, + customSlug: $this->customSlug, + maxVisits: $this->maxVisits, + findIfExists: $this->findIfExists, + domain: $this->domain, + shortCodeLength: $this->shortCodeLength, + validateUrl: $this->validateUrl, + apiKey: $this->apiKey, + tags: $this->tags, + title: $title, + titleWasAutoResolved: true, + crawlable: $this->crawlable, + forwardQuery: $this->forwardQuery, ); } diff --git a/module/Core/src/ShortUrl/Model/ShortUrlEdition.php b/module/Core/src/ShortUrl/Model/ShortUrlEdition.php index fadc9b1e..32451d2e 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlEdition.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlEdition.php @@ -16,77 +16,93 @@ use function Shlinkio\Shlink\Core\normalizeOptionalDate; final class ShortUrlEdition implements TitleResolutionModelInterface { - private bool $longUrlPropWasProvided = false; - private ?string $longUrl = null; - private bool $validSincePropWasProvided = false; - private ?Chronos $validSince = null; - private bool $validUntilPropWasProvided = false; - private ?Chronos $validUntil = null; - private bool $maxVisitsPropWasProvided = false; - private ?int $maxVisits = null; - private bool $tagsPropWasProvided = false; - private array $tags = []; - private bool $titlePropWasProvided = false; - private ?string $title = null; - private bool $titleWasAutoResolved = false; - private bool $validateUrl = false; - private bool $crawlablePropWasProvided = false; - private bool $crawlable = false; - private bool $forwardQueryPropWasProvided = false; - private bool $forwardQuery = true; - - private function __construct() - { + /** + * @param string[] $tags + */ + private function __construct( + private readonly bool $longUrlPropWasProvided = false, + public readonly ?string $longUrl = null, + public readonly array $deviceLongUrls = [], + 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, + public readonly bool $validateUrl = false, + private readonly bool $crawlablePropWasProvided = false, + public readonly bool $crawlable = false, + private readonly bool $forwardQueryPropWasProvided = false, + public readonly bool $forwardQuery = true, + ) { } /** * @throws ValidationException */ public static function fromRawData(array $data): self - { - $instance = new self(); - $instance->validateAndInit($data); - return $instance; - } - - /** - * @throws ValidationException - */ - private function validateAndInit(array $data): void { $inputFilter = ShortUrlInputFilter::withNonRequiredLongUrl($data); if (! $inputFilter->isValid()) { throw ValidationException::fromInputFilter($inputFilter); } - $this->longUrlPropWasProvided = array_key_exists(ShortUrlInputFilter::LONG_URL, $data); - $this->validSincePropWasProvided = array_key_exists(ShortUrlInputFilter::VALID_SINCE, $data); - $this->validUntilPropWasProvided = array_key_exists(ShortUrlInputFilter::VALID_UNTIL, $data); - $this->maxVisitsPropWasProvided = array_key_exists(ShortUrlInputFilter::MAX_VISITS, $data); - $this->tagsPropWasProvided = array_key_exists(ShortUrlInputFilter::TAGS, $data); - $this->titlePropWasProvided = array_key_exists(ShortUrlInputFilter::TITLE, $data); - $this->crawlablePropWasProvided = array_key_exists(ShortUrlInputFilter::CRAWLABLE, $data); - $this->forwardQueryPropWasProvided = array_key_exists(ShortUrlInputFilter::FORWARD_QUERY, $data); - - $this->longUrl = $inputFilter->getValue(ShortUrlInputFilter::LONG_URL); - $this->validSince = normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)); - $this->validUntil = normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_UNTIL)); - $this->maxVisits = getOptionalIntFromInputFilter($inputFilter, ShortUrlInputFilter::MAX_VISITS); - $this->validateUrl = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::VALIDATE_URL) ?? false; - $this->tags = $inputFilter->getValue(ShortUrlInputFilter::TAGS); - $this->title = $inputFilter->getValue(ShortUrlInputFilter::TITLE); - $this->crawlable = $inputFilter->getValue(ShortUrlInputFilter::CRAWLABLE); - $this->forwardQuery = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::FORWARD_QUERY) ?? true; + return new self( + longUrlPropWasProvided: array_key_exists(ShortUrlInputFilter::LONG_URL, $data), + longUrl: $inputFilter->getValue(ShortUrlInputFilter::LONG_URL), + deviceLongUrls: DeviceLongUrlPair::fromMapToList( + $inputFilter->getValue(ShortUrlInputFilter::DEVICE_LONG_URLS) ?? [], + ), + validSincePropWasProvided: array_key_exists(ShortUrlInputFilter::VALID_SINCE, $data), + validSince: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)), + validUntilPropWasProvided: array_key_exists(ShortUrlInputFilter::VALID_UNTIL, $data), + validUntil: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_UNTIL)), + maxVisitsPropWasProvided: array_key_exists(ShortUrlInputFilter::MAX_VISITS, $data), + maxVisits: getOptionalIntFromInputFilter($inputFilter, ShortUrlInputFilter::MAX_VISITS), + tagsPropWasProvided: array_key_exists(ShortUrlInputFilter::TAGS, $data), + tags: $inputFilter->getValue(ShortUrlInputFilter::TAGS), + titlePropWasProvided: array_key_exists(ShortUrlInputFilter::TITLE, $data), + title: $inputFilter->getValue(ShortUrlInputFilter::TITLE), + validateUrl: getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::VALIDATE_URL) ?? false, + crawlablePropWasProvided: array_key_exists(ShortUrlInputFilter::CRAWLABLE, $data), + crawlable: $inputFilter->getValue(ShortUrlInputFilter::CRAWLABLE), + forwardQueryPropWasProvided: array_key_exists(ShortUrlInputFilter::FORWARD_QUERY, $data), + forwardQuery: getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::FORWARD_QUERY) ?? true, + ); } - public function longUrl(): ?string + public function withResolvedTitle(string $title): self { - return $this->longUrl; + return new self( + longUrlPropWasProvided: $this->longUrlPropWasProvided, + longUrl: $this->longUrl, + validSincePropWasProvided: $this->validSincePropWasProvided, + validSince: $this->validSince, + validUntilPropWasProvided: $this->validUntilPropWasProvided, + validUntil: $this->validUntil, + maxVisitsPropWasProvided: $this->maxVisitsPropWasProvided, + maxVisits: $this->maxVisits, + tagsPropWasProvided: $this->tagsPropWasProvided, + tags: $this->tags, + titlePropWasProvided: $this->titlePropWasProvided, + title: $title, + titleWasAutoResolved: true, + validateUrl: $this->validateUrl, + crawlablePropWasProvided: $this->crawlablePropWasProvided, + crawlable: $this->crawlable, + forwardQueryPropWasProvided: $this->forwardQueryPropWasProvided, + forwardQuery: $this->forwardQuery, + ); } public function getLongUrl(): string { - return $this->longUrl() ?? ''; + return $this->longUrl ?? ''; } public function longUrlWasProvided(): bool @@ -94,54 +110,26 @@ final class ShortUrlEdition implements TitleResolutionModelInterface return $this->longUrlPropWasProvided && $this->longUrl !== null; } - public function validSince(): ?Chronos - { - return $this->validSince; - } - public function validSinceWasProvided(): bool { return $this->validSincePropWasProvided; } - public function validUntil(): ?Chronos - { - return $this->validUntil; - } - public function validUntilWasProvided(): bool { return $this->validUntilPropWasProvided; } - public function maxVisits(): ?int - { - return $this->maxVisits; - } - public function maxVisitsWasProvided(): bool { return $this->maxVisitsPropWasProvided; } - /** - * @return string[] - */ - public function tags(): array - { - return $this->tags; - } - public function tagsWereProvided(): bool { return $this->tagsPropWasProvided; } - public function title(): ?string - { - return $this->title; - } - public function titleWasProvided(): bool { return $this->titlePropWasProvided; @@ -157,35 +145,16 @@ final class ShortUrlEdition implements TitleResolutionModelInterface return $this->titleWasAutoResolved; } - public function withResolvedTitle(string $title): self - { - $copy = clone $this; - $copy->title = $title; - $copy->titleWasAutoResolved = true; - - return $copy; - } - public function doValidateUrl(): bool { return $this->validateUrl; } - public function crawlable(): bool - { - return $this->crawlable; - } - public function crawlableWasProvided(): bool { return $this->crawlablePropWasProvided; } - public function forwardQuery(): bool - { - return $this->forwardQuery; - } - public function forwardQueryWasProvided(): bool { return $this->forwardQueryPropWasProvided; diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php index f31ee294..72708250 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php @@ -78,7 +78,7 @@ class ShortUrlInputFilter extends InputFilter $this->add($longUrlInput); $deviceLongUrlsInput = $this->createInput(self::DEVICE_LONG_URLS, false); - $deviceLongUrlsInput->getValidatorChain()->attach( + $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 diff --git a/module/Core/test/ShortUrl/ShortUrlServiceTest.php b/module/Core/test/ShortUrl/ShortUrlServiceTest.php index 9cc0d955..96e0c9f5 100644 --- a/module/Core/test/ShortUrl/ShortUrlServiceTest.php +++ b/module/Core/test/ShortUrl/ShortUrlServiceTest.php @@ -73,10 +73,10 @@ class ShortUrlServiceTest extends TestCase ); self::assertSame($shortUrl, $result); - self::assertEquals($shortUrlEdit->validSince(), $shortUrl->getValidSince()); - self::assertEquals($shortUrlEdit->validUntil(), $shortUrl->getValidUntil()); - self::assertEquals($shortUrlEdit->maxVisits(), $shortUrl->getMaxVisits()); - self::assertEquals($shortUrlEdit->longUrl() ?? $originalLongUrl, $shortUrl->getLongUrl()); + self::assertEquals($shortUrlEdit->validSince, $shortUrl->getValidSince()); + self::assertEquals($shortUrlEdit->validUntil, $shortUrl->getValidUntil()); + self::assertEquals($shortUrlEdit->maxVisits, $shortUrl->getMaxVisits()); + self::assertEquals($shortUrlEdit->longUrl ?? $originalLongUrl, $shortUrl->getLongUrl()); } public function provideShortUrlEdits(): iterable