Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

III-4896 Add PSR router #1102

Merged
merged 40 commits into from
Aug 23, 2022
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
392e5b1
Add league/route package
bertramakers Aug 22, 2022
bfa5ef4
Rename LegacyRoutesServiceProvider to CatchAllRouteServiceProvider
bertramakers Aug 22, 2022
8240ebe
Store the original request
bertramakers Aug 22, 2022
96767d8
Register the league router
bertramakers Aug 22, 2022
68afe45
Dispatch unmatched requests to PSR router
bertramakers Aug 22, 2022
6e67940
Also catch MethodNotAllowedException
bertramakers Aug 22, 2022
e9e2f28
Let the PSR router handle the original request
bertramakers Aug 22, 2022
1bd51d0
Move rewrite logic to function
bertramakers Aug 22, 2022
465157f
Rename $pathHasBeenRewritten to $pathHasBeenRewrittenBefore
bertramakers Aug 22, 2022
ac0da1d
Make the event/place pluralisation work if there is a slash prefixed
bertramakers Aug 22, 2022
3f2b1ef
Always immediately rewrite the path before sending the request to the…
bertramakers Aug 22, 2022
8dfa62c
Only add a trailing slash in case of a rewrite for the Silex router
bertramakers Aug 22, 2022
d77442d
Update comment
bertramakers Aug 22, 2022
6e35ee3
Move Silex sub-request dispatching higher up for readability
bertramakers Aug 22, 2022
97c0a62
Move PSR router dispatching out of else
bertramakers Aug 22, 2022
a6e6aa3
Explain flow
bertramakers Aug 22, 2022
5e981b6
Rename $pathHasBeenRewrittenBefore for clarity
bertramakers Aug 22, 2022
8308986
(Better) Explain why we rewrite the path before dispatching it on the…
bertramakers Aug 22, 2022
5c0c2ff
Move route to get event/place JSON to new router as proof-of-concept
bertramakers Aug 22, 2022
52a1210
Fix for route parameters that are not found in new router
bertramakers Aug 22, 2022
4ddcbab
Merge branch 'master' into III-4896-psr-router
bertramakers Aug 22, 2022
3b792e7
Update RouteParameters tests to reflect new logic
bertramakers Aug 22, 2022
d9f558d
Follow approach of the new PSR router in Psr7RequestBuilder for tests
bertramakers Aug 22, 2022
f6c9e57
Match OPTIONS requests in new router
bertramakers Aug 22, 2022
323973a
Add comment to move CORS headers middleware to PSR-15 middleware later
bertramakers Aug 22, 2022
602c20e
Merge branch 'master' into III-4896-psr-router
bertramakers Aug 22, 2022
7de8cef
Move inline $rewrite function to class
bertramakers Aug 22, 2022
309cfd9
Add tests for LegacyPathRewriter
bertramakers Aug 22, 2022
ffa6ed2
Move PSR-router logic into an ApplicationRequestHandler
bertramakers Aug 22, 2022
0a0e33c
Rename rewrite() to rewritePath()
bertramakers Aug 22, 2022
b372a9d
Move request rewrite logic to LegacyPathRewriter
bertramakers Aug 22, 2022
b66e3d1
Move error handling from ApplicationRequestHandler to WebErrorHandler…
bertramakers Aug 22, 2022
4e86b14
Remove ApplicationRequestHandler again now that is basically does not…
bertramakers Aug 22, 2022
ac33cdf
Linting™
bertramakers Aug 22, 2022
2f616ee
Add comment about PSR router
bertramakers Aug 22, 2022
885bf4b
New router does not ignore trailing slashes after all
bertramakers Aug 22, 2022
02a0feb
Use a custom strategy to handle OPTIONS requests
bertramakers Aug 22, 2022
b5f02e8
Add catch-all route in Silex router again for OPTIONS requests
bertramakers Aug 22, 2022
21a4c9f
Linting™ in code copied from ApplicationStrategy
bertramakers Aug 22, 2022
4d1d2fe
Do not match with a URL like /foo/bar
bertramakers Aug 23, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions app/CatchAllRouteServiceProvider.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php

declare(strict_types=1);

namespace CultuurNet\UDB3\Silex;

use CultuurNet\UDB3\Http\LegacyPathRewriter;
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;

final class CatchAllRouteServiceProvider implements ServiceProviderInterface
{
public function register(Application $app): void
{
}

public function boot(Application $app): void
{
$pathHasBeenRewrittenForSilex = 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.
// 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.
// This makes it possible to support old endpoint names without having to register controllers/request handlers
// 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, &$pathHasBeenRewrittenForSilex, &$originalRequest) {
if (!$pathHasBeenRewrittenForSilex) {
// If the path has not been rewritten before, rewrite it and dispatch the request again to the Silex
// router.
$rewrittenPath = (new LegacyPathRewriter())->rewritePath($path);
$pathHasBeenRewrittenForSilex = 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
$rewrittenRequest = Request::create(
$rewrittenPath,
$request->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.
// 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
// $pathHasBeenRewrittenForSilex will be set to true.
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);
$psrRequest = (new LegacyPathRewriter())->rewriteRequest($psrRequest);
$psrResponse = $psrRouter->handle($psrRequest);
return (new HttpFoundationFactory())->createResponse($psrResponse);
}
)->assert('path', '^.+$');
}
}
14 changes: 14 additions & 0 deletions app/Error/WebErrorHandlerProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
32 changes: 32 additions & 0 deletions app/Http/PsrRouterServiceProvider.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

declare(strict_types=1);

namespace CultuurNet\UDB3\Silex\Http;

use CultuurNet\UDB3\Http\CustomLeagueRouterStrategy;
use CultuurNet\UDB3\Http\Offer\GetDetailRequestHandler;
use League\Route\Router;
use Silex\Application;
use Silex\ServiceProviderInterface;

final class PsrRouterServiceProvider implements ServiceProviderInterface
{
public function register(Application $app): void
{
$app[Router::class] = $app::share(
function (Application $app) {
$router = new Router();
$router->setStrategy(new CustomLeagueRouterStrategy());

$router->get('/{offerType}/{offerId}/', [$app[GetDetailRequestHandler::class], 'handle']);

return $router;
}
);
}

public function boot(Application $app): void
{
}
}
90 changes: 0 additions & 90 deletions app/LegacyRoutesServiceProvider.php

This file was deleted.

1 change: 0 additions & 1 deletion app/Offer/OfferControllerProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading