From 2f39aff2fe84eaaaa317b5078ce4a4e0d3fca1a5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 Dec 2024 12:42:06 +0100 Subject: [PATCH 1/3] Implement logic to import redirect rules from other Shlink instances --- composer.json | 28 ++++++------- module/Core/config/dependencies.config.php | 1 + .../src/Importer/ImportedLinksProcessor.php | 3 ++ .../Core/src/Importer/ShortUrlImporting.php | 42 ++++++++++++++++++- .../RedirectRule/Entity/RedirectCondition.php | 18 ++++++++ .../ShortUrlRedirectRuleService.php | 8 ++-- .../ShortUrlRedirectRuleServiceInterface.php | 2 + module/Core/src/ShortUrl/Entity/ShortUrl.php | 1 - .../Importer/ImportedLinksProcessorTest.php | 6 ++- 9 files changed, 87 insertions(+), 22 deletions(-) diff --git a/composer.json b/composer.json index ca6cab07..15ba3b70 100644 --- a/composer.json +++ b/composer.json @@ -26,15 +26,15 @@ "donatj/phpuseragentparser": "^1.10", "endroid/qr-code": "^6.0", "friendsofphp/proxy-manager-lts": "^1.0", - "geoip2/geoip2": "^3.0", + "geoip2/geoip2": "^3.1", "guzzlehttp/guzzle": "^7.9", "hidehalo/nanoid-php": "^2.0", "jaybizzle/crawler-detect": "^1.3", - "laminas/laminas-config-aggregator": "^1.15", + "laminas/laminas-config-aggregator": "^1.17", "laminas/laminas-diactoros": "^3.5", - "laminas/laminas-inputfilter": "^2.30", - "laminas/laminas-servicemanager": "^3.22", - "laminas/laminas-stdlib": "^3.19", + "laminas/laminas-inputfilter": "^2.31", + "laminas/laminas-servicemanager": "^3.23", + "laminas/laminas-stdlib": "^3.20", "matomo/matomo-php-tracker": "^3.3", "mezzio/mezzio": "^3.20", "mezzio/mezzio-fastroute": "^3.12", @@ -46,7 +46,7 @@ "shlinkio/shlink-common": "^6.6", "shlinkio/shlink-config": "^3.4", "shlinkio/shlink-event-dispatcher": "^4.1", - "shlinkio/shlink-importer": "^5.3.2", + "shlinkio/shlink-importer": "dev-main#6c305ee as 5.5", "shlinkio/shlink-installer": "dev-develop#3675f6d as 9.4", "shlinkio/shlink-ip-geolocation": "^4.2", "shlinkio/shlink-json": "^1.1", @@ -54,14 +54,14 @@ "spiral/roadrunner-cli": "^2.6", "spiral/roadrunner-http": "^3.5", "spiral/roadrunner-jobs": "^4.5", - "symfony/console": "^7.1", - "symfony/filesystem": "^7.1", - "symfony/lock": "^7.1", - "symfony/process": "^7.1", - "symfony/string": "^7.1" + "symfony/console": "^7.2", + "symfony/filesystem": "^7.2", + "symfony/lock": "^7.2", + "symfony/process": "^7.2", + "symfony/string": "^7.2" }, "require-dev": { - "devizzent/cebe-php-openapi": "^1.1.1", + "devizzent/cebe-php-openapi": "^1.1.2", "devster/ubench": "^2.1", "phpstan/phpstan": "^2.0", "phpstan/phpstan-doctrine": "^2.0", @@ -69,11 +69,11 @@ "phpstan/phpstan-symfony": "^2.0", "phpunit/php-code-coverage": "^11.0", "phpunit/phpcov": "^10.0", - "phpunit/phpunit": "^11.4", + "phpunit/phpunit": "^11.5", "roave/security-advisories": "dev-master", "shlinkio/php-coding-standard": "~2.4.0", "shlinkio/shlink-test-utils": "^4.2", - "symfony/var-dumper": "^7.1", + "symfony/var-dumper": "^7.2", "veewee/composer-run-parallel": "^1.4" }, "conflict": { diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index adc9ae2a..eda556e9 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -262,6 +262,7 @@ return [ ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class, ShortUrl\Helper\ShortCodeUniquenessHelper::class, Util\DoctrineBatchHelper::class, + RedirectRule\ShortUrlRedirectRuleService::class, ], Crawling\CrawlingHelper::class => [ShortUrl\Repository\CrawlableShortCodesQuery::class], diff --git a/module/Core/src/Importer/ImportedLinksProcessor.php b/module/Core/src/Importer/ImportedLinksProcessor.php index e8434d4f..af4ce917 100644 --- a/module/Core/src/Importer/ImportedLinksProcessor.php +++ b/module/Core/src/Importer/ImportedLinksProcessor.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\Importer; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; +use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortCodeUniquenessHelperInterface; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository; @@ -32,6 +33,7 @@ readonly class ImportedLinksProcessor implements ImportedLinksProcessorInterface private ShortUrlRelationResolverInterface $relationResolver, private ShortCodeUniquenessHelperInterface $shortCodeHelper, private DoctrineBatchHelperInterface $batchHelper, + private ShortUrlRedirectRuleServiceInterface $redirectRuleService, ) { } @@ -80,6 +82,7 @@ readonly class ImportedLinksProcessor implements ImportedLinksProcessorInterface continue; } + $shortUrlImporting->importRedirectRules($importedUrl->redirectRules, $this->em, $this->redirectRuleService); $resultMessage = $shortUrlImporting->importVisits( $this->batchHelper->wrapIterable($importedUrl->visits, 100), $this->em, diff --git a/module/Core/src/Importer/ShortUrlImporting.php b/module/Core/src/Importer/ShortUrlImporting.php index ad812e8c..cc534fc2 100644 --- a/module/Core/src/Importer/ShortUrlImporting.php +++ b/module/Core/src/Importer/ShortUrlImporting.php @@ -4,11 +4,17 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Importer; +use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; +use Shlinkio\Shlink\Core\RedirectRule\Entity\RedirectCondition; +use Shlinkio\Shlink\Core\RedirectRule\Entity\ShortUrlRedirectRule; +use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\Visit\Entity\Visit; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectRule; use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisit; +use function Shlinkio\Shlink\Core\ArrayUtils\map; use function Shlinkio\Shlink\Core\normalizeDate; use function sprintf; @@ -20,12 +26,12 @@ final readonly class ShortUrlImporting public static function fromExistingShortUrl(ShortUrl $shortUrl): self { - return new self($shortUrl, false); + return new self($shortUrl, isNew: false); } public static function fromNewShortUrl(ShortUrl $shortUrl): self { - return new self($shortUrl, true); + return new self($shortUrl, isNew: true); } /** @@ -55,6 +61,38 @@ final readonly class ShortUrlImporting : sprintf('Skipped. Imported %s visits', $importedVisits); } + /** + * @param ImportedShlinkRedirectRule[] $rules + */ + public function importRedirectRules( + array $rules, + EntityManagerInterface $em, + ShortUrlRedirectRuleServiceInterface $redirectRuleService, + ): void { + $shortUrl = $this->resolveShortUrl($em); + $redirectRules = map( + $rules, + function (ImportedShlinkRedirectRule $rule, int|string|float $index) use ($shortUrl): ShortUrlRedirectRule { + $conditions = new ArrayCollection(); + foreach ($rule->conditions as $cond) { + $redirectCondition = RedirectCondition::fromImport($cond); + if ($redirectCondition !== null) { + $conditions->add($redirectCondition); + } + } + + return new ShortUrlRedirectRule( + shortUrl: $shortUrl, + priority: ((int) $index) + 1, + longUrl:$rule->longUrl, + conditions: $conditions, + ); + }, + ); + + $redirectRuleService->saveRulesForShortUrl($shortUrl, $redirectRules); + } + private function resolveShortUrl(EntityManagerInterface $em): ShortUrl { // If wrapped ShortUrl has no ID, avoid trying to query the EM, as it would fail in Postgres. diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index cf1e134b..602d07b7 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -9,6 +9,7 @@ use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; use Shlinkio\Shlink\Core\RedirectRule\Model\Validation\RedirectRulesInputFilter; use Shlinkio\Shlink\Core\Util\IpAddressUtils; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectCondition; use function Shlinkio\Shlink\Core\acceptLanguageToLocales; use function Shlinkio\Shlink\Core\ArrayUtils\some; @@ -72,6 +73,23 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable return new self($type, $value, $key); } + public static function fromImport(ImportedShlinkRedirectCondition $cond): self|null + { + $type = RedirectConditionType::tryFrom($cond->type); + if ($type === null) { + return null; + } + + return match ($type) { + RedirectConditionType::QUERY_PARAM => self::forQueryParam($cond->matchKey ?? '', $cond->matchValue), + RedirectConditionType::LANGUAGE => self::forLanguage($cond->matchValue), + RedirectConditionType::DEVICE => self::forDevice(DeviceType::from($cond->matchValue)), + RedirectConditionType::IP_ADDRESS => self::forIpAddress($cond->matchValue), + RedirectConditionType::GEOLOCATION_COUNTRY_CODE => self::forGeolocationCountryCode($cond->matchValue), + RedirectConditionType::GEOLOCATION_CITY_NAME => self::forGeolocationCityName($cond->matchValue), + }; + } + /** * Tells if this condition matches provided request */ diff --git a/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php b/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php index 01ba0a8f..dac0dc61 100644 --- a/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php +++ b/module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php @@ -20,7 +20,7 @@ readonly class ShortUrlRedirectRuleService implements ShortUrlRedirectRuleServic } /** - * @return ShortUrlRedirectRule[] + * @inheritDoc */ public function rulesForShortUrl(ShortUrl $shortUrl): array { @@ -31,7 +31,7 @@ readonly class ShortUrlRedirectRuleService implements ShortUrlRedirectRuleServic } /** - * @return ShortUrlRedirectRule[] + * @inheritDoc */ public function setRulesForShortUrl(ShortUrl $shortUrl, RedirectRulesData $data): array { @@ -55,7 +55,7 @@ readonly class ShortUrlRedirectRuleService implements ShortUrlRedirectRuleServic } /** - * @param ShortUrlRedirectRule[] $rules + * @inheritDoc */ public function saveRulesForShortUrl(ShortUrl $shortUrl, array $rules): void { @@ -74,7 +74,7 @@ readonly class ShortUrlRedirectRuleService implements ShortUrlRedirectRuleServic /** * @param ShortUrlRedirectRule[] $rules */ - public function doSetRulesForShortUrl(ShortUrl $shortUrl, array $rules): void + private function doSetRulesForShortUrl(ShortUrl $shortUrl, array $rules): void { $this->em->wrapInTransaction(function () use ($shortUrl, $rules): void { // First, delete existing rules for the short URL diff --git a/module/Core/src/RedirectRule/ShortUrlRedirectRuleServiceInterface.php b/module/Core/src/RedirectRule/ShortUrlRedirectRuleServiceInterface.php index 186be87e..e05c6ab8 100644 --- a/module/Core/src/RedirectRule/ShortUrlRedirectRuleServiceInterface.php +++ b/module/Core/src/RedirectRule/ShortUrlRedirectRuleServiceInterface.php @@ -14,11 +14,13 @@ interface ShortUrlRedirectRuleServiceInterface public function rulesForShortUrl(ShortUrl $shortUrl): array; /** + * Resolve a set of redirect rules and attach them to a short URL, replacing any already existing rules. * @return ShortUrlRedirectRule[] */ public function setRulesForShortUrl(ShortUrl $shortUrl, RedirectRulesData $data): array; /** + * Save provided set of rules for a short URL, replacing any already existing rules. * @param ShortUrlRedirectRule[] $rules */ public function saveRulesForShortUrl(ShortUrl $shortUrl, array $rules): void; diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index b7fb6c56..086a47bb 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -76,7 +76,6 @@ class ShortUrl extends AbstractEntity /** * @param non-empty-string $longUrl - * @internal */ public static function withLongUrl(string $longUrl): self { diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index 36265aa3..8fb63e68 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -14,6 +14,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use RuntimeException; use Shlinkio\Shlink\Core\Importer\ImportedLinksProcessor; +use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortCodeUniquenessHelperInterface; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepository; @@ -44,13 +45,15 @@ class ImportedLinksProcessorTest extends TestCase private MockObject & ShortCodeUniquenessHelperInterface $shortCodeHelper; private MockObject & ShortUrlRepository $repo; private MockObject & StyleInterface $io; + private MockObject & ShortUrlRedirectRuleServiceInterface $redirectRuleService; protected function setUp(): void { $this->em = $this->createMock(EntityManagerInterface::class); $this->repo = $this->createMock(ShortUrlRepository::class); - $this->shortCodeHelper = $this->createMock(ShortCodeUniquenessHelperInterface::class); + $this->redirectRuleService = $this->createMock(ShortUrlRedirectRuleServiceInterface::class); + $batchHelper = $this->createMock(DoctrineBatchHelperInterface::class); $batchHelper->method('wrapIterable')->willReturnArgument(0); @@ -59,6 +62,7 @@ class ImportedLinksProcessorTest extends TestCase new SimpleShortUrlRelationResolver(), $this->shortCodeHelper, $batchHelper, + $this->redirectRuleService, ); $this->io = $this->createMock(StyleInterface::class); From 2807b9ce2fedd0150c0c68c4ca5117d6f10f75c8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 Dec 2024 18:24:56 +0100 Subject: [PATCH 2/3] Fix ImportedLinksProcessorTest --- .../ShortUrl/DeleteShortUrlCommandTest.php | 2 +- .../Command/Visit/LocateVisitsCommandTest.php | 2 +- .../RedirectRule/RedirectRuleHandlerTest.php | 2 +- .../Core/src/Importer/ShortUrlImporting.php | 5 ++ .../RedirectRule/Entity/RedirectCondition.php | 2 +- .../Geolocation/GeolocationDbUpdaterTest.php | 2 +- .../Importer/ImportedLinksProcessorTest.php | 49 +++++++++++++++++-- .../Entity/RedirectConditionTest.php | 20 ++++++++ ...ersistenceShortUrlRelationResolverTest.php | 6 +-- .../Core/test/ShortUrl/UrlShortenerTest.php | 2 +- .../Rest/test/Service/ApiKeyServiceTest.php | 4 +- 11 files changed, 80 insertions(+), 16 deletions(-) diff --git a/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php index 0402dc8c..6ffec859 100644 --- a/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php @@ -74,7 +74,7 @@ class DeleteShortUrlCommandTest extends TestCase $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($shortCode); $this->service->expects($this->exactly($expectedDeleteCalls))->method('deleteByShortCode')->with( $identifier, - $this->isType('bool'), + $this->isBool(), )->willReturnCallback(function ($_, bool $ignoreThreshold) use ($shortCode): void { if (!$ignoreThreshold) { throw Exception\DeleteShortUrlException::fromVisitsThreshold( diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php index 0f24a603..9b35324a 100644 --- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php @@ -47,7 +47,7 @@ class LocateVisitsCommandTest extends TestCase $locker = $this->createMock(Lock\LockFactory::class); $this->lock = $this->createMock(Lock\SharedLockInterface::class); - $locker->method('createLock')->with($this->isType('string'), 600.0, false)->willReturn($this->lock); + $locker->method('createLock')->with($this->isString(), 600.0, false)->willReturn($this->lock); $command = new LocateVisitsCommand($this->visitService, $this->visitToLocation, $locker); diff --git a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php index eb78da61..a7811060 100644 --- a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php +++ b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php @@ -77,7 +77,7 @@ class RedirectRuleHandlerTest extends TestCase $this->io->expects($this->once())->method('choice')->willReturn($action->value); $this->io->expects($this->never())->method('newLine'); $this->io->expects($this->never())->method('text'); - $this->io->expects($this->once())->method('table')->with($this->isType('array'), [ + $this->io->expects($this->once())->method('table')->with($this->isArray(), [ ['1', $comment($this->cond1->toHumanFriendly()), 'https://example.com/one'], [ '2', diff --git a/module/Core/src/Importer/ShortUrlImporting.php b/module/Core/src/Importer/ShortUrlImporting.php index cc534fc2..aeb24cce 100644 --- a/module/Core/src/Importer/ShortUrlImporting.php +++ b/module/Core/src/Importer/ShortUrlImporting.php @@ -14,6 +14,7 @@ use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectRule; use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisit; +use function count; use function Shlinkio\Shlink\Core\ArrayUtils\map; use function Shlinkio\Shlink\Core\normalizeDate; use function sprintf; @@ -69,6 +70,10 @@ final readonly class ShortUrlImporting EntityManagerInterface $em, ShortUrlRedirectRuleServiceInterface $redirectRuleService, ): void { + if ($this->isNew && count($rules) === 0) { + return; + } + $shortUrl = $this->resolveShortUrl($em); $redirectRules = map( $rules, diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index 602d07b7..255cb19e 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -24,7 +24,7 @@ use function trim; class RedirectCondition extends AbstractEntity implements JsonSerializable { private function __construct( - private readonly RedirectConditionType $type, + public readonly RedirectConditionType $type, private readonly string $matchValue, private readonly string|null $matchKey = null, ) { diff --git a/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php index c2983030..8c9f5a1b 100644 --- a/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php +++ b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php @@ -291,7 +291,7 @@ class GeolocationDbUpdaterTest extends TestCase private function geolocationDbUpdater(TrackingOptions|null $options = null): GeolocationDbUpdater { $locker = $this->createMock(Lock\LockFactory::class); - $locker->method('createLock')->with($this->isType('string'))->willReturn($this->lock); + $locker->method('createLock')->with($this->isString())->willReturn($this->lock); return new GeolocationDbUpdater($this->dbUpdater, $locker, $options ?? new TrackingOptions(), $this->em, 3); } diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index 8fb63e68..e8d800a1 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -14,6 +14,9 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use RuntimeException; use Shlinkio\Shlink\Core\Importer\ImportedLinksProcessor; +use Shlinkio\Shlink\Core\Model\DeviceType; +use Shlinkio\Shlink\Core\RedirectRule\Entity\ShortUrlRedirectRule; +use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortCodeUniquenessHelperInterface; @@ -24,6 +27,8 @@ use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\Visitor; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepository; use Shlinkio\Shlink\Importer\Model\ImportedShlinkOrphanVisit; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectCondition; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectRule; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisit; use Shlinkio\Shlink\Importer\Model\ImportResult; @@ -71,10 +76,31 @@ class ImportedLinksProcessorTest extends TestCase #[Test] public function newUrlsWithNoErrorsAreAllPersisted(): void { + $now = Chronos::now(); $urls = [ - new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], Chronos::now(), null, 'foo', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'https://bar', [], Chronos::now(), null, 'bar', 'foo'), - new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz', [], Chronos::now(), null, 'baz', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], $now, null, 'foo', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://bar', [], $now, null, 'bar', 'foo'), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz', [], $now, null, 'baz', null, redirectRules: [ + new ImportedShlinkRedirectRule( + longUrl: 'https://example.com/android', + conditions: [ + new ImportedShlinkRedirectCondition( + RedirectConditionType::DEVICE->value, + DeviceType::ANDROID->value, + ), + ], + ), + new ImportedShlinkRedirectRule( + longUrl: 'https://example.com/spain', + conditions: [ + new ImportedShlinkRedirectCondition( + RedirectConditionType::GEOLOCATION_COUNTRY_CODE->value, + 'ES', + ), + new ImportedShlinkRedirectCondition(RedirectConditionType::LANGUAGE->value, 'es-ES'), + ], + ), + ]), ]; $expectedCalls = count($urls); @@ -86,7 +112,19 @@ class ImportedLinksProcessorTest extends TestCase $this->em->expects($this->exactly($expectedCalls))->method('persist')->with( $this->isInstanceOf(ShortUrl::class), ); - $this->io->expects($this->exactly($expectedCalls))->method('text')->with($this->isType('string')); + $this->io->expects($this->exactly($expectedCalls))->method('text')->with($this->isString()); + $this->redirectRuleService->expects($this->once())->method('saveRulesForShortUrl')->with( + $this->isInstanceOf(ShortUrl::class), + $this->callback(function (array $rules): bool { + Assert::assertCount(2, $rules); + Assert::assertInstanceOf(ShortUrlRedirectRule::class, $rules[0]); + Assert::assertInstanceOf(ShortUrlRedirectRule::class, $rules[1]); + Assert::assertCount(1, $rules[0]->mapConditions(fn ($c) => $c)); + Assert::assertCount(2, $rules[1]->mapConditions(fn ($c) => $c)); + + return true; + }), + ); $this->processor->process($this->io, ImportResult::withShortUrls($urls), $this->buildParams()); } @@ -243,7 +281,8 @@ class ImportedLinksProcessorTest extends TestCase if (!$originalShortUrl->getId()) { $this->em->expects($this->never())->method('find'); } else { - $this->em->expects($this->exactly(2))->method('find')->willReturn($foundShortUrl); + // 3 times: Initial short URL checking, before creating redirect rules, before creating visits + $this->em->expects($this->exactly(3))->method('find')->willReturn($foundShortUrl); } $this->em->expects($this->once())->method('persist')->willReturnCallback( static fn (Visit $visit) => Assert::assertSame( diff --git a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php index 5a4a2e2b..bec6d263 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -9,6 +9,8 @@ use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\RedirectRule\Entity\RedirectCondition; +use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; +use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectCondition; use Shlinkio\Shlink\IpGeolocation\Model\Location; use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE; @@ -133,4 +135,22 @@ class RedirectConditionTest extends TestCase yield 'matching location' => [new Location(city: 'Madrid'), 'Madrid', true]; yield 'matching case-insensitive' => [new Location(city: 'Los Angeles'), 'los angeles', true]; } + + #[Test] + #[TestWith(['invalid', null])] + #[TestWith([RedirectConditionType::DEVICE->value, RedirectConditionType::DEVICE])] + #[TestWith([RedirectConditionType::LANGUAGE->value, RedirectConditionType::LANGUAGE])] + #[TestWith([RedirectConditionType::QUERY_PARAM->value, RedirectConditionType::QUERY_PARAM])] + #[TestWith([RedirectConditionType::IP_ADDRESS->value, RedirectConditionType::IP_ADDRESS])] + #[TestWith( + [RedirectConditionType::GEOLOCATION_COUNTRY_CODE->value, RedirectConditionType::GEOLOCATION_COUNTRY_CODE], + )] + #[TestWith([RedirectConditionType::GEOLOCATION_CITY_NAME->value, RedirectConditionType::GEOLOCATION_CITY_NAME])] + public function canBeCreatedFromImport(string $type, RedirectConditionType|null $expectedType): void + { + $condition = RedirectCondition::fromImport( + new ImportedShlinkRedirectCondition($type, DeviceType::ANDROID->value, ''), + ); + self::assertEquals($expectedType, $condition?->type); + } } diff --git a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php index 934d8511..31f42bc7 100644 --- a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php @@ -78,7 +78,7 @@ class PersistenceShortUrlRelationResolverTest extends TestCase $tagRepo = $this->createMock(TagRepository::class); $tagRepo->expects($this->exactly($expectedLookedOutTags))->method('findOneBy')->with( - $this->isType('array'), + $this->isArray(), )->willReturnCallback(function (array $criteria): Tag|null { ['name' => $name] = $criteria; return $name === 'foo' ? new Tag($name) : null; @@ -115,7 +115,7 @@ class PersistenceShortUrlRelationResolverTest extends TestCase public function newDomainsAreMemoizedUntilStateIsCleared(): void { $repo = $this->createMock(DomainRepository::class); - $repo->expects($this->exactly(3))->method('findOneBy')->with($this->isType('array'))->willReturn(null); + $repo->expects($this->exactly(3))->method('findOneBy')->with($this->isArray())->willReturn(null); $this->em->method('getRepository')->with(Domain::class)->willReturn($repo); $authority = 'foo.com'; @@ -134,7 +134,7 @@ class PersistenceShortUrlRelationResolverTest extends TestCase public function newTagsAreMemoizedUntilStateIsCleared(): void { $tagRepo = $this->createMock(TagRepository::class); - $tagRepo->expects($this->exactly(6))->method('findOneBy')->with($this->isType('array'))->willReturn(null); + $tagRepo->expects($this->exactly(6))->method('findOneBy')->with($this->isArray())->willReturn(null); $this->em->method('getRepository')->with(Tag::class)->willReturn($tagRepo); $tags = ['foo', 'bar']; diff --git a/module/Core/test/ShortUrl/UrlShortenerTest.php b/module/Core/test/ShortUrl/UrlShortenerTest.php index a6cacc46..fb191e95 100644 --- a/module/Core/test/ShortUrl/UrlShortenerTest.php +++ b/module/Core/test/ShortUrl/UrlShortenerTest.php @@ -38,7 +38,7 @@ class UrlShortenerTest extends TestCase // FIXME Should use the interface, but it doe snot define wrapInTransaction explicitly $this->em = $this->createMock(EntityManager::class); $this->em->method('persist')->willReturnCallback(fn (ShortUrl $shortUrl) => $shortUrl->setId('10')); - $this->em->method('wrapInTransaction')->with($this->isType('callable'))->willReturnCallback( + $this->em->method('wrapInTransaction')->with($this->isCallable())->willReturnCallback( fn (callable $callback) => $callback(), ); diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php index 81bec4ea..a5b6cecb 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -45,7 +45,7 @@ class ApiKeyServiceTest extends TestCase public function apiKeyIsProperlyCreated(Chronos|null $date, string|null $name, array $roles): void { $this->repo->expects($this->once())->method('nameExists')->with( - ! empty($name) ? $name : $this->isType('string'), + ! empty($name) ? $name : $this->isString(), )->willReturn(false); $this->em->expects($this->once())->method('persist')->with($this->isInstanceOf(ApiKey::class)); @@ -83,7 +83,7 @@ class ApiKeyServiceTest extends TestCase { $callCount = 0; $this->repo->expects($this->exactly(3))->method('nameExists')->with( - $this->isType('string'), + $this->isString(), )->willReturnCallback(function () use (&$callCount): bool { $callCount++; return $callCount < 3; From 9c251b364618b6a56656b8a5dbb27ea6ed514484 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 22 Dec 2024 18:41:58 +0100 Subject: [PATCH 3/3] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3cdeab8..2f906363 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#2229](https://github.com/shlinkio/shlink/issues/2229) Add `logo=disabled` query param to dynamically disable the default logo on QR codes. * [#2206](https://github.com/shlinkio/shlink/issues/2206) Add new `DB_USE_ENCRYPTION` config option to enable SSL database connections trusting any server certificate. +* [#2209](https://github.com/shlinkio/shlink/issues/2209) Redirect rules are now imported when importing short URLs from a Shlink >=4.0 instance. ### Changed * [#2281](https://github.com/shlinkio/shlink/issues/2281) Update docker image to PHP 8.4