Merge pull request #1404 from acelaya-forks/feature/fix-double-paths

Feature/fix double paths

CHANGELOG.md

@@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 
 ### Fixed
 * [#1397](https://github.com/shlinkio/shlink/issues/1397) Fixed `db:create` command always reporting the schema exists if the `db:migrate` command has been run before by mistake.
+* [#1402](https://github.com/shlinkio/shlink/issues/1402) Fixed the base path getting appended with the default domain by mistake, causing multiple side effects in several places.
 
 
 ## [3.0.3] - 2022-02-19

composer.json

@@ -51,7 +51,7 @@
         "shlinkio/shlink-config": "^1.6",
         "shlinkio/shlink-event-dispatcher": "^2.3",
         "shlinkio/shlink-importer": "^2.5",
-        "shlinkio/shlink-installer": "dev-develop#ea5e967 as 7.1",
+        "shlinkio/shlink-installer": "dev-develop#d02f256 as 7.1",
         "shlinkio/shlink-ip-geolocation": "^2.2",
         "symfony/console": "^6.0",
         "symfony/filesystem": "^6.0",
@@ -139,7 +139,7 @@
         "test:api": "bin/test/run-api-tests.sh",
         "test:api:ci": "GENERATE_COVERAGE=yes composer test:api",
         "infect:ci:base": "infection --threads=4 --log-verbosity=default --only-covered --only-covering-test-cases --skip-initial-tests",
-        "infect:ci:unit": "@infect:ci:base --coverage=build/coverage-unit --min-msi=85",
+        "infect:ci:unit": "@infect:ci:base --coverage=build/coverage-unit --min-msi=84",
         "infect:ci:db": "@infect:ci:base --coverage=build/coverage-db --min-msi=95 --configuration=infection-db.json",
         "infect:ci:api": "@infect:ci:base --coverage=build/coverage-api --min-msi=80 --configuration=infection-api.json",
         "infect:ci": "@parallel infect:ci:unit infect:ci:db infect:ci:api",

module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php

@@ -209,7 +209,7 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand
         }
         if ($input->getOption('show-api-key')) {
             $columnsMap['API Key'] = static fn (array $_, ShortUrl $shortUrl): string =>
-                (string) $shortUrl->authorApiKey();
+                $shortUrl->authorApiKey()?->__toString() ?? '';
         }
         if ($input->getOption('show-api-key-name')) {
             $columnsMap['API Key Name'] = static fn (array $_, ShortUrl $shortUrl): ?string =>

module/CLI/src/Exception/GeolocationDbUpdateFailedException.php

@@ -13,7 +13,7 @@ class GeolocationDbUpdateFailedException extends RuntimeException implements Exc
 {
     private bool $olderDbExists;
 
-    private function __construct(string $message, int $code = 0, ?Throwable $previous = null)
+    private function __construct(string $message, int $code, ?Throwable $previous)
     {
         parent::__construct($message, $code, $previous);
     }
@@ -47,7 +47,7 @@ class GeolocationDbUpdateFailedException extends RuntimeException implements Exc
         $e = new self(sprintf(
             'Build epoch with value "%s" from existing geolocation database, could not be parsed to integer.',
             $buildEpoch,
-        ));
+        ), 0, null);
         $e->olderDbExists = true;
 
         return $e;

module/CLI/test/Util/GeolocationDbUpdaterTest.php

@@ -30,6 +30,7 @@ class GeolocationDbUpdaterTest extends TestCase
     private ObjectProphecy $dbUpdater;
     private ObjectProphecy $geoLiteDbReader;
     private TrackingOptions $trackingOptions;
+    private ObjectProphecy $lock;
 
     public function setUp(): void
     {
@@ -38,11 +39,11 @@ class GeolocationDbUpdaterTest extends TestCase
         $this->trackingOptions = new TrackingOptions();
 
         $locker = $this->prophesize(Lock\LockFactory::class);
-        $lock = $this->prophesize(Lock\LockInterface::class);
+        $this->lock = $this->prophesize(Lock\LockInterface::class);
-        $lock->acquire(true)->willReturn(true);
+        $this->lock->acquire(true)->willReturn(true);
-        $lock->release()->will(function (): void {
+        $this->lock->release()->will(function (): void {
         });
-        $locker->createLock(Argument::type('string'))->willReturn($lock->reveal());
+        $locker->createLock(Argument::type('string'))->willReturn($this->lock->reveal());
 
         $this->geolocationDbUpdater = new GeolocationDbUpdater(
             $this->dbUpdater->reveal(),
@@ -75,6 +76,8 @@ class GeolocationDbUpdaterTest extends TestCase
         $fileExists->shouldHaveBeenCalledOnce();
         $getMeta->shouldNotHaveBeenCalled();
         $download->shouldHaveBeenCalledOnce();
+        $this->lock->acquire(true)->shouldHaveBeenCalledOnce();
+        $this->lock->release()->shouldHaveBeenCalledOnce();
     }
 
     /**

module/Core/src/Config/BasePathPrefixer.php

@@ -13,7 +13,6 @@ class BasePathPrefixer
     public function __invoke(array $config): array
     {
         $basePath = $config['router']['base_path'] ?? '';
-        $config['url_shortener']['domain']['hostname'] .= $basePath;
 
         foreach (self::ELEMENTS_WITH_PATH as $configKey) {
             $config[$configKey] = $this->prefixPathsWithBasePath($configKey, $config, $basePath);

module/Core/test/Config/BasePathPrefixerTest.php

@@ -24,42 +24,16 @@ class BasePathPrefixerTest extends TestCase
         array $originalConfig,
         array $expectedRoutes,
         array $expectedMiddlewares,
-        string $expectedHostname,
     ): void {
-        [
+        ['routes' => $routes, 'middleware_pipeline' => $middlewares] = ($this->prefixer)($originalConfig);
-            'routes' => $routes,
-            'middleware_pipeline' => $middlewares,
-            'url_shortener' => $urlShortener,
-        ] = ($this->prefixer)($originalConfig);
 
         self::assertEquals($expectedRoutes, $routes);
         self::assertEquals($expectedMiddlewares, $middlewares);
-        self::assertEquals([
-            'domain' => [
-                'hostname' => $expectedHostname,
-            ],
-        ], $urlShortener);
     }
 
     public function provideConfig(): iterable
     {
-        $urlShortener = [
+        yield 'with empty options' => [['routes' => []], [], []];
-            'domain' => [
-                'hostname' => null,
-            ],
-        ];
-
-        yield 'without anything' => [['url_shortener' => $urlShortener], [], [], ''];
-        yield 'with empty options' => [
-            [
-                'routes' => [],
-                'middleware_pipeline' => [],
-                'url_shortener' => $urlShortener,
-            ],
-            [],
-            [],
-            '',
-        ];
         yield 'with non-empty options' => [
             [
                 'routes' => [
@@ -70,11 +44,6 @@ class BasePathPrefixerTest extends TestCase
                     ['with' => 'no_path'],
                     ['path' => '/rest', 'middleware' => []],
                 ],
-                'url_shortener' => [
-                    'domain' => [
-                        'hostname' => 'doma.in',
-                    ],
-                ],
                 'router' => ['base_path' => '/foo/bar'],
             ],
             [
@@ -85,7 +54,6 @@ class BasePathPrefixerTest extends TestCase
                 ['with' => 'no_path'],
                 ['path' => '/foo/bar/rest', 'middleware' => []],
             ],
-            'doma.in/foo/bar',
         ];
     }
 }