From f700abd65d9c474ff5a1346f9fa2695cd9532dd9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 29 Feb 2024 19:55:34 +0100 Subject: [PATCH] Add tests for ShortUrlRedirectionRuleService::setRulesForShortUrl --- .../RedirectRule/Model/RedirectRulesData.php | 4 +- .../Validation/RedirectRulesInputFilter.php | 3 - .../ShortUrlRedirectRuleService.php | 4 +- .../Model/RedirectRulesDataTest.php | 7 -- .../ShortUrlRedirectRuleServiceTest.php | 82 ++++++++++++++++++- 5 files changed, 86 insertions(+), 14 deletions(-) diff --git a/module/Core/src/RedirectRule/Model/RedirectRulesData.php b/module/Core/src/RedirectRule/Model/RedirectRulesData.php index cf660ccd..d9a9db18 100644 --- a/module/Core/src/RedirectRule/Model/RedirectRulesData.php +++ b/module/Core/src/RedirectRule/Model/RedirectRulesData.php @@ -8,6 +8,8 @@ use Laminas\InputFilter\Exception\InvalidArgumentException; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\RedirectRule\Model\Validation\RedirectRulesInputFilter; +use function array_values; + readonly class RedirectRulesData { private function __construct(public array $rules) @@ -22,7 +24,7 @@ readonly class RedirectRulesData throw ValidationException::fromInputFilter($inputFilter); } - return new self($inputFilter->getValue(RedirectRulesInputFilter::REDIRECT_RULES)); + return new self(array_values($inputFilter->getValue(RedirectRulesInputFilter::REDIRECT_RULES))); } catch (InvalidArgumentException) { throw ValidationException::fromArray( [RedirectRulesInputFilter::REDIRECT_RULES => RedirectRulesInputFilter::REDIRECT_RULES], diff --git a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php index 745b8914..42b83c76 100644 --- a/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php +++ b/module/Core/src/RedirectRule/Model/Validation/RedirectRulesInputFilter.php @@ -20,7 +20,6 @@ class RedirectRulesInputFilter extends InputFilter { public const REDIRECT_RULES = 'redirectRules'; - public const RULE_PRIORITY = 'priority'; public const RULE_LONG_URL = 'longUrl'; public const RULE_CONDITIONS = 'conditions'; @@ -48,8 +47,6 @@ class RedirectRulesInputFilter extends InputFilter { $redirectRuleInputFilter = new InputFilter(); - $redirectRuleInputFilter->add(InputFactory::numeric(self::RULE_PRIORITY, required: true)); - $longUrl = InputFactory::basic(self::RULE_LONG_URL, required: true); $longUrl->setValidatorChain(ShortUrlInputFilter::longUrlValidators()); $redirectRuleInputFilter->add($longUrl); diff --git a/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php b/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php index 89f61c84..1a770ae9 100644 --- a/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php +++ b/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php @@ -52,10 +52,10 @@ readonly class ShortUrlRedirectRuleService implements ShortUrlRedirectRuleServic // Then insert new rules $rules = []; - foreach ($data->rules as $rule) { + foreach ($data->rules as $index => $rule) { $rule = new ShortUrlRedirectRule( shortUrl: $shortUrl, - priority: $rule[RedirectRulesInputFilter::RULE_PRIORITY], + priority: $index + 1, longUrl: $rule[RedirectRulesInputFilter::RULE_LONG_URL], conditions: new ArrayCollection(array_map( RedirectCondition::fromRawData(...), diff --git a/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php b/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php index 835905ad..f0ded32b 100644 --- a/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php +++ b/module/Core/test/RedirectRule/Model/RedirectRulesDataTest.php @@ -14,18 +14,13 @@ class RedirectRulesDataTest extends TestCase { #[Test] #[TestWith([['redirectRules' => ['foo']]])] - #[TestWith([['redirectRules' => [ - ['priority' => 'foo'], - ]]])] #[TestWith([['redirectRules' => [ [ - 'priority' => 4, 'longUrl' => 34, ], ]]])] #[TestWith([['redirectRules' => [ [ - 'priority' => 4, 'longUrl' => 'https://example.com', 'conditions' => [ [ @@ -36,7 +31,6 @@ class RedirectRulesDataTest extends TestCase ]]])] #[TestWith([['redirectRules' => [ [ - 'priority' => 4, 'longUrl' => 'https://example.com', 'conditions' => [ [ @@ -49,7 +43,6 @@ class RedirectRulesDataTest extends TestCase ]]])] #[TestWith([['redirectRules' => [ [ - 'priority' => 4, 'longUrl' => 'https://example.com', 'conditions' => [ [ diff --git a/module/Core/test/RedirectRule/ShortUrlRedirectRuleServiceTest.php b/module/Core/test/RedirectRule/ShortUrlRedirectRuleServiceTest.php index 016c5453..b0b6d4f2 100644 --- a/module/Core/test/RedirectRule/ShortUrlRedirectRuleServiceTest.php +++ b/module/Core/test/RedirectRule/ShortUrlRedirectRuleServiceTest.php @@ -11,6 +11,9 @@ use PHPUnit\Framework\TestCase; 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\RedirectRule\ShortUrlRedirectRuleService; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; @@ -26,7 +29,7 @@ class ShortUrlRedirectRuleServiceTest extends TestCase } #[Test] - public function delegatesToRepository(): void + public function rulesForShortUrlDelegatesToRepository(): void { $shortUrl = ShortUrl::withLongUrl('https://shlink.io'); $rules = [ @@ -52,4 +55,81 @@ class ShortUrlRedirectRuleServiceTest extends TestCase self::assertSame($rules, $result); } + + #[Test] + public function setRulesForShortUrlParsesProvidedData(): void + { + $shortUrl = ShortUrl::withLongUrl('https://example.com'); + $data = RedirectRulesData::fromRawData([ + RedirectRulesInputFilter::REDIRECT_RULES => [ + [ + RedirectRulesInputFilter::RULE_LONG_URL => 'https://example.com/first', + RedirectRulesInputFilter::RULE_CONDITIONS => [ + [ + RedirectRulesInputFilter::CONDITION_TYPE => RedirectConditionType::DEVICE->value, + RedirectRulesInputFilter::CONDITION_MATCH_KEY => null, + RedirectRulesInputFilter::CONDITION_MATCH_VALUE => DeviceType::ANDROID->value, + ], + [ + RedirectRulesInputFilter::CONDITION_TYPE => RedirectConditionType::QUERY_PARAM->value, + RedirectRulesInputFilter::CONDITION_MATCH_KEY => 'foo', + RedirectRulesInputFilter::CONDITION_MATCH_VALUE => 'bar', + ], + ], + ], + [ + RedirectRulesInputFilter::RULE_LONG_URL => 'https://example.com/second', + RedirectRulesInputFilter::RULE_CONDITIONS => [ + [ + RedirectRulesInputFilter::CONDITION_TYPE => RedirectConditionType::DEVICE->value, + RedirectRulesInputFilter::CONDITION_MATCH_KEY => null, + RedirectRulesInputFilter::CONDITION_MATCH_VALUE => DeviceType::IOS->value, + ], + ], + ], + ], + ]); + + $this->em->expects($this->once())->method('wrapInTransaction')->willReturnCallback( + fn (callable $callback) => $callback(), + ); + $this->em->expects($this->exactly(2))->method('persist'); + $this->em->expects($this->never())->method('remove'); + + $result = $this->ruleService->setRulesForShortUrl($shortUrl, $data); + + self::assertCount(2, $result); + self::assertInstanceOf(ShortUrlRedirectRule::class, $result[0]); + self::assertInstanceOf(ShortUrlRedirectRule::class, $result[1]); + } + + #[Test] + public function setRulesForShortUrlRemovesOldRules(): void + { + $shortUrl = ShortUrl::withLongUrl('https://example.com'); + $data = RedirectRulesData::fromRawData([ + RedirectRulesInputFilter::REDIRECT_RULES => [], + ]); + + $repo = $this->createMock(EntityRepository::class); + $repo->expects($this->once())->method('findBy')->with( + ['shortUrl' => $shortUrl], + ['priority' => 'ASC'], + )->willReturn([ + new ShortUrlRedirectRule($shortUrl, 1, 'https://example.com'), + new ShortUrlRedirectRule($shortUrl, 2, 'https://example.com'), + ]); + $this->em->expects($this->once())->method('getRepository')->with(ShortUrlRedirectRule::class)->willReturn( + $repo, + ); + $this->em->expects($this->once())->method('wrapInTransaction')->willReturnCallback( + fn (callable $callback) => $callback(), + ); + $this->em->expects($this->never())->method('persist'); + $this->em->expects($this->exactly(2))->method('remove'); + + $result = $this->ruleService->setRulesForShortUrl($shortUrl, $data); + + self::assertCount(0, $result); + } }