diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php index 13aa36f6..746ac3fd 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php @@ -70,6 +70,8 @@ return static function (ClassMetadata $metadata, array $emConfig): void { $builder->createOneToMany('deviceLongUrls', ShortUrl\Entity\DeviceLongUrl::class) ->mappedBy('shortUrl') ->cascadePersist() + ->orphanRemoval() + ->setIndexBy('deviceType') ->build(); $builder->createManyToMany('tags', Tag\Entity\Tag::class) diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index 4e93a916..e6da743e 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -40,7 +40,7 @@ class ShortUrl extends AbstractEntity private Chronos $dateCreated; /** @var Collection */ private Collection $visits; - /** @var Collection */ + /** @var Collection */ private Collection $deviceLongUrls; /** @var Collection */ private Collection $tags; @@ -171,10 +171,13 @@ class ShortUrl extends AbstractEntity if ($shortUrlEdit->forwardQueryWasProvided()) { $this->forwardQuery = $shortUrlEdit->forwardQuery; } + + // Update device long URLs, removing, editing or creating where appropriate + foreach ($shortUrlEdit->devicesToRemove as $deviceType) { + $this->deviceLongUrls->remove($deviceType->value); + } foreach ($shortUrlEdit->deviceLongUrls as $deviceLongUrlPair) { - $deviceLongUrl = $this->deviceLongUrls->findFirst( - fn ($_, DeviceLongUrl $d) => $d->deviceType === $deviceLongUrlPair->deviceType, - ); + $deviceLongUrl = $this->deviceLongUrls->get($deviceLongUrlPair->deviceType->value); if ($deviceLongUrl !== null) { $deviceLongUrl->updateLongUrl($deviceLongUrlPair->longUrl); @@ -191,10 +194,7 @@ class ShortUrl extends AbstractEntity public function longUrlForDevice(?DeviceType $deviceType): string { - $deviceLongUrl = $this->deviceLongUrls->findFirst( - static fn ($_, DeviceLongUrl $longUrl) => $longUrl->deviceType === $deviceType, - ); - + $deviceLongUrl = $deviceType === null ? null : $this->deviceLongUrls->get($deviceType->value); return $deviceLongUrl?->longUrl() ?? $this->longUrl; } diff --git a/module/Core/src/ShortUrl/Model/DeviceLongUrlPair.php b/module/Core/src/ShortUrl/Model/DeviceLongUrlPair.php index 6d0234ec..d017c7e5 100644 --- a/module/Core/src/ShortUrl/Model/DeviceLongUrlPair.php +++ b/module/Core/src/ShortUrl/Model/DeviceLongUrlPair.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Model; use Shlinkio\Shlink\Core\Model\DeviceType; use function array_values; +use function Functional\group; use function Functional\map; use function trim; @@ -22,14 +23,25 @@ final class DeviceLongUrlPair } /** + * Returns an array with two values. + * * The first one is a list of mapped instances for those entries in the map with non-null value + * * The second is a list of DeviceTypes which have been provided with value null + * * @param array $map - * @return self[] + * @return array{array, DeviceType[]} */ - public static function fromMapToList(array $map): array + public static function fromMapToChangeSet(array $map): array { - return array_values(map( - $map, - fn (string $longUrl, string $deviceType) => self::fromRawTypeAndLongUrl($deviceType, $longUrl), + $typesWithNullUrl = group($map, static fn (?string $longUrl) => $longUrl === null ? 'remove' : 'keep'); + $deviceTypesToRemove = array_values(map( + $typesWithNullUrl['remove'] ?? [], + static fn ($_, string $deviceType) => DeviceType::from($deviceType), )); + $pairsToKeep = map( + $typesWithNullUrl['keep'] ?? [], + fn (string $longUrl, string $deviceType) => self::fromRawTypeAndLongUrl($deviceType, $longUrl), + ); + + return [$pairsToKeep, $deviceTypesToRemove]; } } diff --git a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php index f2e156f4..a5d20bfb 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php @@ -61,11 +61,13 @@ final class ShortUrlCreation implements TitleResolutionModelInterface throw ValidationException::fromInputFilter($inputFilter); } + [$deviceLongUrls] = DeviceLongUrlPair::fromMapToChangeSet( + $inputFilter->getValue(ShortUrlInputFilter::DEVICE_LONG_URLS) ?? [], + ); + return new self( longUrl: $inputFilter->getValue(ShortUrlInputFilter::LONG_URL), - deviceLongUrls: DeviceLongUrlPair::fromMapToList( - $inputFilter->getValue(ShortUrlInputFilter::DEVICE_LONG_URLS) ?? [], - ), + deviceLongUrls: $deviceLongUrls, validSince: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)), validUntil: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_UNTIL)), customSlug: $inputFilter->getValue(ShortUrlInputFilter::CUSTOM_SLUG), diff --git a/module/Core/src/ShortUrl/Model/ShortUrlEdition.php b/module/Core/src/ShortUrl/Model/ShortUrlEdition.php index 25645437..6bc157c7 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlEdition.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlEdition.php @@ -6,6 +6,7 @@ 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; @@ -19,11 +20,13 @@ final class ShortUrlEdition implements TitleResolutionModelInterface /** * @param string[] $tags * @param DeviceLongUrlPair[] $deviceLongUrls + * @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, @@ -53,12 +56,15 @@ final class ShortUrlEdition implements TitleResolutionModelInterface throw ValidationException::fromInputFilter($inputFilter); } + [$deviceLongUrls, $devicesToRemove] = DeviceLongUrlPair::fromMapToChangeSet( + $inputFilter->getValue(ShortUrlInputFilter::DEVICE_LONG_URLS) ?? [], + ); + 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) ?? [], - ), + deviceLongUrls: $deviceLongUrls, + devicesToRemove: $devicesToRemove, validSincePropWasProvided: array_key_exists(ShortUrlInputFilter::VALID_SINCE, $data), validSince: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)), validUntilPropWasProvided: array_key_exists(ShortUrlInputFilter::VALID_UNTIL, $data), @@ -82,6 +88,8 @@ final class ShortUrlEdition implements TitleResolutionModelInterface return new self( longUrlPropWasProvided: $this->longUrlPropWasProvided, longUrl: $this->longUrl, + deviceLongUrls: $this->deviceLongUrls, + devicesToRemove: $this->devicesToRemove, validSincePropWasProvided: $this->validSincePropWasProvided, validSince: $this->validSince, validUntilPropWasProvided: $this->validUntilPropWasProvided, diff --git a/module/Core/src/ShortUrl/Model/Validation/DeviceLongUrlsValidator.php b/module/Core/src/ShortUrl/Model/Validation/DeviceLongUrlsValidator.php index 1e9d9824..9fda1809 100644 --- a/module/Core/src/ShortUrl/Model/Validation/DeviceLongUrlsValidator.php +++ b/module/Core/src/ShortUrl/Model/Validation/DeviceLongUrlsValidator.php @@ -5,7 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Model\Validation; use Laminas\Validator\AbstractValidator; -use Laminas\Validator\ValidatorChain; +use Laminas\Validator\ValidatorInterface; use Shlinkio\Shlink\Core\Model\DeviceType; use function array_keys; @@ -27,7 +27,7 @@ class DeviceLongUrlsValidator extends AbstractValidator self::INVALID_LONG_URL => 'At least one of the long URLs are invalid.', ]; - public function __construct(private readonly ValidatorChain $longUrlValidators) + public function __construct(private readonly ValidatorInterface $longUrlValidators) { parent::__construct(); } diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php index 7b01841b..4a0e2d7b 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Model\Validation; -use DateTime; +use DateTimeInterface; use Laminas\Filter; use Laminas\InputFilter\InputFilter; use Laminas\Validator; @@ -41,6 +41,7 @@ class ShortUrlInputFilter extends InputFilter private function __construct(array $data, bool $requireLongUrl) { + // FIXME The multi-segment slug option should be injected $this->initialize($requireLongUrl, $data[EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->value] ?? false); $this->setData($data); } @@ -57,29 +58,36 @@ class ShortUrlInputFilter extends InputFilter private function initialize(bool $requireLongUrl, bool $multiSegmentEnabled): void { - $longUrlInput = $this->createInput(self::LONG_URL, $requireLongUrl); - $longUrlInput->getValidatorChain()->attach(new Validator\NotEmpty([ + $longUrlNotEmptyCommonOptions = [ 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(new Validator\NotEmpty([ + ...$longUrlNotEmptyCommonOptions, + Validator\NotEmpty::NULL, ])); $this->add($longUrlInput); $deviceLongUrlsInput = $this->createInput(self::DEVICE_LONG_URLS, false); $deviceLongUrlsInput->getValidatorChain()->attach( - new DeviceLongUrlsValidator($longUrlInput->getValidatorChain()), + new DeviceLongUrlsValidator(new Validator\NotEmpty([ + ...$longUrlNotEmptyCommonOptions, + ...($requireLongUrl ? [Validator\NotEmpty::NULL] : []), + ])), ); $this->add($deviceLongUrlsInput); $validSince = $this->createInput(self::VALID_SINCE, false); - $validSince->getValidatorChain()->attach(new Validator\Date(['format' => DateTime::ATOM])); + $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' => DateTime::ATOM])); + $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 diff --git a/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php b/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php index 33380ecf..4d11289c 100644 --- a/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php +++ b/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php @@ -8,6 +8,7 @@ use Cake\Chronos\Chronos; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Config\EnvVars; use Shlinkio\Shlink\Core\Exception\ValidationException; +use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; use stdClass; @@ -69,6 +70,40 @@ class ShortUrlCreationTest extends TestCase yield [[ ShortUrlInputFilter::LONG_URL => [], ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => null, + ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + 'invalid' => 'https://shlink.io', + ], + ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + DeviceType::DESKTOP->value => '', + ], + ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + DeviceType::DESKTOP->value => null, + ], + ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + DeviceType::IOS->value => ' ', + ], + ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + DeviceType::IOS->value => 'bar', + DeviceType::ANDROID->value => [], + ], + ]]; } /** diff --git a/module/Core/test/ShortUrl/Model/ShortUrlEditionTest.php b/module/Core/test/ShortUrl/Model/ShortUrlEditionTest.php new file mode 100644 index 00000000..e03bb1ac --- /dev/null +++ b/module/Core/test/ShortUrl/Model/ShortUrlEditionTest.php @@ -0,0 +1,54 @@ + $deviceLongUrls]); + + self::assertEquals($expectedDeviceLongUrls, $edition->deviceLongUrls); + self::assertEquals($expectedDevicesToRemove, $edition->devicesToRemove); + } + + public function provideDeviceLongUrls(): iterable + { + yield 'null' => [null, [], []]; + yield 'empty' => [[], [], []]; + yield 'only new urls' => [[ + DeviceType::DESKTOP->value => 'foo', + DeviceType::IOS->value => 'bar', + ], [ + DeviceType::DESKTOP->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::DESKTOP->value, 'foo'), + DeviceType::IOS->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::IOS->value, 'bar'), + ], []]; + yield 'only urls to remove' => [[ + DeviceType::ANDROID->value => null, + DeviceType::IOS->value => null, + ], [], [DeviceType::ANDROID, DeviceType::IOS]]; + yield 'both' => [[ + DeviceType::DESKTOP->value => 'bar', + DeviceType::IOS->value => 'foo', + DeviceType::ANDROID->value => null, + ], [ + DeviceType::DESKTOP->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::DESKTOP->value, 'bar'), + DeviceType::IOS->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::IOS->value, 'foo'), + ], [DeviceType::ANDROID]]; + } +} diff --git a/module/Core/test/ShortUrl/Model/Validation/DeviceLongUrlsValidatorTest.php b/module/Core/test/ShortUrl/Model/Validation/DeviceLongUrlsValidatorTest.php index 42ad720b..8bac2f98 100644 --- a/module/Core/test/ShortUrl/Model/Validation/DeviceLongUrlsValidatorTest.php +++ b/module/Core/test/ShortUrl/Model/Validation/DeviceLongUrlsValidatorTest.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\ShortUrl\Model\Validation; use Laminas\Validator\NotEmpty; -use Laminas\Validator\ValidatorChain; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\DeviceLongUrlsValidator; @@ -17,10 +16,7 @@ class DeviceLongUrlsValidatorTest extends TestCase protected function setUp(): void { - $longUrlValidators = new ValidatorChain(); - $longUrlValidators->attach(new NotEmpty()); - - $this->validator = new DeviceLongUrlsValidator($longUrlValidators); + $this->validator = new DeviceLongUrlsValidator(new NotEmpty()); } /**