diff --git a/composer.json b/composer.json index a09eafa0..65605957 100644 --- a/composer.json +++ b/composer.json @@ -19,6 +19,7 @@ "ext-pdo": "*", "akrabat/ip-address-middleware": "^2.1", "cakephp/chronos": "^3.0.2", + "doctrine/dbal": "^4.0", "doctrine/migrations": "^3.6", "doctrine/orm": "^3.0", "endroid/qr-code": "^5.0", diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index e5f0bae7..72cfdf49 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -7,6 +7,7 @@ use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Entity\AbstractEntity; use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; +use Shlinkio\Shlink\Core\RedirectRule\Model\Validation\RedirectRulesInputFilter; use function Shlinkio\Shlink\Core\acceptLanguageToLocales; use function Shlinkio\Shlink\Core\ArrayUtils\some; @@ -39,6 +40,15 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable return new self(RedirectConditionType::DEVICE, $device->value); } + public static function fromRawData(array $rawData): self + { + $type = RedirectConditionType::from($rawData[RedirectRulesInputFilter::CONDITION_TYPE]); + $value = $rawData[RedirectRulesInputFilter::CONDITION_MATCH_VALUE]; + $key = $rawData[RedirectRulesInputFilter::CONDITION_MATCH_KEY] ?? null; + + return new self($type, $value, $key); + } + /** * Tells if this condition matches provided request */ diff --git a/module/Core/src/RedirectRule/Model/RedirectRulesData.php b/module/Core/src/RedirectRule/Model/RedirectRulesData.php index 6eb7dada..cf660ccd 100644 --- a/module/Core/src/RedirectRule/Model/RedirectRulesData.php +++ b/module/Core/src/RedirectRule/Model/RedirectRulesData.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\RedirectRule\Model; +use Laminas\InputFilter\Exception\InvalidArgumentException; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\RedirectRule\Model\Validation\RedirectRulesInputFilter; @@ -15,11 +16,17 @@ readonly class RedirectRulesData public static function fromRawData(array $rawData): self { - $inputFilter = RedirectRulesInputFilter::initialize($rawData); - if (! $inputFilter->isValid()) { - throw ValidationException::fromInputFilter($inputFilter); - } + try { + $inputFilter = RedirectRulesInputFilter::initialize($rawData); + if (! $inputFilter->isValid()) { + throw ValidationException::fromInputFilter($inputFilter); + } - return new self($inputFilter->getValue(RedirectRulesInputFilter::REDIRECT_RULES)); + return new self($inputFilter->getValue(RedirectRulesInputFilter::REDIRECT_RULES)); + } catch (InvalidArgumentException) { + throw ValidationException::fromArray( + [RedirectRulesInputFilter::REDIRECT_RULES => RedirectRulesInputFilter::REDIRECT_RULES], + ); + } } } diff --git a/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php b/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php index b3ad1f07..89f61c84 100644 --- a/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php +++ b/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php @@ -4,10 +4,8 @@ namespace Shlinkio\Shlink\Core\RedirectRule; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; -use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Entity\RedirectCondition; use Shlinkio\Shlink\Core\RedirectRule\Entity\ShortUrlRedirectRule; -use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectRulesData; use Shlinkio\Shlink\Core\RedirectRule\Model\Validation\RedirectRulesInputFilter; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; @@ -36,46 +34,39 @@ readonly class ShortUrlRedirectRuleService implements ShortUrlRedirectRuleServic */ public function setRulesForShortUrl(ShortUrl $shortUrl, RedirectRulesData $data): array { - return $this->em->wrapInTransaction(function () use ($shortUrl, $data): array { - // First, delete existing rules for the short URL - $oldRules = $this->rulesForShortUrl($shortUrl); - foreach ($oldRules as $oldRule) { - $oldRule->clearConditions(); // This will trigger the orphan removal of old conditions - $this->em->remove($oldRule); - } - $this->em->flush(); - - // Then insert new rules - $rules = []; - foreach ($data->rules as $rule) { - $rule = new ShortUrlRedirectRule( - shortUrl: $shortUrl, - priority: $rule[RedirectRulesInputFilter::RULE_PRIORITY], - longUrl: $rule[RedirectRulesInputFilter::RULE_LONG_URL], - conditions: new ArrayCollection(array_map( - fn (array $conditionData) => $this->createCondition($conditionData), - $rule[RedirectRulesInputFilter::RULE_CONDITIONS], - )), - ); - - $rules[] = $rule; - $this->em->persist($rule); - } - - return $rules; - }); + return $this->em->wrapInTransaction(fn () => $this->doSetRulesForShortUrl($shortUrl, $data)); } - private function createCondition(array $rawConditionData): RedirectCondition + /** + * @return ShortUrlRedirectRule[] + */ + private function doSetRulesForShortUrl(ShortUrl $shortUrl, RedirectRulesData $data): array { - $type = RedirectConditionType::from($rawConditionData[RedirectRulesInputFilter::CONDITION_TYPE]); - $value = $rawConditionData[RedirectRulesInputFilter::CONDITION_MATCH_VALUE]; - $key = $rawConditionData[RedirectRulesInputFilter::CONDITION_MATCH_KEY]; + // First, delete existing rules for the short URL + $oldRules = $this->rulesForShortUrl($shortUrl); + foreach ($oldRules as $oldRule) { + $oldRule->clearConditions(); // This will trigger the orphan removal of old conditions + $this->em->remove($oldRule); + } + $this->em->flush(); - return match ($type) { - RedirectConditionType::DEVICE => RedirectCondition::forDevice(DeviceType::from($value)), - RedirectConditionType::LANGUAGE => RedirectCondition::forLanguage($value), - RedirectConditionType::QUERY_PARAM => RedirectCondition::forQueryParam($key, $value), - }; + // Then insert new rules + $rules = []; + foreach ($data->rules as $rule) { + $rule = new ShortUrlRedirectRule( + shortUrl: $shortUrl, + priority: $rule[RedirectRulesInputFilter::RULE_PRIORITY], + longUrl: $rule[RedirectRulesInputFilter::RULE_LONG_URL], + conditions: new ArrayCollection(array_map( + RedirectCondition::fromRawData(...), + $rule[RedirectRulesInputFilter::RULE_CONDITIONS], + )), + ); + + $rules[] = $rule; + $this->em->persist($rule); + } + + return $rules; } } diff --git a/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php b/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php index 244e4f2f..19e431db 100644 --- a/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php +++ b/module/Core/test/RedirectRule/Entity/ShortUrlRedirectRuleTest.php @@ -20,7 +20,7 @@ class ShortUrlRedirectRuleTest extends TestCase ->withHeader('Accept-Language', 'en-UK') ->withQueryParams(['foo' => 'bar']); - $result = $this->createRule($conditions)->matchesRequest($request); + $result = $this->createRule(new ArrayCollection($conditions))->matchesRequest($request); self::assertEquals($expectedResult, $result); } @@ -38,12 +38,25 @@ class ShortUrlRedirectRuleTest extends TestCase ]; } + #[Test] + public function conditionsCanBeCleared(): void + { + $conditions = new ArrayCollection( + [RedirectCondition::forLanguage('en-UK'), RedirectCondition::forQueryParam('foo', 'bar')], + ); + $rule = $this->createRule($conditions); + + self::assertNotEmpty($conditions); + $rule->clearConditions(); + self::assertEmpty($conditions); + } + /** - * @param RedirectCondition[] $conditions + * @param ArrayCollection $conditions */ - private function createRule(array $conditions): ShortUrlRedirectRule + private function createRule(ArrayCollection $conditions): ShortUrlRedirectRule { $shortUrl = ShortUrl::withLongUrl('https://s.test'); - return new ShortUrlRedirectRule($shortUrl, 1, '', new ArrayCollection($conditions)); + return new ShortUrlRedirectRule($shortUrl, 1, '', $conditions); } } diff --git a/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php b/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php new file mode 100644 index 00000000..835905ad --- /dev/null +++ b/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php @@ -0,0 +1,66 @@ + ['foo']]])] + #[TestWith([['redirectRules' => [ + ['priority' => 'foo'], + ]]])] + #[TestWith([['redirectRules' => [ + [ + 'priority' => 4, + 'longUrl' => 34, + ], + ]]])] + #[TestWith([['redirectRules' => [ + [ + 'priority' => 4, + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'invalid', + ], + ], + ], + ]]])] + #[TestWith([['redirectRules' => [ + [ + 'priority' => 4, + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'device', + 'matchValue' => 'invalid-device', + 'matchKey' => null, + ], + ], + ], + ]]])] + #[TestWith([['redirectRules' => [ + [ + 'priority' => 4, + 'longUrl' => 'https://example.com', + 'conditions' => [ + [ + 'type' => 'language', + ], + ], + ], + ]]])] + public function throwsWhenProvidedDataIsInvalid(array $invalidData): void + { + $this->expectException(ValidationException::class); + RedirectRulesData::fromRawData($invalidData); + } +}