Repository: shlinkio/shlink (php)

Issue:
Migrate config options objects to something that allows hydrating public readonly props
<!--
Before opening an issue, just take into account that this is a completely free of charge and open source project.
I'm always happy to help and provide support, but some understanding will be expected.
I do this in my own free time, so expect some delays when implementing new features and fixing bugs, and don't take it personal if an issue gets eventually closed.
You may also be asked to provide tests or ways to reproduce reported bugs.
Try to be polite, and understand it is impossible for an OSS project to cover all use cases.
-->

Currently, all config options objects extend Laminas' AbstractOptions, which requires a setter to hydrate the properties, and therefore, a getter to ensure readonly access.

Find a way to hydrate objects via constructor to ensure all props can be set as `public readonly`.

Pull request:
Feature/centralize multi segment
Closes #1492 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c11faf60..d32bb1b9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org).
 
+## [Unreleased]
+### Added
+* *Nothing*
+
+### Changed
+* [#1495](https://github.com/shlinkio/shlink/issues/1495) Centralized how routes are configured to support multi-segment slugs.
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* *Nothing*
+
+
 ## [3.2.0] - 2022-08-05
 ### Added
 * [#854](https://github.com/shlinkio/shlink/issues/854) Added support for multi-segment custom slugs.
diff --git a/composer.json b/composer.json
index c6e293bd..a2e6c96c 100644
--- a/composer.json
+++ b/composer.json
@@ -43,7 +43,7 @@
         "php-middleware/request-id": "^4.1",
         "pugx/shortid-php": "^1.0",
         "ramsey/uuid": "^4.3",
-        "shlinkio/shlink-common": "^4.5",
+        "shlinkio/shlink-common": "dev-main#6163a03 as 5.0",
         "shlinkio/shlink-config": "^1.6",
         "shlinkio/shlink-event-dispatcher": "^2.4",
         "shlinkio/shlink-importer": "^3.0",
diff --git a/config/autoload/routes.config.php b/config/autoload/routes.config.php
index e7e24916..298b9349 100644
--- a/config/autoload/routes.config.php
+++ b/config/autoload/routes.config.php
@@ -7,23 +7,19 @@ namespace Shlinkio\Shlink;
 use Fig\Http\Message\RequestMethodInterface;
 use RKA\Middleware\IpAddress;
 use Shlinkio\Shlink\Core\Action as CoreAction;
-use Shlinkio\Shlink\Core\Config\EnvVars;
 use Shlinkio\Shlink\Rest\Action;
 use Shlinkio\Shlink\Rest\ConfigProvider;
 use Shlinkio\Shlink\Rest\Middleware;
 use Shlinkio\Shlink\Rest\Middleware\Mercure\NotConfiguredMercureErrorHandler;
 
-use function sprintf;
-
-// The order of the routes defined here matters. Changing it might cause path conflicts
 return (static function (): array {
     $contentNegotiationMiddleware = Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class;
     $dropDomainMiddleware = Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class;
     $overrideDomainMiddleware = Middleware\ShortUrl\OverrideDomainMiddleware::class;
-    $multiSegment = (bool) EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->loadFromEnv(false);
 
     return [
 
+        // The order of the routes defined here matters. Changing it might cause path conflicts
         'routes' => [
             // Rest
             ...ConfigProvider::applyRoutesPrefix([
@@ -64,7 +60,7 @@ return (static function (): array {
                 Action\Domain\DomainRedirectsAction::getRouteDef(),
 
                 Action\MercureInfoAction::getRouteDef([NotConfiguredMercureErrorHandler::class]),
-            ], $multiSegment),
+            ]),
 
             // Non-rest
             [
@@ -77,7 +73,7 @@ return (static function (): array {
             ],
             [
                 'name' => CoreAction\PixelAction::class,
-                'path' => sprintf('/{shortCode%s}/track', $multiSegment ? ':.+' : ''),
+                'path' => '/{shortCode}/track',
                 'middleware' => [
                     IpAddress::class,
                     CoreAction\PixelAction::class,
@@ -86,7 +82,7 @@ return (static function (): array {
             ],
             [
                 'name' => CoreAction\QrCodeAction::class,
-                'path' => sprintf('/{shortCode%s}/qr-code', $multiSegment ? ':.+' : ''),
+                'path' => '/{shortCode}/qr-code',
                 'middleware' => [
                     CoreAction\QrCodeAction::class,
                 ],
@@ -94,7 +90,7 @@ return (static function (): array {
             ],
             [
                 'name' => CoreAction\RedirectAction::class,
-                'path' => sprintf('/{shortCode%s}', $multiSegment ? ':.+' : ''),
+                'path' => '/{shortCode}',
                 'middleware' => [
                     IpAddress::class,
                     CoreAction\RedirectAction::class,
diff --git a/config/autoload/url-shortener.local.php.dist b/config/autoload/url-shortener.local.php.dist
index 20140a9b..0069ffa9 100644
--- a/config/autoload/url-shortener.local.php.dist
+++ b/config/autoload/url-shortener.local.php.dist
@@ -12,6 +12,7 @@ return [
             'hostname' => sprintf('localhost:%s', $isSwoole ? '8080' : '8000'),
         ],
         'auto_resolve_titles' => true,
+//        'multi_segment_slugs_enabled' => true,
     ],
 
 ];
diff --git a/config/config.php b/config/config.php
index a1e0428a..201c1a17 100644
--- a/config/config.php
+++ b/config/config.php
@@ -47,4 +47,5 @@ return (new ConfigAggregator\ConfigAggregator([
     new ConfigAggregator\PhpFileProvider('config/autoload/routes.config.php'),
 ], 'data/cache/app_config.php', [
     Core\Config\BasePathPrefixer::class,
+    Core\Config\MultiSegmentSlugProcessor::class,
 ]))->getMergedConfig();
diff --git a/module/Core/src/Config/MultiSegmentSlugProcessor.php b/module/Core/src/Config/MultiSegmentSlugProcessor.php
new file mode 100644
index 00000000..b9cf2457
--- /dev/null
+++ b/module/Core/src/Config/MultiSegmentSlugProcessor.php
@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Config;
+
+use function Functional\map;
+use function str_replace;
+
+class MultiSegmentSlugProcessor
+{
+    private const SINGLE_SHORT_CODE_PATTERN = '{shortCode}';
+    private const MULTI_SHORT_CODE_PATTERN = '{shortCode:.+}';
+
+    public function __invoke(array $config): array
+    {
+        $multiSegmentEnabled = $config['url_shortener']['multi_segment_slugs_enabled'] ?? false;
+        if (! $multiSegmentEnabled) {
+            return $config;
+        }
+
+        $config['routes'] = map($config['routes'] ?? [], static function (array $route): array {
+            ['path' => $path] = $route;
+            $route['path'] = str_replace(self::SINGLE_SHORT_CODE_PATTERN, self::MULTI_SHORT_CODE_PATTERN, $path);
+            return $route;
+        });
+
+        return $config;
+    }
+}
diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php
index 5946f255..af4f662d 100644
--- a/module/Core/src/Repository/ShortUrlRepository.php
+++ b/module/Core/src/Repository/ShortUrlRepository.php
@@ -84,13 +84,13 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU
            ->where('1=1');
 
         $dateRange = $filtering->dateRange();
-        if ($dateRange?->startDate() !== null) {
+        if ($dateRange?->startDate !== null) {
             $qb->andWhere($qb->expr()->gte('s.dateCreated', ':startDate'));
-            $qb->setParameter('startDate', $dateRange->startDate(), ChronosDateTimeType::CHRONOS_DATETIME);
+            $qb->setParameter('startDate', $dateRange->startDate, ChronosDateTimeType::CHRONOS_DATETIME);
         }
-        if ($dateRange?->endDate() !== null) {
+        if ($dateRange?->endDate !== null) {
             $qb->andWhere($qb->expr()->lte('s.dateCreated', ':endDate'));
-            $qb->setParameter('endDate', $dateRange->endDate(), ChronosDateTimeType::CHRONOS_DATETIME);
+            $qb->setParameter('endDate', $dateRange->endDate, ChronosDateTimeType::CHRONOS_DATETIME);
         }
 
         $searchTerm = $filtering->searchTerm();
diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php
index 3e33d60a..f24153fc 100644
--- a/module/Core/src/Repository/VisitRepository.php
+++ b/module/Core/src/Repository/VisitRepository.php
@@ -245,11 +245,11 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo
     {
         $conn = $this->getEntityManager()->getConnection();
 
-        if ($dateRange?->startDate() !== null) {
-            $qb->andWhere($qb->expr()->gte('v.date', $conn->quote($dateRange->startDate()->toDateTimeString())));
+        if ($dateRange?->startDate !== null) {
+            $qb->andWhere($qb->expr()->gte('v.date', $conn->quote($dateRange->startDate->toDateTimeString())));
         }
-        if ($dateRange?->endDate() !== null) {
-            $qb->andWhere($qb->expr()->lte('v.date', $conn->quote($dateRange->endDate()->toDateTimeString())));
+        if ($dateRange?->endDate !== null) {
+            $qb->andWhere($qb->expr()->lte('v.date', $conn->quote($dateRange->endDate->toDateTimeString())));
         }
     }
 
diff --git a/module/Core/src/Spec/InDateRange.php b/module/Core/src/Spec/InDateRange.php
index 05cb6f0a..994e6d63 100644
--- a/module/Core/src/Spec/InDateRange.php
+++ b/module/Core/src/Spec/InDateRange.php
@@ -20,12 +20,12 @@ class InDateRange extends BaseSpecification
     {
         $criteria = [];
 
-        if ($this->dateRange?->startDate() !== null) {
-            $criteria[] = Spec::gte($this->field, $this->dateRange->startDate()->toDateTimeString());
+        if ($this->dateRange?->startDate !== null) {
+            $criteria[] = Spec::gte($this->field, $this->dateRange->startDate->toDateTimeString());
         }
 
-        if ($this->dateRange?->endDate() !== null) {
-            $criteria[] = Spec::lte($this->field, $this->dateRange->endDate()->toDateTimeString());
+        if ($this->dateRange?->endDate !== null) {
+            $criteria[] = Spec::lte($this->field, $this->dateRange->endDate->toDateTimeString());
         }
 
         return Spec::andX(...$criteria);
diff --git a/module/Rest/src/ConfigProvider.php b/module/Rest/src/ConfigProvider.php
index 3304ce4d..6d389038 100644
--- a/module/Rest/src/ConfigProvider.php
+++ b/module/Rest/src/ConfigProvider.php
@@ -8,7 +8,6 @@ use function Functional\first;
 use function Functional\map;
 use function Shlinkio\Shlink\Config\loadConfigFromGlob;
 use function sprintf;
-use function str_replace;
 
 class ConfigProvider
 {
@@ -21,16 +20,12 @@ class ConfigProvider
         return loadConfigFromGlob(__DIR__ . '/../config/{,*.}config.php');
     }
 
-    public static function applyRoutesPrefix(array $routes, bool $multiSegmentEnabled): array
+    public static function applyRoutesPrefix(array $routes): array
     {
         $healthRoute = self::buildUnversionedHealthRouteFromExistingRoutes($routes);
-        $prefixedRoutes = map($routes, static function (array $route) use ($multiSegmentEnabled) {
+        $prefixedRoutes = map($routes, static function (array $route) {
             ['path' => $path] = $route;
-            if ($multiSegmentEnabled) {
-                $path = str_replace('{shortCode}', '{shortCode:.+}', $path);
-            }
             $route['path'] = sprintf('%s%s', self::ROUTES_PREFIX, $path);
-
             return $route;
         });
 


Test patch:
diff --git a/module/Core/test/Config/MultiSegmentSlugProcessorTest.php b/module/Core/test/Config/MultiSegmentSlugProcessorTest.php
new file mode 100644
index 00000000..630a5d90
--- /dev/null
+++ b/module/Core/test/Config/MultiSegmentSlugProcessorTest.php
@@ -0,0 +1,60 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ShlinkioTest\Shlink\Core\Config;
+
+use PHPUnit\Framework\TestCase;
+use Shlinkio\Shlink\Core\Config\MultiSegmentSlugProcessor;
+
+class MultiSegmentSlugProcessorTest extends TestCase
+{
+    private MultiSegmentSlugProcessor $processor;
+
+    protected function setUp(): void
+    {
+        $this->processor = new MultiSegmentSlugProcessor();
+    }
+
+    /**
+     * @test
+     * @dataProvider provideConfigs
+     */
+    public function parsesRoutesAsExpected(array $config, array $expectedRoutes): void
+    {
+        self::assertEquals($expectedRoutes, ($this->processor)($config)['routes'] ?? []);
+    }
+
+    public function provideConfigs(): iterable
+    {
+        yield [[], []];
+        yield [['url_shortener' => []], []];
+        yield [['url_shortener' => ['multi_segment_slugs_enabled' => false]], []];
+        yield [
+            [
+                'url_shortener' => ['multi_segment_slugs_enabled' => false],
+                'routes' => $routes = [
+                    ['path' => '/foo'],
+                    ['path' => '/bar/{shortCode}'],
+                    ['path' => '/baz/{shortCode}/foo'],
+                ],
+            ],
+            $routes,
+        ];
+        yield [
+            [
+                'url_shortener' => ['multi_segment_slugs_enabled' => true],
+                'routes' => [
+                    ['path' => '/foo'],
+                    ['path' => '/bar/{shortCode}'],
+                    ['path' => '/baz/{shortCode}/foo'],
+                ],
+            ],
+            [
+                ['path' => '/foo'],
+                ['path' => '/bar/{shortCode:.+}'],
+                ['path' => '/baz/{shortCode:.+}/foo'],
+            ],
+        ];
+    }
+}
diff --git a/module/Rest/test/ConfigProviderTest.php b/module/Rest/test/ConfigProviderTest.php
index 07fa4e43..a3f7d0c9 100644
--- a/module/Rest/test/ConfigProviderTest.php
+++ b/module/Rest/test/ConfigProviderTest.php
@@ -33,9 +33,9 @@ class ConfigProviderTest extends TestCase
      * @test
      * @dataProvider provideRoutesConfig
      */
-    public function routesAreProperlyPrefixed(array $routes, bool $multiSegmentEnabled, array $expected): void
+    public function routesAreProperlyPrefixed(array $routes, array $expected): void
     {
-        self::assertEquals($expected, ConfigProvider::applyRoutesPrefix($routes, $multiSegmentEnabled));
+        self::assertEquals($expected, ConfigProvider::applyRoutesPrefix($routes));
     }
 
     public function provideRoutesConfig(): iterable
@@ -47,7 +47,6 @@ class ConfigProviderTest extends TestCase
                 ['path' => '/baz/foo'],
                 ['path' => '/health'],
             ],
-            false,
             [
                 ['path' => '/rest/v{version:1|2}/foo'],
                 ['path' => '/rest/v{version:1|2}/bar'],
@@ -62,25 +61,11 @@ class ConfigProviderTest extends TestCase
                 ['path' => '/bar'],
                 ['path' => '/baz/foo'],
             ],
-            false,
             [
                 ['path' => '/rest/v{version:1|2}/foo'],
                 ['path' => '/rest/v{version:1|2}/bar'],
                 ['path' => '/rest/v{version:1|2}/baz/foo'],
             ],
         ];
-        yield 'multi-segment enabled' => [
-            [
-                ['path' => '/foo'],
-                ['path' => '/bar/{shortCode}'],
-                ['path' => '/baz/{shortCode}/foo'],
-            ],
-            true,
-            [
-                ['path' => '/rest/v{version:1|2}/foo'],
-                ['path' => '/rest/v{version:1|2}/bar/{shortCode:.+}'],
-                ['path' => '/rest/v{version:1|2}/baz/{shortCode:.+}/foo'],
-            ],
-        ];
     }
 }