diff --git a/CHANGELOG.md b/CHANGELOG.md index 52a0ed40..c11faf60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,12 @@ 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. + + 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. 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/composer.json b/composer.json index 9add38d7..c6e293bd 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", - "shlinkio/shlink-common": "dev-main#b3848ad as 4.5", + "ramsey/uuid": "^4.3", + "shlinkio/shlink-common": "^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": "^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", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index c82b4a97..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, @@ -65,13 +66,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/routes.config.php b/config/autoload/routes.config.php new file mode 100644 index 00000000..e7e24916 --- /dev/null +++ b/config/autoload/routes.config.php @@ -0,0 +1,107 @@ +loadFromEnv(false); + + return [ + + 'routes' => [ + // Rest + ...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]), + ], $multiSegment), + + // Non-rest + [ + 'name' => CoreAction\RobotsAction::class, + 'path' => '/robots.txt', + 'middleware' => [ + CoreAction\RobotsAction::class, + ], + 'allowed_methods' => [RequestMethodInterface::METHOD_GET], + ], + [ + 'name' => CoreAction\PixelAction::class, + 'path' => sprintf('/{shortCode%s}/track', $multiSegment ? ':.+' : ''), + 'middleware' => [ + IpAddress::class, + CoreAction\PixelAction::class, + ], + 'allowed_methods' => [RequestMethodInterface::METHOD_GET], + ], + [ + 'name' => CoreAction\QrCodeAction::class, + 'path' => sprintf('/{shortCode%s}/qr-code', $multiSegment ? ':.+' : ''), + 'middleware' => [ + CoreAction\QrCodeAction::class, + ], + 'allowed_methods' => [RequestMethodInterface::METHOD_GET], + ], + [ + 'name' => CoreAction\RedirectAction::class, + 'path' => sprintf('/{shortCode%s}', $multiSegment ? ':.+' : ''), + 'middleware' => [ + IpAddress::class, + CoreAction\RedirectAction::class, + ], + 'allowed_methods' => [RequestMethodInterface::METHOD_GET], + ], + ], + + ]; +})(); 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/config/config.php b/config/config.php index 3dad2105..a1e0428a 100644 --- a/config/config.php +++ b/config/config.php @@ -43,6 +43,8 @@ return (new ConfigAggregator\ConfigAggregator([ $isTestEnv ? 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/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) 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/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/config/routes.config.php b/module/Core/config/routes.config.php deleted file mode 100644 index 07e33c73..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\RedirectAction::class, - 'path' => '/{shortCode}', - 'middleware' => [ - IpAddress::class, - Action\RedirectAction::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], - ], - ], - -]; 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/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/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/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/src/Options/UrlShortenerOptions.php b/module/Core/src/Options/UrlShortenerOptions.php index 57f4bc37..38e185c2 100644 --- a/module/Core/src/Options/UrlShortenerOptions.php +++ b/module/Core/src/Options/UrlShortenerOptions.php @@ -6,12 +6,39 @@ 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 { @@ -32,4 +59,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 9d92067c..bb350aa2 100644 --- a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php @@ -18,19 +18,21 @@ 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; 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, ) { } @@ -38,15 +40,36 @@ 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}/[...] - if (! $notFoundType?->isRegularNotFound() || ! $this->urlShortenerOptions->appendExtraPath()) { + if (! $this->shouldApplyLogic($notFoundType)) { return $handler->handle($request); } + return $this->tryToResolveRedirect($request, $handler); + } + + 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() + ); + } + + private function tryToResolveRedirect( + ServerRequestInterface $request, + RequestHandlerInterface $handler, + int $shortCodeSegments = 1, + ): ResponseInterface { $uri = $request->getUri(); $query = $request->getQueryParams(); - [$potentialShortCode, $extraPath] = $this->resolvePotentialShortCodeAndExtraPath($uri); + [$potentialShortCode, $extraPath] = $this->resolvePotentialShortCodeAndExtraPath($uri, $shortCodeSegments); $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($potentialShortCode, $uri->getAuthority()); try { @@ -56,18 +79,23 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $query, $extraPath); return $this->redirectResponseHelper->buildRedirectResponse($longUrl); } catch (ShortUrlNotFoundException) { - return $handler->handle($request); + 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/src/Validation/ShortUrlInputFilter.php b/module/Core/src/Validation/ShortUrlInputFilter.php index 6cd578fb..283d9a94 100644 --- a/module/Core/src/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/Validation/ShortUrlInputFilter.php @@ -10,11 +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 str_replace; use function substr; +use function trim; use const Shlinkio\Shlink\MIN_SHORT_CODES_LENGTH; @@ -39,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); } @@ -53,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([ @@ -76,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) ? 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/src/Visit/RequestTracker.php b/module/Core/src/Visit/RequestTracker.php index dc45e12f..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 @@ -45,10 +47,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, }; } 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/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/Core/test/Model/ShortUrlMetaTest.php b/module/Core/test/Model/ShortUrlMetaTest.php index 1933b3b6..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,7 +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', 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/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php index d8997524..4099faea 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; @@ -27,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; @@ -65,12 +68,15 @@ 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()); + $this->handler->handle($request)->shouldHaveBeenCalledOnce(); $this->resolver->resolveEnabledShortUrl(Argument::cetera())->shouldNotHaveBeenCalled(); $this->requestTracker->trackIfApplicable(Argument::cetera())->shouldNotHaveBeenCalled(); $this->redirectionBuilder->buildShortUrlRedirect(Argument::cetera())->shouldNotHaveBeenCalled(); @@ -83,65 +89,109 @@ 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 */ - 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]; + } } 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/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/config/routes.config.php b/module/Rest/config/routes.config.php deleted file mode 100644 index f318664f..00000000 --- a/module/Rest/config/routes.config.php +++ /dev/null @@ -1,57 +0,0 @@ - [ - Action\HealthAction::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(), - - // 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(), - 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/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/src/Action/ShortUrl/EditShortUrlAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php index 87c21aec..71cf8bf3 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php @@ -17,7 +17,7 @@ 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_ALLOWED_METHODS = [self::METHOD_PATCH]; public function __construct( private ShortUrlServiceInterface $shortUrlService, diff --git a/module/Rest/src/ConfigProvider.php b/module/Rest/src/ConfigProvider.php index 130617d6..3304ce4d 100644 --- a/module/Rest/src/ConfigProvider.php +++ b/module/Rest/src/ConfigProvider.php @@ -4,12 +4,11 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest; -use Closure; - use function Functional\first; use function Functional\map; use function Shlinkio\Shlink\Config\loadConfigFromGlob; use function sprintf; +use function str_replace; class ConfigProvider { @@ -17,45 +16,35 @@ 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, bool $multiSegmentEnabled): array { - $routes = $config['routes'] ?? []; - $healthRoute = $this->buildUnversionedHealthRouteFromExistingRoutes($routes); - - $prefixRoute = static function (array $route) { + $healthRoute = self::buildUnversionedHealthRouteFromExistingRoutes($routes); + $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; - }; - $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) { 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 3efbeacb..b09e2b3b 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/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(), ); } 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)) diff --git a/module/Rest/test/ConfigProviderTest.php b/module/Rest/test/ConfigProviderTest.php index 462947c9..07fa4e43 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); @@ -34,13 +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 { - $configProvider = new ConfigProvider(fn () => ['routes' => $routes]); - - $config = $configProvider(); - - self::assertEquals($expected, $config['routes']); + self::assertEquals($expected, ConfigProvider::applyRoutesPrefix($routes, $multiSegmentEnabled)); } public function provideRoutesConfig(): iterable @@ -52,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'], @@ -66,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'], + ], + ]; } }