Allow device long URLs to be removed from short URLs by providing null value

This commit is contained in:
Alejandro Celaya 2023-01-22 11:03:05 +01:00
parent 45961144b9
commit 13e443880a
10 changed files with 150 additions and 33 deletions

View File

@ -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)

View File

@ -40,7 +40,7 @@ class ShortUrl extends AbstractEntity
private Chronos $dateCreated;
/** @var Collection<int, Visit> */
private Collection $visits;
/** @var Collection<int, DeviceLongUrl> */
/** @var Collection<string, DeviceLongUrl> */
private Collection $deviceLongUrls;
/** @var Collection<int, Tag> */
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;
}

View File

@ -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<string, string> $map
* @return self[]
* @return array{array<string, self>, 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];
}
}

View File

@ -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),

View File

@ -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,

View File

@ -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();
}

View File

@ -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

View File

@ -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 => [],
],
]];
}
/**

View File

@ -0,0 +1,54 @@
<?php
declare(strict_types=1);
namespace ShlinkioTest\Shlink\Core\ShortUrl\Model;
use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\Core\Model\DeviceType;
use Shlinkio\Shlink\Core\ShortUrl\Model\DeviceLongUrlPair;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlEdition;
use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter;
class ShortUrlEditionTest extends TestCase
{
/**
* @test
* @dataProvider provideDeviceLongUrls
*/
public function expectedDeviceLongUrlsAreResolved(
?array $deviceLongUrls,
array $expectedDeviceLongUrls,
array $expectedDevicesToRemove,
): void {
$edition = ShortUrlEdition::fromRawData([ShortUrlInputFilter::DEVICE_LONG_URLS => $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]];
}
}

View File

@ -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());
}
/**