From 36d5e057d02e0bd9ad25c22a673ecbe4e4a12016 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 6 Jan 2020 23:32:43 +0100 Subject: [PATCH 1/3] Ensured the health action is registered bit with and without version --- module/Rest/config/auth.config.php | 1 + module/Rest/src/ConfigProvider.php | 14 +++++++++----- module/Rest/test/ConfigProviderTest.php | 3 ++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/module/Rest/config/auth.config.php b/module/Rest/config/auth.config.php index dd0d5369..99141364 100644 --- a/module/Rest/config/auth.config.php +++ b/module/Rest/config/auth.config.php @@ -12,6 +12,7 @@ return [ 'routes_whitelist' => [ Action\HealthAction::class, Action\ShortUrl\SingleStepCreateShortUrlAction::class, + ConfigProvider::UNVERSIONED_HEALTH_ENDPOINT_NAME, ], 'plugins' => [ diff --git a/module/Rest/src/ConfigProvider.php b/module/Rest/src/ConfigProvider.php index b94991ca..98ad8b4c 100644 --- a/module/Rest/src/ConfigProvider.php +++ b/module/Rest/src/ConfigProvider.php @@ -13,6 +13,7 @@ class ConfigProvider { private const ROUTES_PREFIX = '/rest/v{version:1|2}'; private const UNVERSIONED_ROUTES_PREFIX = '/rest'; + public const UNVERSIONED_HEALTH_ENDPOINT_NAME = 'unversioned_health'; private Closure $loadConfig; @@ -34,11 +35,14 @@ class ConfigProvider // Prepend the routes prefix to every path foreach ($routes as $key => $route) { ['path' => $path] = $route; - $routes[$key]['path'] = sprintf( - '%s%s', - $path === '/health' ? self::UNVERSIONED_ROUTES_PREFIX : self::ROUTES_PREFIX, - $path, - ); + $routes[$key]['path'] = sprintf('%s%s', self::ROUTES_PREFIX, $path); + + // Also append the health route so that it works without version + if ($path === '/health') { + $route['path'] = sprintf('%s%s', self::UNVERSIONED_ROUTES_PREFIX, $path); + $route['name'] = self::UNVERSIONED_HEALTH_ENDPOINT_NAME; + $routes[] = $route; + } } return $config; diff --git a/module/Rest/test/ConfigProviderTest.php b/module/Rest/test/ConfigProviderTest.php index 80919c30..48c71716 100644 --- a/module/Rest/test/ConfigProviderTest.php +++ b/module/Rest/test/ConfigProviderTest.php @@ -43,7 +43,8 @@ class ConfigProviderTest extends TestCase ['path' => '/rest/v{version:1|2}/foo'], ['path' => '/rest/v{version:1|2}/bar'], ['path' => '/rest/v{version:1|2}/baz/foo'], - ['path' => '/rest/health'], + ['path' => '/rest/v{version:1|2}/health'], + ['path' => '/rest/health', 'name' => ConfigProvider::UNVERSIONED_HEALTH_ENDPOINT_NAME], ], $config['routes']); } } From 2b544ad141e493489d83c4a65e156a1576eaa2c7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 7 Jan 2020 18:07:51 +0100 Subject: [PATCH 2/3] Refactored Rest ConfigProvider so that it appends the health action with and without version --- module/Rest/src/ConfigProvider.php | 36 +++++++++++------ module/Rest/test/ConfigProviderTest.php | 51 +++++++++++++++++-------- 2 files changed, 61 insertions(+), 26 deletions(-) diff --git a/module/Rest/src/ConfigProvider.php b/module/Rest/src/ConfigProvider.php index 98ad8b4c..570eab85 100644 --- a/module/Rest/src/ConfigProvider.php +++ b/module/Rest/src/ConfigProvider.php @@ -6,6 +6,8 @@ namespace Shlinkio\Shlink\Rest; use Closure; +use function Functional\first; +use function Functional\map; use function Shlinkio\Shlink\Common\loadConfigFromGlob; use function sprintf; @@ -30,21 +32,33 @@ class ConfigProvider private function applyRoutesPrefix(array $config): array { - $routes =& $config['routes'] ?? []; + $routes = $config['routes'] ?? []; + $healthRoute = $this->buildUnversionedHealthRouteFromExistingRoutes($routes); - // Prepend the routes prefix to every path - foreach ($routes as $key => $route) { + $prefixRoute = static function (array $route) { ['path' => $path] = $route; - $routes[$key]['path'] = sprintf('%s%s', self::ROUTES_PREFIX, $path); + $route['path'] = sprintf('%s%s', self::ROUTES_PREFIX, $path); - // Also append the health route so that it works without version - if ($path === '/health') { - $route['path'] = sprintf('%s%s', self::UNVERSIONED_ROUTES_PREFIX, $path); - $route['name'] = self::UNVERSIONED_HEALTH_ENDPOINT_NAME; - $routes[] = $route; - } - } + return $route; + }; + $prefixedRoutes = map($routes, $prefixRoute); + + $config['routes'] = $healthRoute !== null ? [...$prefixedRoutes, $healthRoute] : $prefixedRoutes; return $config; } + + private function buildUnversionedHealthRouteFromExistingRoutes(array $routes): ?array + { + $healthRoute = first($routes, fn (array $route) => $route['path'] === '/health'); + if ($healthRoute === null) { + return null; + } + + $path = $healthRoute['path']; + $healthRoute['path'] = sprintf('%s%s', self::UNVERSIONED_ROUTES_PREFIX, $path); + $healthRoute['name'] = self::UNVERSIONED_HEALTH_ENDPOINT_NAME; + + return $healthRoute; + } } diff --git a/module/Rest/test/ConfigProviderTest.php b/module/Rest/test/ConfigProviderTest.php index 48c71716..8032a854 100644 --- a/module/Rest/test/ConfigProviderTest.php +++ b/module/Rest/test/ConfigProviderTest.php @@ -25,26 +25,47 @@ class ConfigProviderTest extends TestCase $this->assertArrayHasKey('dependencies', $config); } - /** @test */ - public function routesAreProperlyPrefixed(): void + /** + * @test + * @dataProvider provideRoutesConfig + */ + public function routesAreProperlyPrefixed(array $routes, array $expected): void { - $configProvider = new ConfigProvider(fn () => [ - 'routes' => [ + $configProvider = new ConfigProvider(fn () => ['routes' => $routes]); + + $config = $configProvider(); + + $this->assertEquals($expected, $config['routes']); + } + + public function provideRoutesConfig(): iterable + { + yield 'health action present' => [ + [ ['path' => '/foo'], ['path' => '/bar'], ['path' => '/baz/foo'], ['path' => '/health'], ], - ]); - - $config = $configProvider(); - - $this->assertEquals([ - ['path' => '/rest/v{version:1|2}/foo'], - ['path' => '/rest/v{version:1|2}/bar'], - ['path' => '/rest/v{version:1|2}/baz/foo'], - ['path' => '/rest/v{version:1|2}/health'], - ['path' => '/rest/health', 'name' => ConfigProvider::UNVERSIONED_HEALTH_ENDPOINT_NAME], - ], $config['routes']); + [ + ['path' => '/rest/v{version:1|2}/foo'], + ['path' => '/rest/v{version:1|2}/bar'], + ['path' => '/rest/v{version:1|2}/baz/foo'], + ['path' => '/rest/v{version:1|2}/health'], + ['path' => '/rest/health', 'name' => ConfigProvider::UNVERSIONED_HEALTH_ENDPOINT_NAME], + ], + ]; + yield 'health action not present' => [ + [ + ['path' => '/foo'], + ['path' => '/bar'], + ['path' => '/baz/foo'], + ], + [ + ['path' => '/rest/v{version:1|2}/foo'], + ['path' => '/rest/v{version:1|2}/bar'], + ['path' => '/rest/v{version:1|2}/baz/foo'], + ], + ]; } } From 302a77b9ddc93a2a5735168b3e0122b8291c4392 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 7 Jan 2020 18:10:09 +0100 Subject: [PATCH 3/3] Updated changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb9c59e8..d1b9f221 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,7 +34,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this #### Fixed -* *Nothing* +* [#600](https://github.com/shlinkio/shlink/issues/600) Fixed health action so that it works with and without version in the path. ## 1.21.1 - 2020-01-02