From 16bd368a58ec10bfda81771849385c19ea7ec563 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Aug 2022 09:30:13 +0200 Subject: [PATCH 1/7] Centralized how routes are configured to support multi-segment slugs --- composer.json | 2 +- config/autoload/routes.config.php | 14 ++--- config/config.php | 1 + .../src/Config/MultiSegmentSlugProcessor.php | 30 ++++++++++ .../src/Repository/ShortUrlRepository.php | 8 +-- .../Core/src/Repository/VisitRepository.php | 8 +-- module/Core/src/Spec/InDateRange.php | 8 +-- .../Config/MultiSegmentSlugProcessorTest.php | 60 +++++++++++++++++++ module/Rest/src/ConfigProvider.php | 9 +-- module/Rest/test/ConfigProviderTest.php | 19 +----- 10 files changed, 113 insertions(+), 46 deletions(-) create mode 100644 module/Core/src/Config/MultiSegmentSlugProcessor.php create mode 100644 module/Core/test/Config/MultiSegmentSlugProcessorTest.php diff --git a/composer.json b/composer.json index c6e293bd..a2e6c96c 100644 --- a/composer.json +++ b/composer.json @@ -43,7 +43,7 @@ "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.0", "ramsey/uuid": "^4.3", - "shlinkio/shlink-common": "^4.5", + "shlinkio/shlink-common": "dev-main#6163a03 as 5.0", "shlinkio/shlink-config": "^1.6", "shlinkio/shlink-event-dispatcher": "^2.4", "shlinkio/shlink-importer": "^3.0", diff --git a/config/autoload/routes.config.php b/config/autoload/routes.config.php index e7e24916..298b9349 100644 --- a/config/autoload/routes.config.php +++ b/config/autoload/routes.config.php @@ -7,23 +7,19 @@ namespace Shlinkio\Shlink; use Fig\Http\Message\RequestMethodInterface; use RKA\Middleware\IpAddress; use Shlinkio\Shlink\Core\Action as CoreAction; -use Shlinkio\Shlink\Core\Config\EnvVars; use Shlinkio\Shlink\Rest\Action; use Shlinkio\Shlink\Rest\ConfigProvider; use Shlinkio\Shlink\Rest\Middleware; use Shlinkio\Shlink\Rest\Middleware\Mercure\NotConfiguredMercureErrorHandler; -use function sprintf; - -// The order of the routes defined here matters. Changing it might cause path conflicts return (static function (): array { $contentNegotiationMiddleware = Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class; $dropDomainMiddleware = Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class; $overrideDomainMiddleware = Middleware\ShortUrl\OverrideDomainMiddleware::class; - $multiSegment = (bool) EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->loadFromEnv(false); return [ + // The order of the routes defined here matters. Changing it might cause path conflicts 'routes' => [ // Rest ...ConfigProvider::applyRoutesPrefix([ @@ -64,7 +60,7 @@ return (static function (): array { Action\Domain\DomainRedirectsAction::getRouteDef(), Action\MercureInfoAction::getRouteDef([NotConfiguredMercureErrorHandler::class]), - ], $multiSegment), + ]), // Non-rest [ @@ -77,7 +73,7 @@ return (static function (): array { ], [ 'name' => CoreAction\PixelAction::class, - 'path' => sprintf('/{shortCode%s}/track', $multiSegment ? ':.+' : ''), + 'path' => '/{shortCode}/track', 'middleware' => [ IpAddress::class, CoreAction\PixelAction::class, @@ -86,7 +82,7 @@ return (static function (): array { ], [ 'name' => CoreAction\QrCodeAction::class, - 'path' => sprintf('/{shortCode%s}/qr-code', $multiSegment ? ':.+' : ''), + 'path' => '/{shortCode}/qr-code', 'middleware' => [ CoreAction\QrCodeAction::class, ], @@ -94,7 +90,7 @@ return (static function (): array { ], [ 'name' => CoreAction\RedirectAction::class, - 'path' => sprintf('/{shortCode%s}', $multiSegment ? ':.+' : ''), + 'path' => '/{shortCode}', 'middleware' => [ IpAddress::class, CoreAction\RedirectAction::class, diff --git a/config/config.php b/config/config.php index a1e0428a..201c1a17 100644 --- a/config/config.php +++ b/config/config.php @@ -47,4 +47,5 @@ return (new ConfigAggregator\ConfigAggregator([ new ConfigAggregator\PhpFileProvider('config/autoload/routes.config.php'), ], 'data/cache/app_config.php', [ Core\Config\BasePathPrefixer::class, + Core\Config\MultiSegmentSlugProcessor::class, ]))->getMergedConfig(); diff --git a/module/Core/src/Config/MultiSegmentSlugProcessor.php b/module/Core/src/Config/MultiSegmentSlugProcessor.php new file mode 100644 index 00000000..b9cf2457 --- /dev/null +++ b/module/Core/src/Config/MultiSegmentSlugProcessor.php @@ -0,0 +1,30 @@ + $path] = $route; + $route['path'] = str_replace(self::SINGLE_SHORT_CODE_PATTERN, self::MULTI_SHORT_CODE_PATTERN, $path); + return $route; + }); + + return $config; + } +} diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 5946f255..af4f662d 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -84,13 +84,13 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU ->where('1=1'); $dateRange = $filtering->dateRange(); - if ($dateRange?->startDate() !== null) { + if ($dateRange?->startDate !== null) { $qb->andWhere($qb->expr()->gte('s.dateCreated', ':startDate')); - $qb->setParameter('startDate', $dateRange->startDate(), ChronosDateTimeType::CHRONOS_DATETIME); + $qb->setParameter('startDate', $dateRange->startDate, ChronosDateTimeType::CHRONOS_DATETIME); } - if ($dateRange?->endDate() !== null) { + if ($dateRange?->endDate !== null) { $qb->andWhere($qb->expr()->lte('s.dateCreated', ':endDate')); - $qb->setParameter('endDate', $dateRange->endDate(), ChronosDateTimeType::CHRONOS_DATETIME); + $qb->setParameter('endDate', $dateRange->endDate, ChronosDateTimeType::CHRONOS_DATETIME); } $searchTerm = $filtering->searchTerm(); diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index 3e33d60a..f24153fc 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -245,11 +245,11 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo { $conn = $this->getEntityManager()->getConnection(); - if ($dateRange?->startDate() !== null) { - $qb->andWhere($qb->expr()->gte('v.date', $conn->quote($dateRange->startDate()->toDateTimeString()))); + if ($dateRange?->startDate !== null) { + $qb->andWhere($qb->expr()->gte('v.date', $conn->quote($dateRange->startDate->toDateTimeString()))); } - if ($dateRange?->endDate() !== null) { - $qb->andWhere($qb->expr()->lte('v.date', $conn->quote($dateRange->endDate()->toDateTimeString()))); + if ($dateRange?->endDate !== null) { + $qb->andWhere($qb->expr()->lte('v.date', $conn->quote($dateRange->endDate->toDateTimeString()))); } } diff --git a/module/Core/src/Spec/InDateRange.php b/module/Core/src/Spec/InDateRange.php index 05cb6f0a..994e6d63 100644 --- a/module/Core/src/Spec/InDateRange.php +++ b/module/Core/src/Spec/InDateRange.php @@ -20,12 +20,12 @@ class InDateRange extends BaseSpecification { $criteria = []; - if ($this->dateRange?->startDate() !== null) { - $criteria[] = Spec::gte($this->field, $this->dateRange->startDate()->toDateTimeString()); + if ($this->dateRange?->startDate !== null) { + $criteria[] = Spec::gte($this->field, $this->dateRange->startDate->toDateTimeString()); } - if ($this->dateRange?->endDate() !== null) { - $criteria[] = Spec::lte($this->field, $this->dateRange->endDate()->toDateTimeString()); + if ($this->dateRange?->endDate !== null) { + $criteria[] = Spec::lte($this->field, $this->dateRange->endDate->toDateTimeString()); } return Spec::andX(...$criteria); diff --git a/module/Core/test/Config/MultiSegmentSlugProcessorTest.php b/module/Core/test/Config/MultiSegmentSlugProcessorTest.php new file mode 100644 index 00000000..630a5d90 --- /dev/null +++ b/module/Core/test/Config/MultiSegmentSlugProcessorTest.php @@ -0,0 +1,60 @@ +processor = new MultiSegmentSlugProcessor(); + } + + /** + * @test + * @dataProvider provideConfigs + */ + public function parsesRoutesAsExpected(array $config, array $expectedRoutes): void + { + self::assertEquals($expectedRoutes, ($this->processor)($config)['routes'] ?? []); + } + + public function provideConfigs(): iterable + { + yield [[], []]; + yield [['url_shortener' => []], []]; + yield [['url_shortener' => ['multi_segment_slugs_enabled' => false]], []]; + yield [ + [ + 'url_shortener' => ['multi_segment_slugs_enabled' => false], + 'routes' => $routes = [ + ['path' => '/foo'], + ['path' => '/bar/{shortCode}'], + ['path' => '/baz/{shortCode}/foo'], + ], + ], + $routes, + ]; + yield [ + [ + 'url_shortener' => ['multi_segment_slugs_enabled' => true], + 'routes' => [ + ['path' => '/foo'], + ['path' => '/bar/{shortCode}'], + ['path' => '/baz/{shortCode}/foo'], + ], + ], + [ + ['path' => '/foo'], + ['path' => '/bar/{shortCode:.+}'], + ['path' => '/baz/{shortCode:.+}/foo'], + ], + ]; + } +} diff --git a/module/Rest/src/ConfigProvider.php b/module/Rest/src/ConfigProvider.php index 3304ce4d..6d389038 100644 --- a/module/Rest/src/ConfigProvider.php +++ b/module/Rest/src/ConfigProvider.php @@ -8,7 +8,6 @@ use function Functional\first; use function Functional\map; use function Shlinkio\Shlink\Config\loadConfigFromGlob; use function sprintf; -use function str_replace; class ConfigProvider { @@ -21,16 +20,12 @@ class ConfigProvider return loadConfigFromGlob(__DIR__ . '/../config/{,*.}config.php'); } - public static function applyRoutesPrefix(array $routes, bool $multiSegmentEnabled): array + public static function applyRoutesPrefix(array $routes): array { $healthRoute = self::buildUnversionedHealthRouteFromExistingRoutes($routes); - $prefixedRoutes = map($routes, static function (array $route) use ($multiSegmentEnabled) { + $prefixedRoutes = map($routes, static function (array $route) { ['path' => $path] = $route; - if ($multiSegmentEnabled) { - $path = str_replace('{shortCode}', '{shortCode:.+}', $path); - } $route['path'] = sprintf('%s%s', self::ROUTES_PREFIX, $path); - return $route; }); diff --git a/module/Rest/test/ConfigProviderTest.php b/module/Rest/test/ConfigProviderTest.php index 07fa4e43..a3f7d0c9 100644 --- a/module/Rest/test/ConfigProviderTest.php +++ b/module/Rest/test/ConfigProviderTest.php @@ -33,9 +33,9 @@ class ConfigProviderTest extends TestCase * @test * @dataProvider provideRoutesConfig */ - public function routesAreProperlyPrefixed(array $routes, bool $multiSegmentEnabled, array $expected): void + public function routesAreProperlyPrefixed(array $routes, array $expected): void { - self::assertEquals($expected, ConfigProvider::applyRoutesPrefix($routes, $multiSegmentEnabled)); + self::assertEquals($expected, ConfigProvider::applyRoutesPrefix($routes)); } public function provideRoutesConfig(): iterable @@ -47,7 +47,6 @@ class ConfigProviderTest extends TestCase ['path' => '/baz/foo'], ['path' => '/health'], ], - false, [ ['path' => '/rest/v{version:1|2}/foo'], ['path' => '/rest/v{version:1|2}/bar'], @@ -62,25 +61,11 @@ class ConfigProviderTest extends TestCase ['path' => '/bar'], ['path' => '/baz/foo'], ], - false, [ ['path' => '/rest/v{version:1|2}/foo'], ['path' => '/rest/v{version:1|2}/bar'], ['path' => '/rest/v{version:1|2}/baz/foo'], ], ]; - yield 'multi-segment enabled' => [ - [ - ['path' => '/foo'], - ['path' => '/bar/{shortCode}'], - ['path' => '/baz/{shortCode}/foo'], - ], - true, - [ - ['path' => '/rest/v{version:1|2}/foo'], - ['path' => '/rest/v{version:1|2}/bar/{shortCode:.+}'], - ['path' => '/rest/v{version:1|2}/baz/{shortCode:.+}/foo'], - ], - ]; } } From 334aee64ad01782625c7b8f5bb7bbf740630e859 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 Aug 2022 09:37:15 +0200 Subject: [PATCH 2/7] Updated changelog --- CHANGELOG.md | 17 +++++++++++++++++ config/autoload/url-shortener.local.php.dist | 1 + 2 files changed, 18 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c11faf60..d32bb1b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). +## [Unreleased] +### Added +* *Nothing* + +### Changed +* [#1495](https://github.com/shlinkio/shlink/issues/1495) Centralized how routes are configured to support multi-segment slugs. + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* *Nothing* + + ## [3.2.0] - 2022-08-05 ### Added * [#854](https://github.com/shlinkio/shlink/issues/854) Added support for multi-segment custom slugs. diff --git a/config/autoload/url-shortener.local.php.dist b/config/autoload/url-shortener.local.php.dist index 20140a9b..0069ffa9 100644 --- a/config/autoload/url-shortener.local.php.dist +++ b/config/autoload/url-shortener.local.php.dist @@ -12,6 +12,7 @@ return [ 'hostname' => sprintf('localhost:%s', $isSwoole ? '8080' : '8000'), ], 'auto_resolve_titles' => true, +// 'multi_segment_slugs_enabled' => true, ], ]; From a03f32f521ef1aeea3fddc753a3ef01e3c409c9b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 Aug 2022 09:36:51 +0200 Subject: [PATCH 3/7] Updated to latest shlink dependencies --- composer.json | 14 +++--- module/Core/src/Entity/ShortUrl.php | 22 +++++----- module/Core/src/Entity/Visit.php | 8 ++-- module/Core/src/Entity/VisitLocation.php | 28 ++++++------ .../src/Exception/NonUniqueSlugException.php | 2 +- .../src/Importer/ImportedLinksProcessor.php | 24 +++++----- .../Core/src/Importer/ShortUrlImporting.php | 2 +- .../src/Repository/ShortUrlRepository.php | 6 +-- .../Repository/ShortUrlRepositoryTest.php | 3 +- module/Core/test/Entity/ShortUrlTest.php | 8 ++-- .../Importer/ImportedLinksProcessorTest.php | 44 +++++++++---------- 11 files changed, 81 insertions(+), 80 deletions(-) diff --git a/composer.json b/composer.json index a2e6c96c..27d7ec3b 100644 --- a/composer.json +++ b/composer.json @@ -43,12 +43,12 @@ "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.0", "ramsey/uuid": "^4.3", - "shlinkio/shlink-common": "dev-main#6163a03 as 5.0", - "shlinkio/shlink-config": "^1.6", - "shlinkio/shlink-event-dispatcher": "^2.4", - "shlinkio/shlink-importer": "^3.0", - "shlinkio/shlink-installer": "^8.0", - "shlinkio/shlink-ip-geolocation": "^2.2", + "shlinkio/shlink-common": "dev-main#5251f37 as 5.0", + "shlinkio/shlink-config": "^2.0", + "shlinkio/shlink-event-dispatcher": "^2.5", + "shlinkio/shlink-importer": "^4.0", + "shlinkio/shlink-installer": "dev-develop#c6032b1 as 8.1", + "shlinkio/shlink-ip-geolocation": "^3.0", "symfony/console": "^6.1", "symfony/filesystem": "^6.1", "symfony/lock": "^6.1", @@ -69,7 +69,7 @@ "phpunit/phpunit": "^9.5", "roave/security-advisories": "dev-master", "shlinkio/php-coding-standard": "~2.3.0", - "shlinkio/shlink-test-utils": "^3.0.1", + "shlinkio/shlink-test-utils": "^3.1.0", "symfony/var-dumper": "^6.1", "veewee/composer-run-parallel": "^1.1" }, diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index 28c4c446..6a146372 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -94,31 +94,31 @@ class ShortUrl extends AbstractEntity ): self { $meta = [ ShortUrlInputFilter::VALIDATE_URL => false, - ShortUrlInputFilter::LONG_URL => $url->longUrl(), - ShortUrlInputFilter::DOMAIN => $url->domain(), - ShortUrlInputFilter::TAGS => $url->tags(), - ShortUrlInputFilter::TITLE => $url->title(), - ShortUrlInputFilter::MAX_VISITS => $url->meta()->maxVisits(), + ShortUrlInputFilter::LONG_URL => $url->longUrl, + ShortUrlInputFilter::DOMAIN => $url->domain, + ShortUrlInputFilter::TAGS => $url->tags, + ShortUrlInputFilter::TITLE => $url->title, + ShortUrlInputFilter::MAX_VISITS => $url->meta->maxVisits, ]; if ($importShortCode) { - $meta[ShortUrlInputFilter::CUSTOM_SLUG] = $url->shortCode(); + $meta[ShortUrlInputFilter::CUSTOM_SLUG] = $url->shortCode; } $instance = self::fromMeta(ShortUrlMeta::fromRawData($meta), $relationResolver); - $validSince = $url->meta()->validSince(); + $validSince = $url->meta->validSince; if ($validSince !== null) { $instance->validSince = Chronos::instance($validSince); } - $validUntil = $url->meta()->validUntil(); + $validUntil = $url->meta->validUntil; if ($validUntil !== null) { $instance->validUntil = Chronos::instance($validUntil); } - $instance->importSource = $url->source(); - $instance->importOriginalShortCode = $url->shortCode(); - $instance->dateCreated = Chronos::instance($url->createdAt()); + $instance->importSource = $url->source->value; + $instance->importOriginalShortCode = $url->shortCode; + $instance->dateCreated = Chronos::instance($url->createdAt); return $instance; } diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index 9bff9db9..fd53ab9b 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -45,12 +45,12 @@ class Visit extends AbstractEntity implements JsonSerializable public static function fromImport(ShortUrl $shortUrl, ImportedShlinkVisit $importedVisit): self { $instance = new self($shortUrl, VisitType::IMPORTED); - $instance->userAgent = $importedVisit->userAgent(); + $instance->userAgent = $importedVisit->userAgent; $instance->potentialBot = isCrawler($instance->userAgent); - $instance->referer = $importedVisit->referer(); - $instance->date = Chronos::instance($importedVisit->date()); + $instance->referer = $importedVisit->referer; + $instance->date = Chronos::instance($importedVisit->date); - $importedLocation = $importedVisit->location(); + $importedLocation = $importedVisit->location; $instance->visitLocation = $importedLocation !== null ? VisitLocation::fromImport($importedLocation) : null; return $instance; diff --git a/module/Core/src/Entity/VisitLocation.php b/module/Core/src/Entity/VisitLocation.php index 239338d6..bcd8e95d 100644 --- a/module/Core/src/Entity/VisitLocation.php +++ b/module/Core/src/Entity/VisitLocation.php @@ -28,13 +28,13 @@ class VisitLocation extends AbstractEntity implements JsonSerializable { $instance = new self(); - $instance->countryCode = $location->countryCode(); - $instance->countryName = $location->countryName(); - $instance->regionName = $location->regionName(); - $instance->cityName = $location->city(); - $instance->latitude = $location->latitude(); - $instance->longitude = $location->longitude(); - $instance->timezone = $location->timeZone(); + $instance->countryCode = $location->countryCode; + $instance->countryName = $location->countryName; + $instance->regionName = $location->regionName; + $instance->cityName = $location->city; + $instance->latitude = $location->latitude; + $instance->longitude = $location->longitude; + $instance->timezone = $location->timeZone; $instance->computeIsEmpty(); return $instance; @@ -44,13 +44,13 @@ class VisitLocation extends AbstractEntity implements JsonSerializable { $instance = new self(); - $instance->countryCode = $location->countryCode(); - $instance->countryName = $location->countryName(); - $instance->regionName = $location->regionName(); - $instance->cityName = $location->cityName(); - $instance->latitude = $location->latitude(); - $instance->longitude = $location->longitude(); - $instance->timezone = $location->timeZone(); + $instance->countryCode = $location->countryCode; + $instance->countryName = $location->countryName; + $instance->regionName = $location->regionName; + $instance->cityName = $location->cityName; + $instance->latitude = $location->latitude; + $instance->longitude = $location->longitude; + $instance->timezone = $location->timezone; $instance->computeIsEmpty(); return $instance; diff --git a/module/Core/src/Exception/NonUniqueSlugException.php b/module/Core/src/Exception/NonUniqueSlugException.php index 3b8b0b15..f61c480f 100644 --- a/module/Core/src/Exception/NonUniqueSlugException.php +++ b/module/Core/src/Exception/NonUniqueSlugException.php @@ -38,6 +38,6 @@ class NonUniqueSlugException extends InvalidArgumentException implements Problem public static function fromImport(ImportedShlinkUrl $importedUrl): self { - return self::fromSlug($importedUrl->shortCode(), $importedUrl->domain()); + return self::fromSlug($importedUrl->shortCode, $importedUrl->domain); } } diff --git a/module/Core/src/Importer/ImportedLinksProcessor.php b/module/Core/src/Importer/ImportedLinksProcessor.php index 6b33c586..f5818252 100644 --- a/module/Core/src/Importer/ImportedLinksProcessor.php +++ b/module/Core/src/Importer/ImportedLinksProcessor.php @@ -14,7 +14,7 @@ use Shlinkio\Shlink\Core\Util\DoctrineBatchHelperInterface; use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Importer\Params\ImportParams; -use Shlinkio\Shlink\Importer\Sources\ImportSources; +use Shlinkio\Shlink\Importer\Sources\ImportSource; use Symfony\Component\Console\Style\OutputStyle; use Symfony\Component\Console\Style\StyleInterface; use Throwable; @@ -26,10 +26,10 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface private ShortUrlRepositoryInterface $shortUrlRepo; public function __construct( - private EntityManagerInterface $em, - private ShortUrlRelationResolverInterface $relationResolver, - private ShortCodeUniquenessHelperInterface $shortCodeHelper, - private DoctrineBatchHelperInterface $batchHelper, + private readonly EntityManagerInterface $em, + private readonly ShortUrlRelationResolverInterface $relationResolver, + private readonly ShortCodeUniquenessHelperInterface $shortCodeHelper, + private readonly DoctrineBatchHelperInterface $batchHelper, ) { $this->shortUrlRepo = $this->em->getRepository(ShortUrl::class); } @@ -39,19 +39,19 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface */ public function process(StyleInterface $io, iterable $shlinkUrls, ImportParams $params): void { - $importShortCodes = $params->importShortCodes(); - $source = $params->source(); - $iterable = $this->batchHelper->wrapIterable($shlinkUrls, $source === ImportSources::SHLINK ? 10 : 100); + $importShortCodes = $params->importShortCodes; + $source = $params->source; + $iterable = $this->batchHelper->wrapIterable($shlinkUrls, $source === ImportSource::SHLINK ? 10 : 100); /** @var ImportedShlinkUrl $importedUrl */ foreach ($iterable as $importedUrl) { $skipOnShortCodeConflict = static fn (): bool => $io->choice(sprintf( 'Failed to import URL "%s" because its short-code "%s" is already in use. Do you want to generate ' . 'a new one or skip it?', - $importedUrl->longUrl(), - $importedUrl->shortCode(), + $importedUrl->longUrl, + $importedUrl->shortCode, ), ['Generate new short-code', 'Skip'], 1) === 'Skip'; - $longUrl = $importedUrl->longUrl(); + $longUrl = $importedUrl->longUrl; try { $shortUrlImporting = $this->resolveShortUrl($importedUrl, $importShortCodes, $skipOnShortCodeConflict); @@ -68,7 +68,7 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface continue; } - $resultMessage = $shortUrlImporting->importVisits($importedUrl->visits(), $this->em); + $resultMessage = $shortUrlImporting->importVisits($importedUrl->visits, $this->em); $io->text(sprintf('%s: %s', $longUrl, $resultMessage)); } } diff --git a/module/Core/src/Importer/ShortUrlImporting.php b/module/Core/src/Importer/ShortUrlImporting.php index 4aa87166..60209fa1 100644 --- a/module/Core/src/Importer/ShortUrlImporting.php +++ b/module/Core/src/Importer/ShortUrlImporting.php @@ -38,7 +38,7 @@ final class ShortUrlImporting $importedVisits = 0; foreach ($visits as $importedVisit) { // Skip visits which are older than the most recent already imported visit's date - if ($mostRecentImportedDate?->gte(Chronos::instance($importedVisit->date()))) { + if ($mostRecentImportedDate?->gte(Chronos::instance($importedVisit->date))) { continue; } diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index af4f662d..406d2b80 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -284,12 +284,12 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU { $qb = $this->createQueryBuilder('s'); $qb->andWhere($qb->expr()->eq('s.importOriginalShortCode', ':shortCode')) - ->setParameter('shortCode', $url->shortCode()) + ->setParameter('shortCode', $url->shortCode) ->andWhere($qb->expr()->eq('s.importSource', ':importSource')) - ->setParameter('importSource', $url->source()) + ->setParameter('importSource', $url->source->value) ->setMaxResults(1); - $this->whereDomainIs($qb, $url->domain()); + $this->whereDomainIs($qb, $url->domain); return $qb->getQuery()->getOneOrNullResult(); } diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index 3143e90c..b538f422 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -21,6 +21,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; +use Shlinkio\Shlink\Importer\Sources\ImportSource; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -601,7 +602,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase public function importedShortUrlsAreFoundWhenExpected(): void { $buildImported = static fn (string $shortCode, ?String $domain = null) => - new ImportedShlinkUrl('', 'foo', [], Chronos::now(), $domain, $shortCode, null); + new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), $domain, $shortCode, null); $shortUrlWithoutDomain = ShortUrl::fromImport($buildImported('my-cool-slug'), true); $this->getEntityManager()->persist($shortUrlWithoutDomain); diff --git a/module/Core/test/Entity/ShortUrlTest.php b/module/Core/test/Entity/ShortUrlTest.php index d41357cd..b55281ab 100644 --- a/module/Core/test/Entity/ShortUrlTest.php +++ b/module/Core/test/Entity/ShortUrlTest.php @@ -11,6 +11,7 @@ use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Validation\ShortUrlInputFilter; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; +use Shlinkio\Shlink\Importer\Sources\ImportSource; use function Functional\map; use function range; @@ -63,9 +64,10 @@ class ShortUrlTest extends TestCase public function provideValidShortUrls(): iterable { yield 'no custom slug' => [ShortUrl::createEmpty()]; - yield 'imported with custom slug' => [ - ShortUrl::fromImport(new ImportedShlinkUrl('', '', [], Chronos::now(), null, 'custom-slug', null), true), - ]; + yield 'imported with custom slug' => [ShortUrl::fromImport( + new ImportedShlinkUrl(ImportSource::BITLY, '', [], Chronos::now(), null, 'custom-slug', null), + true, + )]; } /** diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index 341b32bc..29111928 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -22,7 +22,7 @@ use Shlinkio\Shlink\Core\Util\DoctrineBatchHelperInterface; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisit; use Shlinkio\Shlink\Importer\Params\ImportParams; -use Shlinkio\Shlink\Importer\Sources\ImportSources; +use Shlinkio\Shlink\Importer\Sources\ImportSource; use Symfony\Component\Console\Style\StyleInterface; use function count; @@ -64,9 +64,9 @@ class ImportedLinksProcessorTest extends TestCase public function newUrlsWithNoErrorsAreAllPersisted(): void { $urls = [ - new ImportedShlinkUrl('', 'foo', [], Chronos::now(), null, 'foo', null), - new ImportedShlinkUrl('', 'bar', [], Chronos::now(), null, 'bar', 'foo'), - new ImportedShlinkUrl('', 'baz', [], Chronos::now(), null, 'baz', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), null, 'foo', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'bar', [], Chronos::now(), null, 'bar', 'foo'), + new ImportedShlinkUrl(ImportSource::BITLY, 'baz', [], Chronos::now(), null, 'baz', null), ]; $expectedCalls = count($urls); @@ -86,9 +86,9 @@ class ImportedLinksProcessorTest extends TestCase public function newUrlsWithErrorsAreSkipped(): void { $urls = [ - new ImportedShlinkUrl('', 'foo', [], Chronos::now(), null, 'foo', null), - new ImportedShlinkUrl('', 'bar', [], Chronos::now(), null, 'bar', 'foo'), - new ImportedShlinkUrl('', 'baz', [], Chronos::now(), null, 'baz', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), null, 'foo', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'bar', [], Chronos::now(), null, 'bar', 'foo'), + new ImportedShlinkUrl(ImportSource::BITLY, 'baz', [], Chronos::now(), null, 'baz', null), ]; $importedUrlExists = $this->repo->findOneByImportedUrl(Argument::cetera())->willReturn(null); @@ -117,11 +117,11 @@ class ImportedLinksProcessorTest extends TestCase public function alreadyImportedUrlsAreSkipped(): void { $urls = [ - new ImportedShlinkUrl('', 'foo', [], Chronos::now(), null, 'foo', null), - new ImportedShlinkUrl('', 'bar', [], Chronos::now(), null, 'bar', null), - new ImportedShlinkUrl('', 'baz', [], Chronos::now(), null, 'baz', null), - new ImportedShlinkUrl('', 'baz2', [], Chronos::now(), null, 'baz2', null), - new ImportedShlinkUrl('', 'baz3', [], Chronos::now(), null, 'baz3', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), null, 'foo', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'bar', [], Chronos::now(), null, 'bar', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'baz', [], Chronos::now(), null, 'baz', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'baz2', [], Chronos::now(), null, 'baz2', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'baz3', [], Chronos::now(), null, 'baz3', null), ]; $importedUrlExists = $this->repo->findOneByImportedUrl(Argument::cetera())->will( @@ -129,7 +129,7 @@ class ImportedLinksProcessorTest extends TestCase /** @var ImportedShlinkUrl $url */ [$url] = $args; - return contains(['foo', 'baz2', 'baz3'], $url->longUrl()) ? ShortUrl::fromImport($url, true) : null; + return contains(['foo', 'baz2', 'baz3'], $url->longUrl) ? ShortUrl::fromImport($url, true) : null; }, ); $ensureUniqueness = $this->shortCodeHelper->ensureShortCodeUniqueness(Argument::cetera())->willReturn(true); @@ -148,11 +148,11 @@ class ImportedLinksProcessorTest extends TestCase public function nonUniqueShortCodesAreAskedToUser(): void { $urls = [ - new ImportedShlinkUrl('', 'foo', [], Chronos::now(), null, 'foo', null), - new ImportedShlinkUrl('', 'bar', [], Chronos::now(), null, 'bar', null), - new ImportedShlinkUrl('', 'baz', [], Chronos::now(), null, 'baz', 'foo'), - new ImportedShlinkUrl('', 'baz2', [], Chronos::now(), null, 'baz2', null), - new ImportedShlinkUrl('', 'baz3', [], Chronos::now(), null, 'baz3', 'bar'), + new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), null, 'foo', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'bar', [], Chronos::now(), null, 'bar', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'baz', [], Chronos::now(), null, 'baz', 'foo'), + new ImportedShlinkUrl(ImportSource::BITLY, 'baz2', [], Chronos::now(), null, 'baz2', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'baz3', [], Chronos::now(), null, 'baz3', 'bar'), ]; $importedUrlExists = $this->repo->findOneByImportedUrl(Argument::cetera())->willReturn(null); @@ -210,7 +210,8 @@ class ImportedLinksProcessorTest extends TestCase public function provideUrlsWithVisits(): iterable { $now = Chronos::now(); - $createImportedUrl = fn (array $visits) => new ImportedShlinkUrl('', 's', [], $now, null, 's', null, $visits); + $createImportedUrl = static fn (array $visits) => + new ImportedShlinkUrl(ImportSource::BITLY, 's', [], $now, null, 's', null, $visits); yield 'new short URL' => [$createImportedUrl([ new ImportedShlinkVisit('', '', $now, null), @@ -248,9 +249,6 @@ class ImportedLinksProcessorTest extends TestCase private function buildParams(): ImportParams { - return ImportParams::fromSourceAndCallableMap( - ImportSources::BITLY, - ['import_short_codes' => static fn () => true], - ); + return ImportSource::BITLY->toParamsWithCallableMap(['import_short_codes' => static fn () => true]); } } From b116a57aa7f13fa578777a57ca4cec235bccf58a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 Aug 2022 09:37:49 +0200 Subject: [PATCH 4/7] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d32bb1b9..cb4bfa6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Changed * [#1495](https://github.com/shlinkio/shlink/issues/1495) Centralized how routes are configured to support multi-segment slugs. +* [#1497](https://github.com/shlinkio/shlink/issues/1497) Updated to latest shlink dependencies with support for PHP 8.1 only. ### Deprecated * *Nothing* From a2f9742cfcd9858462b41171b272df7790c3a241 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 8 Aug 2022 19:23:16 +0200 Subject: [PATCH 5/7] Fix loading of config options as env vars --- config/config.php | 2 +- module/Core/src/Config/EnvVars.php | 10 ++++++++++ module/Core/test/Config/EnvVarsTest.php | 8 ++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/config/config.php b/config/config.php index 201c1a17..6c38707d 100644 --- a/config/config.php +++ b/config/config.php @@ -21,7 +21,7 @@ $isTestEnv = env('APP_ENV') === 'test'; return (new ConfigAggregator\ConfigAggregator([ ! $isTestEnv - ? new EnvVarLoaderProvider('config/params/generated_config.php', Core\Config\EnvVars::cases()) + ? new EnvVarLoaderProvider('config/params/generated_config.php', Core\Config\EnvVars::values()) : new ConfigAggregator\ArrayProvider([]), Mezzio\ConfigProvider::class, Mezzio\Router\ConfigProvider::class, diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 8f8689be..ae93e4da 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Config; +use function Functional\map; use function Shlinkio\Shlink\Config\env; enum EnvVars: string @@ -74,4 +75,13 @@ enum EnvVars: string { return $this->loadFromEnv() !== null; } + + /** + * @return string[] + */ + public static function values(): array + { + static $values; + return $values ?? ($values = map(self::cases(), static fn (EnvVars $envVar) => $envVar->value)); + } } diff --git a/module/Core/test/Config/EnvVarsTest.php b/module/Core/test/Config/EnvVarsTest.php index 6d4b1394..ff4878de 100644 --- a/module/Core/test/Config/EnvVarsTest.php +++ b/module/Core/test/Config/EnvVarsTest.php @@ -7,6 +7,7 @@ namespace ShlinkioTest\Shlink\Core\Config; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Config\EnvVars; +use function Functional\map; use function putenv; class EnvVarsTest extends TestCase @@ -58,4 +59,11 @@ class EnvVarsTest extends TestCase yield 'DB_DRIVER without default' => [EnvVars::DB_DRIVER, null, null]; yield 'DB_DRIVER with default' => [EnvVars::DB_DRIVER, 'foobar', 'foobar']; } + + /** @test */ + public function allValuesCanBeListed(): void + { + $expected = map(EnvVars::cases(), static fn (EnvVars $envVar) => $envVar->value); + self::assertEquals(EnvVars::values(), $expected); + } } From 23138dc0b4195662d57ba5cab86ce4a910f5074c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 8 Aug 2022 19:24:51 +0200 Subject: [PATCH 6/7] Updated changelog --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb4bfa6f..71842087 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## [3.2.1] - 2022-08-08 ### Added * *Nothing* @@ -19,7 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * *Nothing* ### Fixed -* *Nothing* +* [#1499](https://github.com/shlinkio/shlink/issues/1499) Fixed loading of config options as env vars, which was making all default configurations to be loaded unless env vars were explicitly provided. ## [3.2.0] - 2022-08-05 From bd2cd18916d62036caae56b078851589454c97e5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 8 Aug 2022 19:33:59 +0200 Subject: [PATCH 7/7] Tagged stable releases for all shlink teps --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 27d7ec3b..3cba02fe 100644 --- a/composer.json +++ b/composer.json @@ -43,11 +43,11 @@ "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.0", "ramsey/uuid": "^4.3", - "shlinkio/shlink-common": "dev-main#5251f37 as 5.0", + "shlinkio/shlink-common": "^5.0", "shlinkio/shlink-config": "^2.0", "shlinkio/shlink-event-dispatcher": "^2.5", "shlinkio/shlink-importer": "^4.0", - "shlinkio/shlink-installer": "dev-develop#c6032b1 as 8.1", + "shlinkio/shlink-installer": "^8.1", "shlinkio/shlink-ip-geolocation": "^3.0", "symfony/console": "^6.1", "symfony/filesystem": "^6.1",