diff --git a/packages/core/src/Priority.php b/packages/core/src/Priority.php index 0bf263053..87d13a60b 100644 --- a/packages/core/src/Priority.php +++ b/packages/core/src/Priority.php @@ -7,6 +7,8 @@ #[Attribute(Attribute::TARGET_CLASS)] final readonly class Priority { + public const int EXCEPTION_HANDLING = -100; + public const int FRAMEWORK = -1; public const int HIGHEST = 0; diff --git a/packages/http/src/HttpException.php b/packages/http/src/HttpException.php index 7d29aaf27..bd15deb69 100644 --- a/packages/http/src/HttpException.php +++ b/packages/http/src/HttpException.php @@ -14,7 +14,6 @@ public function __construct( public readonly Status $status, ?string $message = null, public readonly ?Response $cause = null, - public ?Response $response = null, ?\Throwable $previous = null, ) { parent::__construct($message ?: '', $status->value, $previous); diff --git a/packages/router/src/Exceptions/HttpErrorResponseProcessor.php b/packages/router/src/Exceptions/HttpErrorResponseProcessor.php deleted file mode 100644 index 9562431ce..000000000 --- a/packages/router/src/Exceptions/HttpErrorResponseProcessor.php +++ /dev/null @@ -1,43 +0,0 @@ -status->isServerError() && ! $response->status->isClientError()) { - return $response; - } - - // If the response already has a body, it means it is most likely - // meant to be returned as-is, so we don't have to throw an exception. - if ($response->body) { - return $response; - } - - // During tests, the router is generally configured to not throw HTTP exceptions in order - // to perform assertions on the responses. In this case, we return the response as is. - if (! $this->routeConfig->throwHttpExceptions) { - return $response; - } - - throw new HttpException( - status: $response->status, - cause: $response, - ); - } -} diff --git a/packages/router/src/Exceptions/HttpExceptionHandler.php b/packages/router/src/Exceptions/HttpExceptionHandler.php index f1d6191e2..37fbe9fa3 100644 --- a/packages/router/src/Exceptions/HttpExceptionHandler.php +++ b/packages/router/src/Exceptions/HttpExceptionHandler.php @@ -45,10 +45,6 @@ public function handle(Throwable $throwable): void private function renderErrorResponse(Status $status, ?HttpException $exception = null): Response { - if ($exception?->response) { - return $exception->response; - } - return new GenericResponse( status: $status, body: new GenericView(__DIR__ . '/HttpErrorResponse/error.view.php', [ diff --git a/packages/router/src/Exceptions/NoMatchedRoute.php b/packages/router/src/Exceptions/NoMatchedRoute.php new file mode 100644 index 000000000..cdb05ec33 --- /dev/null +++ b/packages/router/src/Exceptions/NoMatchedRoute.php @@ -0,0 +1,13 @@ +processResponse( - $this->processRequest($request), - ); - } - - private function processRequest(Request|PsrRequest $request): Response - { - if (! ($request instanceof PsrRequest)) { - $request = map($request)->with(RequestToPsrRequestMapper::class)->do(); - } - - $matchedRoute = $this->routeMatcher->match($request); - - if ($matchedRoute === null) { - return new NotFound(); + if (! ($request instanceof Request)) { + $request = map($request)->with(PsrRequestToGenericRequestMapper::class)->do(); } - $this->container->singleton(MatchedRoute::class, fn () => $matchedRoute); + $callable = $this->getCallable(); - try { - $callable = $this->getCallable($matchedRoute); - $request = $this->resolveRequest($request, $matchedRoute); - $response = $callable($request); - } catch (NotFoundException) { - return new NotFound(); - } catch (ValidationException $validationException) { - return new Invalid($validationException->subject, $validationException->failingRules); - } - - return $response; + return $this->processResponse($callable($request)); } - private function getCallable(MatchedRoute $matchedRoute): HttpMiddlewareCallable + private function getCallable(): HttpMiddlewareCallable { - $route = $matchedRoute->route; + $callControllerAction = function (Request $_) { + $matchedRoute = $this->container->get(MatchedRoute::class); + + if ($matchedRoute === null) { + // At this point, the `MatchRouteMiddleware` should have run. + // If that's not the case, then someone messed up by clearing all HTTP middleware + throw new NoMatchedRoute(); + } + + $route = $matchedRoute->route; - $callControllerAction = function (Request $_) use ($route, $matchedRoute) { $response = $this->container->invoke( $route->handler, ...$matchedRoute->params, @@ -93,10 +71,7 @@ private function getCallable(MatchedRoute $matchedRoute): HttpMiddlewareCallable $callable = new HttpMiddlewareCallable(fn (Request $request) => $this->createResponse($callControllerAction($request))); - $middlewareStack = $this->routeConfig - ->middleware - ->clone() - ->add(...$route->middleware); + $middlewareStack = $this->routeConfig->middleware; foreach ($middlewareStack->unwrap() as $middlewareClass) { $callable = new HttpMiddlewareCallable(function (Request $request) use ($middlewareClass, $callable) { @@ -194,38 +169,4 @@ private function processResponse(Response $response): Response return $response; } - - // TODO: could in theory be moved to a dynamic initializer - private function resolveRequest(PsrRequest|ObjectFactory $psrRequest, MatchedRoute $matchedRoute): Request - { - // Let's find out if our input request data matches what the route's action needs - $requestClass = GenericRequest::class; - - // We'll loop over all the handler's parameters - foreach ($matchedRoute->route->handler->getParameters() as $parameter) { - // If the parameter's type is an instance of Request… - if ($parameter->getType()->matches(Request::class)) { - // We'll use that specific request class - $requestClass = $parameter->getType()->getName(); - - break; - } - } - - // We map the original request we got into this method to the right request class - /** @var \Tempest\Http\GenericRequest $request */ - $request = map($psrRequest)->with(PsrRequestToGenericRequestMapper::class)->do(); - - if ($requestClass !== Request::class && $requestClass !== GenericRequest::class) { - $request = map($request)->with(RequestToObjectMapper::class)->to($requestClass); - } - - // Next, we register this newly created request object in the container - // This makes it so that RequestInitializer is bypassed entirely when the controller action needs the request class - // Making it so that we don't need to set any $_SERVER variables and stuff like that - $this->container->singleton(Request::class, fn () => $request); - $this->container->singleton($request::class, fn () => $request); - - return $request; - } } diff --git a/packages/router/src/HandleRouteExceptionMiddleware.php b/packages/router/src/HandleRouteExceptionMiddleware.php new file mode 100644 index 000000000..628f3933d --- /dev/null +++ b/packages/router/src/HandleRouteExceptionMiddleware.php @@ -0,0 +1,44 @@ +routeConfig->throwHttpExceptions === true) { + $response = $next($request); + + if ($response->status->isServerError() || $response->status->isClientError()) { + throw new HttpException( + status: $response->status, + cause: $response, + ); + } + + return $response; + } + + try { + return $next($request); + } catch (NotFoundException) { + return new NotFound(); + } catch (ValidationException $validationException) { + return new Invalid($validationException->subject, $validationException->failingRules); + } + } +} diff --git a/packages/router/src/HandleRouteSpecificMiddleware.php b/packages/router/src/HandleRouteSpecificMiddleware.php new file mode 100644 index 000000000..9e27c96da --- /dev/null +++ b/packages/router/src/HandleRouteSpecificMiddleware.php @@ -0,0 +1,36 @@ +matchedRoute->route->middleware); + + $callable = new HttpMiddlewareCallable(fn (Request $request) => $next($request)); + + foreach ($middlewareStack->unwrap() as $middlewareClass) { + $callable = new HttpMiddlewareCallable(function (Request $request) use ($middlewareClass, $callable) { + /** @var HttpMiddleware $middleware */ + $middleware = $this->container->get($middlewareClass->getName()); + + return $middleware($request, $callable); + }); + } + + return $callable($request); + } +} diff --git a/packages/router/src/MatchRouteMiddleware.php b/packages/router/src/MatchRouteMiddleware.php new file mode 100644 index 000000000..d596c6117 --- /dev/null +++ b/packages/router/src/MatchRouteMiddleware.php @@ -0,0 +1,69 @@ +routeMatcher->match($request); + + if ($matchedRoute === null) { + return new NotFound(); + } + + // We register the matched route in the container, some internal framework components will need it + $this->container->singleton(MatchedRoute::class, fn () => $matchedRoute); + + // Convert the request to a specific request implementation, if needed + $request = $this->resolveRequest($request, $matchedRoute); + + // We register this newly created request object in the container + // This makes it so that RequestInitializer is bypassed entirely when the controller action needs the request class + // Making it so that we don't need to set any $_SERVER variables and stuff like that + $this->container->singleton(Request::class, fn () => $request); + $this->container->singleton($request::class, fn () => $request); + + return $next($request); + } + + private function resolveRequest(Request $request, MatchedRoute $matchedRoute): Request + { + // Let's find out if our input request data matches what the route's action needs + $requestClass = GenericRequest::class; + + // We'll loop over all the handler's parameters + foreach ($matchedRoute->route->handler->getParameters() as $parameter) { + // If the parameter's type is an instance of Request… + if ($parameter->getType()->matches(Request::class)) { + // We'll use that specific request class + $requestClass = $parameter->getType()->getName(); + + break; + } + } + + if ($requestClass !== Request::class && $requestClass !== GenericRequest::class) { + $request = map($request)->with(RequestToObjectMapper::class)->to($requestClass); + } + + return $request; + } +} diff --git a/packages/router/src/RouteConfig.php b/packages/router/src/RouteConfig.php index 349ab8f74..d149ef266 100644 --- a/packages/router/src/RouteConfig.php +++ b/packages/router/src/RouteConfig.php @@ -23,7 +23,10 @@ public function __construct( /** @var Middleware<\Tempest\Router\HttpMiddleware> */ public Middleware $middleware = new Middleware( + HandleRouteExceptionMiddleware::class, + MatchRouteMiddleware::class, SetCookieMiddleware::class, + HandleRouteSpecificMiddleware::class, ), public bool $throwHttpExceptions = true, diff --git a/packages/router/src/Routing/Matching/GenericRouteMatcher.php b/packages/router/src/Routing/Matching/GenericRouteMatcher.php index 4b73138f2..8774e16e3 100644 --- a/packages/router/src/Routing/Matching/GenericRouteMatcher.php +++ b/packages/router/src/Routing/Matching/GenericRouteMatcher.php @@ -4,9 +4,8 @@ namespace Tempest\Router\Routing\Matching; -use Psr\Http\Message\ServerRequestInterface as PsrRequest; +use Tempest\Http\Request; use Tempest\Router\Exceptions\InvalidEnumParameterException; -use Tempest\Router\Exceptions\NotFoundException; use Tempest\Router\MatchedRoute; use Tempest\Router\RouteConfig; use Tempest\Router\Routing\Construction\DiscoveredRoute; @@ -17,7 +16,7 @@ public function __construct( private RouteConfig $routeConfig, ) {} - public function match(PsrRequest $request): ?MatchedRoute + public function match(Request $request): ?MatchedRoute { // Try to match routes without any parameters if (($staticRoute = $this->matchStaticRoute($request)) !== null) { @@ -28,9 +27,9 @@ public function match(PsrRequest $request): ?MatchedRoute return $this->matchDynamicRoute($request); } - private function matchStaticRoute(PsrRequest $request): ?MatchedRoute + private function matchStaticRoute(Request $request): ?MatchedRoute { - $staticRoute = $this->routeConfig->staticRoutes[$request->getMethod()][$request->getUri()->getPath()] ?? null; + $staticRoute = $this->routeConfig->staticRoutes[$request->method->value][$request->path] ?? null; if ($staticRoute === null) { return null; @@ -39,19 +38,19 @@ private function matchStaticRoute(PsrRequest $request): ?MatchedRoute return new MatchedRoute($staticRoute, []); } - private function matchDynamicRoute(PsrRequest $request): ?MatchedRoute + private function matchDynamicRoute(Request $request): ?MatchedRoute { // If there are no routes for the given request method, we immediately stop - $routesForMethod = $this->routeConfig->dynamicRoutes[$request->getMethod()] ?? null; + $routesForMethod = $this->routeConfig->dynamicRoutes[$request->method->value] ?? null; if ($routesForMethod === null) { return null; } // Get matching regex for route - $matchingRegexForMethod = $this->routeConfig->matchingRegexes[$request->getMethod()]; + $matchingRegexForMethod = $this->routeConfig->matchingRegexes[$request->method->value]; // Then we'll use this regex to see whether we have a match or not - $matchResult = $matchingRegexForMethod->match($request->getUri()->getPath()); + $matchResult = $matchingRegexForMethod->match($request->path); if ($matchResult === null) { return null; diff --git a/packages/router/src/Routing/Matching/RouteMatcher.php b/packages/router/src/Routing/Matching/RouteMatcher.php index 89cce0537..ddaf83548 100644 --- a/packages/router/src/Routing/Matching/RouteMatcher.php +++ b/packages/router/src/Routing/Matching/RouteMatcher.php @@ -4,10 +4,10 @@ namespace Tempest\Router\Routing\Matching; -use Psr\Http\Message\ServerRequestInterface as PsrRequest; +use Tempest\Http\Request; use Tempest\Router\MatchedRoute; interface RouteMatcher { - public function match(PsrRequest $request): ?MatchedRoute; + public function match(Request $request): ?MatchedRoute; } diff --git a/packages/router/tests/Routing/Matching/GenericRouteMatcherTest.php b/packages/router/tests/Routing/Matching/GenericRouteMatcherTest.php index 124c40f9a..e2c626f9d 100644 --- a/packages/router/tests/Routing/Matching/GenericRouteMatcherTest.php +++ b/packages/router/tests/Routing/Matching/GenericRouteMatcherTest.php @@ -6,6 +6,7 @@ use Laminas\Diactoros\ServerRequest; use PHPUnit\Framework\TestCase; +use Tempest\Http\GenericRequest; use Tempest\Http\Method; use Tempest\Router\RouteConfig; use Tempest\Router\Routing\Matching\GenericRouteMatcher; @@ -57,7 +58,7 @@ protected function setUp(): void public function test_match_on_static_route(): void { - $request = new ServerRequest(uri: '/static', method: 'GET'); + $request = new GenericRequest(method: Method::GET, uri: '/static'); $matchedRoute = $this->subject->match($request); @@ -68,7 +69,7 @@ public function test_match_on_static_route(): void public function test_match_returns_null_on_unknown_route(): void { - $request = new ServerRequest(uri: '/non-existing', method: 'GET'); + $request = new GenericRequest(Method::GET, '/non-existing'); $matchedRoute = $this->subject->match($request); @@ -77,7 +78,7 @@ public function test_match_returns_null_on_unknown_route(): void public function test_match_returns_null_on_unconfigured_method(): void { - $request = new ServerRequest(uri: '/static', method: 'POST'); + $request = new GenericRequest(method: Method::POST, uri: '/static'); $matchedRoute = $this->subject->match($request); @@ -86,7 +87,7 @@ public function test_match_returns_null_on_unconfigured_method(): void public function test_match_on_dynamic_route(): void { - $request = new ServerRequest(uri: '/dynamic/5', method: 'GET'); + $request = new GenericRequest(method: Method::GET, uri: '/dynamic/5'); $matchedRoute = $this->subject->match($request); @@ -97,7 +98,7 @@ public function test_match_on_dynamic_route(): void public function test_match_on_dynamic_route_with_many_parameters(): void { - $request = new ServerRequest(uri: '/dynamic/5/brendt/brent/6', method: 'GET'); + $request = new GenericRequest(method: Method::GET, uri: '/dynamic/5/brendt/brent/6'); $matchedRoute = $this->subject->match($request); diff --git a/tests/Benchmark/Http/Routing/Matching/GenericRouteMatcherBench.php b/tests/Benchmark/Http/Routing/Matching/GenericRouteMatcherBench.php index 34b8eb56e..c5a10adce 100644 --- a/tests/Benchmark/Http/Routing/Matching/GenericRouteMatcherBench.php +++ b/tests/Benchmark/Http/Routing/Matching/GenericRouteMatcherBench.php @@ -9,6 +9,8 @@ use PhpBench\Attributes\ParamProviders; use PhpBench\Attributes\Revs; use PhpBench\Attributes\Warmup; +use Tempest\Http\GenericRequest; +use Tempest\Http\Method; use Tempest\Router\RouteConfig; use Tempest\Router\Routing\Construction\RouteConfigurator; use Tempest\Router\Routing\Matching\GenericRouteMatcher; @@ -31,7 +33,7 @@ public function __construct() public function benchMatch(array $params): void { $this->matcher->match( - new ServerRequest(uri: $params['uri'], method: 'GET'), + new GenericRequest(Method::GET, $params['uri']), ); } diff --git a/tests/Integration/Route/Fixtures/CustomNotFoundMiddleware.php b/tests/Integration/Route/Fixtures/CustomNotFoundMiddleware.php new file mode 100644 index 000000000..f33e6750c --- /dev/null +++ b/tests/Integration/Route/Fixtures/CustomNotFoundMiddleware.php @@ -0,0 +1,22 @@ +addHeader('x-not-found', 'indeed'); + + return $response; + } +} diff --git a/tests/Integration/Route/NotFoundTest.php b/tests/Integration/Route/NotFoundTest.php new file mode 100644 index 000000000..45a1c1ec0 --- /dev/null +++ b/tests/Integration/Route/NotFoundTest.php @@ -0,0 +1,26 @@ +http->get('unknown-route')->assertNotFound(); + } + + public function test_custom_not_found_middleware(): void + { + $routeConfig = $this->container->get(RouteConfig::class); + $routeConfig->middleware->add(CustomNotFoundMiddleware::class); + + $this->http->get('unknown-route')->assertHasHeader('x-not-found'); + } +}