Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement logic to import redirect rules from other Shlink instances #2315

Merged
merged 3 commits into from
Dec 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 14 additions & 14 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -46,34 +46,34 @@
"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",
"spiral/roadrunner": "^2024.1",
"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",
"phpstan/phpstan-phpunit": "^2.0",
"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": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public function deleteIsRetriedWhenThresholdIsReachedAndQuestionIsAccepted(
$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(
Expand Down
2 changes: 1 addition & 1 deletion module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ protected function setUp(): void

$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);

Expand Down
2 changes: 1 addition & 1 deletion module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public function rulesAreDisplayedWhenRulesListIsEmpty(
$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',
Expand Down
1 change: 1 addition & 0 deletions module/Core/config/dependencies.config.php
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@
ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class,
ShortUrl\Helper\ShortCodeUniquenessHelper::class,
Util\DoctrineBatchHelper::class,
RedirectRule\ShortUrlRedirectRuleService::class,
],

Crawling\CrawlingHelper::class => [ShortUrl\Repository\CrawlableShortCodesQuery::class],
Expand Down
3 changes: 3 additions & 0 deletions module/Core/src/Importer/ImportedLinksProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

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;
Expand All @@ -32,6 +33,7 @@ public function __construct(
private ShortUrlRelationResolverInterface $relationResolver,
private ShortCodeUniquenessHelperInterface $shortCodeHelper,
private DoctrineBatchHelperInterface $batchHelper,
private ShortUrlRedirectRuleServiceInterface $redirectRuleService,
) {
}

Expand Down Expand Up @@ -80,6 +82,7 @@ private function importShortUrls(StyleInterface $io, iterable $shlinkUrls, Impor
continue;
}

$shortUrlImporting->importRedirectRules($importedUrl->redirectRules, $this->em, $this->redirectRuleService);
$resultMessage = $shortUrlImporting->importVisits(
$this->batchHelper->wrapIterable($importedUrl->visits, 100),
$this->em,
Expand Down
47 changes: 45 additions & 2 deletions module/Core/src/Importer/ShortUrlImporting.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,18 @@

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 count;
use function Shlinkio\Shlink\Core\ArrayUtils\map;
use function Shlinkio\Shlink\Core\normalizeDate;
use function sprintf;

Expand All @@ -20,12 +27,12 @@ private function __construct(private ShortUrl $shortUrl, private bool $isNew)

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

/**
Expand Down Expand Up @@ -55,6 +62,42 @@ public function importVisits(iterable $visits, EntityManagerInterface $em): stri
: sprintf('<comment>Skipped</comment>. Imported <info>%s</info> visits', $importedVisits);
}

/**
* @param ImportedShlinkRedirectRule[] $rules
*/
public function importRedirectRules(
array $rules,
EntityManagerInterface $em,
ShortUrlRedirectRuleServiceInterface $redirectRuleService,
): void {
if ($this->isNew && count($rules) === 0) {
return;
}

$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.
Expand Down
20 changes: 19 additions & 1 deletion module/Core/src/RedirectRule/Entity/RedirectCondition.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
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;
Expand All @@ -23,7 +24,7 @@
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,
) {
Expand Down Expand Up @@ -72,6 +73,23 @@ public static function fromRawData(array $rawData): self
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
*/
Expand Down
8 changes: 4 additions & 4 deletions module/Core/src/RedirectRule/ShortUrlRedirectRuleService.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public function __construct(private EntityManagerInterface $em)
}

/**
* @return ShortUrlRedirectRule[]
* @inheritDoc
*/
public function rulesForShortUrl(ShortUrl $shortUrl): array
{
Expand All @@ -31,7 +31,7 @@ public function rulesForShortUrl(ShortUrl $shortUrl): array
}

/**
* @return ShortUrlRedirectRule[]
* @inheritDoc
*/
public function setRulesForShortUrl(ShortUrl $shortUrl, RedirectRulesData $data): array
{
Expand All @@ -55,7 +55,7 @@ public function setRulesForShortUrl(ShortUrl $shortUrl, RedirectRulesData $data)
}

/**
* @param ShortUrlRedirectRule[] $rules
* @inheritDoc
*/
public function saveRulesForShortUrl(ShortUrl $shortUrl, array $rules): void
{
Expand All @@ -74,7 +74,7 @@ public function saveRulesForShortUrl(ShortUrl $shortUrl, array $rules): void
/**
* @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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion module/Core/src/ShortUrl/Entity/ShortUrl.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ public static function createFake(): self

/**
* @param non-empty-string $longUrl
* @internal
*/
public static function withLongUrl(string $longUrl): self
{
Expand Down
2 changes: 1 addition & 1 deletion module/Core/test/Geolocation/GeolocationDbUpdaterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ public static function provideTrackingOptions(): iterable
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);
}
Expand Down
Loading