From 392e5b178b0de6c73889deed2842c9ddfad73cbd Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 12:13:10 +0200 Subject: [PATCH 01/38] Add league/route package --- composer.json | 1 + composer.lock | 206 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 206 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index b95c9d27f1..17118fc622 100644 --- a/composer.json +++ b/composer.json @@ -46,6 +46,7 @@ "league/flysystem": "^2.2.3", "league/flysystem-aws-s3-v3": "^2.1", "league/period": "^3.3", + "league/route": "^5.1", "league/uri": "^6.3", "league/uri-components": "^2.4", "marvin_b8/psr-7-service-provider": "^1.0", diff --git a/composer.lock b/composer.lock index 8287bc6b92..94a2da85e0 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "3f99b2736ddcaab57c3a49c7cb561d73", + "content-hash": "10e1021e42625df8920dc2562009b76b", "packages": [ { "name": "auth0/auth0-php", @@ -2681,6 +2681,95 @@ ], "time": "2017-11-17T11:28:33+00:00" }, + { + "name": "league/route", + "version": "5.1.2", + "source": { + "type": "git", + "url": "https://github.com/thephpleague/route.git", + "reference": "adf9b961dc5ffdbcffb2b8d7963c7978f2794c92" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/thephpleague/route/zipball/adf9b961dc5ffdbcffb2b8d7963c7978f2794c92", + "reference": "adf9b961dc5ffdbcffb2b8d7963c7978f2794c92", + "shasum": "" + }, + "require": { + "nikic/fast-route": "^1.3", + "opis/closure": "^3.5.5", + "php": "^7.2 || ^8.0", + "psr/container": "^1.0|^2.0", + "psr/http-factory": "^1.0", + "psr/http-message": "^1.0.1", + "psr/http-server-handler": "^1.0.1", + "psr/http-server-middleware": "^1.0.1", + "psr/simple-cache": "^1.0" + }, + "replace": { + "orno/http": "~1.0", + "orno/route": "~1.0" + }, + "require-dev": { + "laminas/laminas-diactoros": "^2.3", + "phpstan/phpstan": "^0.12", + "phpstan/phpstan-phpunit": "^0.12", + "phpunit/phpunit": "^8.5", + "roave/security-advisories": "dev-master", + "scrutinizer/ocular": "^1.8", + "squizlabs/php_codesniffer": "^3.5" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "5.x-dev", + "dev-5.x": "5.x-dev", + "dev-4.x": "4.x-dev", + "dev-3.x": "3.x-dev", + "dev-2.x": "2.x-dev", + "dev-1.x": "1.x-dev" + } + }, + "autoload": { + "psr-4": { + "League\\Route\\": "src" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Phil Bennett", + "email": "philipobenito@gmail.com", + "role": "Developer" + } + ], + "description": "Fast routing and dispatch component including PSR-15 middleware, built on top of FastRoute.", + "homepage": "https://github.com/thephpleague/route", + "keywords": [ + "dispatcher", + "league", + "psr-15", + "psr-7", + "psr15", + "psr7", + "route", + "router" + ], + "support": { + "issues": "https://github.com/thephpleague/route/issues", + "source": "https://github.com/thephpleague/route/tree/5.1.2" + }, + "funding": [ + { + "url": "https://github.com/philipobenito", + "type": "github" + } + ], + "time": "2021-07-30T08:33:09+00:00" + }, { "name": "league/uri", "version": "6.5.0", @@ -3549,6 +3638,56 @@ ], "time": "2019-10-14T05:51:36+00:00" }, + { + "name": "nikic/fast-route", + "version": "v1.3.0", + "source": { + "type": "git", + "url": "https://github.com/nikic/FastRoute.git", + "reference": "181d480e08d9476e61381e04a71b34dc0432e812" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/nikic/FastRoute/zipball/181d480e08d9476e61381e04a71b34dc0432e812", + "reference": "181d480e08d9476e61381e04a71b34dc0432e812", + "shasum": "" + }, + "require": { + "php": ">=5.4.0" + }, + "require-dev": { + "phpunit/phpunit": "^4.8.35|~5.7" + }, + "type": "library", + "autoload": { + "files": [ + "src/functions.php" + ], + "psr-4": { + "FastRoute\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-3-Clause" + ], + "authors": [ + { + "name": "Nikita Popov", + "email": "nikic@php.net" + } + ], + "description": "Fast request router for PHP", + "keywords": [ + "router", + "routing" + ], + "support": { + "issues": "https://github.com/nikic/FastRoute/issues", + "source": "https://github.com/nikic/FastRoute/tree/master" + }, + "time": "2018-02-13T20:26:39+00:00" + }, { "name": "ocramius/proxy-manager", "version": "2.1.1", @@ -3618,6 +3757,71 @@ ], "time": "2017-05-04T11:12:50+00:00" }, + { + "name": "opis/closure", + "version": "3.6.3", + "source": { + "type": "git", + "url": "https://github.com/opis/closure.git", + "reference": "3d81e4309d2a927abbe66df935f4bb60082805ad" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/opis/closure/zipball/3d81e4309d2a927abbe66df935f4bb60082805ad", + "reference": "3d81e4309d2a927abbe66df935f4bb60082805ad", + "shasum": "" + }, + "require": { + "php": "^5.4 || ^7.0 || ^8.0" + }, + "require-dev": { + "jeremeamia/superclosure": "^2.0", + "phpunit/phpunit": "^4.0 || ^5.0 || ^6.0 || ^7.0 || ^8.0 || ^9.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "3.6.x-dev" + } + }, + "autoload": { + "files": [ + "functions.php" + ], + "psr-4": { + "Opis\\Closure\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Marius Sarca", + "email": "marius.sarca@gmail.com" + }, + { + "name": "Sorin Sarca", + "email": "sarca_sorin@hotmail.com" + } + ], + "description": "A library that can be used to serialize closures (anonymous functions) and arbitrary objects.", + "homepage": "https://opis.io/closure", + "keywords": [ + "anonymous functions", + "closure", + "function", + "serializable", + "serialization", + "serialize" + ], + "support": { + "issues": "https://github.com/opis/closure/issues", + "source": "https://github.com/opis/closure/tree/3.6.3" + }, + "time": "2022-01-27T09:35:39+00:00" + }, { "name": "opis/json-schema", "version": "2.1.0", From bfa5ef403e1d83e4f17a65d9275178e1bc326256 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 12:13:55 +0200 Subject: [PATCH 02/38] Rename LegacyRoutesServiceProvider to CatchAllRouteServiceProvider --- ...esServiceProvider.php => CatchAllRouteServiceProvider.php} | 2 +- web/index.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename app/{LegacyRoutesServiceProvider.php => CatchAllRouteServiceProvider.php} (98%) diff --git a/app/LegacyRoutesServiceProvider.php b/app/CatchAllRouteServiceProvider.php similarity index 98% rename from app/LegacyRoutesServiceProvider.php rename to app/CatchAllRouteServiceProvider.php index f645f6ca9d..d2da5db707 100644 --- a/app/LegacyRoutesServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -11,7 +11,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\HttpKernelInterface; -final class LegacyRoutesServiceProvider implements ServiceProviderInterface +final class CatchAllRouteServiceProvider implements ServiceProviderInterface { public function register(Application $app): void { diff --git a/web/index.php b/web/index.php index 90e304af8e..c48a330014 100644 --- a/web/index.php +++ b/web/index.php @@ -18,7 +18,7 @@ use CultuurNet\UDB3\Silex\Event\EventControllerProvider; use CultuurNet\UDB3\Silex\Http\RequestHandlerControllerServiceProvider; use CultuurNet\UDB3\Silex\Import\ImportControllerProvider; -use CultuurNet\UDB3\Silex\LegacyRoutesServiceProvider; +use CultuurNet\UDB3\Silex\CatchAllRouteServiceProvider; use CultuurNet\UDB3\Silex\Offer\DeprecatedOfferControllerProvider; use CultuurNet\UDB3\Silex\Offer\OfferControllerProvider; use CultuurNet\UDB3\Silex\Place\PlaceControllerProvider; @@ -273,7 +273,7 @@ function (Request $request, Response $response) { } ); -$app->register(new LegacyRoutesServiceProvider()); +$app->register(new CatchAllRouteServiceProvider()); JsonSchemaLocator::setSchemaDirectory(__DIR__ . '/../vendor/publiq/udb3-json-schemas'); From 8240ebee18c7e5fdc2c80e33407f4f7be4d9670f Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 12:16:37 +0200 Subject: [PATCH 03/38] Store the original request --- app/CatchAllRouteServiceProvider.php | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index d2da5db707..71f0b89b9c 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -20,6 +20,7 @@ public function register(Application $app): void public function boot(Application $app): void { $pathHasBeenRewritten = false; + $originalRequest = null; // NOTE: THIS CATCH-ALL ROUTE HAS TO BE REGISTERED INSIDE boot() SO THAT (DYNAMICALLY GENERATED) OPTIONS ROUTES // FOR CORS GET REGISTERED FIRST BEFORE THIS ONE. @@ -31,7 +32,7 @@ public function boot(Application $app): void // PSR-15 middleware instead. $app->match( '/{path}', - function (Request $originalRequest, string $path) use ($app, &$pathHasBeenRewritten) { + function (Request $request, string $path) use ($app, &$pathHasBeenRewritten, &$originalRequest) { if ($pathHasBeenRewritten) { return new ApiProblemJsonResponse(ApiProblem::urlNotFound()); } @@ -63,27 +64,28 @@ function (Request $originalRequest, string $path) use ($app, &$pathHasBeenRewrit $rewrittenPath = preg_replace(array_keys($rewrites), array_values($rewrites), $path); $pathHasBeenRewritten = true; + $originalRequest = $request; // Create a new Request object with the rewritten path, because it's basically impossible to overwrite // the path of an existing Request object even with initialize() or duplicate(). Approach copied from // https://github.com/graze/silex-trailing-slash-handler/blob/1.x/src/TrailingSlashControllerProvider.php - $request = Request::create( + $rewrittenRequest = Request::create( $rewrittenPath, - $originalRequest->getMethod(), + $request->getMethod(), [], - $originalRequest->cookies->all(), - $originalRequest->files->all(), - $originalRequest->server->all(), - $originalRequest->getContent() + $request->cookies->all(), + $request->files->all(), + $request->server->all(), + $request->getContent() ); - $request = $request->duplicate( - $originalRequest->query->all(), - $originalRequest->request->all() + $rewrittenRequest = $rewrittenRequest->duplicate( + $request->query->all(), + $request->request->all() ); - $request->headers->replace($app['request']->headers->all()); + $rewrittenRequest->headers->replace($app['request']->headers->all()); // Handle the request with the rewritten path. - return $app->handle($request, HttpKernelInterface::SUB_REQUEST); + return $app->handle($rewrittenRequest, HttpKernelInterface::SUB_REQUEST); } )->assert('path', '^.+$'); } From 96767d8387a7c03455a3aa04cd45dbf6d6794656 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 12:21:10 +0200 Subject: [PATCH 04/38] Register the league router --- app/Http/PsrRouterServiceProvider.php | 23 +++++++++++++++++++++++ web/index.php | 8 ++++++++ 2 files changed, 31 insertions(+) create mode 100644 app/Http/PsrRouterServiceProvider.php diff --git a/app/Http/PsrRouterServiceProvider.php b/app/Http/PsrRouterServiceProvider.php new file mode 100644 index 0000000000..f0d7db40d1 --- /dev/null +++ b/app/Http/PsrRouterServiceProvider.php @@ -0,0 +1,23 @@ + new Router() + ); + } + + public function boot(Application $app): void + { + } +} diff --git a/web/index.php b/web/index.php index c48a330014..22e591b16f 100644 --- a/web/index.php +++ b/web/index.php @@ -12,6 +12,7 @@ use CultuurNet\UDB3\Role\ValueObjects\Permission; use CultuurNet\UDB3\Silex\ApiName; use CultuurNet\UDB3\Silex\Curators\CuratorsControllerProvider; +use CultuurNet\UDB3\Silex\Http\PsrRouterServiceProvider; use CultuurNet\UDB3\Silex\Udb3ControllerCollection; use CultuurNet\UDB3\Silex\Error\WebErrorHandlerProvider; use CultuurNet\UDB3\Silex\Error\ErrorLogger; @@ -57,6 +58,13 @@ */ $app->register(new RequestHandlerControllerServiceProvider()); +/** + * Register a PSR-7 / PSR-15 compatible router. + * Will be used in CatchAllRouteServiceProvider to route unmatched requests from Silex to the PSR router, until we can + * completely by-pass the Silex router. + */ +$app->register(new PsrRouterServiceProvider()); + /** * Firewall configuration. * From 68afe459ead26d5b5f6f9e0fca32063eae8a7dd8 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 12:33:32 +0200 Subject: [PATCH 05/38] Dispatch unmatched requests to PSR router --- app/CatchAllRouteServiceProvider.php | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 71f0b89b9c..c71644f2ec 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -6,8 +6,12 @@ use CultuurNet\UDB3\Http\ApiProblem\ApiProblem; use CultuurNet\UDB3\Http\Response\ApiProblemJsonResponse; +use League\Route\Http\Exception\NotFoundException; +use League\Route\Router; use Silex\Application; use Silex\ServiceProviderInterface; +use Symfony\Bridge\PsrHttpMessage\Factory\DiactorosFactory; +use Symfony\Bridge\PsrHttpMessage\Factory\HttpFoundationFactory; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\HttpKernelInterface; @@ -34,7 +38,17 @@ public function boot(Application $app): void '/{path}', function (Request $request, string $path) use ($app, &$pathHasBeenRewritten, &$originalRequest) { if ($pathHasBeenRewritten) { - return new ApiProblemJsonResponse(ApiProblem::urlNotFound()); + /** @var Router $router */ + $router = $app[Router::class]; + $psrRequest = (new DiactorosFactory())->createRequest($request); + + try { + $psrResponse = $router->handle($psrRequest); + } catch (NotFoundException $e) { + return new ApiProblemJsonResponse(ApiProblem::urlNotFound()); + } + + return (new HttpFoundationFactory())->createResponse($psrResponse); } $rewrites = [ From 6e6794099fd5a490b3ec64384498c33752482f0a Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 12:33:46 +0200 Subject: [PATCH 06/38] Also catch MethodNotAllowedException --- app/CatchAllRouteServiceProvider.php | 9 +++++++++ src/Http/ApiProblem/ApiProblem.php | 10 ++++++++++ 2 files changed, 19 insertions(+) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index c71644f2ec..1a161d7960 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -6,6 +6,7 @@ use CultuurNet\UDB3\Http\ApiProblem\ApiProblem; use CultuurNet\UDB3\Http\Response\ApiProblemJsonResponse; +use League\Route\Http\Exception\MethodNotAllowedException; use League\Route\Http\Exception\NotFoundException; use League\Route\Router; use Silex\Application; @@ -46,6 +47,14 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewritten, &$o $psrResponse = $router->handle($psrRequest); } catch (NotFoundException $e) { return new ApiProblemJsonResponse(ApiProblem::urlNotFound()); + } catch (MethodNotAllowedException $e) { + $details = null; + $headers = $e->getHeaders(); + $allowed = $headers['Allow'] ?? null; + if ($allowed !== null) { + $details = 'Allowed: ' . $allowed; + } + return new ApiProblemJsonResponse(ApiProblem::methodNotAllowed($details)); } return (new HttpFoundationFactory())->createResponse($psrResponse); diff --git a/src/Http/ApiProblem/ApiProblem.php b/src/Http/ApiProblem/ApiProblem.php index beff2eb75b..d62ad842ca 100644 --- a/src/Http/ApiProblem/ApiProblem.php +++ b/src/Http/ApiProblem/ApiProblem.php @@ -222,6 +222,16 @@ public static function notAcceptable(string $detail = null): self ); } + public static function methodNotAllowed(string $detail = null): self + { + return self::create( + 'https://api.publiq.be/probs/method/not-allowed', + 'Method not allowed', + 405, + $detail + ); + } + public static function urlNotFound(string $detail = null): self { return self::create( From e9e2f2858f2a4a25c1d4df7ccf5037be4dd3e77d Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 12:40:37 +0200 Subject: [PATCH 07/38] Let the PSR router handle the original request --- app/CatchAllRouteServiceProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 1a161d7960..317af1434b 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -41,7 +41,7 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewritten, &$o if ($pathHasBeenRewritten) { /** @var Router $router */ $router = $app[Router::class]; - $psrRequest = (new DiactorosFactory())->createRequest($request); + $psrRequest = (new DiactorosFactory())->createRequest($originalRequest); try { $psrResponse = $router->handle($psrRequest); From 1bd51d09851f80558abe8b02f7b5065252860996 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 12:42:47 +0200 Subject: [PATCH 08/38] Move rewrite logic to function --- app/CatchAllRouteServiceProvider.php | 55 +++++++++++++++------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 317af1434b..e660f8ad51 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -38,6 +38,34 @@ public function boot(Application $app): void $app->match( '/{path}', function (Request $request, string $path) use ($app, &$pathHasBeenRewritten, &$originalRequest) { + $rewritePath = static function (string $originalPath): string { + $rewrites = [ + // Pluralize /event and /place + '/^(event|place)($|\/.*)/' => '${1}s${2}', + + // Convert known legacy camelCase resource/collection names to kebab-case + '/bookingAvailability/' => 'booking-availability', + '/bookingInfo/' => 'booking-info', + '/cardSystems/' => 'card-systems', + '/contactPoint/' => 'contact-point', + '/distributionKey/' => 'distribution-key', + '/majorInfo/' => 'major-info', + '/priceInfo/' => 'price-info', + '/subEvents/' => 'sub-events', + '/typicalAgeRange/' => 'typical-age-range', + + // Convert old "calsum" path to "calendar-summary" + '/\/calsum/' => '/calendar-summary', + + // Convert old "news_articles" path to "news-articles" + '/news_articles/' => 'news-articles', + + // Add trailing slash if missing + '/^(.*)(? '${1}/', + ]; + return preg_replace(array_keys($rewrites), array_values($rewrites), $originalPath); + }; + if ($pathHasBeenRewritten) { /** @var Router $router */ $router = $app[Router::class]; @@ -60,32 +88,7 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewritten, &$o return (new HttpFoundationFactory())->createResponse($psrResponse); } - $rewrites = [ - // Pluralize /event and /place - '/^(event|place)($|\/.*)/' => '${1}s${2}', - - // Convert known legacy camelCase resource/collection names to kebab-case - '/bookingAvailability/' => 'booking-availability', - '/bookingInfo/' => 'booking-info', - '/cardSystems/' => 'card-systems', - '/contactPoint/' => 'contact-point', - '/distributionKey/' => 'distribution-key', - '/majorInfo/' => 'major-info', - '/priceInfo/' => 'price-info', - '/subEvents/' => 'sub-events', - '/typicalAgeRange/' => 'typical-age-range', - - // Convert old "calsum" path to "calendar-summary" - '/\/calsum/' => '/calendar-summary', - - // Convert old "news_articles" path to "news-articles" - '/news_articles/' => 'news-articles', - - // Add trailing slash if missing - '/^(.*)(? '${1}/', - ]; - $rewrittenPath = preg_replace(array_keys($rewrites), array_values($rewrites), $path); - + $rewrittenPath = $rewritePath($path); $pathHasBeenRewritten = true; $originalRequest = $request; From 465157f05617ab1b3c8d9061ddcd5b7505ff79f1 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 12:43:20 +0200 Subject: [PATCH 09/38] Rename $pathHasBeenRewritten to $pathHasBeenRewrittenBefore --- app/CatchAllRouteServiceProvider.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index e660f8ad51..168eae1e20 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -24,7 +24,7 @@ public function register(Application $app): void public function boot(Application $app): void { - $pathHasBeenRewritten = false; + $pathHasBeenRewrittenBefore = false; $originalRequest = null; // NOTE: THIS CATCH-ALL ROUTE HAS TO BE REGISTERED INSIDE boot() SO THAT (DYNAMICALLY GENERATED) OPTIONS ROUTES @@ -37,7 +37,7 @@ public function boot(Application $app): void // PSR-15 middleware instead. $app->match( '/{path}', - function (Request $request, string $path) use ($app, &$pathHasBeenRewritten, &$originalRequest) { + function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenBefore, &$originalRequest) { $rewritePath = static function (string $originalPath): string { $rewrites = [ // Pluralize /event and /place @@ -66,7 +66,7 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewritten, &$o return preg_replace(array_keys($rewrites), array_values($rewrites), $originalPath); }; - if ($pathHasBeenRewritten) { + if ($pathHasBeenRewrittenBefore) { /** @var Router $router */ $router = $app[Router::class]; $psrRequest = (new DiactorosFactory())->createRequest($originalRequest); @@ -89,7 +89,7 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewritten, &$o } $rewrittenPath = $rewritePath($path); - $pathHasBeenRewritten = true; + $pathHasBeenRewrittenBefore = true; $originalRequest = $request; // Create a new Request object with the rewritten path, because it's basically impossible to overwrite From ac0da1ddb56f01e1ac92e2834f5f6cfe54cd8aa9 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 13:47:25 +0200 Subject: [PATCH 10/38] Make the event/place pluralisation work if there is a slash prefixed --- app/CatchAllRouteServiceProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 168eae1e20..5937399bc8 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -41,7 +41,7 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenBefor $rewritePath = static function (string $originalPath): string { $rewrites = [ // Pluralize /event and /place - '/^(event|place)($|\/.*)/' => '${1}s${2}', + '/^(\/)?(event|place)($|\/.*)/' => '${1}${2}s${3}', // Convert known legacy camelCase resource/collection names to kebab-case '/bookingAvailability/' => 'booking-availability', From 3f2b1ef833a6f89533559012a444ac378369455d Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 13:48:00 +0200 Subject: [PATCH 11/38] Always immediately rewrite the path before sending the request to the PSR router --- app/CatchAllRouteServiceProvider.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 5937399bc8..e4d05ae71c 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -11,6 +11,7 @@ use League\Route\Router; use Silex\Application; use Silex\ServiceProviderInterface; +use Slim\Psr7\Factory\UriFactory; use Symfony\Bridge\PsrHttpMessage\Factory\DiactorosFactory; use Symfony\Bridge\PsrHttpMessage\Factory\HttpFoundationFactory; use Symfony\Component\HttpFoundation\Request; @@ -71,8 +72,17 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenBefor $router = $app[Router::class]; $psrRequest = (new DiactorosFactory())->createRequest($originalRequest); + // Always rewrite the request before dispatching the request on the PSR router. + // The only case that this could be a problem is if there is a route that is registered with an + // outdated name, but it makes the logic a lot easier. So we should just make sure to use the newer + // names when registering the routes on the new router. + $path = $psrRequest->getUri()->getPath(); + $rewrittenPath = $rewritePath($path); + $rewrittenUri = (new UriFactory())->createUri($rewrittenPath); + $rewrittenPsrRequest = $psrRequest->withUri($rewrittenUri); + try { - $psrResponse = $router->handle($psrRequest); + $psrResponse = $router->handle($rewrittenPsrRequest); } catch (NotFoundException $e) { return new ApiProblemJsonResponse(ApiProblem::urlNotFound()); } catch (MethodNotAllowedException $e) { From 8dfa62ca2af254d21e1b5a4308d09a3f071fb849 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 13:48:47 +0200 Subject: [PATCH 12/38] Only add a trailing slash in case of a rewrite for the Silex router --- app/CatchAllRouteServiceProvider.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index e4d05ae71c..09b6679afd 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -60,9 +60,6 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenBefor // Convert old "news_articles" path to "news-articles" '/news_articles/' => 'news-articles', - - // Add trailing slash if missing - '/^(.*)(? '${1}/', ]; return preg_replace(array_keys($rewrites), array_values($rewrites), $originalPath); }; @@ -98,7 +95,11 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenBefor return (new HttpFoundationFactory())->createResponse($psrResponse); } + // If the path has not been rewritten before, rewrite it and dispatch the request again to the Silex + // router. Note that the Silex router also requires us to append a trailing slash if it's missing, + // whereas the PSR router treats paths with or without trailing slash the same. $rewrittenPath = $rewritePath($path); + $rewrittenPath = preg_replace('/^(.*)(? Date: Mon, 22 Aug 2022 13:53:04 +0200 Subject: [PATCH 13/38] Update comment --- app/CatchAllRouteServiceProvider.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 09b6679afd..eb558519de 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -31,11 +31,11 @@ public function boot(Application $app): void // NOTE: THIS CATCH-ALL ROUTE HAS TO BE REGISTERED INSIDE boot() SO THAT (DYNAMICALLY GENERATED) OPTIONS ROUTES // FOR CORS GET REGISTERED FIRST BEFORE THIS ONE. // Matches any path that does not match a registered route, and rewrites it using a set of predefined pattern - // replacements and sends an internal sub-request to try and match an existing route. If the sub-request does - // not return a response either an error response will be returned. + // replacements and sends an internal sub-request to try and match an existing route. // This makes it possible to support old endpoint names without having to register controllers/request handlers - // twice. When we have a router with support for PSR-15 middlewares, we should refactor this URL rewriting to a - // PSR-15 middleware instead. + // twice. + // If the sub-request does not return a response either, it will be converted to a PSR request and dispatched on + // a new PSR router. $app->match( '/{path}', function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenBefore, &$originalRequest) { From 6e35ee3e3001a2b7b37f8d2938561b94a2aadbfe Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 13:55:21 +0200 Subject: [PATCH 14/38] Move Silex sub-request dispatching higher up for readability --- app/CatchAllRouteServiceProvider.php | 60 ++++++++++++++-------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index eb558519de..3b9a5abdc3 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -64,7 +64,36 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenBefor return preg_replace(array_keys($rewrites), array_values($rewrites), $originalPath); }; - if ($pathHasBeenRewrittenBefore) { + if (!$pathHasBeenRewrittenBefore) { + // If the path has not been rewritten before, rewrite it and dispatch the request again to the Silex + // router. Note that the Silex router also requires us to append a trailing slash if it's missing, + // whereas the PSR router treats paths with or without trailing slash the same. + $rewrittenPath = $rewritePath($path); + $rewrittenPath = preg_replace('/^(.*)(?getMethod(), + [], + $request->cookies->all(), + $request->files->all(), + $request->server->all(), + $request->getContent() + ); + $rewrittenRequest = $rewrittenRequest->duplicate( + $request->query->all(), + $request->request->all() + ); + $rewrittenRequest->headers->replace($app['request']->headers->all()); + + // Handle the request with the rewritten path. + return $app->handle($rewrittenRequest, HttpKernelInterface::SUB_REQUEST); + } else { /** @var Router $router */ $router = $app[Router::class]; $psrRequest = (new DiactorosFactory())->createRequest($originalRequest); @@ -94,35 +123,6 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenBefor return (new HttpFoundationFactory())->createResponse($psrResponse); } - - // If the path has not been rewritten before, rewrite it and dispatch the request again to the Silex - // router. Note that the Silex router also requires us to append a trailing slash if it's missing, - // whereas the PSR router treats paths with or without trailing slash the same. - $rewrittenPath = $rewritePath($path); - $rewrittenPath = preg_replace('/^(.*)(?getMethod(), - [], - $request->cookies->all(), - $request->files->all(), - $request->server->all(), - $request->getContent() - ); - $rewrittenRequest = $rewrittenRequest->duplicate( - $request->query->all(), - $request->request->all() - ); - $rewrittenRequest->headers->replace($app['request']->headers->all()); - - // Handle the request with the rewritten path. - return $app->handle($rewrittenRequest, HttpKernelInterface::SUB_REQUEST); } )->assert('path', '^.+$'); } From 97c0a6248b3d3e98277ea1b927427135039fc629 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 13:55:42 +0200 Subject: [PATCH 15/38] Move PSR router dispatching out of else --- app/CatchAllRouteServiceProvider.php | 56 ++++++++++++++-------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 3b9a5abdc3..8bea2006be 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -93,36 +93,36 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenBefor // Handle the request with the rewritten path. return $app->handle($rewrittenRequest, HttpKernelInterface::SUB_REQUEST); - } else { - /** @var Router $router */ - $router = $app[Router::class]; - $psrRequest = (new DiactorosFactory())->createRequest($originalRequest); - - // Always rewrite the request before dispatching the request on the PSR router. - // The only case that this could be a problem is if there is a route that is registered with an - // outdated name, but it makes the logic a lot easier. So we should just make sure to use the newer - // names when registering the routes on the new router. - $path = $psrRequest->getUri()->getPath(); - $rewrittenPath = $rewritePath($path); - $rewrittenUri = (new UriFactory())->createUri($rewrittenPath); - $rewrittenPsrRequest = $psrRequest->withUri($rewrittenUri); - - try { - $psrResponse = $router->handle($rewrittenPsrRequest); - } catch (NotFoundException $e) { - return new ApiProblemJsonResponse(ApiProblem::urlNotFound()); - } catch (MethodNotAllowedException $e) { - $details = null; - $headers = $e->getHeaders(); - $allowed = $headers['Allow'] ?? null; - if ($allowed !== null) { - $details = 'Allowed: ' . $allowed; - } - return new ApiProblemJsonResponse(ApiProblem::methodNotAllowed($details)); - } + } + + /** @var Router $router */ + $router = $app[Router::class]; + $psrRequest = (new DiactorosFactory())->createRequest($originalRequest); - return (new HttpFoundationFactory())->createResponse($psrResponse); + // Always rewrite the request before dispatching the request on the PSR router. + // The only case that this could be a problem is if there is a route that is registered with an + // outdated name, but it makes the logic a lot easier. So we should just make sure to use the newer + // names when registering the routes on the new router. + $path = $psrRequest->getUri()->getPath(); + $rewrittenPath = $rewritePath($path); + $rewrittenUri = (new UriFactory())->createUri($rewrittenPath); + $rewrittenPsrRequest = $psrRequest->withUri($rewrittenUri); + + try { + $psrResponse = $router->handle($rewrittenPsrRequest); + } catch (NotFoundException $e) { + return new ApiProblemJsonResponse(ApiProblem::urlNotFound()); + } catch (MethodNotAllowedException $e) { + $details = null; + $headers = $e->getHeaders(); + $allowed = $headers['Allow'] ?? null; + if ($allowed !== null) { + $details = 'Allowed: ' . $allowed; + } + return new ApiProblemJsonResponse(ApiProblem::methodNotAllowed($details)); } + + return (new HttpFoundationFactory())->createResponse($psrResponse); } )->assert('path', '^.+$'); } From a6e6aa3bcd245cb4c516cf9d9be86cbb13801c77 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 13:56:37 +0200 Subject: [PATCH 16/38] Explain flow --- app/CatchAllRouteServiceProvider.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 8bea2006be..134f1ada7a 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -92,6 +92,9 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenBefor $rewrittenRequest->headers->replace($app['request']->headers->all()); // Handle the request with the rewritten path. + // If the Silex app still cannot match the new path to a route, this catch-all route will be matched + // again and that time we will dispatch it to the new PSR router because $pathHasBeenRewrittenBefore + // will be set to true. return $app->handle($rewrittenRequest, HttpKernelInterface::SUB_REQUEST); } From 5e981b61021f8b62afb8b8ea0a098d0d402ad15b Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 13:57:40 +0200 Subject: [PATCH 17/38] Rename $pathHasBeenRewrittenBefore for clarity --- app/CatchAllRouteServiceProvider.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 134f1ada7a..62a9e78fc7 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -25,7 +25,7 @@ public function register(Application $app): void public function boot(Application $app): void { - $pathHasBeenRewrittenBefore = false; + $pathHasBeenRewrittenForSilex = false; $originalRequest = null; // NOTE: THIS CATCH-ALL ROUTE HAS TO BE REGISTERED INSIDE boot() SO THAT (DYNAMICALLY GENERATED) OPTIONS ROUTES @@ -38,7 +38,7 @@ public function boot(Application $app): void // a new PSR router. $app->match( '/{path}', - function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenBefore, &$originalRequest) { + function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenForSilex, &$originalRequest) { $rewritePath = static function (string $originalPath): string { $rewrites = [ // Pluralize /event and /place @@ -64,13 +64,13 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenBefor return preg_replace(array_keys($rewrites), array_values($rewrites), $originalPath); }; - if (!$pathHasBeenRewrittenBefore) { + if (!$pathHasBeenRewrittenForSilex) { // If the path has not been rewritten before, rewrite it and dispatch the request again to the Silex // router. Note that the Silex router also requires us to append a trailing slash if it's missing, // whereas the PSR router treats paths with or without trailing slash the same. $rewrittenPath = $rewritePath($path); $rewrittenPath = preg_replace('/^(.*)(?handle($rewrittenRequest, HttpKernelInterface::SUB_REQUEST); } From 83089863a17addc89e1b3ef5c7b4a88d78b23aed Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 14:05:52 +0200 Subject: [PATCH 18/38] (Better) Explain why we rewrite the path before dispatching it on the PSR router --- app/CatchAllRouteServiceProvider.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 62a9e78fc7..09782ba0d0 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -100,12 +100,12 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenForSi /** @var Router $router */ $router = $app[Router::class]; - $psrRequest = (new DiactorosFactory())->createRequest($originalRequest); - // Always rewrite the request before dispatching the request on the PSR router. + // Always start from the original request and rewrite the path before dispatching it on the PSR router. // The only case that this could be a problem is if there is a route that is registered with an - // outdated name, but it makes the logic a lot easier. So we should just make sure to use the newer - // names when registering the routes on the new router. + // outdated name in the PSR router, but it makes the logic a lot easier. So we should just make sure to + // use the newer names when registering the routes on the new router. + $psrRequest = (new DiactorosFactory())->createRequest($originalRequest); $path = $psrRequest->getUri()->getPath(); $rewrittenPath = $rewritePath($path); $rewrittenUri = (new UriFactory())->createUri($rewrittenPath); From 5c0c2ff43dcaf50d70a7aafeabd6008c3b06e1c4 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 14:25:25 +0200 Subject: [PATCH 19/38] Move route to get event/place JSON to new router as proof-of-concept --- app/Http/PsrRouterServiceProvider.php | 9 ++++++++- app/Offer/OfferControllerProvider.php | 1 - 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/Http/PsrRouterServiceProvider.php b/app/Http/PsrRouterServiceProvider.php index f0d7db40d1..b3c5bfae10 100644 --- a/app/Http/PsrRouterServiceProvider.php +++ b/app/Http/PsrRouterServiceProvider.php @@ -4,6 +4,7 @@ namespace CultuurNet\UDB3\Silex\Http; +use CultuurNet\UDB3\Http\Offer\GetDetailRequestHandler; use League\Route\Router; use Silex\Application; use Silex\ServiceProviderInterface; @@ -13,7 +14,13 @@ final class PsrRouterServiceProvider implements ServiceProviderInterface public function register(Application $app): void { $app[Router::class] = $app::share( - fn () => new Router() + function (Application $app) { + $router = new Router(); + + $router->get('/{offerType}/{offerId}', [$app[GetDetailRequestHandler::class], 'handle']); + + return $router; + } ); } diff --git a/app/Offer/OfferControllerProvider.php b/app/Offer/OfferControllerProvider.php index a04dda0ed1..bb4d9c1078 100644 --- a/app/Offer/OfferControllerProvider.php +++ b/app/Offer/OfferControllerProvider.php @@ -34,7 +34,6 @@ public function connect(Application $app): ControllerCollection /** @var ControllerCollection $controllers */ $controllers = $app['controllers_factory']; - $controllers->get('/{offerType}/{offerId}/', GetDetailRequestHandler::class); $controllers->delete('/{offerType}/{offerId}/', DeleteRequestHandler::class); $controllers->put('/{offerType}/{offerId}/name/{language}/', UpdateTitleRequestHandler::class); From 52a12100ae0b3926a4ec2bd859416f33ecc33f28 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 14:25:46 +0200 Subject: [PATCH 20/38] Fix for route parameters that are not found in new router --- src/Http/Request/RouteParameters.php | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/Http/Request/RouteParameters.php b/src/Http/Request/RouteParameters.php index b10725bae9..e160bcf1c0 100644 --- a/src/Http/Request/RouteParameters.php +++ b/src/Http/Request/RouteParameters.php @@ -14,25 +14,31 @@ final class RouteParameters { - private array $routeParameters; + private array $attributes; public function __construct(ServerRequestInterface $request) { - $attributes = $request->getAttributes(); - $this->routeParameters = $attributes['_route_params'] ?? []; + $this->attributes = $request->getAttributes(); } public function get(string $parameterName): string { - if (!isset($this->routeParameters[$parameterName])) { - throw new RuntimeException('Route parameter ' . $parameterName . ' not found in given ServerRequestInterface!'); + // The League router puts the parameters directly in the request attributes. + if (isset($this->attributes[$parameterName])) { + return (string) $this->attributes[$parameterName]; } - return (string) $this->routeParameters[$parameterName]; + // The Silex router puts the parameters in a "_route_params" nested array. + if (isset($this->attributes['_route_params'][$parameterName])) { + return (string) $this->attributes['_route_params'][$parameterName]; + } + throw new RuntimeException('Route parameter ' . $parameterName . ' not found in given ServerRequestInterface!'); } public function has(string $parameterName): bool { - return isset($this->routeParameters[$parameterName]); + // The League router puts the parameters directly in the request attributes. + // The Silex router puts the parameters in a "_route_params" nested array. + return isset($this->attributes[$parameterName]) || isset($this->attributes['_route_params'][$parameterName]); } public function getEventId(): string From 3b792e75d377d05a3bdf728e9d6306f16b1c59af Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 14:29:22 +0200 Subject: [PATCH 21/38] Update RouteParameters tests to reflect new logic --- tests/Http/Request/RouteParametersTest.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/Http/Request/RouteParametersTest.php b/tests/Http/Request/RouteParametersTest.php index cc4635a24c..b83a6c5f36 100644 --- a/tests/Http/Request/RouteParametersTest.php +++ b/tests/Http/Request/RouteParametersTest.php @@ -18,10 +18,10 @@ class RouteParametersTest extends TestCase /** * @test */ - public function it_should_return_an_existing_route_parameter_from_the_request_as_string(): void + public function it_should_return_an_existing_route_parameter_from_the_psr_request_as_string(): void { $request = (new Psr7RequestBuilder())->build('PUT'); - $request = $request->withAttribute('_route_params', ['foo' => 'bar']); + $request = $request->withAttribute('foo', 'bar'); $routeParameters = new RouteParameters($request); $this->assertEquals('bar', $routeParameters->get('foo')); @@ -30,14 +30,13 @@ public function it_should_return_an_existing_route_parameter_from_the_request_as /** * @test */ - public function it_should_throw_a_runtime_exception_if_a_parameter_is_requested_that_is_not_set(): void + public function it_should_return_an_existing_route_parameter_from_the_silex_request_as_string(): void { $request = (new Psr7RequestBuilder())->build('PUT'); - $request = $request->withAttribute('_route_params', []); + $request = $request->withAttribute('_route_params', ['foo' => 'bar']); $routeParameters = new RouteParameters($request); - $this->expectException(RuntimeException::class); - $routeParameters->get('foo'); + $this->assertEquals('bar', $routeParameters->get('foo')); } /** From d9f558d1f11eba8e7f4b9b4ce2ca671842159600 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 14:31:21 +0200 Subject: [PATCH 22/38] Follow approach of the new PSR router in Psr7RequestBuilder for tests --- tests/Http/Request/Psr7RequestBuilder.php | 29 +++++++++++++---------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/tests/Http/Request/Psr7RequestBuilder.php b/tests/Http/Request/Psr7RequestBuilder.php index 3abccdf474..5cb0d976c9 100644 --- a/tests/Http/Request/Psr7RequestBuilder.php +++ b/tests/Http/Request/Psr7RequestBuilder.php @@ -99,19 +99,22 @@ public function withRouteParameter(string $parameterName, string $parameterValue public function build(string $method): ServerRequestInterface { - return ( - new Request( - $method, - $this->uri ?? self::getUriFactory()->createUri(), - $this->headers ?? new Headers(), - [], - [], - $this->body ?? self::getStreamFactory()->createStream(), - $this->files, - ) - ) - ->withAttribute('_route_params', $this->routeParameters) - ->withParsedBody($this->parsedBody); + $request = new Request( + $method, + $this->uri ?? self::getUriFactory()->createUri(), + $this->headers ?? new Headers(), + [], + [], + $this->body ?? self::getStreamFactory()->createStream(), + $this->files, + ); + + foreach ($this->routeParameters as $routeParameter => $value) { + $request = $request->withAttribute($routeParameter, $value); + } + + $request = $request->withParsedBody($this->parsedBody); + return $request; } private static function getUriFactory(): UriFactory From f6c9e571ac4ed7d537d006f937b36fe064c13534 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 15:25:55 +0200 Subject: [PATCH 23/38] Match OPTIONS requests in new router --- app/Http/PsrRouterServiceProvider.php | 5 +++++ web/index.php | 5 ----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/Http/PsrRouterServiceProvider.php b/app/Http/PsrRouterServiceProvider.php index b3c5bfae10..e86cc51dfe 100644 --- a/app/Http/PsrRouterServiceProvider.php +++ b/app/Http/PsrRouterServiceProvider.php @@ -5,6 +5,7 @@ namespace CultuurNet\UDB3\Silex\Http; use CultuurNet\UDB3\Http\Offer\GetDetailRequestHandler; +use CultuurNet\UDB3\Http\Response\NoContentResponse; use League\Route\Router; use Silex\Application; use Silex\ServiceProviderInterface; @@ -17,6 +18,10 @@ public function register(Application $app): void function (Application $app) { $router = new Router(); + // Return 204 No Content for every OPTIONS request to support CORS. + // The necessary CORS headers will be added by a Middleware that adds them for every response. + $router->options('/{path:.*}', fn () => new NoContentResponse()); + $router->get('/{offerType}/{offerId}', [$app[GetDetailRequestHandler::class], 'handle']); return $router; diff --git a/web/index.php b/web/index.php index fa97a8e432..e52ab7df68 100644 --- a/web/index.php +++ b/web/index.php @@ -252,11 +252,6 @@ function () { $app->mount(ImportControllerProvider::PATH, new ImportControllerProvider()); -// Match with any OPTIONS request with any URL and return a 204 No Content. Actual CORS headers will be added by an -// ->after() middleware, which adds CORS headers to every request (so non-preflighted requests like simple GETs also get -// the needed CORS headers). -$app->options('/{path}', fn () => new NoContentResponse())->assert('path', '^.+$'); - // Add CORS headers to every request. We explicitly allow everything, because we don't use cookies and our API is not on // an internal network, so CORS requests are never a security issue in our case. This greatly reduces the risk of CORS // bugs in our frontend and other integrations. From 323973a947177c5fe39bdb9357803767570863ae Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 15:29:04 +0200 Subject: [PATCH 24/38] Add comment to move CORS headers middleware to PSR-15 middleware later It still works for all routes, also those matched by the new router. And if we move it now it won't work anymore for routes matched by the Silex router. So we need to move all the routes to the new router first --- web/index.php | 1 + 1 file changed, 1 insertion(+) diff --git a/web/index.php b/web/index.php index e52ab7df68..973d82cd00 100644 --- a/web/index.php +++ b/web/index.php @@ -255,6 +255,7 @@ function () { // Add CORS headers to every request. We explicitly allow everything, because we don't use cookies and our API is not on // an internal network, so CORS requests are never a security issue in our case. This greatly reduces the risk of CORS // bugs in our frontend and other integrations. +// @todo III-4235 Move to Middleware in new PSR router when all routes are registered on the new router. $app->after( function (Request $request, Response $response) { // Allow any known method regardless of the URL. From 7de8cef1bc17531585e1846dfaf82d204b19526b Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 15:36:09 +0200 Subject: [PATCH 25/38] Move inline $rewrite function to class --- app/CatchAllRouteServiceProvider.php | 30 +++--------------------- src/Http/LegacyPathRewriter.php | 35 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 27 deletions(-) create mode 100644 src/Http/LegacyPathRewriter.php diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 09782ba0d0..1276c91bc6 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -5,6 +5,7 @@ namespace CultuurNet\UDB3\Silex; use CultuurNet\UDB3\Http\ApiProblem\ApiProblem; +use CultuurNet\UDB3\Http\LegacyPathRewriter; use CultuurNet\UDB3\Http\Response\ApiProblemJsonResponse; use League\Route\Http\Exception\MethodNotAllowedException; use League\Route\Http\Exception\NotFoundException; @@ -39,36 +40,11 @@ public function boot(Application $app): void $app->match( '/{path}', function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenForSilex, &$originalRequest) { - $rewritePath = static function (string $originalPath): string { - $rewrites = [ - // Pluralize /event and /place - '/^(\/)?(event|place)($|\/.*)/' => '${1}${2}s${3}', - - // Convert known legacy camelCase resource/collection names to kebab-case - '/bookingAvailability/' => 'booking-availability', - '/bookingInfo/' => 'booking-info', - '/cardSystems/' => 'card-systems', - '/contactPoint/' => 'contact-point', - '/distributionKey/' => 'distribution-key', - '/majorInfo/' => 'major-info', - '/priceInfo/' => 'price-info', - '/subEvents/' => 'sub-events', - '/typicalAgeRange/' => 'typical-age-range', - - // Convert old "calsum" path to "calendar-summary" - '/\/calsum/' => '/calendar-summary', - - // Convert old "news_articles" path to "news-articles" - '/news_articles/' => 'news-articles', - ]; - return preg_replace(array_keys($rewrites), array_values($rewrites), $originalPath); - }; - if (!$pathHasBeenRewrittenForSilex) { // If the path has not been rewritten before, rewrite it and dispatch the request again to the Silex // router. Note that the Silex router also requires us to append a trailing slash if it's missing, // whereas the PSR router treats paths with or without trailing slash the same. - $rewrittenPath = $rewritePath($path); + $rewrittenPath = (new LegacyPathRewriter())->rewrite($path); $rewrittenPath = preg_replace('/^(.*)(?createRequest($originalRequest); $path = $psrRequest->getUri()->getPath(); - $rewrittenPath = $rewritePath($path); + $rewrittenPath = (new LegacyPathRewriter())->rewrite($path); $rewrittenUri = (new UriFactory())->createUri($rewrittenPath); $rewrittenPsrRequest = $psrRequest->withUri($rewrittenUri); diff --git a/src/Http/LegacyPathRewriter.php b/src/Http/LegacyPathRewriter.php new file mode 100644 index 0000000000..c17380a9b1 --- /dev/null +++ b/src/Http/LegacyPathRewriter.php @@ -0,0 +1,35 @@ + '${1}${2}s${3}', + + // Convert known legacy camelCase resource/collection names to kebab-case + '/bookingAvailability/' => 'booking-availability', + '/bookingInfo/' => 'booking-info', + '/cardSystems/' => 'card-systems', + '/contactPoint/' => 'contact-point', + '/distributionKey/' => 'distribution-key', + '/majorInfo/' => 'major-info', + '/priceInfo/' => 'price-info', + '/subEvents/' => 'sub-events', + '/typicalAgeRange/' => 'typical-age-range', + + // Convert old "calsum" path to "calendar-summary" + '/\/calsum/' => '/calendar-summary', + + // Convert old "news_articles" path to "news-articles" + '/news_articles/' => 'news-articles', + ]; + + public function rewrite(string $path): string + { + return preg_replace(array_keys(self::REWRITES), array_values(self::REWRITES), $path); + } +} From 309cfd985e688cfd71c41fa0e2a1d9504e38aed2 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 15:56:54 +0200 Subject: [PATCH 26/38] Add tests for LegacyPathRewriter --- tests/Http/LegacyPathRewriterTest.php | 136 ++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 tests/Http/LegacyPathRewriterTest.php diff --git a/tests/Http/LegacyPathRewriterTest.php b/tests/Http/LegacyPathRewriterTest.php new file mode 100644 index 0000000000..c9f0e2b5e0 --- /dev/null +++ b/tests/Http/LegacyPathRewriterTest.php @@ -0,0 +1,136 @@ +rewrite($originalPath); + $this->assertEquals($expectedRewrite, $actualRewrite); + } + + public function rewritesDataProvider(): array + { + return [ + 'event_detail_singular_to_pluralized' => [ + 'original' => '/event/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + ], + 'place_detail_singular_to_pluralized' => [ + 'original' => '/place/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + ], + 'event_detail_untouched' => [ + 'original' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + ], + 'place_detail_untouched' => [ + 'original' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + ], + 'event_detail_without_prefixed_slash_singular_to_pluralized' => [ + 'original' => 'event/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'rewrite' => 'events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + ], + 'place_detail_without_prefixed_slash_singular_to_pluralized' => [ + 'original' => 'place/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'rewrite' => 'places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + ], + 'event_detail_without_prefixed_slash_untouched' => [ + 'original' => 'events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'rewrite' => 'events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + ], + 'place_detail_without_prefixed_slash_untouched' => [ + 'original' => 'places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'rewrite' => 'places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + ], + 'event_update_name_singular_to_pluralized' => [ + 'original' => '/event/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', + 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', + ], + 'place_update_name_singular_to_pluralized' => [ + 'original' => '/place/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', + 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', + ], + 'event_update_name_untouched' => [ + 'original' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', + 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', + ], + 'place_update_name_untouched' => [ + 'original' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', + 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', + ], + 'event_singular_calsum_to_pluralized_calendar_summary' => [ + 'original' => '/event/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calsum', + 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calendar-summary', + ], + 'place_singular_calsum_to_pluralized_calendar_summary' => [ + 'original' => '/place/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calsum', + 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calendar-summary', + ], + 'event_calsum_to_calendar_summary' => [ + 'original' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calsum', + 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calendar-summary', + ], + 'place_calsum_to_calendar_summary' => [ + 'original' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calsum', + 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calendar-summary', + ], + 'news_articles_with_underscore_to_hyphen' => [ + 'original' => '/news_articles', + 'rewrite' => '/news-articles', + ], + 'news_articles_detail_with_underscore_to_kebab_case' => [ + 'original' => '/news_articles/8a5fcfae-e698-437a-87a5-32cd4ac61076', + 'rewrite' => '/news-articles/8a5fcfae-e698-437a-87a5-32cd4ac61076', + ], + 'event_update_booking_availability_camel_case_to_kebab_case' => [ + 'original' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/bookingAvailability', + 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/booking-availability', + ], + 'event_update_booking_info_camel_case_to_kebab_case' => [ + 'original' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/bookingInfo', + 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/booking-info', + ], + 'event_update_contact_point_camel_case_to_kebab_case' => [ + 'original' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/contactPoint', + 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/contact-point', + ], + 'event_update_major_info_camel_case_to_kebab_case' => [ + 'original' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/majorInfo', + 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/major-info', + ], + 'event_update_price_info_camel_case_to_kebab_case' => [ + 'original' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/priceInfo', + 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/price-info', + ], + 'event_update_sub_events_camel_case_to_kebab_case' => [ + 'original' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/subEvents', + 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/sub-events', + ], + 'event_update_typical_age_range_camel_case_to_kebab_case' => [ + 'original' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/typicalAgeRange', + 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/typical-age-range', + ], + 'event_uitpas_card_systems_camel_case_to_kebab_case' => [ + 'original' => '/uitpas/events/08a70475-4ffe-44b9-b0b9-256c82e7d747/cardSystems', + 'rewrite' => '/uitpas/events/08a70475-4ffe-44b9-b0b9-256c82e7d747/card-systems', + ], + 'event_uitpas_distribution_keys_camel_case_to_kebab_case' => [ + 'original' => '/uitpas/events/08a70475-4ffe-44b9-b0b9-256c82e7d747/cardSystems/1/distributionKey', + 'rewrite' => '/uitpas/events/08a70475-4ffe-44b9-b0b9-256c82e7d747/card-systems/1/distribution-key', + ], + ]; + } +} From ffa6ed2fc98cd70d7114ede5e706f7396bce3b3b Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 16:15:08 +0200 Subject: [PATCH 27/38] Move PSR-router logic into an ApplicationRequestHandler --- app/CatchAllRouteServiceProvider.php | 30 ++--------- app/Http/PsrRouterServiceProvider.php | 7 +++ src/Http/ApplicationRequestHandler.php | 71 ++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 26 deletions(-) create mode 100644 src/Http/ApplicationRequestHandler.php diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 1276c91bc6..061d6a284c 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -5,6 +5,7 @@ namespace CultuurNet\UDB3\Silex; use CultuurNet\UDB3\Http\ApiProblem\ApiProblem; +use CultuurNet\UDB3\Http\ApplicationRequestHandler; use CultuurNet\UDB3\Http\LegacyPathRewriter; use CultuurNet\UDB3\Http\Response\ApiProblemJsonResponse; use League\Route\Http\Exception\MethodNotAllowedException; @@ -74,33 +75,10 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenForSi return $app->handle($rewrittenRequest, HttpKernelInterface::SUB_REQUEST); } - /** @var Router $router */ - $router = $app[Router::class]; - - // Always start from the original request and rewrite the path before dispatching it on the PSR router. - // The only case that this could be a problem is if there is a route that is registered with an - // outdated name in the PSR router, but it makes the logic a lot easier. So we should just make sure to - // use the newer names when registering the routes on the new router. + /** @var ApplicationRequestHandler $applicationRequestHandler */ + $psrApplicationRequestHandler = $app[ApplicationRequestHandler::class]; $psrRequest = (new DiactorosFactory())->createRequest($originalRequest); - $path = $psrRequest->getUri()->getPath(); - $rewrittenPath = (new LegacyPathRewriter())->rewrite($path); - $rewrittenUri = (new UriFactory())->createUri($rewrittenPath); - $rewrittenPsrRequest = $psrRequest->withUri($rewrittenUri); - - try { - $psrResponse = $router->handle($rewrittenPsrRequest); - } catch (NotFoundException $e) { - return new ApiProblemJsonResponse(ApiProblem::urlNotFound()); - } catch (MethodNotAllowedException $e) { - $details = null; - $headers = $e->getHeaders(); - $allowed = $headers['Allow'] ?? null; - if ($allowed !== null) { - $details = 'Allowed: ' . $allowed; - } - return new ApiProblemJsonResponse(ApiProblem::methodNotAllowed($details)); - } - + $psrResponse = $psrApplicationRequestHandler->handle($psrRequest); return (new HttpFoundationFactory())->createResponse($psrResponse); } )->assert('path', '^.+$'); diff --git a/app/Http/PsrRouterServiceProvider.php b/app/Http/PsrRouterServiceProvider.php index e86cc51dfe..3236247c62 100644 --- a/app/Http/PsrRouterServiceProvider.php +++ b/app/Http/PsrRouterServiceProvider.php @@ -4,6 +4,7 @@ namespace CultuurNet\UDB3\Silex\Http; +use CultuurNet\UDB3\Http\ApplicationRequestHandler; use CultuurNet\UDB3\Http\Offer\GetDetailRequestHandler; use CultuurNet\UDB3\Http\Response\NoContentResponse; use League\Route\Router; @@ -27,6 +28,12 @@ function (Application $app) { return $router; } ); + + $app[ApplicationRequestHandler::class] = $app::share( + function (Application $app) { + return new ApplicationRequestHandler($app[Router::class]); + } + ); } public function boot(Application $app): void diff --git a/src/Http/ApplicationRequestHandler.php b/src/Http/ApplicationRequestHandler.php new file mode 100644 index 0000000000..c1dfed62d6 --- /dev/null +++ b/src/Http/ApplicationRequestHandler.php @@ -0,0 +1,71 @@ +router = $router; + } + + public function handle(ServerRequestInterface $request): ResponseInterface + { + $request = $this->rewriteRequestUri($request); + + try { + return $this->router->handle($request); + } catch (Throwable $e) { + return $this->handleError($e); + } + } + + private function rewriteRequestUri(ServerRequestInterface $request): ServerRequestInterface + { + $path = $request->getUri()->getPath(); + $rewrittenPath = (new LegacyPathRewriter())->rewrite($path); + $rewrittenUri = (new UriFactory())->createUri($rewrittenPath); + return $request->withUri($rewrittenUri); + } + + private function handleError(Throwable $e): ResponseInterface + { + switch (true) { + case $e instanceof NotFoundException: + return new ApiProblemJsonResponse(ApiProblem::urlNotFound()); + + case $e instanceof MethodNotAllowedException: + $details = null; + $headers = $e->getHeaders(); + $allowed = $headers['Allow'] ?? null; + if ($allowed !== null) { + $details = 'Allowed: ' . $allowed; + } + return new ApiProblemJsonResponse(ApiProblem::methodNotAllowed($details)); + + default: + throw $e; + } + } +} From 0a0e33c38b01a8b3130378d50f935a7ddbad27b9 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 16:29:07 +0200 Subject: [PATCH 28/38] Rename rewrite() to rewritePath() --- app/CatchAllRouteServiceProvider.php | 2 +- src/Http/ApplicationRequestHandler.php | 2 +- src/Http/LegacyPathRewriter.php | 2 +- tests/Http/LegacyPathRewriterTest.php | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 061d6a284c..3862b0f299 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -45,7 +45,7 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenForSi // If the path has not been rewritten before, rewrite it and dispatch the request again to the Silex // router. Note that the Silex router also requires us to append a trailing slash if it's missing, // whereas the PSR router treats paths with or without trailing slash the same. - $rewrittenPath = (new LegacyPathRewriter())->rewrite($path); + $rewrittenPath = (new LegacyPathRewriter())->rewritePath($path); $rewrittenPath = preg_replace('/^(.*)(?getUri()->getPath(); - $rewrittenPath = (new LegacyPathRewriter())->rewrite($path); + $rewrittenPath = (new LegacyPathRewriter())->rewritePath($path); $rewrittenUri = (new UriFactory())->createUri($rewrittenPath); return $request->withUri($rewrittenUri); } diff --git a/src/Http/LegacyPathRewriter.php b/src/Http/LegacyPathRewriter.php index c17380a9b1..593b88ee43 100644 --- a/src/Http/LegacyPathRewriter.php +++ b/src/Http/LegacyPathRewriter.php @@ -28,7 +28,7 @@ final class LegacyPathRewriter '/news_articles/' => 'news-articles', ]; - public function rewrite(string $path): string + public function rewritePath(string $path): string { return preg_replace(array_keys(self::REWRITES), array_values(self::REWRITES), $path); } diff --git a/tests/Http/LegacyPathRewriterTest.php b/tests/Http/LegacyPathRewriterTest.php index c9f0e2b5e0..7ec28648c8 100644 --- a/tests/Http/LegacyPathRewriterTest.php +++ b/tests/Http/LegacyPathRewriterTest.php @@ -16,7 +16,7 @@ public function it_rewrites_legacy_paths_and_leaves_others_as_is( string $originalPath, string $expectedRewrite ): void { - $actualRewrite = (new LegacyPathRewriter())->rewrite($originalPath); + $actualRewrite = (new LegacyPathRewriter())->rewritePath($originalPath); $this->assertEquals($expectedRewrite, $actualRewrite); } From b372a9d4e4a5c129cd3f50b4f9cd6a78246557b1 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 16:31:04 +0200 Subject: [PATCH 29/38] Move request rewrite logic to LegacyPathRewriter --- src/Http/ApplicationRequestHandler.php | 11 +---------- src/Http/LegacyPathRewriter.php | 11 +++++++++++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/Http/ApplicationRequestHandler.php b/src/Http/ApplicationRequestHandler.php index 214ac36744..b69087c2d7 100644 --- a/src/Http/ApplicationRequestHandler.php +++ b/src/Http/ApplicationRequestHandler.php @@ -11,7 +11,6 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; -use Slim\Psr7\Factory\UriFactory; use Throwable; /** @@ -32,7 +31,7 @@ public function __construct(RequestHandlerInterface $router) public function handle(ServerRequestInterface $request): ResponseInterface { - $request = $this->rewriteRequestUri($request); + $request = (new LegacyPathRewriter())->rewriteRequest($request); try { return $this->router->handle($request); @@ -41,14 +40,6 @@ public function handle(ServerRequestInterface $request): ResponseInterface } } - private function rewriteRequestUri(ServerRequestInterface $request): ServerRequestInterface - { - $path = $request->getUri()->getPath(); - $rewrittenPath = (new LegacyPathRewriter())->rewritePath($path); - $rewrittenUri = (new UriFactory())->createUri($rewrittenPath); - return $request->withUri($rewrittenUri); - } - private function handleError(Throwable $e): ResponseInterface { switch (true) { diff --git a/src/Http/LegacyPathRewriter.php b/src/Http/LegacyPathRewriter.php index 593b88ee43..dbe7d078ad 100644 --- a/src/Http/LegacyPathRewriter.php +++ b/src/Http/LegacyPathRewriter.php @@ -4,6 +4,8 @@ namespace CultuurNet\UDB3\Http; +use Psr\Http\Message\ServerRequestInterface; + final class LegacyPathRewriter { private const REWRITES = [ @@ -32,4 +34,13 @@ public function rewritePath(string $path): string { return preg_replace(array_keys(self::REWRITES), array_values(self::REWRITES), $path); } + + public function rewriteRequest(ServerRequestInterface $request): ServerRequestInterface + { + $uri = $request->getUri(); + $path = $uri->getPath(); + $rewrittenPath = $this->rewritePath($path); + $rewrittenUri = $uri->withPath($rewrittenPath); + return $request->withUri($rewrittenUri); + } } From b66e3d1a7b4c4ab8230af1efe146aea7ce309472 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 18:19:52 +0200 Subject: [PATCH 30/38] Move error handling from ApplicationRequestHandler to WebErrorHandlerProvider Better to stay consistent, and we will refactor the error handling in a later ticket --- app/Error/WebErrorHandlerProvider.php | 14 +++++++++++ src/Http/ApplicationRequestHandler.php | 32 +------------------------- 2 files changed, 15 insertions(+), 31 deletions(-) diff --git a/app/Error/WebErrorHandlerProvider.php b/app/Error/WebErrorHandlerProvider.php index 35b92084b2..72c7887561 100644 --- a/app/Error/WebErrorHandlerProvider.php +++ b/app/Error/WebErrorHandlerProvider.php @@ -16,6 +16,8 @@ use CultuurNet\UDB3\Security\CommandAuthorizationException; use Error; use Exception; +use League\Route\Http\Exception\MethodNotAllowedException; +use League\Route\Http\Exception\NotFoundException; use Respect\Validation\Exceptions\GroupedValidationException; use Silex\Application; use Silex\ServiceProviderInterface; @@ -89,6 +91,18 @@ private static function convertThrowableToApiProblem(Request $request, Throwable ) ); + case $e instanceof NotFoundException: + return ApiProblem::urlNotFound(); + + case $e instanceof MethodNotAllowedException: + $details = null; + $headers = $e->getHeaders(); + $allowed = $headers['Allow'] ?? null; + if ($allowed !== null) { + $details = 'Allowed: ' . $allowed; + } + return ApiProblem::methodNotAllowed($details); + // Do a best effort to convert "not found" exceptions into an ApiProblem with preferably a detail mentioning // what kind of resource and with what id could not be found. Since the exceptions themselves do not contain // enough info to detect this, we need to get this info from the current request. However this is not diff --git a/src/Http/ApplicationRequestHandler.php b/src/Http/ApplicationRequestHandler.php index b69087c2d7..670ff90da4 100644 --- a/src/Http/ApplicationRequestHandler.php +++ b/src/Http/ApplicationRequestHandler.php @@ -4,14 +4,9 @@ namespace CultuurNet\UDB3\Http; -use CultuurNet\UDB3\Http\ApiProblem\ApiProblem; -use CultuurNet\UDB3\Http\Response\ApiProblemJsonResponse; -use League\Route\Http\Exception\MethodNotAllowedException; -use League\Route\Http\Exception\NotFoundException; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; -use Throwable; /** * Request handler that handles any incoming request by delegating it to a router that implements @@ -32,31 +27,6 @@ public function __construct(RequestHandlerInterface $router) public function handle(ServerRequestInterface $request): ResponseInterface { $request = (new LegacyPathRewriter())->rewriteRequest($request); - - try { - return $this->router->handle($request); - } catch (Throwable $e) { - return $this->handleError($e); - } - } - - private function handleError(Throwable $e): ResponseInterface - { - switch (true) { - case $e instanceof NotFoundException: - return new ApiProblemJsonResponse(ApiProblem::urlNotFound()); - - case $e instanceof MethodNotAllowedException: - $details = null; - $headers = $e->getHeaders(); - $allowed = $headers['Allow'] ?? null; - if ($allowed !== null) { - $details = 'Allowed: ' . $allowed; - } - return new ApiProblemJsonResponse(ApiProblem::methodNotAllowed($details)); - - default: - throw $e; - } + return $this->router->handle($request); } } From 4e86b14e6474e8508e7af49f42e2011c1cbcb09d Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 18:21:15 +0200 Subject: [PATCH 31/38] Remove ApplicationRequestHandler again now that is basically does nothing --- app/CatchAllRouteServiceProvider.php | 7 +++--- app/Http/PsrRouterServiceProvider.php | 6 ----- src/Http/ApplicationRequestHandler.php | 32 -------------------------- 3 files changed, 4 insertions(+), 41 deletions(-) delete mode 100644 src/Http/ApplicationRequestHandler.php diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 3862b0f299..a014434b9c 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -75,10 +75,11 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenForSi return $app->handle($rewrittenRequest, HttpKernelInterface::SUB_REQUEST); } - /** @var ApplicationRequestHandler $applicationRequestHandler */ - $psrApplicationRequestHandler = $app[ApplicationRequestHandler::class]; + /** @var Router $psrRouter */ + $psrRouter = $app[Router::class]; $psrRequest = (new DiactorosFactory())->createRequest($originalRequest); - $psrResponse = $psrApplicationRequestHandler->handle($psrRequest); + $psrRequest = (new LegacyPathRewriter())->rewriteRequest($psrRequest); + $psrResponse = $psrRouter->handle($psrRequest); return (new HttpFoundationFactory())->createResponse($psrResponse); } )->assert('path', '^.+$'); diff --git a/app/Http/PsrRouterServiceProvider.php b/app/Http/PsrRouterServiceProvider.php index 3236247c62..d1fd55fc20 100644 --- a/app/Http/PsrRouterServiceProvider.php +++ b/app/Http/PsrRouterServiceProvider.php @@ -28,12 +28,6 @@ function (Application $app) { return $router; } ); - - $app[ApplicationRequestHandler::class] = $app::share( - function (Application $app) { - return new ApplicationRequestHandler($app[Router::class]); - } - ); } public function boot(Application $app): void diff --git a/src/Http/ApplicationRequestHandler.php b/src/Http/ApplicationRequestHandler.php deleted file mode 100644 index 670ff90da4..0000000000 --- a/src/Http/ApplicationRequestHandler.php +++ /dev/null @@ -1,32 +0,0 @@ -router = $router; - } - - public function handle(ServerRequestInterface $request): ResponseInterface - { - $request = (new LegacyPathRewriter())->rewriteRequest($request); - return $this->router->handle($request); - } -} From ac33cdf1a5ba06c0266e6ff6677f7f7c45ebd872 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 18:51:41 +0200 Subject: [PATCH 32/38] =?UTF-8?q?Linting=E2=84=A2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/CatchAllRouteServiceProvider.php | 6 ------ app/Http/PsrRouterServiceProvider.php | 1 - 2 files changed, 7 deletions(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index a014434b9c..52e8d4ac9d 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -4,16 +4,10 @@ namespace CultuurNet\UDB3\Silex; -use CultuurNet\UDB3\Http\ApiProblem\ApiProblem; -use CultuurNet\UDB3\Http\ApplicationRequestHandler; use CultuurNet\UDB3\Http\LegacyPathRewriter; -use CultuurNet\UDB3\Http\Response\ApiProblemJsonResponse; -use League\Route\Http\Exception\MethodNotAllowedException; -use League\Route\Http\Exception\NotFoundException; use League\Route\Router; use Silex\Application; use Silex\ServiceProviderInterface; -use Slim\Psr7\Factory\UriFactory; use Symfony\Bridge\PsrHttpMessage\Factory\DiactorosFactory; use Symfony\Bridge\PsrHttpMessage\Factory\HttpFoundationFactory; use Symfony\Component\HttpFoundation\Request; diff --git a/app/Http/PsrRouterServiceProvider.php b/app/Http/PsrRouterServiceProvider.php index d1fd55fc20..e86cc51dfe 100644 --- a/app/Http/PsrRouterServiceProvider.php +++ b/app/Http/PsrRouterServiceProvider.php @@ -4,7 +4,6 @@ namespace CultuurNet\UDB3\Silex\Http; -use CultuurNet\UDB3\Http\ApplicationRequestHandler; use CultuurNet\UDB3\Http\Offer\GetDetailRequestHandler; use CultuurNet\UDB3\Http\Response\NoContentResponse; use League\Route\Router; From 2f616ee567cbc0f8ead3728fb7dc105d6023eb67 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 18:53:32 +0200 Subject: [PATCH 33/38] Add comment about PSR router --- app/CatchAllRouteServiceProvider.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 52e8d4ac9d..42f5542453 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -69,6 +69,8 @@ function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenForSi return $app->handle($rewrittenRequest, HttpKernelInterface::SUB_REQUEST); } + // If the request is still not matched with a route after it has been rewritten and dispatched on the + // Silex router a second time, convert it to a PSR request and dispatch it on the PSR router instead. /** @var Router $psrRouter */ $psrRouter = $app[Router::class]; $psrRequest = (new DiactorosFactory())->createRequest($originalRequest); From 885bf4b32a8259d8d019f4b174672cb88806c4c5 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 18:58:29 +0200 Subject: [PATCH 34/38] New router does not ignore trailing slashes after all --- app/CatchAllRouteServiceProvider.php | 4 +- app/Http/PsrRouterServiceProvider.php | 2 +- src/Http/LegacyPathRewriter.php | 3 + tests/Http/LegacyPathRewriterTest.php | 108 +++++++++++++------------- 4 files changed, 59 insertions(+), 58 deletions(-) diff --git a/app/CatchAllRouteServiceProvider.php b/app/CatchAllRouteServiceProvider.php index 42f5542453..fd9ab4292b 100644 --- a/app/CatchAllRouteServiceProvider.php +++ b/app/CatchAllRouteServiceProvider.php @@ -37,10 +37,8 @@ public function boot(Application $app): void function (Request $request, string $path) use ($app, &$pathHasBeenRewrittenForSilex, &$originalRequest) { if (!$pathHasBeenRewrittenForSilex) { // If the path has not been rewritten before, rewrite it and dispatch the request again to the Silex - // router. Note that the Silex router also requires us to append a trailing slash if it's missing, - // whereas the PSR router treats paths with or without trailing slash the same. + // router. $rewrittenPath = (new LegacyPathRewriter())->rewritePath($path); - $rewrittenPath = preg_replace('/^(.*)(?options('/{path:.*}', fn () => new NoContentResponse()); - $router->get('/{offerType}/{offerId}', [$app[GetDetailRequestHandler::class], 'handle']); + $router->get('/{offerType}/{offerId}/', [$app[GetDetailRequestHandler::class], 'handle']); return $router; } diff --git a/src/Http/LegacyPathRewriter.php b/src/Http/LegacyPathRewriter.php index dbe7d078ad..dbe1c3941f 100644 --- a/src/Http/LegacyPathRewriter.php +++ b/src/Http/LegacyPathRewriter.php @@ -28,6 +28,9 @@ final class LegacyPathRewriter // Convert old "news_articles" path to "news-articles" '/news_articles/' => 'news-articles', + + // Add trailing slash if missing + '/^(.*)(? '${1}/', ]; public function rewritePath(string $path): string diff --git a/tests/Http/LegacyPathRewriterTest.php b/tests/Http/LegacyPathRewriterTest.php index 7ec28648c8..f9a50ddea5 100644 --- a/tests/Http/LegacyPathRewriterTest.php +++ b/tests/Http/LegacyPathRewriterTest.php @@ -23,113 +23,113 @@ public function it_rewrites_legacy_paths_and_leaves_others_as_is( public function rewritesDataProvider(): array { return [ - 'event_detail_singular_to_pluralized' => [ + 'event_detail_singular_to_pluralized_and_trailing_slash' => [ 'original' => '/event/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', - 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/', ], - 'place_detail_singular_to_pluralized' => [ + 'place_detail_singular_to_pluralized_and_trailing_slash' => [ 'original' => '/place/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', - 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/', ], 'event_detail_untouched' => [ - 'original' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', - 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'original' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/', + 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/', ], 'place_detail_untouched' => [ - 'original' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', - 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'original' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/', + 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/', ], - 'event_detail_without_prefixed_slash_singular_to_pluralized' => [ + 'event_detail_without_prefixed_slash_singular_to_pluralized_and_trailing_slash' => [ 'original' => 'event/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', - 'rewrite' => 'events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'rewrite' => 'events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/', ], - 'place_detail_without_prefixed_slash_singular_to_pluralized' => [ + 'place_detail_without_prefixed_slash_singular_to_pluralized_and_trailing_slash' => [ 'original' => 'place/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', - 'rewrite' => 'places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'rewrite' => 'places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/', ], 'event_detail_without_prefixed_slash_untouched' => [ - 'original' => 'events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', - 'rewrite' => 'events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'original' => 'events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/', + 'rewrite' => 'events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/', ], 'place_detail_without_prefixed_slash_untouched' => [ - 'original' => 'places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', - 'rewrite' => 'places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3', + 'original' => 'places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/', + 'rewrite' => 'places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/', ], - 'event_update_name_singular_to_pluralized' => [ + 'event_update_name_singular_to_pluralized_and_trailing_slash' => [ 'original' => '/event/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', - 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', + 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl/', ], - 'place_update_name_singular_to_pluralized' => [ + 'place_update_name_singular_to_pluralized_and_trailing_slash' => [ 'original' => '/place/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', - 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', + 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl/', ], 'event_update_name_untouched' => [ - 'original' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', - 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', + 'original' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl/', + 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl/', ], 'place_update_name_untouched' => [ - 'original' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', - 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl', + 'original' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl/', + 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/name/nl/', ], - 'event_singular_calsum_to_pluralized_calendar_summary' => [ + 'event_singular_calsum_to_pluralized_calendar_summary_and_trailing_slash' => [ 'original' => '/event/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calsum', - 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calendar-summary', + 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calendar-summary/', ], - 'place_singular_calsum_to_pluralized_calendar_summary' => [ + 'place_singular_calsum_to_pluralized_calendar_summary_and_trailing_slash' => [ 'original' => '/place/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calsum', - 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calendar-summary', + 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calendar-summary/', ], - 'event_calsum_to_calendar_summary' => [ + 'event_calsum_to_calendar_summary_and_trailing_slash' => [ 'original' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calsum', - 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calendar-summary', + 'rewrite' => '/events/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calendar-summary/', ], - 'place_calsum_to_calendar_summary' => [ + 'place_calsum_to_calendar_summary_and_trailing_slash' => [ 'original' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calsum', - 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calendar-summary', + 'rewrite' => '/places/ccb8f44b-d316-41f0-aeec-2b4149be6cd3/calendar-summary/', ], - 'news_articles_with_underscore_to_hyphen' => [ + 'news_articles_with_underscore_to_hyphen_and_trailing_slash' => [ 'original' => '/news_articles', - 'rewrite' => '/news-articles', + 'rewrite' => '/news-articles/', ], - 'news_articles_detail_with_underscore_to_kebab_case' => [ + 'news_articles_detail_with_underscore_to_kebab_case_and_trailing_slash' => [ 'original' => '/news_articles/8a5fcfae-e698-437a-87a5-32cd4ac61076', - 'rewrite' => '/news-articles/8a5fcfae-e698-437a-87a5-32cd4ac61076', + 'rewrite' => '/news-articles/8a5fcfae-e698-437a-87a5-32cd4ac61076/', ], - 'event_update_booking_availability_camel_case_to_kebab_case' => [ + 'event_update_booking_availability_camel_case_to_kebab_case_and_trailing_slash' => [ 'original' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/bookingAvailability', - 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/booking-availability', + 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/booking-availability/', ], - 'event_update_booking_info_camel_case_to_kebab_case' => [ + 'event_update_booking_info_camel_case_to_kebab_case_and_trailing_slash' => [ 'original' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/bookingInfo', - 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/booking-info', + 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/booking-info/', ], - 'event_update_contact_point_camel_case_to_kebab_case' => [ + 'event_update_contact_point_camel_case_to_kebab_case_and_trailing_slash' => [ 'original' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/contactPoint', - 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/contact-point', + 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/contact-point/', ], - 'event_update_major_info_camel_case_to_kebab_case' => [ + 'event_update_major_info_camel_case_to_kebab_case_and_trailing_slash' => [ 'original' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/majorInfo', - 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/major-info', + 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/major-info/', ], - 'event_update_price_info_camel_case_to_kebab_case' => [ + 'event_update_price_info_camel_case_to_kebab_case_and_trailing_slash' => [ 'original' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/priceInfo', - 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/price-info', + 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/price-info/', ], - 'event_update_sub_events_camel_case_to_kebab_case' => [ + 'event_update_sub_events_camel_case_to_kebab_case_and_trailing_slash' => [ 'original' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/subEvents', - 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/sub-events', + 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/sub-events/', ], - 'event_update_typical_age_range_camel_case_to_kebab_case' => [ + 'event_update_typical_age_range_camel_case_to_kebab_case_and_trailing_slash' => [ 'original' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/typicalAgeRange', - 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/typical-age-range', + 'rewrite' => '/events/8a5fcfae-e698-437a-87a5-32cd4ac61076/typical-age-range/', ], - 'event_uitpas_card_systems_camel_case_to_kebab_case' => [ + 'event_uitpas_card_systems_camel_case_to_kebab_case_and_trailing_slash' => [ 'original' => '/uitpas/events/08a70475-4ffe-44b9-b0b9-256c82e7d747/cardSystems', - 'rewrite' => '/uitpas/events/08a70475-4ffe-44b9-b0b9-256c82e7d747/card-systems', + 'rewrite' => '/uitpas/events/08a70475-4ffe-44b9-b0b9-256c82e7d747/card-systems/', ], - 'event_uitpas_distribution_keys_camel_case_to_kebab_case' => [ + 'event_uitpas_distribution_keys_camel_case_to_kebab_case_and_trailing_slash' => [ 'original' => '/uitpas/events/08a70475-4ffe-44b9-b0b9-256c82e7d747/cardSystems/1/distributionKey', - 'rewrite' => '/uitpas/events/08a70475-4ffe-44b9-b0b9-256c82e7d747/card-systems/1/distribution-key', + 'rewrite' => '/uitpas/events/08a70475-4ffe-44b9-b0b9-256c82e7d747/card-systems/1/distribution-key/', ], ]; } From 02a0febc2966fba2174a40ec22cfad40be533334 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 19:09:10 +0200 Subject: [PATCH 35/38] Use a custom strategy to handle OPTIONS requests --- app/Http/PsrRouterServiceProvider.php | 7 +- src/Http/CustomLeagueRouterStrategy.php | 98 +++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 src/Http/CustomLeagueRouterStrategy.php diff --git a/app/Http/PsrRouterServiceProvider.php b/app/Http/PsrRouterServiceProvider.php index 3e71c113bf..049d6b6d78 100644 --- a/app/Http/PsrRouterServiceProvider.php +++ b/app/Http/PsrRouterServiceProvider.php @@ -4,8 +4,8 @@ namespace CultuurNet\UDB3\Silex\Http; +use CultuurNet\UDB3\Http\CustomLeagueRouterStrategy; use CultuurNet\UDB3\Http\Offer\GetDetailRequestHandler; -use CultuurNet\UDB3\Http\Response\NoContentResponse; use League\Route\Router; use Silex\Application; use Silex\ServiceProviderInterface; @@ -17,10 +17,7 @@ public function register(Application $app): void $app[Router::class] = $app::share( function (Application $app) { $router = new Router(); - - // Return 204 No Content for every OPTIONS request to support CORS. - // The necessary CORS headers will be added by a Middleware that adds them for every response. - $router->options('/{path:.*}', fn () => new NoContentResponse()); + $router->setStrategy(new CustomLeagueRouterStrategy()); $router->get('/{offerType}/{offerId}/', [$app[GetDetailRequestHandler::class], 'handle']); diff --git a/src/Http/CustomLeagueRouterStrategy.php b/src/Http/CustomLeagueRouterStrategy.php new file mode 100644 index 0000000000..8aecf02f3a --- /dev/null +++ b/src/Http/CustomLeagueRouterStrategy.php @@ -0,0 +1,98 @@ +throwThrowableMiddleware($exception); + } + + public function getNotFoundDecorator(NotFoundException $exception): MiddlewareInterface + { + return $this->throwThrowableMiddleware($exception); + } + + public function getThrowableHandler(): MiddlewareInterface + { + return new class implements MiddlewareInterface + { + public function process( + ServerRequestInterface $request, + RequestHandlerInterface $handler + ): ResponseInterface { + try { + return $handler->handle($request); + } catch (Throwable $e) { + throw $e; + } + } + }; + } + + public function invokeRouteCallable(Route $route, ServerRequestInterface $request): ResponseInterface + { + $controller = $route->getCallable($this->getContainer()); + $response = $controller($request, $route->getVars()); + return $this->decorateResponse($response); + } + + protected function throwThrowableMiddleware(Throwable $error): MiddlewareInterface + { + return new class ($error) implements MiddlewareInterface + { + protected $error; + + public function __construct(Throwable $error) + { + $this->error = $error; + } + + public function process( + ServerRequestInterface $request, + RequestHandlerInterface $handler + ): ResponseInterface { + throw $this->error; + } + }; + } +} From b5f02e88dd6450c7ca4d838be6129ee9d8b29e56 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 19:11:43 +0200 Subject: [PATCH 36/38] Add catch-all route in Silex router again for OPTIONS requests Otherwise routes that are not registered in the new router yet will not work with CORS anymore --- web/index.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/web/index.php b/web/index.php index 973d82cd00..039a25c4d7 100644 --- a/web/index.php +++ b/web/index.php @@ -252,6 +252,14 @@ function () { $app->mount(ImportControllerProvider::PATH, new ImportControllerProvider()); +// Match with any OPTIONS request with any URL and return a 204 No Content. Actual CORS headers will be added by an +// ->after() middleware, which adds CORS headers to every request (so non-preflighted requests like simple GETs also get +// the needed CORS headers). +// Note that the new PSR router in PsrRouterServiceProvider already supports OPTIONS requests for all routes registered +// in the new router, so this can be removed completely once all route definitions and handlers are moved to the new +// router. +$app->options('/{path}', fn () => new NoContentResponse())->assert('path', '^.+$'); + // Add CORS headers to every request. We explicitly allow everything, because we don't use cookies and our API is not on // an internal network, so CORS requests are never a security issue in our case. This greatly reduces the risk of CORS // bugs in our frontend and other integrations. From 21a4c9fd59642e0c1bb1d7cac505b75758c837c7 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Mon, 22 Aug 2022 19:48:37 +0200 Subject: [PATCH 37/38] =?UTF-8?q?Linting=E2=84=A2=20in=20code=20copied=20f?= =?UTF-8?q?rom=20ApplicationStrategy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Http/CustomLeagueRouterStrategy.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Http/CustomLeagueRouterStrategy.php b/src/Http/CustomLeagueRouterStrategy.php index 8aecf02f3a..ebaf1c3f3c 100644 --- a/src/Http/CustomLeagueRouterStrategy.php +++ b/src/Http/CustomLeagueRouterStrategy.php @@ -54,8 +54,7 @@ public function getNotFoundDecorator(NotFoundException $exception): MiddlewareIn public function getThrowableHandler(): MiddlewareInterface { - return new class implements MiddlewareInterface - { + return new class() implements MiddlewareInterface { public function process( ServerRequestInterface $request, RequestHandlerInterface $handler @@ -78,8 +77,7 @@ public function invokeRouteCallable(Route $route, ServerRequestInterface $reques protected function throwThrowableMiddleware(Throwable $error): MiddlewareInterface { - return new class ($error) implements MiddlewareInterface - { + return new class($error) implements MiddlewareInterface { protected $error; public function __construct(Throwable $error) From 4d1d2fe9f2cee565df34a9f9426ffcda8f0f06c1 Mon Sep 17 00:00:00 2001 From: Bert Ramakers Date: Tue, 23 Aug 2022 09:52:55 +0200 Subject: [PATCH 38/38] Do not match with a URL like /foo/bar --- app/Http/PsrRouterServiceProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Http/PsrRouterServiceProvider.php b/app/Http/PsrRouterServiceProvider.php index 049d6b6d78..f1d61ac0c7 100644 --- a/app/Http/PsrRouterServiceProvider.php +++ b/app/Http/PsrRouterServiceProvider.php @@ -19,7 +19,7 @@ function (Application $app) { $router = new Router(); $router->setStrategy(new CustomLeagueRouterStrategy()); - $router->get('/{offerType}/{offerId}/', [$app[GetDetailRequestHandler::class], 'handle']); + $router->get('/{offerType:events|places}/{offerId}/', [$app[GetDetailRequestHandler::class], 'handle']); return $router; }