From f801f265ed3615d51faa5be725303cf33a75a5c4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 25 Jul 2022 22:48:41 +0200 Subject: [PATCH 01/16] Added comments on places to change --- config/autoload/middleware-pipeline.global.php | 1 + module/Core/src/Visit/RequestTracker.php | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index c628c4fd..a5446671 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -68,6 +68,7 @@ return [ // This middleware is in front of tracking actions explicitly. Putting here for orphan visits tracking IpAddress::class, Core\ErrorHandler\NotFoundTypeResolverMiddleware::class, + // TODO MultiSegmentCustomSlugRedirectMiddleware Core\ShortUrl\Middleware\ExtraPathRedirectMiddleware::class, Core\ErrorHandler\NotFoundTrackerMiddleware::class, Core\ErrorHandler\NotFoundRedirectHandler::class, diff --git a/module/Core/src/Visit/RequestTracker.php b/module/Core/src/Visit/RequestTracker.php index dc45e12f..3177d08c 100644 --- a/module/Core/src/Visit/RequestTracker.php +++ b/module/Core/src/Visit/RequestTracker.php @@ -45,10 +45,11 @@ class RequestTracker implements RequestTrackerInterface, RequestMethodInterface $notFoundType = $request->getAttribute(NotFoundType::class); $visitor = Visitor::fromRequest($request); - match (true) { // @phpstan-ignore-line + match (true) { $notFoundType?->isBaseUrl() => $this->visitsTracker->trackBaseUrlVisit($visitor), $notFoundType?->isRegularNotFound() => $this->visitsTracker->trackRegularNotFoundVisit($visitor), $notFoundType?->isInvalidShortUrl() => $this->visitsTracker->trackInvalidShortUrlVisit($visitor), + default => null, }; } From d375dece0e33622093734a4aedfd0e70e186ac19 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 29 Jul 2022 12:04:30 +0200 Subject: [PATCH 02/16] Updated required deps --- composer.json | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/composer.json b/composer.json index 9add38d7..38a4f960 100644 --- a/composer.json +++ b/composer.json @@ -17,37 +17,37 @@ "ext-pdo": "*", "akrabat/ip-address-middleware": "^2.1", "cakephp/chronos": "^2.3", - "doctrine/migrations": "^3.3", - "doctrine/orm": "^2.11", + "doctrine/migrations": "^3.5", + "doctrine/orm": "^2.12", "endroid/qr-code": "^4.4", "geoip2/geoip2": "^2.12", "guzzlehttp/guzzle": "^7.4", "happyr/doctrine-specification": "^2.0", "jaybizzle/crawler-detect": "^1.2.110", "laminas/laminas-config": "^3.7", - "laminas/laminas-config-aggregator": "^1.7", - "laminas/laminas-diactoros": "^2.8", - "laminas/laminas-inputfilter": "^2.13", - "laminas/laminas-servicemanager": "^3.11.2", - "laminas/laminas-stdlib": "^3.6", + "laminas/laminas-config-aggregator": "^1.8", + "laminas/laminas-diactoros": "^2.14", + "laminas/laminas-inputfilter": "^2.19", + "laminas/laminas-servicemanager": "^3.16", + "laminas/laminas-stdlib": "^3.11", "lcobucci/jwt": "^4.1", - "league/uri": "^6.4", + "league/uri": "^6.7", "lstrojny/functional-php": "^1.17", - "mezzio/mezzio": "^3.7", - "mezzio/mezzio-fastroute": "^3.3", - "mezzio/mezzio-problem-details": "^1.5", + "mezzio/mezzio": "^3.11", + "mezzio/mezzio-fastroute": "^3.5", + "mezzio/mezzio-problem-details": "^1.6", "mezzio/mezzio-swoole": "^4.3", - "mlocati/ip-lib": "^1.17", - "ocramius/proxy-manager": "^2.11", - "pagerfanta/core": "^3.5", + "mlocati/ip-lib": "^1.18", + "ocramius/proxy-manager": "^2.14", + "pagerfanta/core": "^3.6", "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.0", - "ramsey/uuid": "^4.2", + "ramsey/uuid": "^4.3", "shlinkio/shlink-common": "dev-main#b3848ad as 4.5", "shlinkio/shlink-config": "^1.6", "shlinkio/shlink-event-dispatcher": "^2.4", "shlinkio/shlink-importer": "^3.0", - "shlinkio/shlink-installer": "dev-develop#f76e9aa as 7.2", + "shlinkio/shlink-installer": "dev-develop#8a44b6e as 8.0", "shlinkio/shlink-ip-geolocation": "^2.2", "symfony/console": "^6.1", "symfony/filesystem": "^6.1", @@ -62,9 +62,9 @@ "infection/infection": "^0.26.5", "openswoole/ide-helper": "~4.11.1", "phpspec/prophecy-phpunit": "^2.0", - "phpstan/phpstan": "^1.2", - "phpstan/phpstan-doctrine": "^1.0", - "phpstan/phpstan-symfony": "^1.0", + "phpstan/phpstan": "^1.8", + "phpstan/phpstan-doctrine": "^1.3", + "phpstan/phpstan-symfony": "^1.2", "phpunit/php-code-coverage": "^9.2", "phpunit/phpunit": "^9.5", "roave/security-advisories": "dev-master", From e0e511f56d8232f52551c05eec32e66db648df68 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 29 Jul 2022 17:02:00 +0200 Subject: [PATCH 03/16] Some improvements and comments in preparation of multi-segment slugs --- config/autoload/installer.global.php | 6 +++--- config/autoload/middleware-pipeline.global.php | 2 +- module/Core/src/ErrorHandler/Model/NotFoundType.php | 6 +++--- .../Middleware/ExtraPathRedirectMiddleware.php | 13 +++++++------ module/Core/src/Visit/RequestTracker.php | 6 ++++-- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index c82b4a97..8a452d92 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -65,13 +65,13 @@ return [ ], 'installation_commands' => [ - InstallationCommand::DB_CREATE_SCHEMA => [ + InstallationCommand::DB_CREATE_SCHEMA->value => [ 'command' => 'bin/cli ' . Command\Db\CreateDatabaseCommand::NAME, ], - InstallationCommand::DB_MIGRATE => [ + InstallationCommand::DB_MIGRATE->value => [ 'command' => 'bin/cli ' . Command\Db\MigrateDatabaseCommand::NAME, ], - InstallationCommand::GEOLITE_DOWNLOAD_DB => [ + InstallationCommand::GEOLITE_DOWNLOAD_DB->value => [ 'command' => 'bin/cli ' . Command\Visit\DownloadGeoLiteDbCommand::NAME, ], ], diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index a5446671..9c0e2978 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -68,7 +68,7 @@ return [ // This middleware is in front of tracking actions explicitly. Putting here for orphan visits tracking IpAddress::class, Core\ErrorHandler\NotFoundTypeResolverMiddleware::class, - // TODO MultiSegmentCustomSlugRedirectMiddleware + // TODO MultiSegmentSlugRedirectMiddleware Core\ShortUrl\Middleware\ExtraPathRedirectMiddleware::class, Core\ErrorHandler\NotFoundTrackerMiddleware::class, Core\ErrorHandler\NotFoundRedirectHandler::class, diff --git a/module/Core/src/ErrorHandler/Model/NotFoundType.php b/module/Core/src/ErrorHandler/Model/NotFoundType.php index f95368cb..99f7fbe6 100644 --- a/module/Core/src/ErrorHandler/Model/NotFoundType.php +++ b/module/Core/src/ErrorHandler/Model/NotFoundType.php @@ -13,21 +13,21 @@ use function rtrim; class NotFoundType { - private function __construct(private readonly VisitType $type) + private function __construct(private readonly ?VisitType $type) { } public static function fromRequest(ServerRequestInterface $request, string $basePath): self { /** @var RouteResult $routeResult */ - $routeResult = $request->getAttribute(RouteResult::class, RouteResult::fromRouteFailure(null)); + $routeResult = $request->getAttribute(RouteResult::class) ?? RouteResult::fromRouteFailure(null); $isBaseUrl = rtrim($request->getUri()->getPath(), '/') === $basePath; $type = match (true) { $isBaseUrl => VisitType::BASE_URL, $routeResult->isFailure() => VisitType::REGULAR_404, $routeResult->getMatchedRouteName() === RedirectAction::class => VisitType::INVALID_SHORT_URL, - default => VisitType::VALID_SHORT_URL, + default => null, }; return new self($type); diff --git a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php index 9d92067c..2704cadb 100644 --- a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php @@ -26,11 +26,11 @@ use function trim; class ExtraPathRedirectMiddleware implements MiddlewareInterface { public function __construct( - private ShortUrlResolverInterface $resolver, - private RequestTrackerInterface $requestTracker, - private ShortUrlRedirectionBuilderInterface $redirectionBuilder, - private RedirectResponseHelperInterface $redirectResponseHelper, - private UrlShortenerOptions $urlShortenerOptions, + private readonly ShortUrlResolverInterface $resolver, + private readonly RequestTrackerInterface $requestTracker, + private readonly ShortUrlRedirectionBuilderInterface $redirectionBuilder, + private readonly RedirectResponseHelperInterface $redirectResponseHelper, + private readonly UrlShortenerOptions $urlShortenerOptions, ) { } @@ -39,7 +39,7 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface /** @var NotFoundType|null $notFoundType */ $notFoundType = $request->getAttribute(NotFoundType::class); - // We'll apply this logic only if actively opted in and current URL is potentially /{shortCode}/[...] + // This logic is applied only if actively opted in and current URL is potentially /{shortCode}/[...] if (! $notFoundType?->isRegularNotFound() || ! $this->urlShortenerOptions->appendExtraPath()) { return $handler->handle($request); } @@ -50,6 +50,7 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($potentialShortCode, $uri->getAuthority()); try { + // TODO Try pieces of the URL in order to match multi-segment slugs too $shortUrl = $this->resolver->resolveEnabledShortUrl($identifier); $this->requestTracker->trackIfApplicable($shortUrl, $request); diff --git a/module/Core/src/Visit/RequestTracker.php b/module/Core/src/Visit/RequestTracker.php index 3177d08c..1887dbfd 100644 --- a/module/Core/src/Visit/RequestTracker.php +++ b/module/Core/src/Visit/RequestTracker.php @@ -24,8 +24,10 @@ use function str_contains; class RequestTracker implements RequestTrackerInterface, RequestMethodInterface { - public function __construct(private VisitsTrackerInterface $visitsTracker, private TrackingOptions $trackingOptions) - { + public function __construct( + private readonly VisitsTrackerInterface $visitsTracker, + private readonly TrackingOptions $trackingOptions, + ) { } public function trackIfApplicable(ShortUrl $shortUrl, ServerRequestInterface $request): void From 0a220bbc7aad0ffab647b9d8d6bbe6e6ed324f50 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 1 Aug 2022 10:12:08 +0200 Subject: [PATCH 04/16] Allowed slashes on custom slugs during short URL creation --- module/Core/src/Validation/ShortUrlInputFilter.php | 3 ++- module/Core/test/Model/ShortUrlMetaTest.php | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/module/Core/src/Validation/ShortUrlInputFilter.php b/module/Core/src/Validation/ShortUrlInputFilter.php index 6cd578fb..d4d0f818 100644 --- a/module/Core/src/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/Validation/ShortUrlInputFilter.php @@ -13,6 +13,7 @@ use Shlinkio\Shlink\Common\Validation; use Shlinkio\Shlink\Rest\Entity\ApiKey; use function is_string; +use function ltrim; use function str_replace; use function substr; @@ -77,7 +78,7 @@ class ShortUrlInputFilter extends InputFilter // empty, is by using the deprecated setContinueIfEmpty $customSlug = $this->createInput(self::CUSTOM_SLUG, false)->setContinueIfEmpty(true); $customSlug->getFilterChain()->attach(new Filter\Callback( - static fn (mixed $value) => is_string($value) ? str_replace([' ', '/'], '-', $value) : $value, + static fn (mixed $value) => is_string($value) ? ltrim(str_replace(' ', '-', $value), '/') : $value, )); $customSlug->getValidatorChain()->attach(new Validator\NotEmpty([ Validator\NotEmpty::STRING, diff --git a/module/Core/test/Model/ShortUrlMetaTest.php b/module/Core/test/Model/ShortUrlMetaTest.php index 1933b3b6..833a25bb 100644 --- a/module/Core/test/Model/ShortUrlMetaTest.php +++ b/module/Core/test/Model/ShortUrlMetaTest.php @@ -103,7 +103,8 @@ class ShortUrlMetaTest extends TestCase yield ['foo bar', 'foo-bar']; yield ['foo bar baz', 'foo-bar-baz']; yield ['foo bar-baz', 'foo-bar-baz']; - yield ['foo/bar/baz', 'foo-bar-baz']; + yield ['foo/bar/baz', 'foo/bar/baz']; + yield ['/foo/bar/baz', 'foo/bar/baz']; yield ['wp-admin.php', 'wp-admin.php']; yield ['UPPER_lower', 'UPPER_lower']; yield ['more~url_special.chars', 'more~url_special.chars']; From a570ce202a150ba7ceb65d7daa3e524ffbb8fbea Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 3 Aug 2022 12:59:09 +0200 Subject: [PATCH 05/16] Updated to latest common --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 38a4f960..60b95c0e 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": "dev-main#b3848ad as 4.5", + "shlinkio/shlink-common": "dev-main#b395e99 as 4.5", "shlinkio/shlink-config": "^1.6", "shlinkio/shlink-event-dispatcher": "^2.4", "shlinkio/shlink-importer": "^3.0", From fdd3e249676bfd35c98540a0ba985a558a013321 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 3 Aug 2022 19:32:59 +0200 Subject: [PATCH 06/16] Added support for multi-segment slugs --- UPGRADE.md | 2 +- .../autoload/middleware-pipeline.global.php | 1 - config/config.php | 5 +++-- module/Core/config/routes.config.php | 22 +++++++++---------- module/Rest/config/routes.config.php | 16 +++++++------- .../Action/ShortUrl/DeleteShortUrlAction.php | 2 +- .../Action/ShortUrl/EditShortUrlAction.php | 4 ++-- .../Action/ShortUrl/ResolveShortUrlAction.php | 2 +- .../src/Action/Visit/ShortUrlVisitsAction.php | 2 +- 9 files changed, 28 insertions(+), 28 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index bce1bdde..6bef9dbc 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -76,7 +76,7 @@ These routes have been removed, but have a direct replacement: * `/qr/{shortCode}[/{size}]` -> `/{shortCode}/qr-code[/{size}]` * `PUT /rest/v{version}/short-urls/{shortCode}` -> `PATCH /rest/v{version}/short-urls/{shortCode}` -When using the old ones, a 404 status will me returned now. +When using the old ones, a 404 status will be returned now. ### Removed command and route aliases diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index 9c0e2978..c628c4fd 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -68,7 +68,6 @@ return [ // This middleware is in front of tracking actions explicitly. Putting here for orphan visits tracking IpAddress::class, Core\ErrorHandler\NotFoundTypeResolverMiddleware::class, - // TODO MultiSegmentSlugRedirectMiddleware Core\ShortUrl\Middleware\ExtraPathRedirectMiddleware::class, Core\ErrorHandler\NotFoundTrackerMiddleware::class, Core\ErrorHandler\NotFoundRedirectHandler::class, diff --git a/config/config.php b/config/config.php index 3dad2105..19e3a82d 100644 --- a/config/config.php +++ b/config/config.php @@ -36,11 +36,12 @@ return (new ConfigAggregator\ConfigAggregator([ Importer\ConfigProvider::class, IpGeolocation\ConfigProvider::class, EventDispatcher\ConfigProvider::class, - Core\ConfigProvider::class, CLI\ConfigProvider::class, - Rest\ConfigProvider::class, + Rest\ConfigProvider::class, // Load rest before Core, to prevent conflicting routes when multi-segment is enabled + Core\ConfigProvider::class, new ConfigAggregator\PhpFileProvider('config/autoload/{{,*.}global,{,*.}local}.php'), $isTestEnv + // TODO Test routes must be loaded before core config ? new ConfigAggregator\PhpFileProvider('config/test/*.global.php') : new ConfigAggregator\ArrayProvider([]), ], 'data/cache/app_config.php', [ diff --git a/module/Core/config/routes.config.php b/module/Core/config/routes.config.php index 07e33c73..bb7e94f5 100644 --- a/module/Core/config/routes.config.php +++ b/module/Core/config/routes.config.php @@ -17,18 +17,9 @@ return [ ], 'allowed_methods' => [RequestMethod::METHOD_GET], ], - [ - 'name' => Action\RedirectAction::class, - 'path' => '/{shortCode}', - 'middleware' => [ - IpAddress::class, - Action\RedirectAction::class, - ], - 'allowed_methods' => [RequestMethod::METHOD_GET], - ], [ 'name' => Action\PixelAction::class, - 'path' => '/{shortCode}/track', + 'path' => '/{shortCode:.+}/track', 'middleware' => [ IpAddress::class, Action\PixelAction::class, @@ -37,12 +28,21 @@ return [ ], [ 'name' => Action\QrCodeAction::class, - 'path' => '/{shortCode}/qr-code', + 'path' => '/{shortCode:.+}/qr-code', 'middleware' => [ Action\QrCodeAction::class, ], 'allowed_methods' => [RequestMethod::METHOD_GET], ], + [ + 'name' => Action\RedirectAction::class, + 'path' => '/{shortCode:.+}', + 'middleware' => [ + IpAddress::class, + Action\RedirectAction::class, + ], + 'allowed_methods' => [RequestMethod::METHOD_GET], + ], ], ]; diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index f318664f..dbc10ba8 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -16,6 +16,14 @@ return (static function (): array { 'routes' => [ Action\HealthAction::getRouteDef(), + // Visits + Action\Visit\ShortUrlVisitsAction::getRouteDef([$dropDomainMiddleware]), + Action\Visit\TagVisitsAction::getRouteDef(), + Action\Visit\DomainVisitsAction::getRouteDef(), + Action\Visit\GlobalVisitsAction::getRouteDef(), + Action\Visit\OrphanVisitsAction::getRouteDef(), + Action\Visit\NonOrphanVisitsAction::getRouteDef(), + // Short URLs Action\ShortUrl\CreateShortUrlAction::getRouteDef([ $contentNegotiationMiddleware, @@ -32,14 +40,6 @@ return (static function (): array { Action\ShortUrl\ResolveShortUrlAction::getRouteDef([$dropDomainMiddleware]), Action\ShortUrl\ListShortUrlsAction::getRouteDef(), - // Visits - Action\Visit\ShortUrlVisitsAction::getRouteDef([$dropDomainMiddleware]), - Action\Visit\TagVisitsAction::getRouteDef(), - Action\Visit\DomainVisitsAction::getRouteDef(), - Action\Visit\GlobalVisitsAction::getRouteDef(), - Action\Visit\OrphanVisitsAction::getRouteDef(), - Action\Visit\NonOrphanVisitsAction::getRouteDef(), - // Tags Action\Tag\ListTagsAction::getRouteDef(), Action\Tag\TagsStatsAction::getRouteDef(), diff --git a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php index 8059e5ab..40b03b6e 100644 --- a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php @@ -14,7 +14,7 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; class DeleteShortUrlAction extends AbstractRestAction { - protected const ROUTE_PATH = '/short-urls/{shortCode}'; + protected const ROUTE_PATH = '/short-urls/{shortCode:.+}'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_DELETE]; public function __construct(private DeleteShortUrlServiceInterface $deleteShortUrlService) diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php index 87c21aec..d82aef3e 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php @@ -16,8 +16,8 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; class EditShortUrlAction extends AbstractRestAction { - protected const ROUTE_PATH = '/short-urls/{shortCode}'; - protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PATCH, self::METHOD_PUT]; + protected const ROUTE_PATH = '/short-urls/{shortCode:.+}'; + protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PATCH]; public function __construct( private ShortUrlServiceInterface $shortUrlService, diff --git a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php index aae1a895..66719cba 100644 --- a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php @@ -15,7 +15,7 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; class ResolveShortUrlAction extends AbstractRestAction { - protected const ROUTE_PATH = '/short-urls/{shortCode}'; + protected const ROUTE_PATH = '/short-urls/{shortCode:.+}'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; public function __construct( diff --git a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php index 5496ba35..45254f18 100644 --- a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php +++ b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php @@ -18,7 +18,7 @@ class ShortUrlVisitsAction extends AbstractRestAction { use PagerfantaUtilsTrait; - protected const ROUTE_PATH = '/short-urls/{shortCode}/visits'; + protected const ROUTE_PATH = '/short-urls/{shortCode:.+}/visits'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; public function __construct(private VisitsStatsHelperInterface $visitsHelper) From ba517eeeb58d2d304b5642017cd07ae9e303aa06 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 4 Aug 2022 11:14:26 +0200 Subject: [PATCH 07/16] Moved routes config together, and ensure they are loaded last --- config/autoload/routes.config.php | 101 +++++++++++++++++++ config/config.php | 7 +- module/Core/config/routes.config.php | 48 --------- module/Core/test/ConfigProviderTest.php | 3 +- module/Rest/config/routes.config.php | 57 ----------- module/Rest/src/ConfigProvider.php | 29 ++---- module/Rest/test-api/Middleware/CorsTest.php | 4 +- module/Rest/test/ConfigProviderTest.php | 9 +- 8 files changed, 117 insertions(+), 141 deletions(-) create mode 100644 config/autoload/routes.config.php delete mode 100644 module/Core/config/routes.config.php delete mode 100644 module/Rest/config/routes.config.php diff --git a/config/autoload/routes.config.php b/config/autoload/routes.config.php new file mode 100644 index 00000000..38b4edc7 --- /dev/null +++ b/config/autoload/routes.config.php @@ -0,0 +1,101 @@ + [ + ...ConfigProvider::applyRoutesPrefix([ + Action\HealthAction::getRouteDef(), + + // Visits + Action\Visit\ShortUrlVisitsAction::getRouteDef([$dropDomainMiddleware]), + Action\Visit\TagVisitsAction::getRouteDef(), + Action\Visit\DomainVisitsAction::getRouteDef(), + Action\Visit\GlobalVisitsAction::getRouteDef(), + Action\Visit\OrphanVisitsAction::getRouteDef(), + Action\Visit\NonOrphanVisitsAction::getRouteDef(), + + // Short URLs + Action\ShortUrl\CreateShortUrlAction::getRouteDef([ + $contentNegotiationMiddleware, + $dropDomainMiddleware, + $overrideDomainMiddleware, + Middleware\ShortUrl\DefaultShortCodesLengthMiddleware::class, + ]), + Action\ShortUrl\SingleStepCreateShortUrlAction::getRouteDef([ + $contentNegotiationMiddleware, + $overrideDomainMiddleware, + ]), + Action\ShortUrl\EditShortUrlAction::getRouteDef([$dropDomainMiddleware]), + Action\ShortUrl\DeleteShortUrlAction::getRouteDef([$dropDomainMiddleware]), + Action\ShortUrl\ResolveShortUrlAction::getRouteDef([$dropDomainMiddleware]), + Action\ShortUrl\ListShortUrlsAction::getRouteDef(), + + // Tags + Action\Tag\ListTagsAction::getRouteDef(), + Action\Tag\TagsStatsAction::getRouteDef(), + Action\Tag\DeleteTagsAction::getRouteDef(), + Action\Tag\UpdateTagAction::getRouteDef(), + + // Domains + Action\Domain\ListDomainsAction::getRouteDef(), + Action\Domain\DomainRedirectsAction::getRouteDef(), + + Action\MercureInfoAction::getRouteDef([NotConfiguredMercureErrorHandler::class]), + ]), + + // Non-rest + [ + 'name' => CoreAction\RobotsAction::class, + 'path' => '/robots.txt', + 'middleware' => [ + CoreAction\RobotsAction::class, + ], + 'allowed_methods' => [RequestMethod::METHOD_GET], + ], + [ + 'name' => CoreAction\PixelAction::class, + 'path' => '/{shortCode:.+}/track', + 'middleware' => [ + IpAddress::class, + CoreAction\PixelAction::class, + ], + 'allowed_methods' => [RequestMethod::METHOD_GET], + ], + [ + 'name' => CoreAction\QrCodeAction::class, + 'path' => '/{shortCode:.+}/qr-code', + 'middleware' => [ + CoreAction\QrCodeAction::class, + ], + 'allowed_methods' => [RequestMethod::METHOD_GET], + ], + [ + 'name' => CoreAction\RedirectAction::class, + 'path' => '/{shortCode:.+}', + 'middleware' => [ + IpAddress::class, + CoreAction\RedirectAction::class, + ], + 'allowed_methods' => [RequestMethod::METHOD_GET], + ], + ], + + ]; +})(); diff --git a/config/config.php b/config/config.php index 19e3a82d..a1e0428a 100644 --- a/config/config.php +++ b/config/config.php @@ -36,14 +36,15 @@ return (new ConfigAggregator\ConfigAggregator([ Importer\ConfigProvider::class, IpGeolocation\ConfigProvider::class, EventDispatcher\ConfigProvider::class, - CLI\ConfigProvider::class, - Rest\ConfigProvider::class, // Load rest before Core, to prevent conflicting routes when multi-segment is enabled Core\ConfigProvider::class, + CLI\ConfigProvider::class, + Rest\ConfigProvider::class, new ConfigAggregator\PhpFileProvider('config/autoload/{{,*.}global,{,*.}local}.php'), $isTestEnv - // TODO Test routes must be loaded before core config ? new ConfigAggregator\PhpFileProvider('config/test/*.global.php') : new ConfigAggregator\ArrayProvider([]), + // Routes have to be loaded last + new ConfigAggregator\PhpFileProvider('config/autoload/routes.config.php'), ], 'data/cache/app_config.php', [ Core\Config\BasePathPrefixer::class, ]))->getMergedConfig(); diff --git a/module/Core/config/routes.config.php b/module/Core/config/routes.config.php deleted file mode 100644 index bb7e94f5..00000000 --- a/module/Core/config/routes.config.php +++ /dev/null @@ -1,48 +0,0 @@ - [ - [ - 'name' => Action\RobotsAction::class, - 'path' => '/robots.txt', - 'middleware' => [ - Action\RobotsAction::class, - ], - 'allowed_methods' => [RequestMethod::METHOD_GET], - ], - [ - 'name' => Action\PixelAction::class, - 'path' => '/{shortCode:.+}/track', - 'middleware' => [ - IpAddress::class, - Action\PixelAction::class, - ], - 'allowed_methods' => [RequestMethod::METHOD_GET], - ], - [ - 'name' => Action\QrCodeAction::class, - 'path' => '/{shortCode:.+}/qr-code', - 'middleware' => [ - Action\QrCodeAction::class, - ], - 'allowed_methods' => [RequestMethod::METHOD_GET], - ], - [ - 'name' => Action\RedirectAction::class, - 'path' => '/{shortCode:.+}', - 'middleware' => [ - IpAddress::class, - Action\RedirectAction::class, - ], - 'allowed_methods' => [RequestMethod::METHOD_GET], - ], - ], - -]; diff --git a/module/Core/test/ConfigProviderTest.php b/module/Core/test/ConfigProviderTest.php index 4044446a..33714f88 100644 --- a/module/Core/test/ConfigProviderTest.php +++ b/module/Core/test/ConfigProviderTest.php @@ -22,8 +22,7 @@ class ConfigProviderTest extends TestCase { $config = ($this->configProvider)(); - self::assertCount(5, $config); - self::assertArrayHasKey('routes', $config); + self::assertCount(4, $config); self::assertArrayHasKey('dependencies', $config); self::assertArrayHasKey('entity_manager', $config); self::assertArrayHasKey('events', $config); diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php deleted file mode 100644 index dbc10ba8..00000000 --- a/module/Rest/config/routes.config.php +++ /dev/null @@ -1,57 +0,0 @@ - [ - Action\HealthAction::getRouteDef(), - - // Visits - Action\Visit\ShortUrlVisitsAction::getRouteDef([$dropDomainMiddleware]), - Action\Visit\TagVisitsAction::getRouteDef(), - Action\Visit\DomainVisitsAction::getRouteDef(), - Action\Visit\GlobalVisitsAction::getRouteDef(), - Action\Visit\OrphanVisitsAction::getRouteDef(), - Action\Visit\NonOrphanVisitsAction::getRouteDef(), - - // Short URLs - Action\ShortUrl\CreateShortUrlAction::getRouteDef([ - $contentNegotiationMiddleware, - $dropDomainMiddleware, - $overrideDomainMiddleware, - Middleware\ShortUrl\DefaultShortCodesLengthMiddleware::class, - ]), - Action\ShortUrl\SingleStepCreateShortUrlAction::getRouteDef([ - $contentNegotiationMiddleware, - $overrideDomainMiddleware, - ]), - Action\ShortUrl\EditShortUrlAction::getRouteDef([$dropDomainMiddleware]), - Action\ShortUrl\DeleteShortUrlAction::getRouteDef([$dropDomainMiddleware]), - Action\ShortUrl\ResolveShortUrlAction::getRouteDef([$dropDomainMiddleware]), - Action\ShortUrl\ListShortUrlsAction::getRouteDef(), - - // Tags - Action\Tag\ListTagsAction::getRouteDef(), - Action\Tag\TagsStatsAction::getRouteDef(), - Action\Tag\DeleteTagsAction::getRouteDef(), - Action\Tag\UpdateTagAction::getRouteDef(), - - // Domains - Action\Domain\ListDomainsAction::getRouteDef(), - Action\Domain\DomainRedirectsAction::getRouteDef(), - - Action\MercureInfoAction::getRouteDef([NotConfiguredMercureErrorHandler::class]), - ], - - ]; -})(); diff --git a/module/Rest/src/ConfigProvider.php b/module/Rest/src/ConfigProvider.php index 130617d6..b8d5f9cc 100644 --- a/module/Rest/src/ConfigProvider.php +++ b/module/Rest/src/ConfigProvider.php @@ -4,8 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest; -use Closure; - use function Functional\first; use function Functional\map; use function Shlinkio\Shlink\Config\loadConfigFromGlob; @@ -17,38 +15,25 @@ class ConfigProvider private const UNVERSIONED_ROUTES_PREFIX = '/rest'; public const UNVERSIONED_HEALTH_ENDPOINT_NAME = 'unversioned_health'; - private Closure $loadConfig; - - public function __construct(?callable $loadConfig = null) - { - $this->loadConfig = Closure::fromCallable($loadConfig ?? fn (string $glob) => loadConfigFromGlob($glob)); - } - public function __invoke(): array { - $config = ($this->loadConfig)(__DIR__ . '/../config/{,*.}config.php'); - return $this->applyRoutesPrefix($config); + return loadConfigFromGlob(__DIR__ . '/../config/{,*.}config.php'); } - private function applyRoutesPrefix(array $config): array + public static function applyRoutesPrefix(array $routes): array { - $routes = $config['routes'] ?? []; - $healthRoute = $this->buildUnversionedHealthRouteFromExistingRoutes($routes); - - $prefixRoute = static function (array $route) { + $healthRoute = self::buildUnversionedHealthRouteFromExistingRoutes($routes); + $prefixedRoutes = map($routes, static function (array $route) { ['path' => $path] = $route; $route['path'] = sprintf('%s%s', self::ROUTES_PREFIX, $path); return $route; - }; - $prefixedRoutes = map($routes, $prefixRoute); + }); - $config['routes'] = $healthRoute !== null ? [...$prefixedRoutes, $healthRoute] : $prefixedRoutes; - - return $config; + return $healthRoute !== null ? [...$prefixedRoutes, $healthRoute] : $prefixedRoutes; } - private function buildUnversionedHealthRouteFromExistingRoutes(array $routes): ?array + private static function buildUnversionedHealthRouteFromExistingRoutes(array $routes): ?array { $healthRoute = first($routes, fn (array $route) => $route['path'] === '/health'); if ($healthRoute === null) { diff --git a/module/Rest/test-api/Middleware/CorsTest.php b/module/Rest/test-api/Middleware/CorsTest.php index 3efbeacb..bffb0421 100644 --- a/module/Rest/test-api/Middleware/CorsTest.php +++ b/module/Rest/test-api/Middleware/CorsTest.php @@ -71,9 +71,9 @@ class CorsTest extends ApiTestCase public function providePreflightEndpoints(): iterable { - yield 'invalid route' => ['/foo/bar', 'GET,POST,PUT,PATCH,DELETE']; +// yield 'invalid route' => ['/foo/bar', 'GET,POST,PUT,PATCH,DELETE']; // TODO This won't work with multi-segment yield 'short URLs route' => ['/short-urls', 'GET,POST']; - yield 'tags route' => ['/tags', 'GET,PUT,DELETE']; + yield 'tags route' => ['/tags', 'GET,DELETE,PUT']; yield 'health route' => ['/health', 'GET']; } } diff --git a/module/Rest/test/ConfigProviderTest.php b/module/Rest/test/ConfigProviderTest.php index 462947c9..a3f7d0c9 100644 --- a/module/Rest/test/ConfigProviderTest.php +++ b/module/Rest/test/ConfigProviderTest.php @@ -22,8 +22,7 @@ class ConfigProviderTest extends TestCase { $config = ($this->configProvider)(); - self::assertCount(5, $config); - self::assertArrayHasKey('routes', $config); + self::assertCount(4, $config); self::assertArrayHasKey('dependencies', $config); self::assertArrayHasKey('auth', $config); self::assertArrayHasKey('entity_manager', $config); @@ -36,11 +35,7 @@ class ConfigProviderTest extends TestCase */ public function routesAreProperlyPrefixed(array $routes, array $expected): void { - $configProvider = new ConfigProvider(fn () => ['routes' => $routes]); - - $config = $configProvider(); - - self::assertEquals($expected, $config['routes']); + self::assertEquals($expected, ConfigProvider::applyRoutesPrefix($routes)); } public function provideRoutesConfig(): iterable From 7acf27dd38dc6125922616547082486578f72d2d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 4 Aug 2022 11:27:33 +0200 Subject: [PATCH 08/16] Replaced usage of deprecated methods in DateRange class --- config/autoload/routes.config.php | 12 ++-- .../ShortUrl/GetShortUrlVisitsCommandTest.php | 4 +- module/Core/src/Model/VisitsParams.php | 2 +- .../Repository/ShortUrlRepositoryTest.php | 8 +-- .../Repository/VisitRepositoryTest.php | 68 +++++++++---------- .../ShortUrlVisitsPaginatorAdapterTest.php | 4 +- .../VisitsForTagPaginatorAdapterTest.php | 4 +- .../Action/Visit/ShortUrlVisitsActionTest.php | 2 +- 8 files changed, 53 insertions(+), 51 deletions(-) diff --git a/config/autoload/routes.config.php b/config/autoload/routes.config.php index 38b4edc7..f7c515fb 100644 --- a/config/autoload/routes.config.php +++ b/config/autoload/routes.config.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink; -use Fig\Http\Message\RequestMethodInterface as RequestMethod; +use Fig\Http\Message\RequestMethodInterface; use RKA\Middleware\IpAddress; use Shlinkio\Shlink\Core\Action as CoreAction; use Shlinkio\Shlink\Rest\Action; @@ -12,6 +12,7 @@ use Shlinkio\Shlink\Rest\ConfigProvider; use Shlinkio\Shlink\Rest\Middleware; use Shlinkio\Shlink\Rest\Middleware\Mercure\NotConfiguredMercureErrorHandler; +// 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; @@ -20,6 +21,7 @@ return (static function (): array { return [ 'routes' => [ + // Rest ...ConfigProvider::applyRoutesPrefix([ Action\HealthAction::getRouteDef(), @@ -67,7 +69,7 @@ return (static function (): array { 'middleware' => [ CoreAction\RobotsAction::class, ], - 'allowed_methods' => [RequestMethod::METHOD_GET], + 'allowed_methods' => [RequestMethodInterface::METHOD_GET], ], [ 'name' => CoreAction\PixelAction::class, @@ -76,7 +78,7 @@ return (static function (): array { IpAddress::class, CoreAction\PixelAction::class, ], - 'allowed_methods' => [RequestMethod::METHOD_GET], + 'allowed_methods' => [RequestMethodInterface::METHOD_GET], ], [ 'name' => CoreAction\QrCodeAction::class, @@ -84,7 +86,7 @@ return (static function (): array { 'middleware' => [ CoreAction\QrCodeAction::class, ], - 'allowed_methods' => [RequestMethod::METHOD_GET], + 'allowed_methods' => [RequestMethodInterface::METHOD_GET], ], [ 'name' => CoreAction\RedirectAction::class, @@ -93,7 +95,7 @@ return (static function (): array { IpAddress::class, CoreAction\RedirectAction::class, ], - 'allowed_methods' => [RequestMethod::METHOD_GET], + 'allowed_methods' => [RequestMethodInterface::METHOD_GET], ], ], diff --git a/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php index 076eb9b2..316c762e 100644 --- a/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php @@ -46,7 +46,7 @@ class GetShortUrlVisitsCommandTest extends TestCase $shortCode = 'abc123'; $this->visitsHelper->visitsForShortUrl( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), - new VisitsParams(DateRange::emptyInstance()), + new VisitsParams(DateRange::allTime()), ) ->willReturn(new Paginator(new ArrayAdapter([]))) ->shouldBeCalledOnce(); @@ -81,7 +81,7 @@ class GetShortUrlVisitsCommandTest extends TestCase $startDate = 'foo'; $info = $this->visitsHelper->visitsForShortUrl( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), - new VisitsParams(DateRange::emptyInstance()), + new VisitsParams(DateRange::allTime()), )->willReturn(new Paginator(new ArrayAdapter([]))); $this->commandTester->execute([ diff --git a/module/Core/src/Model/VisitsParams.php b/module/Core/src/Model/VisitsParams.php index ab9e8002..dfc4663d 100644 --- a/module/Core/src/Model/VisitsParams.php +++ b/module/Core/src/Model/VisitsParams.php @@ -19,7 +19,7 @@ final class VisitsParams extends AbstractInfinitePaginableListParams public readonly bool $excludeBots = false, ) { parent::__construct($page, $itemsPerPage); - $this->dateRange = $dateRange ?? DateRange::emptyInstance(); + $this->dateRange = $dateRange ?? DateRange::allTime(); } public static function fromRawData(array $query): self diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index bfa38169..3143e90c 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -152,23 +152,23 @@ class ShortUrlRepositoryTest extends DatabaseTestCase self::assertSame($bar, $result[0]); $result = $this->repo->findList( - new ShortUrlsListFiltering(null, null, Ordering::emptyInstance(), null, [], null, DateRange::withEndDate( + new ShortUrlsListFiltering(null, null, Ordering::emptyInstance(), null, [], null, DateRange::until( Chronos::now()->subDays(2), )), ); self::assertCount(1, $result); - self::assertEquals(1, $this->repo->countList(new ShortUrlsCountFiltering(null, [], null, DateRange::withEndDate( + self::assertEquals(1, $this->repo->countList(new ShortUrlsCountFiltering(null, [], null, DateRange::until( Chronos::now()->subDays(2), )))); self::assertSame($foo2, $result[0]); self::assertCount(2, $this->repo->findList( - new ShortUrlsListFiltering(null, null, Ordering::emptyInstance(), null, [], null, DateRange::withStartDate( + new ShortUrlsListFiltering(null, null, Ordering::emptyInstance(), null, [], null, DateRange::since( Chronos::now()->subDays(2), )), )); self::assertEquals(2, $this->repo->countList( - new ShortUrlsCountFiltering(null, [], null, DateRange::withStartDate(Chronos::now()->subDays(2))), + new ShortUrlsCountFiltering(null, [], null, DateRange::since(Chronos::now()->subDays(2))), )); } diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index b16c3382..041f5e4c 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -114,16 +114,16 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertCount(2, $this->repo->findVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), new VisitsListFiltering( - DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), + DateRange::between(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ), )); self::assertCount(4, $this->repo->findVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), - new VisitsListFiltering(DateRange::withStartDate(Chronos::parse('2016-01-03'))), + new VisitsListFiltering(DateRange::since(Chronos::parse('2016-01-03'))), )); self::assertCount(1, $this->repo->findVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, $domain), - new VisitsListFiltering(DateRange::withStartDate(Chronos::parse('2016-01-03'))), + new VisitsListFiltering(DateRange::since(Chronos::parse('2016-01-03'))), )); self::assertCount(3, $this->repo->findVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), @@ -163,16 +163,16 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(2, $this->repo->countVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), new VisitsCountFiltering( - DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), + DateRange::between(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ), )); self::assertEquals(4, $this->repo->countVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), - new VisitsCountFiltering(DateRange::withStartDate(Chronos::parse('2016-01-03'))), + new VisitsCountFiltering(DateRange::since(Chronos::parse('2016-01-03'))), )); self::assertEquals(1, $this->repo->countVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, $domain), - new VisitsCountFiltering(DateRange::withStartDate(Chronos::parse('2016-01-03'))), + new VisitsCountFiltering(DateRange::since(Chronos::parse('2016-01-03'))), )); } @@ -227,10 +227,10 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertCount(18, $this->repo->findVisitsByTag($foo, new VisitsListFiltering())); self::assertCount(12, $this->repo->findVisitsByTag($foo, new VisitsListFiltering(null, true))); self::assertCount(6, $this->repo->findVisitsByTag($foo, new VisitsListFiltering( - DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), + DateRange::between(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ))); self::assertCount(12, $this->repo->findVisitsByTag($foo, new VisitsListFiltering( - DateRange::withStartDate(Chronos::parse('2016-01-03')), + DateRange::since(Chronos::parse('2016-01-03')), ))); } @@ -249,10 +249,10 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(12, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering())); self::assertEquals(8, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering(null, true))); self::assertEquals(4, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering( - DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), + DateRange::between(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ))); self::assertEquals(8, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering( - DateRange::withStartDate(Chronos::parse('2016-01-03')), + DateRange::since(Chronos::parse('2016-01-03')), ))); } @@ -267,16 +267,16 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertCount(3, $this->repo->findVisitsByDomain('doma.in', new VisitsListFiltering())); self::assertCount(1, $this->repo->findVisitsByDomain('doma.in', new VisitsListFiltering(null, true))); self::assertCount(2, $this->repo->findVisitsByDomain('doma.in', new VisitsListFiltering( - DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), + DateRange::between(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ))); self::assertCount(1, $this->repo->findVisitsByDomain('doma.in', new VisitsListFiltering( - DateRange::withStartDate(Chronos::parse('2016-01-03')), + DateRange::since(Chronos::parse('2016-01-03')), ))); self::assertCount(2, $this->repo->findVisitsByDomain('DEFAULT', new VisitsListFiltering( - DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), + DateRange::between(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ))); self::assertCount(4, $this->repo->findVisitsByDomain('DEFAULT', new VisitsListFiltering( - DateRange::withStartDate(Chronos::parse('2016-01-03')), + DateRange::since(Chronos::parse('2016-01-03')), ))); } @@ -291,16 +291,16 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(3, $this->repo->countVisitsByDomain('doma.in', new VisitsListFiltering())); self::assertEquals(1, $this->repo->countVisitsByDomain('doma.in', new VisitsListFiltering(null, true))); self::assertEquals(2, $this->repo->countVisitsByDomain('doma.in', new VisitsListFiltering( - DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), + DateRange::between(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ))); self::assertEquals(1, $this->repo->countVisitsByDomain('doma.in', new VisitsListFiltering( - DateRange::withStartDate(Chronos::parse('2016-01-03')), + DateRange::since(Chronos::parse('2016-01-03')), ))); self::assertEquals(2, $this->repo->countVisitsByDomain('DEFAULT', new VisitsListFiltering( - DateRange::withStartAndEndDate(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), + DateRange::between(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ))); self::assertEquals(4, $this->repo->countVisitsByDomain('DEFAULT', new VisitsListFiltering( - DateRange::withStartDate(Chronos::parse('2016-01-03')), + DateRange::since(Chronos::parse('2016-01-03')), ))); } @@ -349,13 +349,13 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(4, $this->repo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($apiKey1))); self::assertEquals(5 + 7, $this->repo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($apiKey2))); self::assertEquals(4 + 7, $this->repo->countNonOrphanVisits(VisitsCountFiltering::withApiKey($domainApiKey))); - self::assertEquals(4, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::withStartDate( + self::assertEquals(4, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::since( Chronos::parse('2016-01-05')->startOfDay(), )))); - self::assertEquals(2, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::withStartDate( + self::assertEquals(2, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::since( Chronos::parse('2016-01-03')->startOfDay(), ), false, $apiKey1))); - self::assertEquals(1, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::withStartDate( + self::assertEquals(1, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::since( Chronos::parse('2016-01-07')->startOfDay(), ), false, $apiKey2))); self::assertEquals(3 + 5, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(null, true, $apiKey2))); @@ -395,20 +395,20 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertCount(5, $this->repo->findOrphanVisits(new VisitsListFiltering(null, false, null, 5))); self::assertCount(10, $this->repo->findOrphanVisits(new VisitsListFiltering(null, false, null, 15, 8))); self::assertCount(9, $this->repo->findOrphanVisits(new VisitsListFiltering( - DateRange::withStartDate(Chronos::parse('2020-01-04')), + DateRange::since(Chronos::parse('2020-01-04')), false, null, 15, ))); self::assertCount(2, $this->repo->findOrphanVisits(new VisitsListFiltering( - DateRange::withStartAndEndDate(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')), + DateRange::between(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')), false, null, 6, 4, ))); self::assertCount(3, $this->repo->findOrphanVisits(new VisitsListFiltering( - DateRange::withEndDate(Chronos::parse('2020-01-01')), + DateRange::until(Chronos::parse('2020-01-01')), ))); } @@ -437,15 +437,15 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); self::assertEquals(18, $this->repo->countOrphanVisits(new VisitsCountFiltering())); - self::assertEquals(18, $this->repo->countOrphanVisits(new VisitsCountFiltering(DateRange::emptyInstance()))); + self::assertEquals(18, $this->repo->countOrphanVisits(new VisitsCountFiltering(DateRange::allTime()))); self::assertEquals(9, $this->repo->countOrphanVisits( - new VisitsCountFiltering(DateRange::withStartDate(Chronos::parse('2020-01-04'))), + new VisitsCountFiltering(DateRange::since(Chronos::parse('2020-01-04'))), )); self::assertEquals(6, $this->repo->countOrphanVisits(new VisitsCountFiltering( - DateRange::withStartAndEndDate(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')), + DateRange::between(Chronos::parse('2020-01-02'), Chronos::parse('2020-01-03')), ))); self::assertEquals(3, $this->repo->countOrphanVisits( - new VisitsCountFiltering(DateRange::withEndDate(Chronos::parse('2020-01-01'))), + new VisitsCountFiltering(DateRange::until(Chronos::parse('2020-01-01'))), )); } @@ -467,22 +467,22 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); self::assertCount(21, $this->repo->findNonOrphanVisits(new VisitsListFiltering())); - self::assertCount(21, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::emptyInstance()))); - self::assertCount(7, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::withStartDate( + self::assertCount(21, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::allTime()))); + self::assertCount(7, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::since( Chronos::parse('2016-01-05')->endOfDay(), )))); - self::assertCount(12, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::withEndDate( + self::assertCount(12, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::until( Chronos::parse('2016-01-04')->endOfDay(), )))); - self::assertCount(6, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::withStartAndEndDate( + self::assertCount(6, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::between( Chronos::parse('2016-01-03')->startOfDay(), Chronos::parse('2016-01-04')->endOfDay(), )))); - self::assertCount(13, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::withStartAndEndDate( + self::assertCount(13, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::between( Chronos::parse('2016-01-03')->startOfDay(), Chronos::parse('2016-01-08')->endOfDay(), )))); - self::assertCount(3, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::withStartAndEndDate( + self::assertCount(3, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::between( Chronos::parse('2016-01-03')->startOfDay(), Chronos::parse('2016-01-08')->endOfDay(), ), false, null, 10, 10))); diff --git a/module/Core/test/Visit/Paginator/Adapter/ShortUrlVisitsPaginatorAdapterTest.php b/module/Core/test/Visit/Paginator/Adapter/ShortUrlVisitsPaginatorAdapterTest.php index ae9a2a64..7d6d04a6 100644 --- a/module/Core/test/Visit/Paginator/Adapter/ShortUrlVisitsPaginatorAdapterTest.php +++ b/module/Core/test/Visit/Paginator/Adapter/ShortUrlVisitsPaginatorAdapterTest.php @@ -36,7 +36,7 @@ class ShortUrlVisitsPaginatorAdapterTest extends TestCase $adapter = $this->createAdapter(null); $findVisits = $this->repo->findVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain(''), - new VisitsListFiltering(DateRange::emptyInstance(), false, null, $limit, $offset), + new VisitsListFiltering(DateRange::allTime(), false, null, $limit, $offset), )->willReturn([]); for ($i = 0; $i < $count; $i++) { @@ -54,7 +54,7 @@ class ShortUrlVisitsPaginatorAdapterTest extends TestCase $adapter = $this->createAdapter($apiKey); $countVisits = $this->repo->countVisitsByShortCode( ShortUrlIdentifier::fromShortCodeAndDomain(''), - new VisitsCountFiltering(DateRange::emptyInstance(), false, $apiKey), + new VisitsCountFiltering(DateRange::allTime(), false, $apiKey), )->willReturn(3); for ($i = 0; $i < $count; $i++) { diff --git a/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php b/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php index 442e7128..32e1bb85 100644 --- a/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php +++ b/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php @@ -35,7 +35,7 @@ class VisitsForTagPaginatorAdapterTest extends TestCase $adapter = $this->createAdapter(null); $findVisits = $this->repo->findVisitsByTag( 'foo', - new VisitsListFiltering(DateRange::emptyInstance(), false, null, $limit, $offset), + new VisitsListFiltering(DateRange::allTime(), false, null, $limit, $offset), )->willReturn([]); for ($i = 0; $i < $count; $i++) { @@ -53,7 +53,7 @@ class VisitsForTagPaginatorAdapterTest extends TestCase $adapter = $this->createAdapter($apiKey); $countVisits = $this->repo->countVisitsByTag( 'foo', - new VisitsCountFiltering(DateRange::emptyInstance(), false, $apiKey), + new VisitsCountFiltering(DateRange::allTime(), false, $apiKey), )->willReturn(3); for ($i = 0; $i < $count; $i++) { diff --git a/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php b/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php index 50cabe8e..299c42d1 100644 --- a/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/ShortUrlVisitsActionTest.php @@ -53,7 +53,7 @@ class ShortUrlVisitsActionTest extends TestCase { $shortCode = 'abc123'; $this->visitsHelper->visitsForShortUrl(ShortUrlIdentifier::fromShortCodeAndDomain($shortCode), new VisitsParams( - DateRange::withEndDate(Chronos::parse('2016-01-01 00:00:00')), + DateRange::until(Chronos::parse('2016-01-01 00:00:00')), 3, 10, ), Argument::type(ApiKey::class)) From 619999d4f8bddd6ac1ee34042ad26b83d71b8422 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 4 Aug 2022 11:49:33 +0200 Subject: [PATCH 09/16] Added feature flag to enable/disable multi-segment support --- config/autoload/routes.config.php | 12 ++++++++---- module/Core/src/Config/EnvVars.php | 1 + .../Action/ShortUrl/DeleteShortUrlAction.php | 2 +- .../Action/ShortUrl/EditShortUrlAction.php | 2 +- .../Action/ShortUrl/ResolveShortUrlAction.php | 2 +- .../src/Action/Visit/ShortUrlVisitsAction.php | 2 +- module/Rest/src/ConfigProvider.php | 10 +++++++--- module/Rest/test-api/Middleware/CorsTest.php | 2 +- module/Rest/test/ConfigProviderTest.php | 19 +++++++++++++++++-- 9 files changed, 38 insertions(+), 14 deletions(-) diff --git a/config/autoload/routes.config.php b/config/autoload/routes.config.php index f7c515fb..e7e24916 100644 --- a/config/autoload/routes.config.php +++ b/config/autoload/routes.config.php @@ -7,16 +7,20 @@ 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 [ @@ -60,7 +64,7 @@ return (static function (): array { Action\Domain\DomainRedirectsAction::getRouteDef(), Action\MercureInfoAction::getRouteDef([NotConfiguredMercureErrorHandler::class]), - ]), + ], $multiSegment), // Non-rest [ @@ -73,7 +77,7 @@ return (static function (): array { ], [ 'name' => CoreAction\PixelAction::class, - 'path' => '/{shortCode:.+}/track', + 'path' => sprintf('/{shortCode%s}/track', $multiSegment ? ':.+' : ''), 'middleware' => [ IpAddress::class, CoreAction\PixelAction::class, @@ -82,7 +86,7 @@ return (static function (): array { ], [ 'name' => CoreAction\QrCodeAction::class, - 'path' => '/{shortCode:.+}/qr-code', + 'path' => sprintf('/{shortCode%s}/qr-code', $multiSegment ? ':.+' : ''), 'middleware' => [ CoreAction\QrCodeAction::class, ], @@ -90,7 +94,7 @@ return (static function (): array { ], [ 'name' => CoreAction\RedirectAction::class, - 'path' => '/{shortCode:.+}', + 'path' => sprintf('/{shortCode%s}', $multiSegment ? ':.+' : ''), 'middleware' => [ IpAddress::class, CoreAction\RedirectAction::class, diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index a68f24f3..8f8689be 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -59,6 +59,7 @@ enum EnvVars: string case AUTO_RESOLVE_TITLES = 'AUTO_RESOLVE_TITLES'; case REDIRECT_APPEND_EXTRA_PATH = 'REDIRECT_APPEND_EXTRA_PATH'; case TIMEZONE = 'TIMEZONE'; + case MULTI_SEGMENT_SLUGS_ENABLED = 'MULTI_SEGMENT_SLUGS_ENABLED'; /** @deprecated */ case VISITS_WEBHOOKS = 'VISITS_WEBHOOKS'; /** @deprecated */ diff --git a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php index 40b03b6e..8059e5ab 100644 --- a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php @@ -14,7 +14,7 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; class DeleteShortUrlAction extends AbstractRestAction { - protected const ROUTE_PATH = '/short-urls/{shortCode:.+}'; + protected const ROUTE_PATH = '/short-urls/{shortCode}'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_DELETE]; public function __construct(private DeleteShortUrlServiceInterface $deleteShortUrlService) diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php index d82aef3e..71cf8bf3 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php @@ -16,7 +16,7 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; class EditShortUrlAction extends AbstractRestAction { - protected const ROUTE_PATH = '/short-urls/{shortCode:.+}'; + protected const ROUTE_PATH = '/short-urls/{shortCode}'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PATCH]; public function __construct( diff --git a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php index 66719cba..aae1a895 100644 --- a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php @@ -15,7 +15,7 @@ use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; class ResolveShortUrlAction extends AbstractRestAction { - protected const ROUTE_PATH = '/short-urls/{shortCode:.+}'; + protected const ROUTE_PATH = '/short-urls/{shortCode}'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; public function __construct( diff --git a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php index 45254f18..5496ba35 100644 --- a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php +++ b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php @@ -18,7 +18,7 @@ class ShortUrlVisitsAction extends AbstractRestAction { use PagerfantaUtilsTrait; - protected const ROUTE_PATH = '/short-urls/{shortCode:.+}/visits'; + protected const ROUTE_PATH = '/short-urls/{shortCode}/visits'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; public function __construct(private VisitsStatsHelperInterface $visitsHelper) diff --git a/module/Rest/src/ConfigProvider.php b/module/Rest/src/ConfigProvider.php index b8d5f9cc..3304ce4d 100644 --- a/module/Rest/src/ConfigProvider.php +++ b/module/Rest/src/ConfigProvider.php @@ -8,6 +8,7 @@ use function Functional\first; use function Functional\map; use function Shlinkio\Shlink\Config\loadConfigFromGlob; use function sprintf; +use function str_replace; class ConfigProvider { @@ -20,11 +21,14 @@ class ConfigProvider return loadConfigFromGlob(__DIR__ . '/../config/{,*.}config.php'); } - public static function applyRoutesPrefix(array $routes): array + public static function applyRoutesPrefix(array $routes, bool $multiSegmentEnabled): array { $healthRoute = self::buildUnversionedHealthRouteFromExistingRoutes($routes); - $prefixedRoutes = map($routes, static function (array $route) { + $prefixedRoutes = map($routes, static function (array $route) use ($multiSegmentEnabled) { ['path' => $path] = $route; + if ($multiSegmentEnabled) { + $path = str_replace('{shortCode}', '{shortCode:.+}', $path); + } $route['path'] = sprintf('%s%s', self::ROUTES_PREFIX, $path); return $route; @@ -40,7 +44,7 @@ class ConfigProvider return null; } - $path = $healthRoute['path']; + ['path' => $path] = $healthRoute; $healthRoute['path'] = sprintf('%s%s', self::UNVERSIONED_ROUTES_PREFIX, $path); $healthRoute['name'] = self::UNVERSIONED_HEALTH_ENDPOINT_NAME; diff --git a/module/Rest/test-api/Middleware/CorsTest.php b/module/Rest/test-api/Middleware/CorsTest.php index bffb0421..b09e2b3b 100644 --- a/module/Rest/test-api/Middleware/CorsTest.php +++ b/module/Rest/test-api/Middleware/CorsTest.php @@ -71,7 +71,7 @@ class CorsTest extends ApiTestCase public function providePreflightEndpoints(): iterable { -// yield 'invalid route' => ['/foo/bar', 'GET,POST,PUT,PATCH,DELETE']; // TODO This won't work with multi-segment + yield 'invalid route' => ['/foo/bar', 'GET,POST,PUT,PATCH,DELETE']; // TODO This won't work with multi-segment yield 'short URLs route' => ['/short-urls', 'GET,POST']; yield 'tags route' => ['/tags', 'GET,DELETE,PUT']; yield 'health route' => ['/health', 'GET']; diff --git a/module/Rest/test/ConfigProviderTest.php b/module/Rest/test/ConfigProviderTest.php index a3f7d0c9..07fa4e43 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, array $expected): void + public function routesAreProperlyPrefixed(array $routes, bool $multiSegmentEnabled, array $expected): void { - self::assertEquals($expected, ConfigProvider::applyRoutesPrefix($routes)); + self::assertEquals($expected, ConfigProvider::applyRoutesPrefix($routes, $multiSegmentEnabled)); } public function provideRoutesConfig(): iterable @@ -47,6 +47,7 @@ class ConfigProviderTest extends TestCase ['path' => '/baz/foo'], ['path' => '/health'], ], + false, [ ['path' => '/rest/v{version:1|2}/foo'], ['path' => '/rest/v{version:1|2}/bar'], @@ -61,11 +62,25 @@ 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 a3de3e15cb879e2594308a696a6ad7c3820fd8e3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 4 Aug 2022 13:00:09 +0200 Subject: [PATCH 10/16] Updated installer with support for multi-segment slugs flag --- composer.json | 2 +- config/autoload/installer.global.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 60b95c0e..00dfb279 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,7 @@ "shlinkio/shlink-config": "^1.6", "shlinkio/shlink-event-dispatcher": "^2.4", "shlinkio/shlink-importer": "^3.0", - "shlinkio/shlink-installer": "dev-develop#8a44b6e as 8.0", + "shlinkio/shlink-installer": "^8.0", "shlinkio/shlink-ip-geolocation": "^2.2", "symfony/console": "^6.1", "symfony/filesystem": "^6.1", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 8a452d92..2e120e35 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -43,6 +43,7 @@ return [ Option\UrlShortener\RedirectCacheLifeTimeConfigOption::class, Option\UrlShortener\AutoResolveTitlesConfigOption::class, Option\UrlShortener\AppendExtraPathConfigOption::class, + Option\UrlShortener\EnableMultiSegmentSlugsConfigOption::class, Option\Tracking\IpAnonymizationConfigOption::class, Option\Tracking\OrphanVisitsTrackingConfigOption::class, Option\Tracking\DisableTrackParamConfigOption::class, From 3d5ddce621f35fdaac1f0184b91d82ed54cbe656 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 4 Aug 2022 16:10:54 +0200 Subject: [PATCH 11/16] Ensured multi-segment feature flag affects how append_extra_path is checked --- config/autoload/url-shortener.global.php | 1 + .../Core/src/Options/UrlShortenerOptions.php | 11 ++++++++ .../ExtraPathRedirectMiddleware.php | 19 +++++++++++--- .../ExtraPathRedirectMiddlewareTest.php | 25 ++++++++++++++----- 4 files changed, 47 insertions(+), 9 deletions(-) diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index bdd257d5..bf9ecb93 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -23,6 +23,7 @@ return (static function (): array { 'default_short_codes_length' => $shortCodesLength, 'auto_resolve_titles' => (bool) EnvVars::AUTO_RESOLVE_TITLES->loadFromEnv(false), 'append_extra_path' => (bool) EnvVars::REDIRECT_APPEND_EXTRA_PATH->loadFromEnv(false), + 'multi_segment_slugs_enabled' => (bool) EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->loadFromEnv(false), ], ]; diff --git a/module/Core/src/Options/UrlShortenerOptions.php b/module/Core/src/Options/UrlShortenerOptions.php index 57f4bc37..ce4bc86d 100644 --- a/module/Core/src/Options/UrlShortenerOptions.php +++ b/module/Core/src/Options/UrlShortenerOptions.php @@ -12,6 +12,7 @@ class UrlShortenerOptions extends AbstractOptions private bool $autoResolveTitles = false; private bool $appendExtraPath = false; + private bool $multiSegmentSlugsEnabled = false; public function autoResolveTitles(): bool { @@ -32,4 +33,14 @@ class UrlShortenerOptions extends AbstractOptions { $this->appendExtraPath = $appendExtraPath; } + + public function multiSegmentSlugsEnabled(): bool + { + return $this->multiSegmentSlugsEnabled; + } + + protected function setMultiSegmentSlugsEnabled(bool $multiSegmentSlugsEnabled): void + { + $this->multiSegmentSlugsEnabled = $multiSegmentSlugsEnabled; + } } diff --git a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php index 2704cadb..44cab54c 100644 --- a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php @@ -38,9 +38,7 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface { /** @var NotFoundType|null $notFoundType */ $notFoundType = $request->getAttribute(NotFoundType::class); - - // This logic is applied only if actively opted in and current URL is potentially /{shortCode}/[...] - if (! $notFoundType?->isRegularNotFound() || ! $this->urlShortenerOptions->appendExtraPath()) { + if (! $this->shouldApplyLogic($notFoundType)) { return $handler->handle($request); } @@ -61,6 +59,21 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface } } + private function shouldApplyLogic(?NotFoundType $notFoundType): bool + { + if ($notFoundType === null || ! $this->urlShortenerOptions->appendExtraPath()) { + return false; + } + + return ( + // If multi-segment slugs are enabled, the appropriate not-found type is "invalid_short_url" + $this->urlShortenerOptions->multiSegmentSlugsEnabled() && $notFoundType->isInvalidShortUrl() + ) || ( + // If multi-segment slugs are disabled, the appropriate not-found type is "regular_404" + ! $this->urlShortenerOptions->multiSegmentSlugsEnabled() && $notFoundType->isRegularNotFound() + ); + } + /** * @return array{0: string, 1: string|null} */ diff --git a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php index d8997524..f4e31c5d 100644 --- a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php +++ b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php @@ -16,6 +16,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; @@ -65,9 +66,11 @@ class ExtraPathRedirectMiddlewareTest extends TestCase */ public function handlerIsCalledWhenConfigPreventsRedirectWithExtraPath( bool $appendExtraPath, + bool $multiSegmentEnabled, ServerRequestInterface $request, ): void { $this->options->appendExtraPath = $appendExtraPath; + $this->options->multiSegmentSlugsEnabled = $multiSegmentEnabled; $this->middleware->process($request, $this->handler->reveal()); @@ -83,20 +86,30 @@ class ExtraPathRedirectMiddlewareTest extends TestCase $buildReq = static fn (?NotFoundType $type): ServerRequestInterface => $baseReq->withAttribute(NotFoundType::class, $type); - yield 'disabled option' => [false, $buildReq(NotFoundType::fromRequest($baseReq, '/foo/bar'))]; - yield 'base_url error' => [true, $buildReq(NotFoundType::fromRequest($baseReq, ''))]; + yield 'disabled option' => [false, false, $buildReq(NotFoundType::fromRequest($baseReq, '/foo/bar'))]; + yield 'no error type' => [true, false, $buildReq(null)]; + yield 'base_url error' => [true, false, $buildReq(NotFoundType::fromRequest($baseReq, ''))]; yield 'invalid_short_url error' => [ true, - $buildReq(NotFoundType::fromRequest($baseReq, ''))->withAttribute( + false, + $buildReq(NotFoundType::fromRequest($baseReq->withUri(new Uri('/foo'))->withAttribute( RouteResult::class, RouteResult::fromRoute(new Route( - '', + '/foo', $this->prophesize(MiddlewareInterface::class)->reveal(), ['GET'], + RedirectAction::class, )), - ), + ), '')), + ]; + yield 'regular_404 error with multi-segment slugs' => [ + true, + true, + $buildReq(NotFoundType::fromRequest($baseReq->withUri(new Uri('/foo'))->withAttribute( + RouteResult::class, + RouteResult::fromRouteFailure(['GET']), + ), '')), ]; - yield 'no error type' => [true, $buildReq(null)]; } /** @test */ From efe655f880ec7e8d873bea5c8aa88dfaa4305237 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 4 Aug 2022 17:03:08 +0200 Subject: [PATCH 12/16] Enhanced ExtraPathRedirectMiddleware so that it supports multi-segment slugs --- .../ExtraPathRedirectMiddleware.php | 54 +++++++++------ .../ExtraPathRedirectMiddlewareTest.php | 69 ++++++++++++++----- 2 files changed, 87 insertions(+), 36 deletions(-) diff --git a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php index 44cab54c..bb350aa2 100644 --- a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php @@ -18,8 +18,10 @@ use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilderInterface; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; -use function array_pad; +use function array_slice; +use function count; use function explode; +use function implode; use function sprintf; use function trim; @@ -42,21 +44,7 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface return $handler->handle($request); } - $uri = $request->getUri(); - $query = $request->getQueryParams(); - [$potentialShortCode, $extraPath] = $this->resolvePotentialShortCodeAndExtraPath($uri); - $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($potentialShortCode, $uri->getAuthority()); - - try { - // TODO Try pieces of the URL in order to match multi-segment slugs too - $shortUrl = $this->resolver->resolveEnabledShortUrl($identifier); - $this->requestTracker->trackIfApplicable($shortUrl, $request); - - $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $query, $extraPath); - return $this->redirectResponseHelper->buildRedirectResponse($longUrl); - } catch (ShortUrlNotFoundException) { - return $handler->handle($request); - } + return $this->tryToResolveRedirect($request, $handler); } private function shouldApplyLogic(?NotFoundType $notFoundType): bool @@ -74,14 +62,40 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface ); } + private function tryToResolveRedirect( + ServerRequestInterface $request, + RequestHandlerInterface $handler, + int $shortCodeSegments = 1, + ): ResponseInterface { + $uri = $request->getUri(); + $query = $request->getQueryParams(); + [$potentialShortCode, $extraPath] = $this->resolvePotentialShortCodeAndExtraPath($uri, $shortCodeSegments); + $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($potentialShortCode, $uri->getAuthority()); + + try { + $shortUrl = $this->resolver->resolveEnabledShortUrl($identifier); + $this->requestTracker->trackIfApplicable($shortUrl, $request); + + $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $query, $extraPath); + return $this->redirectResponseHelper->buildRedirectResponse($longUrl); + } catch (ShortUrlNotFoundException) { + if ($extraPath === null || ! $this->urlShortenerOptions->multiSegmentSlugsEnabled()) { + return $handler->handle($request); + } + + return $this->tryToResolveRedirect($request, $handler, $shortCodeSegments + 1); + } + } + /** * @return array{0: string, 1: string|null} */ - private function resolvePotentialShortCodeAndExtraPath(UriInterface $uri): array + private function resolvePotentialShortCodeAndExtraPath(UriInterface $uri, int $shortCodeSegments): array { - $pathParts = explode('/', trim($uri->getPath(), '/'), 2); - [$potentialShortCode, $extraPath] = array_pad($pathParts, 2, null); + $parts = explode('/', trim($uri->getPath(), '/')); + $shortCode = array_slice($parts, 0, $shortCodeSegments); + $extraPath = array_slice($parts, $shortCodeSegments); - return [$potentialShortCode, $extraPath === null ? null : sprintf('/%s', $extraPath)]; + return [implode('/', $shortCode), count($extraPath) > 0 ? sprintf('/%s', implode('/', $extraPath)) : null]; } } diff --git a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php index f4e31c5d..4099faea 100644 --- a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php +++ b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php @@ -28,6 +28,8 @@ use Shlinkio\Shlink\Core\ShortUrl\Middleware\ExtraPathRedirectMiddleware; use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface; use Shlinkio\Shlink\Core\Visit\RequestTrackerInterface; +use function str_starts_with; + class ExtraPathRedirectMiddlewareTest extends TestCase { use ProphecyTrait; @@ -74,6 +76,7 @@ class ExtraPathRedirectMiddlewareTest extends TestCase $this->middleware->process($request, $this->handler->reveal()); + $this->handler->handle($request)->shouldHaveBeenCalledOnce(); $this->resolver->resolveEnabledShortUrl(Argument::cetera())->shouldNotHaveBeenCalled(); $this->requestTracker->trackIfApplicable(Argument::cetera())->shouldNotHaveBeenCalled(); $this->redirectionBuilder->buildShortUrlRedirect(Argument::cetera())->shouldNotHaveBeenCalled(); @@ -112,49 +115,83 @@ class ExtraPathRedirectMiddlewareTest extends TestCase ]; } - /** @test */ - public function handlerIsCalledWhenNoShortUrlIsFound(): void - { + /** + * @test + * @dataProvider provideResolves + */ + public function handlerIsCalledWhenNoShortUrlIsFoundAfterExpectedAmountOfIterations( + bool $multiSegmentEnabled, + int $expectedResolveCalls, + ): void { + $this->options->multiSegmentSlugsEnabled = $multiSegmentEnabled; + $type = $this->prophesize(NotFoundType::class); $type->isRegularNotFound()->willReturn(true); + $type->isInvalidShortUrl()->willReturn(true); $request = ServerRequestFactory::fromGlobals()->withAttribute(NotFoundType::class, $type->reveal()) ->withUri(new Uri('/shortCode/bar/baz')); - $resolve = $this->resolver->resolveEnabledShortUrl(Argument::cetera())->willThrow( - ShortUrlNotFoundException::class, - ); + $resolve = $this->resolver->resolveEnabledShortUrl( + Argument::that(fn (ShortUrlIdentifier $identifier) => str_starts_with($identifier->shortCode, 'shortCode')), + )->willThrow(ShortUrlNotFoundException::class); $this->middleware->process($request, $this->handler->reveal()); - $resolve->shouldHaveBeenCalledOnce(); + $resolve->shouldHaveBeenCalledTimes($expectedResolveCalls); $this->requestTracker->trackIfApplicable(Argument::cetera())->shouldNotHaveBeenCalled(); $this->redirectionBuilder->buildShortUrlRedirect(Argument::cetera())->shouldNotHaveBeenCalled(); $this->redirectResponseHelper->buildRedirectResponse(Argument::cetera())->shouldNotHaveBeenCalled(); } - /** @test */ - public function visitIsTrackedAndRedirectIsReturnedWhenShortUrlIsFound(): void - { + /** + * @test + * @dataProvider provideResolves + */ + public function visitIsTrackedAndRedirectIsReturnedWhenShortUrlIsFoundAfterExpectedAmountOfIterations( + bool $multiSegmentEnabled, + int $expectedResolveCalls, + ?string $expectedExtraPath, + ): void { + $this->options->multiSegmentSlugsEnabled = $multiSegmentEnabled; + $type = $this->prophesize(NotFoundType::class); $type->isRegularNotFound()->willReturn(true); + $type->isInvalidShortUrl()->willReturn(true); $request = ServerRequestFactory::fromGlobals()->withAttribute(NotFoundType::class, $type->reveal()) ->withUri(new Uri('https://doma.in/shortCode/bar/baz')); $shortUrl = ShortUrl::withLongUrl(''); - $identifier = ShortUrlIdentifier::fromShortCodeAndDomain('shortCode', 'doma.in'); - - $resolve = $this->resolver->resolveEnabledShortUrl($identifier)->willReturn($shortUrl); - $buildLongUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, [], '/bar/baz')->willReturn( - 'the_built_long_url', + $identifier = Argument::that( + fn (ShortUrlIdentifier $identifier) => str_starts_with($identifier->shortCode, 'shortCode'), ); + + $currentIteration = 1; + $resolve = $this->resolver->resolveEnabledShortUrl($identifier)->will( + function () use ($shortUrl, &$currentIteration, $expectedResolveCalls): ShortUrl { + if ($expectedResolveCalls === $currentIteration) { + return $shortUrl; + } + + $currentIteration++; + throw ShortUrlNotFoundException::fromNotFound(ShortUrlIdentifier::fromShortUrl($shortUrl)); + }, + ); + $buildLongUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, [], $expectedExtraPath) + ->willReturn('the_built_long_url'); $buildResp = $this->redirectResponseHelper->buildRedirectResponse('the_built_long_url')->willReturn( new RedirectResponse(''), ); $this->middleware->process($request, $this->handler->reveal()); - $resolve->shouldHaveBeenCalledOnce(); + $resolve->shouldHaveBeenCalledTimes($expectedResolveCalls); $buildLongUrl->shouldHaveBeenCalledOnce(); $buildResp->shouldHaveBeenCalledOnce(); $this->requestTracker->trackIfApplicable($shortUrl, $request)->shouldHaveBeenCalledOnce(); } + + public function provideResolves(): iterable + { + yield [false, 1, '/bar/baz']; + yield [true, 3, null]; + } } From 6834e72c4af225a557b10e99077e447b8e8384f3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 4 Aug 2022 17:15:35 +0200 Subject: [PATCH 13/16] Updated changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52a0ed40..4531be0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added +* [#854](https://github.com/shlinkio/shlink/issues/854) Added support for multi-segment custom slugs. + + The feature is disabled by default, but you can optionally opt in. If you do, you will be able to create short URLs with multiple segments in the custom slug, like `https://example.com/foo/bar/baz`. + * [#1280](https://github.com/shlinkio/shlink/issues/1280) Added missing visit-related commands. Now you can run `tag:visits`, `domain:visits`, `visit:orphan` or `visit:non-orphan` to get the corresponding list of visits from the command line. From fc0d99be4116e2e3ccdc7dde9fc4bddb0b4652c1 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 5 Aug 2022 08:38:05 +0200 Subject: [PATCH 14/16] Ensure filtering of custom-slug is different depending on the multi-sement lugsfeature flag --- module/CLI/config/dependencies.config.php | 4 +-- .../ShortUrl/CreateShortUrlCommand.php | 17 +++++++----- .../ShortUrl/CreateShortUrlCommandTest.php | 4 +-- module/Core/src/Model/ShortUrlEdit.php | 1 + module/Core/src/Model/ShortUrlMeta.php | 1 + .../Core/src/Options/UrlShortenerOptions.php | 26 +++++++++++++++++++ .../src/Validation/ShortUrlInputFilter.php | 14 +++++----- module/Core/test/Model/ShortUrlMetaTest.php | 15 ++++++++--- module/Rest/config/dependencies.config.php | 7 ++++- .../ShortUrl/AbstractCreateShortUrlAction.php | 6 +++-- .../Action/ShortUrl/CreateShortUrlAction.php | 2 ++ .../ShortUrl/CreateShortUrlActionTest.php | 7 ++++- .../SingleStepCreateShortUrlActionTest.php | 2 ++ 13 files changed, 82 insertions(+), 24 deletions(-) diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 933affd0..6920e839 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -11,6 +11,7 @@ use Laminas\ServiceManager\Factory\InvokableFactory; use Shlinkio\Shlink\Common\Doctrine\NoDbNameConnectionFactory; use Shlinkio\Shlink\Core\Domain\DomainService; use Shlinkio\Shlink\Core\Options\TrackingOptions; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Service; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer; @@ -81,8 +82,7 @@ return [ Command\ShortUrl\CreateShortUrlCommand::class => [ Service\UrlShortener::class, ShortUrlStringifier::class, - 'config.url_shortener.default_short_codes_length', - 'config.url_shortener.domain.hostname', + UrlShortenerOptions::class, ], Command\ShortUrl\ResolveUrlCommand::class => [Service\ShortUrl\ShortUrlResolver::class], Command\ShortUrl\ListShortUrlsCommand::class => [ diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index 3334ae6a..6b4cce1a 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -5,9 +5,11 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\Core\Config\EnvVars; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use Shlinkio\Shlink\Core\Validation\ShortUrlInputFilter; @@ -19,6 +21,7 @@ use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; use function array_map; +use function explode; use function Functional\curry; use function Functional\flatten; use function Functional\unique; @@ -29,14 +32,15 @@ class CreateShortUrlCommand extends Command public const NAME = 'short-url:create'; private ?SymfonyStyle $io; + private string $defaultDomain; public function __construct( - private UrlShortenerInterface $urlShortener, - private ShortUrlStringifierInterface $stringifier, - private int $defaultShortCodeLength, - private string $defaultDomain, + private readonly UrlShortenerInterface $urlShortener, + private readonly ShortUrlStringifierInterface $stringifier, + private readonly UrlShortenerOptions $options, ) { parent::__construct(); + $this->defaultDomain = $this->options->domain()['hostname'] ?? ''; } protected function configure(): void @@ -150,11 +154,11 @@ class CreateShortUrlCommand extends Command return ExitCodes::EXIT_FAILURE; } - $explodeWithComma = curry('explode')(','); + $explodeWithComma = curry(explode(...))(','); $tags = unique(flatten(array_map($explodeWithComma, $input->getOption('tags')))); $customSlug = $input->getOption('custom-slug'); $maxVisits = $input->getOption('max-visits'); - $shortCodeLength = $input->getOption('short-code-length') ?? $this->defaultShortCodeLength; + $shortCodeLength = $input->getOption('short-code-length') ?? $this->options->defaultShortCodesLength(); $doValidateUrl = $input->getOption('validate-url'); try { @@ -171,6 +175,7 @@ class CreateShortUrlCommand extends Command ShortUrlInputFilter::TAGS => $tags, ShortUrlInputFilter::CRAWLABLE => $input->getOption('crawlable'), ShortUrlInputFilter::FORWARD_QUERY => !$input->getOption('no-forward-query'), + EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->value => $this->options->multiSegmentSlugsEnabled(), ])); $io->writeln([ diff --git a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php index 3ec90412..73d2b785 100644 --- a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php @@ -14,6 +14,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use ShlinkioTest\Shlink\CLI\CliTestUtilsTrait; @@ -38,8 +39,7 @@ class CreateShortUrlCommandTest extends TestCase $command = new CreateShortUrlCommand( $this->urlShortener->reveal(), $this->stringifier->reveal(), - 5, - self::DEFAULT_DOMAIN, + new UrlShortenerOptions(['defaultShortCodesLength' => 5, 'domain' => ['hostname' => self::DEFAULT_DOMAIN]]), ); $this->commandTester = $this->testerForCommand($command); } diff --git a/module/Core/src/Model/ShortUrlEdit.php b/module/Core/src/Model/ShortUrlEdit.php index 325ee339..2d39d657 100644 --- a/module/Core/src/Model/ShortUrlEdit.php +++ b/module/Core/src/Model/ShortUrlEdit.php @@ -14,6 +14,7 @@ use function Shlinkio\Shlink\Core\getOptionalBoolFromInputFilter; use function Shlinkio\Shlink\Core\getOptionalIntFromInputFilter; use function Shlinkio\Shlink\Core\normalizeDate; +// TODO Rename to ShortUrlEdition final class ShortUrlEdit implements TitleResolutionModelInterface { private bool $longUrlPropWasProvided = false; diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index f43f929d..e5b621c2 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -16,6 +16,7 @@ use function Shlinkio\Shlink\Core\normalizeDate; use const Shlinkio\Shlink\DEFAULT_SHORT_CODES_LENGTH; +// TODO Rename to ShortUrlCreation final class ShortUrlMeta implements TitleResolutionModelInterface { private string $longUrl; diff --git a/module/Core/src/Options/UrlShortenerOptions.php b/module/Core/src/Options/UrlShortenerOptions.php index ce4bc86d..38e185c2 100644 --- a/module/Core/src/Options/UrlShortenerOptions.php +++ b/module/Core/src/Options/UrlShortenerOptions.php @@ -6,14 +6,40 @@ namespace Shlinkio\Shlink\Core\Options; use Laminas\Stdlib\AbstractOptions; +use const Shlinkio\Shlink\DEFAULT_SHORT_CODES_LENGTH; + class UrlShortenerOptions extends AbstractOptions { protected $__strictMode__ = false; // phpcs:ignore + private array $domain = []; + private int $defaultShortCodesLength = DEFAULT_SHORT_CODES_LENGTH; private bool $autoResolveTitles = false; private bool $appendExtraPath = false; private bool $multiSegmentSlugsEnabled = false; + public function domain(): array + { + return $this->domain; + } + + protected function setDomain(array $domain): self + { + $this->domain = $domain; + return $this; + } + + public function defaultShortCodesLength(): int + { + return $this->defaultShortCodesLength; + } + + protected function setDefaultShortCodesLength(int $defaultShortCodesLength): self + { + $this->defaultShortCodesLength = $defaultShortCodesLength; + return $this; + } + public function autoResolveTitles(): bool { return $this->autoResolveTitles; diff --git a/module/Core/src/Validation/ShortUrlInputFilter.php b/module/Core/src/Validation/ShortUrlInputFilter.php index d4d0f818..283d9a94 100644 --- a/module/Core/src/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/Validation/ShortUrlInputFilter.php @@ -10,12 +10,13 @@ use Laminas\InputFilter\Input; use Laminas\InputFilter\InputFilter; use Laminas\Validator; use Shlinkio\Shlink\Common\Validation; +use Shlinkio\Shlink\Core\Config\EnvVars; use Shlinkio\Shlink\Rest\Entity\ApiKey; use function is_string; -use function ltrim; use function str_replace; use function substr; +use function trim; use const Shlinkio\Shlink\MIN_SHORT_CODES_LENGTH; @@ -40,7 +41,7 @@ class ShortUrlInputFilter extends InputFilter private function __construct(array $data, bool $requireLongUrl) { - $this->initialize($requireLongUrl); + $this->initialize($requireLongUrl, $data[EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->value] ?? false); $this->setData($data); } @@ -54,7 +55,7 @@ class ShortUrlInputFilter extends InputFilter return new self($data, false); } - private function initialize(bool $requireLongUrl): void + private function initialize(bool $requireLongUrl, bool $multiSegmentEnabled): void { $longUrlInput = $this->createInput(self::LONG_URL, $requireLongUrl); $longUrlInput->getValidatorChain()->attach(new Validator\NotEmpty([ @@ -77,9 +78,10 @@ class ShortUrlInputFilter extends InputFilter // FIXME The only way to enforce the NotEmpty validator to be evaluated when the value is provided but it's // empty, is by using the deprecated setContinueIfEmpty $customSlug = $this->createInput(self::CUSTOM_SLUG, false)->setContinueIfEmpty(true); - $customSlug->getFilterChain()->attach(new Filter\Callback( - static fn (mixed $value) => is_string($value) ? ltrim(str_replace(' ', '-', $value), '/') : $value, - )); + $customSlug->getFilterChain()->attach(new Filter\Callback(match ($multiSegmentEnabled) { + true => static fn (mixed $v) => is_string($v) ? trim(str_replace(' ', '-', $v), '/') : $v, + false => static fn (mixed $v) => is_string($v) ? str_replace([' ', '/'], '-', $v) : $v, + })); $customSlug->getValidatorChain()->attach(new Validator\NotEmpty([ Validator\NotEmpty::STRING, Validator\NotEmpty::SPACE, diff --git a/module/Core/test/Model/ShortUrlMetaTest.php b/module/Core/test/Model/ShortUrlMetaTest.php index 833a25bb..975dc372 100644 --- a/module/Core/test/Model/ShortUrlMetaTest.php +++ b/module/Core/test/Model/ShortUrlMetaTest.php @@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Core\Model; use Cake\Chronos\Chronos; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Core\Config\EnvVars; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Validation\ShortUrlInputFilter; @@ -74,12 +75,16 @@ class ShortUrlMetaTest extends TestCase * @test * @dataProvider provideCustomSlugs */ - public function properlyCreatedInstanceReturnsValues(string $customSlug, string $expectedSlug): void - { + public function properlyCreatedInstanceReturnsValues( + string $customSlug, + string $expectedSlug, + bool $multiSegmentEnabled = false, + ): void { $meta = ShortUrlMeta::fromRawData([ 'validSince' => Chronos::parse('2015-01-01')->toAtomString(), 'customSlug' => $customSlug, 'longUrl' => '', + EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->value => $multiSegmentEnabled, ]); self::assertTrue($meta->hasValidSince()); @@ -103,8 +108,10 @@ class ShortUrlMetaTest extends TestCase yield ['foo bar', 'foo-bar']; yield ['foo bar baz', 'foo-bar-baz']; yield ['foo bar-baz', 'foo-bar-baz']; - yield ['foo/bar/baz', 'foo/bar/baz']; - yield ['/foo/bar/baz', 'foo/bar/baz']; + yield ['foo/bar/baz', 'foo/bar/baz', true]; + yield ['/foo/bar/baz', 'foo/bar/baz', true]; + yield ['foo/bar/baz', 'foo-bar-baz']; + yield ['/foo/bar/baz', '-foo-bar-baz']; yield ['wp-admin.php', 'wp-admin.php']; yield ['UPPER_lower', 'UPPER_lower']; yield ['more~url_special.chars', 'more~url_special.chars']; diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 34be71f4..189180b0 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -61,10 +61,15 @@ return [ Action\HealthAction::class => ['em', Options\AppOptions::class], Action\MercureInfoAction::class => [LcobucciJwtProvider::class, 'config.mercure'], - Action\ShortUrl\CreateShortUrlAction::class => [Service\UrlShortener::class, ShortUrlDataTransformer::class], + Action\ShortUrl\CreateShortUrlAction::class => [ + Service\UrlShortener::class, + ShortUrlDataTransformer::class, + Options\UrlShortenerOptions::class, + ], Action\ShortUrl\SingleStepCreateShortUrlAction::class => [ Service\UrlShortener::class, ShortUrlDataTransformer::class, + Options\UrlShortenerOptions::class, ], Action\ShortUrl\EditShortUrlAction::class => [Service\ShortUrlService::class, ShortUrlDataTransformer::class], Action\ShortUrl\DeleteShortUrlAction::class => [Service\ShortUrl\DeleteShortUrlService::class], diff --git a/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php index 90616dc5..f122601b 100644 --- a/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php @@ -10,14 +10,16 @@ use Psr\Http\Message\ServerRequestInterface as Request; use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; abstract class AbstractCreateShortUrlAction extends AbstractRestAction { public function __construct( - private UrlShortenerInterface $urlShortener, - private DataTransformerInterface $transformer, + private readonly UrlShortenerInterface $urlShortener, + private readonly DataTransformerInterface $transformer, + protected readonly UrlShortenerOptions $urlShortenerOptions, ) { } diff --git a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php index d8b873a6..376c6bec 100644 --- a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Action\ShortUrl; use Psr\Http\Message\ServerRequestInterface as Request; +use Shlinkio\Shlink\Core\Config\EnvVars; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Validation\ShortUrlInputFilter; @@ -22,6 +23,7 @@ class CreateShortUrlAction extends AbstractCreateShortUrlAction { $payload = (array) $request->getParsedBody(); $payload[ShortUrlInputFilter::API_KEY] = AuthenticationMiddleware::apiKeyFromRequest($request); + $payload[EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->value] = $this->urlShortenerOptions->multiSegmentSlugsEnabled(); return ShortUrlMeta::fromRawData($payload); } diff --git a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php index ffcd6c62..206b016f 100644 --- a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php @@ -16,6 +16,7 @@ use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Rest\Action\ShortUrl\CreateShortUrlAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -34,7 +35,11 @@ class CreateShortUrlActionTest extends TestCase $this->transformer = $this->prophesize(DataTransformerInterface::class); $this->transformer->transform(Argument::type(ShortUrl::class))->willReturn([]); - $this->action = new CreateShortUrlAction($this->urlShortener->reveal(), $this->transformer->reveal()); + $this->action = new CreateShortUrlAction( + $this->urlShortener->reveal(), + $this->transformer->reveal(), + new UrlShortenerOptions(), + ); } /** @test */ diff --git a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php index 8bb1482a..e3fd3e10 100644 --- a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php @@ -12,6 +12,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; use Shlinkio\Shlink\Rest\Action\ShortUrl\SingleStepCreateShortUrlAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -33,6 +34,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase $this->action = new SingleStepCreateShortUrlAction( $this->urlShortener->reveal(), $this->transformer->reveal(), + new UrlShortenerOptions(), ); } From 8961191b2eca18387051cfc46cc128c416bd2995 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 5 Aug 2022 16:18:53 +0200 Subject: [PATCH 15/16] Documented ADR for multi-segment slugs --- composer.json | 2 +- ...estrictions-and-permissions-in-api-keys.md | 2 +- ...e-url-invalid-short-url-and-regular-404.md | 2 +- ...-08-05-migrate-to-a-new-caching-library.md | 2 +- ...-have-precedence-over-installer-options.md | 2 +- ...8-05-support-multi-segment-custom-slugs.md | 42 +++++++++++++++++++ docs/adr/README.md | 1 + 7 files changed, 48 insertions(+), 5 deletions(-) create mode 100644 docs/adr/2022-08-05-support-multi-segment-custom-slugs.md diff --git a/composer.json b/composer.json index 00dfb279..c6e293bd 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": "dev-main#b395e99 as 4.5", + "shlinkio/shlink-common": "^4.5", "shlinkio/shlink-config": "^1.6", "shlinkio/shlink-event-dispatcher": "^2.4", "shlinkio/shlink-importer": "^3.0", diff --git a/docs/adr/2021-01-17-support-restrictions-and-permissions-in-api-keys.md b/docs/adr/2021-01-17-support-restrictions-and-permissions-in-api-keys.md index 4c3b6c52..16dea9d3 100644 --- a/docs/adr/2021-01-17-support-restrictions-and-permissions-in-api-keys.md +++ b/docs/adr/2021-01-17-support-restrictions-and-permissions-in-api-keys.md @@ -16,7 +16,7 @@ The intention is to implement a system that allows adding to API keys as many of Supporting more restrictions in the future is also desirable. -## Considered option +## Considered options * Using an ACL/RBAC library, and checking roles in a middleware. * Using a service that, provided an API key, tells if certain resource is reachable while it also allows building queries dynamically. diff --git a/docs/adr/2021-02-07-track-visits-to-base-url-invalid-short-url-and-regular-404.md b/docs/adr/2021-02-07-track-visits-to-base-url-invalid-short-url-and-regular-404.md index 983410d1..f4e5a288 100644 --- a/docs/adr/2021-02-07-track-visits-to-base-url-invalid-short-url-and-regular-404.md +++ b/docs/adr/2021-02-07-track-visits-to-base-url-invalid-short-url-and-regular-404.md @@ -11,7 +11,7 @@ However, it does not track visits to any of those, just to valid short URLs. The intention is to change that, and allow users to track the cases mentioned above. -## Considered option +## Considered options * Create a new table to track visits o this kind. * Reuse the existing `visits` table, by making `short_url_id` nullable and adding a couple of other fields. diff --git a/docs/adr/2021-08-05-migrate-to-a-new-caching-library.md b/docs/adr/2021-08-05-migrate-to-a-new-caching-library.md index aa19f160..11cf4fc6 100644 --- a/docs/adr/2021-08-05-migrate-to-a-new-caching-library.md +++ b/docs/adr/2021-08-05-migrate-to-a-new-caching-library.md @@ -13,7 +13,7 @@ However, after the creation of the caching PSRs ([PSR-6 - Cache](https://www.php Also, Shlink needs support for Redis clusters and Redis sentinels, which is not supported by `doctrine/cache` Redis adapters. -## Considered option +## Considered options After some research, the only packages that seem to support the capabilities required by Shlink and also seem healthy, are these: diff --git a/docs/adr/2022-01-15-update-env-vars-behavior-to-have-precedence-over-installer-options.md b/docs/adr/2022-01-15-update-env-vars-behavior-to-have-precedence-over-installer-options.md index df11538c..e5b72b09 100644 --- a/docs/adr/2022-01-15-update-env-vars-behavior-to-have-precedence-over-installer-options.md +++ b/docs/adr/2022-01-15-update-env-vars-behavior-to-have-precedence-over-installer-options.md @@ -11,7 +11,7 @@ It is potentially possible to combine both, but if you do so, you will find out A [Twitter survey](https://twitter.com/shlinkio/status/1480614855006732289) has also showed up all participants also found the behavior should be the opposite. -## Considered option +## Considered options * Move the logic to read env vars to another config file which always overrides installer options. * Move the logic to read env vars to a config post-processor which overrides config dynamically, only if the appropriate env var had been defined. diff --git a/docs/adr/2022-08-05-support-multi-segment-custom-slugs.md b/docs/adr/2022-08-05-support-multi-segment-custom-slugs.md new file mode 100644 index 00000000..99d82668 --- /dev/null +++ b/docs/adr/2022-08-05-support-multi-segment-custom-slugs.md @@ -0,0 +1,42 @@ +# Support multi-segment custom slugs + +* Status: Accepted +* Date: 2022-08-05 + +## Context and problem statement + +There's a new requirement to support multi-segment custom slugs (as in `https://exam.ple/foo/bar/baz`). + +The internal router does not support this at the moment, as it only matches the shortCode in one of the segments. + +## Considered options + +* Tweak the internal router, so that it is capable of matching multiple segments for the slug, in every route that requires it. +* Define a new set of routes with a short prefix that allows configuring multi-segment in those, without touching the existing routes. +* Let the router fail, and use a middleware to fall back to the proper route (similar to what was done for the extra path forwarding feature). + +## Decision outcome + +Even though I was initially inclined to use a fallback middleware, that has turned out to be harder than anticipated, because there are several possible routes where the slug is used, and we would still need some kind of router to determine which one matches. + +Because of that, the selected approach has been to tweak the existing router, so that it can match multiple segments, and moving the configuration of routes to a common place so that they can be defined in the proper order that prevents conflicts. + +## Pros and Cons of the Options + +### Tweaking the router + +* Bad: It requires routes to be defined in a specific order, and remember it in the future if more routes are added. +* Good: It initially requires fewer changes. +* Good: Once routes are defined in the proper order, all the internal logic works out of the box. + +### Defining new routes + +* Bad: The end-user experience gets affected. +* Bad: Probably a lot of side effects would happen when it comes to assembling short URLs. +* Bad: Routing needs to be configured twice, resolving the same logic. +* Bad: It turns out to still conflict with some routes, even with the prefix, which defeats what looked like its main benefit. + +### Let routing fail and fall back in middleware + +* Good: Does not require changing routes configuration, which means less side effects. +* Bad: Since many routes can potentially end up in the middleware, there's still the need to have some kind of routing logic. diff --git a/docs/adr/README.md b/docs/adr/README.md index 8fd4a662..7cfccdf7 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -2,6 +2,7 @@ Here listed you will find the different architectural decisions taken in the project, including all the reasoning behind it, options considered, and final outcome. +* [2022-08-05 Support multi-segment custom slugs](2022-08-05-support-multi-segment-custom-slugs.md) * [2022-01-15 Update env vars behavior to have precedence over installer options](2022-01-15-update-env-vars-behavior-to-have-precedence-over-installer-options.md) * [2021-08-05 Migrate to a new caching library](2021-08-05-migrate-to-a-new-caching-library.md) * [2021-02-07 Track visits to 'base_url', 'invalid_short_url' and 'regular_404'](2021-02-07-track-visits-to-base-url-invalid-short-url-and-regular-404.md) From c061c9c3ffcf8d92effb54eb7a43b49e0e4662d4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 5 Aug 2022 16:19:40 +0200 Subject: [PATCH 16/16] Added v3.2.0 to changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4531be0c..c11faf60 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.0] - 2022-08-05 ### Added * [#854](https://github.com/shlinkio/shlink/issues/854) Added support for multi-segment custom slugs.