From be1cd57eefecf75655089a2ad7cdda7bc3849291 Mon Sep 17 00:00:00 2001 From: loks0n <22452787+loks0n@users.noreply.github.com> Date: Fri, 24 Apr 2026 14:37:07 +0100 Subject: [PATCH 1/7] Refactor: Coroutine-safe request dispatch Extract per-request state from the Http singleton into a Dispatcher and immutable RouteMatch. Under Swoole the Http, Router, and Route objects are shared across every concurrent coroutine; previously matched-route state lived on those shared objects and could be clobbered by a peer coroutine between match and execute. - Http no longer holds \$this->route; the matched route flows through the per-request DI container. - Route is frozen after registration: setMatchedPath/getMatchedPath are gone, and the wildcard route is no longer mutated with the request path on each hit. - Router::matchRoute() returns an immutable RouteMatch (final readonly) instead of writing back onto the shared Route. - RequestContext provides a per-request overlay for label overrides so consumers that tagged the current route (e.g. \$route->label('router', true)) have a safe migration target. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Http/Dispatcher.php | 213 +++++++++++++++++++ src/Http/Http.php | 382 ++++++++++++----------------------- src/Http/RequestContext.php | 47 +++++ src/Http/Route.php | 13 -- src/Http/RouteMatch.php | 21 ++ src/Http/Router.php | 29 ++- tests/HttpTest.php | 15 +- tests/RequestContextTest.php | 58 ++++++ 8 files changed, 495 insertions(+), 283 deletions(-) create mode 100644 src/Http/Dispatcher.php create mode 100644 src/Http/RequestContext.php create mode 100644 src/Http/RouteMatch.php create mode 100644 tests/RequestContextTest.php diff --git a/src/Http/Dispatcher.php b/src/Http/Dispatcher.php new file mode 100644 index 0000000..0da3ba2 --- /dev/null +++ b/src/Http/Dispatcher.php @@ -0,0 +1,213 @@ +match?->route; + } + + public function matchedRouteMatch(): ?RouteMatch + { + return $this->match; + } + + public function handle(): void + { + if ($this->http->isCompressionEnabled()) { + $this->response->setAcceptEncoding($this->request->getHeader('accept-encoding', '')); + $this->response->setCompressionMinSize($this->http->getCompressionMinSize()); + $this->response->setCompressionSupported($this->http->getCompressionSupported()); + } + + $context = new RequestContext(); + $this->http->setRequestResource('request', fn() => $this->request); + $this->http->setRequestResource('response', fn() => $this->response); + $this->http->setRequestResource('context', fn() => $context); + + try { + foreach (Http::getRequestHooks() as $hook) { + $arguments = $this->http->getArguments($hook, [], []); + \call_user_func_array($hook->getAction(), $arguments); + } + } catch (\Exception $e) { + $this->http->setRequestResource('error', fn() => $e); + + foreach (Http::getErrorHooks() as $error) { + if (\in_array('*', $error->getGroups())) { + try { + $arguments = $this->http->getArguments($error, [], []); + \call_user_func_array($error->getAction(), $arguments); + } catch (\Throwable $e) { + throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e); + } + } + } + } + + if ($this->http->isFileLoaded($this->request->getURI())) { + $time = (60 * 60 * 24 * 365 * 2); + + $this->response + ->setContentType($this->http->getFileMimeType($this->request->getURI())) + ->addHeader('Cache-Control', 'public, max-age=' . $time) + ->addHeader('Expires', \date('D, d M Y H:i:s', \time() + $time) . ' GMT') + ->send($this->http->getFileContents($this->request->getURI())); + + return; + } + + $method = $this->request->getMethod(); + $url = \parse_url($this->request->getURI(), PHP_URL_PATH); + $url = \is_string($url) ? ($url === '' ? '/' : $url) : '/'; + $matchMethod = (Http::REQUEST_METHOD_HEAD === $method) ? Http::REQUEST_METHOD_GET : $method; + + $this->match = Router::matchRoute($matchMethod, $url); + + if ($this->match === null && Http::getWildcardRoute() !== null) { + $wildcard = Http::getWildcardRoute(); + $this->match = new RouteMatch($wildcard, $url, Router::WILDCARD_TOKEN, Router::WILDCARD_TOKEN); + } + + $context->setMatch($this->match); + $this->http->setRequestResource('route', fn() => $this->match?->route); + $this->http->setRequestResource('routeMatch', fn() => $this->match); + + $groups = $this->match?->route->getGroups() ?? []; + + if (Http::REQUEST_METHOD_HEAD === $method) { + $method = Http::REQUEST_METHOD_GET; + $this->response->disablePayload(); + } + + if (Http::REQUEST_METHOD_OPTIONS === $method) { + try { + foreach ($groups as $group) { + foreach (Http::getOptionsHooks() as $option) { + if (\in_array($group, $option->getGroups())) { + \call_user_func_array($option->getAction(), $this->http->getArguments($option, [], $this->request->getParams())); + } + } + } + + foreach (Http::getOptionsHooks() as $option) { + if (\in_array('*', $option->getGroups())) { + \call_user_func_array($option->getAction(), $this->http->getArguments($option, [], $this->request->getParams())); + } + } + } catch (\Throwable $e) { + foreach (Http::getErrorHooks() as $error) { + if (\in_array('*', $error->getGroups())) { + $this->http->setRequestResource('error', fn() => $e); + \call_user_func_array($error->getAction(), $this->http->getArguments($error, [], $this->request->getParams())); + } + } + } + + return; + } + + if ($this->match !== null) { + $this->execute($this->match); + + return; + } + + foreach (Http::getErrorHooks() as $error) { + if (\in_array('*', $error->getGroups())) { + $this->http->setRequestResource('error', fn() => new Exception('Not Found', 404)); + \call_user_func_array($error->getAction(), $this->http->getArguments($error, [], $this->request->getParams())); + } + } + } + + public function execute(RouteMatch $match): void + { + $route = $match->route; + $groups = $route->getGroups(); + $pathValues = $route->getPathValues($this->request, $match->preparedPath); + $requestParams = $this->request->getParams(); + + try { + if ($route->getHook()) { + foreach (Http::getInitHooks() as $hook) { + if (\in_array('*', $hook->getGroups())) { + \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams)); + } + } + } + + foreach ($groups as $group) { + foreach (Http::getInitHooks() as $hook) { + if (\in_array($group, $hook->getGroups())) { + \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams)); + } + } + } + + if (!$this->response->isSent()) { + \call_user_func_array($route->getAction(), $this->http->getArguments($route, $pathValues, $requestParams)); + } + + foreach ($groups as $group) { + foreach (Http::getShutdownHooks() as $hook) { + if (\in_array($group, $hook->getGroups())) { + \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams)); + } + } + } + + if ($route->getHook()) { + foreach (Http::getShutdownHooks() as $hook) { + if (\in_array('*', $hook->getGroups())) { + \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams)); + } + } + } + } catch (\Throwable $e) { + $this->http->setRequestResource('error', fn() => $e); + + foreach ($groups as $group) { + foreach (Http::getErrorHooks() as $error) { + if (\in_array($group, $error->getGroups())) { + try { + \call_user_func_array($error->getAction(), $this->http->getArguments($error, $pathValues, $requestParams)); + } catch (\Throwable $e) { + throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e); + } + } + } + } + + foreach (Http::getErrorHooks() as $error) { + if (\in_array('*', $error->getGroups())) { + try { + \call_user_func_array($error->getAction(), $this->http->getArguments($error, $pathValues, $requestParams)); + } catch (\Throwable $e) { + throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e); + } + } + } + } + } +} diff --git a/src/Http/Http.php b/src/Http/Http.php index da93d66..e5a4604 100755 --- a/src/Http/Http.php +++ b/src/Http/Http.php @@ -48,8 +48,6 @@ class Http protected Container $container; - protected ?Container $requestContainer = null; - /** * Current running mode */ @@ -105,13 +103,6 @@ class Http */ protected static array $requestHooks = []; - /** - * Route - * - * Memory cached result for chosen route - */ - protected ?Route $route = null; - /** * Wildcard route * If set, this get's executed if no other route is matched @@ -178,6 +169,11 @@ public function setCompression(bool $compression): void $this->compression = $compression; } + public function isCompressionEnabled(): bool + { + return $this->compression; + } + /** * Set minimum compression size */ @@ -186,6 +182,11 @@ public function setCompressionMinSize(int $compressionMinSize): void $this->compressionMinSize = $compressionMinSize; } + public function getCompressionMinSize(): int + { + return $this->compressionMinSize; + } + /** * Set supported compression algorithms */ @@ -194,6 +195,11 @@ public function setCompressionSupported(mixed $compressionSupported): void $this->compressionSupported = $compressionSupported; } + public function getCompressionSupported(): mixed + { + return $this->compressionSupported; + } + /** * GET * @@ -256,6 +262,18 @@ public static function wildcard(): Route return self::$wildcardRoute; } + /** + * Returns the registered wildcard route, if any. + * + * The returned Route is a shared definition and MUST NOT be mutated by + * request-handling code. Per-request state belongs on {@see RouteMatch} + * or {@see RequestContext}. + */ + public static function getWildcardRoute(): ?Route + { + return self::$wildcardRoute; + } + /** * Init * @@ -316,6 +334,42 @@ public static function error(): Hook return $hook; } + /** @return Hook[] */ + public static function getInitHooks(): array + { + return self::$init; + } + + /** @return Hook[] */ + public static function getShutdownHooks(): array + { + return self::$shutdown; + } + + /** @return Hook[] */ + public static function getOptionsHooks(): array + { + return self::$options; + } + + /** @return Hook[] */ + public static function getErrorHooks(): array + { + return self::$errors; + } + + /** @return Hook[] */ + public static function getStartHooks(): array + { + return self::$startHooks; + } + + /** @return Hook[] */ + public static function getRequestHooks(): array + { + return self::$requestHooks; + } + /** * Get env var * @@ -417,9 +471,14 @@ public function setResource(string $name, callable $callback, array $injections /** * Set a request-scoped resource on the current request's container. * + * Relies on {@see Adapter::getContainer()} returning a container scoped + * to the current request/coroutine. Swoole adapters back this with + * `Coroutine::getContext()`; the FPM adapter has a single request per + * process so the shared container is safe there. + * * @param list $injections */ - protected function setRequestResource(string $name, callable $callback, array $injections = []): void + public function setRequestResource(string $name, callable $callback, array $injections = []): void { $this->server->getContainer()->set($name, $callback, $injections); } @@ -461,19 +520,32 @@ public static function getRoutes(): array } /** - * Get the current route + * @deprecated Read the `routeMatch` or `route` request resource instead + * (e.g. `$http->getResource('routeMatch')?->route`). The per-request + * route lives in the per-request DI container; returning it from the + * shared Http singleton is not safe under concurrent request handling. */ public function getRoute(): ?Route { - return $this->route ?? null; + try { + $match = $this->server->getContainer()->get('routeMatch'); + } catch (ContainerExceptionInterface|NotFoundExceptionInterface) { + return null; + } + + return $match instanceof RouteMatch ? $match->route : null; } /** - * Set the current route + * @deprecated Construct a {@see RouteMatch} and register it via + * `setRequestResource('routeMatch', ...)` instead. Provided as a shim + * for tests and legacy callers only. */ public function setRoute(Route $route): self { - $this->route = $route; + $match = new RouteMatch($route, '', '', ''); + $this->setRequestResource('route', fn() => $route); + $this->setRequestResource('routeMatch', fn() => $match); return $this; } @@ -506,7 +578,7 @@ public function loadFiles(string $directory, ?string $root = null): void /** * Is file loaded. */ - protected function isFileLoaded(string $uri): bool + public function isFileLoaded(string $uri): bool { return $this->files->isFileLoaded($uri); } @@ -514,10 +586,9 @@ protected function isFileLoaded(string $uri): bool /** * Get file contents. * - * @return string * @throws \Exception */ - protected function getFileContents(string $uri): mixed + public function getFileContents(string $uri): mixed { return $this->files->getFileContents($uri); } @@ -525,10 +596,9 @@ protected function getFileContents(string $uri): mixed /** * Get file MIME type. * - * @return string * @throws \Exception */ - protected function getFileMimeType(string $uri): mixed + public function getFileMimeType(string $uri): mixed { return $this->files->getFileMimeType($uri); } @@ -549,7 +619,6 @@ public static function onRequest(): Hook public function start(): void { - $this->server->onRequest( fn(Request $request, Response $response) => $this->run($request, $response), ); @@ -557,7 +626,6 @@ public function start(): void $this->server->onStart(function ($server) { $this->setResource('server', fn() => $server); try { - foreach (self::$startHooks as $hook) { $arguments = $this->getArguments($hook, [], []); \call_user_func_array($hook->getAction(), $arguments); @@ -565,7 +633,7 @@ public function start(): void } catch (\Exception $e) { $this->setResource('error', fn() => $e); - foreach (self::$errors as $error) { // Global error hooks + foreach (self::$errors as $error) { if (in_array('*', $error->getGroups())) { try { $arguments = $this->getArguments($error, [], []); @@ -582,107 +650,52 @@ public function start(): void } /** - * Match - * - * Find matching route given current user request - * - * @param bool $fresh If true, will not match any cached route + * @deprecated Use {@see Router::matchRoute()} which returns a per-request + * {@see RouteMatch}. This shim discards the `$fresh` argument: the + * previous implementation cached the match on the Http singleton, + * which is not safe under concurrent request handling. */ public function match(Request $request, bool $fresh = true): ?Route { - if (null !== $this->route && !$fresh) { - return $this->route; - } - $url = \parse_url($request->getURI(), PHP_URL_PATH); $url = \is_string($url) ? ($url === '' ? '/' : $url) : '/'; $method = $request->getMethod(); $method = (self::REQUEST_METHOD_HEAD === $method) ? self::REQUEST_METHOD_GET : $method; - $this->route = Router::match($method, $url); + $match = Router::matchRoute($method, $url); + if ($match === null) { + return null; + } - return $this->route; + $this->setRequestResource('route', fn() => $match->route); + $this->setRequestResource('routeMatch', fn() => $match); + + return $match->route; } /** - * Execute a given route with middlewares and error handling + * Execute a given route with middlewares and error handling. + * + * @deprecated Internal dispatch moved to {@see Dispatcher}. This shim + * remains for tests and callers that invoke `execute()` directly with a + * Route built outside the router; it synthesises a {@see RouteMatch} + * from the route's registered path. */ public function execute(Route $route, Request $request, Response $response): static { - $arguments = []; - $groups = $route->getGroups(); - - $preparedPath = Router::preparePath($route->getMatchedPath()); - $pathValues = $route->getPathValues($request, $preparedPath[0]); + [$preparedPath] = Router::preparePath($route->getPath()); + $urlPath = \parse_url($request->getURI(), PHP_URL_PATH); + $urlPath = \is_string($urlPath) ? ($urlPath === '' ? '/' : $urlPath) : '/'; + $match = new RouteMatch($route, $urlPath, $preparedPath, $preparedPath); - try { - if ($route->getHook()) { - foreach (self::$init as $hook) { // Global init hooks - if (in_array('*', $hook->getGroups())) { - $arguments = $this->getArguments($hook, $pathValues, $request->getParams()); - \call_user_func_array($hook->getAction(), $arguments); - } - } - } - - foreach ($groups as $group) { - foreach (self::$init as $hook) { // Group init hooks - if (\in_array($group, $hook->getGroups())) { - $arguments = $this->getArguments($hook, $pathValues, $request->getParams()); - \call_user_func_array($hook->getAction(), $arguments); - } - } - } - - if (!$response->isSent()) { - $arguments = $this->getArguments($route, $pathValues, $request->getParams()); - \call_user_func_array($route->getAction(), $arguments); - } - - foreach ($groups as $group) { - foreach (self::$shutdown as $hook) { // Group shutdown hooks - if (\in_array($group, $hook->getGroups())) { - $arguments = $this->getArguments($hook, $pathValues, $request->getParams()); - \call_user_func_array($hook->getAction(), $arguments); - } - } - } - - if ($route->getHook()) { - foreach (self::$shutdown as $hook) { // Group shutdown hooks - if (\in_array('*', $hook->getGroups())) { - $arguments = $this->getArguments($hook, $pathValues, $request->getParams()); - \call_user_func_array($hook->getAction(), $arguments); - } - } - } - } catch (\Throwable $e) { - $this->setRequestResource('error', fn() => $e, []); - - foreach ($groups as $group) { - foreach (self::$errors as $error) { // Group error hooks - if (\in_array($group, $error->getGroups())) { - try { - $arguments = $this->getArguments($error, $pathValues, $request->getParams()); - \call_user_func_array($error->getAction(), $arguments); - } catch (\Throwable $e) { - throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e); - } - } - } - } + $context = new RequestContext($match); + $this->setRequestResource('request', fn() => $request); + $this->setRequestResource('response', fn() => $response); + $this->setRequestResource('context', fn() => $context); + $this->setRequestResource('route', fn() => $route); + $this->setRequestResource('routeMatch', fn() => $match); - foreach (self::$errors as $error) { // Global error hooks - if (\in_array('*', $error->getGroups())) { - try { - $arguments = $this->getArguments($error, $pathValues, $request->getParams()); - \call_user_func_array($error->getAction(), $arguments); - } catch (\Throwable $e) { - throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e); - } - } - } - } + (new Dispatcher($this, $request, $response))->execute($match); return $this; } @@ -695,10 +708,10 @@ public function execute(Route $route, Request $request, Response $response): sta * @return array * @throws Exception */ - protected function getArguments(Hook $hook, array $values, array $requestParams): array + public function getArguments(Hook $hook, array $values, array $requestParams): array { $arguments = []; - foreach ($hook->getParams() as $key => $param) { // Get value from route or request object + foreach ($hook->getParams() as $key => $param) { $existsInRequest = \array_key_exists($key, $requestParams); $existsInValues = \array_key_exists($key, $values); $paramExists = $existsInRequest || $existsInValues; @@ -731,7 +744,7 @@ protected function getArguments(Hook $hook, array $values, array $requestParams) } /** - * Run: wrapper function to record telemetry. All domain logic should happen in `runInternal`. + * Run: wrapper function to record telemetry. Dispatch lives in {@see Dispatcher}. */ public function run(Request $request, Response $response): static { @@ -741,13 +754,15 @@ public function run(Request $request, Response $response): static ]); $start = microtime(true); - $result = $this->runInternal($request, $response); + + $dispatcher = new Dispatcher($this, $request, $response); + $dispatcher->handle(); $requestDuration = microtime(true) - $start; $attributes = [ 'url.scheme' => $request->getProtocol(), 'http.request.method' => $request->getMethod(), - 'http.route' => $this->route?->getPath(), + 'http.route' => $dispatcher->matchedRoute()?->getPath(), 'http.response.status_code' => $response->getStatusCode(), ]; $this->requestDuration->record($requestDuration, $attributes); @@ -758,150 +773,9 @@ public function run(Request $request, Response $response): static 'url.scheme' => $request->getProtocol(), ]); - return $result; - } - - /** - * Run internal - * - * This is the place to initialize any pre routing logic. - * This is where you might want to parse the application current URL by any desired logic - * - * @param Response $response; - */ - private function runInternal(Request $request, Response $response): static - { - if ($this->compression) { - $response->setAcceptEncoding($request->getHeader('accept-encoding', '')); - $response->setCompressionMinSize($this->compressionMinSize); - $response->setCompressionSupported($this->compressionSupported); - } - - $this->setRequestResource('request', fn() => $request); - $this->setRequestResource('response', fn() => $response); - - try { - foreach (self::$requestHooks as $hook) { - $arguments = $this->getArguments($hook, [], []); - \call_user_func_array($hook->getAction(), $arguments); - } - } catch (\Exception $e) { - $this->setRequestResource('error', fn() => $e, []); - - foreach (self::$errors as $error) { // Global error hooks - if (\in_array('*', $error->getGroups())) { - try { - $arguments = $this->getArguments($error, [], []); - \call_user_func_array($error->getAction(), $arguments); - } catch (\Throwable $e) { - throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e); - } - } - } - } - - if ($this->isFileLoaded($request->getURI())) { - $time = (60 * 60 * 24 * 365 * 2); // 45 days cache - - $response - ->setContentType($this->getFileMimeType($request->getURI())) - ->addHeader('Cache-Control', 'public, max-age=' . $time) - ->addHeader('Expires', \date('D, d M Y H:i:s', \time() + $time) . ' GMT') // 45 days cache - ->send($this->getFileContents($request->getURI())); - - return $this; - } - - $method = $request->getMethod(); - $route = $this->match($request); - $groups = ($route instanceof Route) ? $route->getGroups() : []; - - $this->setRequestResource('route', fn() => $route, []); - - if (self::REQUEST_METHOD_HEAD === $method) { - $method = self::REQUEST_METHOD_GET; - $response->disablePayload(); - } - - if (self::REQUEST_METHOD_OPTIONS === $method) { - try { - foreach ($groups as $group) { - foreach (self::$options as $option) { // Group options hooks - /** @var Hook $option */ - if (\in_array($group, $option->getGroups())) { - \call_user_func_array($option->getAction(), $this->getArguments($option, [], $request->getParams())); - } - } - } - - foreach (self::$options as $option) { // Global options hooks - /** @var Hook $option */ - if (\in_array('*', $option->getGroups())) { - \call_user_func_array($option->getAction(), $this->getArguments($option, [], $request->getParams())); - } - } - } catch (\Throwable $e) { - foreach (self::$errors as $error) { // Global error hooks - /** @var Hook $error */ - if (\in_array('*', $error->getGroups())) { - $this->setRequestResource('error', fn() => $e, []); - \call_user_func_array($error->getAction(), $this->getArguments($error, [], $request->getParams())); - } - } - } - - return $this; - } - - if (null === $route && null !== self::$wildcardRoute) { - $route = self::$wildcardRoute; - $this->route = $route; - $path = \parse_url($request->getURI(), PHP_URL_PATH); - $path = \is_string($path) ? ($path === '' ? '/' : $path) : '/'; - $route->path($path); - - $this->setRequestResource('route', fn() => $route, []); - } - if (null !== $route) { - return $this->execute($route, $request, $response); - } - - if (self::REQUEST_METHOD_OPTIONS === $method) { - try { - foreach ($groups as $group) { - foreach (self::$options as $option) { // Group options hooks - if (\in_array($group, $option->getGroups())) { - \call_user_func_array($option->getAction(), $this->getArguments($option, [], $request->getParams())); - } - } - } - - foreach (self::$options as $option) { // Global options hooks - if (\in_array('*', $option->getGroups())) { - \call_user_func_array($option->getAction(), $this->getArguments($option, [], $request->getParams())); - } - } - } catch (\Throwable $e) { - foreach (self::$errors as $error) { // Global error hooks - if (\in_array('*', $error->getGroups())) { - $this->setRequestResource('error', fn() => $e, []); - \call_user_func_array($error->getAction(), $this->getArguments($error, [], $request->getParams())); - } - } - } - } else { - foreach (self::$errors as $error) { // Global error hooks - if (\in_array('*', $error->getGroups())) { - $this->setRequestResource('error', fn() => new Exception('Not Found', 404), []); - \call_user_func_array($error->getAction(), $this->getArguments($error, [], $request->getParams())); - } - } - } - return $this; } - /** * Validate Param * @@ -917,13 +791,13 @@ protected function validate(string $key, array $param, mixed $value): void return; } - $validator = $param['validator']; // checking whether the class exists + $validator = $param['validator']; if (\is_callable($validator)) { $validator = \call_user_func_array($validator, \array_values($this->getResources($param['injections']))); } - if (!$validator instanceof Validator) { // is the validator object an instance of the Validator class + if (!$validator instanceof Validator) { throw new Exception('Validator object is not an instance of the Validator class', 500); } diff --git a/src/Http/RequestContext.php b/src/Http/RequestContext.php new file mode 100644 index 0000000..b2ddf94 --- /dev/null +++ b/src/Http/RequestContext.php @@ -0,0 +1,47 @@ + */ + private array $labels = []; + + public function __construct(private ?RouteMatch $match = null) {} + + public function setMatch(?RouteMatch $match): void + { + $this->match = $match; + } + + public function getMatch(): ?RouteMatch + { + return $this->match; + } + + public function label(string $key, mixed $value): self + { + $this->labels[$key] = $value; + + return $this; + } + + public function getLabel(string $key, mixed $default = null): mixed + { + if (\array_key_exists($key, $this->labels)) { + return $this->labels[$key]; + } + + return $this->match?->route->getLabel($key, $default) ?? $default; + } +} diff --git a/src/Http/Route.php b/src/Http/Route.php index 46bfb29..0c72a19 100755 --- a/src/Http/Route.php +++ b/src/Http/Route.php @@ -38,8 +38,6 @@ class Route extends Hook */ protected int $order; - protected string $matchedPath = ''; - public function __construct(string $method, string $path) { parent::__construct(); @@ -48,17 +46,6 @@ public function __construct(string $method, string $path) $this->order = ++self::$counter; } - public function setMatchedPath(string $path): self - { - $this->matchedPath = $path; - return $this; - } - - public function getMatchedPath(): string - { - return $this->matchedPath; - } - /** * Get Route Order ID */ diff --git a/src/Http/RouteMatch.php b/src/Http/RouteMatch.php new file mode 100644 index 0000000..2aa97c5 --- /dev/null +++ b/src/Http/RouteMatch.php @@ -0,0 +1,21 @@ +setMatchedPath($match); - return $route; + return new RouteMatch(self::$routes[$method][$match], $path, $match, $match); } } @@ -140,9 +142,7 @@ public static function match(string $method, string $path): ?Route */ $match = self::WILDCARD_TOKEN; if (array_key_exists($match, self::$routes[$method])) { - $route = self::$routes[$method][$match]; - $route->setMatchedPath($match); - return $route; + return new RouteMatch(self::$routes[$method][$match], $path, $match, $match); } /** @@ -152,15 +152,24 @@ public static function match(string $method, string $path): ?Route $current = ($current ?? '') . "{$part}/"; $match = $current . self::WILDCARD_TOKEN; if (array_key_exists($match, self::$routes[$method])) { - $route = self::$routes[$method][$match]; - $route->setMatchedPath($match); - return $route; + return new RouteMatch(self::$routes[$method][$match], $path, $match, $match); } } return null; } + /** + * @deprecated Use {@see Router::matchRoute()} which returns a per-request + * {@see RouteMatch}. The old signature returned the shared Route + * definition; under concurrent request handling (Swoole coroutines) + * mutating that return value is racy. + */ + public static function match(string $method, string $path): ?Route + { + return self::matchRoute($method, $path)?->route; + } + /** * Get all combinations of the given set. * diff --git a/tests/HttpTest.php b/tests/HttpTest.php index dbb703c..54a15ba 100755 --- a/tests/HttpTest.php +++ b/tests/HttpTest.php @@ -455,28 +455,31 @@ public function testNoMismatchRoute(): void } } - public function testCanMatchFreshRoute(): void + public function testMatchAlwaysReturnsFreshRoute(): void { + // The previous implementation cached the last matched route on the + // Http singleton and returned it on subsequent match() calls with + // `fresh: false`. That cache was unsafe under concurrent request + // handling and has been removed; match() now always returns the + // fresh result, regardless of the `$fresh` argument. $route1 = Http::get('/path1'); $route2 = Http::get('/path2'); try { - // Match first request $_SERVER['REQUEST_METHOD'] = 'HEAD'; $_SERVER['REQUEST_URI'] = '/path1'; $matched = $this->http->match(new Request()); $this->assertSame($route1, $matched); $this->assertSame($route1, $this->http->getRoute()); - // Second request match returns cached route $_SERVER['REQUEST_METHOD'] = 'HEAD'; $_SERVER['REQUEST_URI'] = '/path2'; $request2 = new Request(); + $matched = $this->http->match($request2, fresh: false); - $this->assertSame($route1, $matched); - $this->assertSame($route1, $this->http->getRoute()); + $this->assertSame($route2, $matched); + $this->assertSame($route2, $this->http->getRoute()); - // Fresh match returns new route $matched = $this->http->match($request2, fresh: true); $this->assertSame($route2, $matched); $this->assertSame($route2, $this->http->getRoute()); diff --git a/tests/RequestContextTest.php b/tests/RequestContextTest.php new file mode 100644 index 0000000..96a2e51 --- /dev/null +++ b/tests/RequestContextTest.php @@ -0,0 +1,58 @@ +label('sdk.platform', 'server'); + + $match = new RouteMatch($route, '/things', '/things', '/things'); + $context = new RequestContext($match); + + $this->assertSame('server', $context->getLabel('sdk.platform')); + $this->assertSame('fallback', $context->getLabel('missing', 'fallback')); + } + + public function testOverridesRouteLabelWithoutMutatingRoute(): void + { + $route = new Route('GET', '/things'); + $route->label('router', false); + + $match = new RouteMatch($route, '/things', '/things', '/things'); + $context = new RequestContext($match); + + $context->label('router', true); + + $this->assertTrue($context->getLabel('router')); + // The shared Route definition is untouched — concurrent requests + // that don't override see the original value. + $this->assertFalse($route->getLabel('router', null)); + } + + public function testNullRouteReturnsDefault(): void + { + $context = new RequestContext(); + $this->assertSame('default', $context->getLabel('anything', 'default')); + } + + public function testRouteMatchIsImmutableAtTypeLevel(): void + { + $route = new Route('GET', '/x'); + $match = new RouteMatch($route, '/x', '/x', '/x'); + + $reflection = new \ReflectionClass($match); + $this->assertTrue($reflection->isReadOnly(), 'RouteMatch must be a readonly class'); + } +} From 42c7d023a7e9d80693ceb54afc5038c7d9cee144 Mon Sep 17 00:00:00 2001 From: loks0n <22452787+loks0n@users.noreply.github.com> Date: Fri, 24 Apr 2026 14:41:30 +0100 Subject: [PATCH 2/7] Test: Cover Dispatcher, RouteMatch, and Router::matchRoute Adds three test files (28 cases, 67 assertions) focused on the no-mutation guarantees that motivate the refactor. - DispatcherTest: sequential FPM dispatch verifies request resources (route/routeMatch/context) are populated, RequestContext is fresh per request, the wildcard Route is never mutated, parameterized routes don't bleed state between requests, and hook/error/OPTIONS/HEAD lifecycle flows. - RouteMatchTest: field access, readonly-class reflection check, and a runtime assertion that fields cannot be reassigned. - RouterMatchRouteTest: matchRoute return shape, exact/param/wildcard branches, explicit no-mutation snapshot across repeated matches, distinct RouteMatch instance per call, and the deprecated match() shim still returns a Route. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/DispatcherTest.php | 273 +++++++++++++++++++++++++++++++++ tests/RouteMatchTest.php | 54 +++++++ tests/RouterMatchRouteTest.php | 138 +++++++++++++++++ 3 files changed, 465 insertions(+) create mode 100644 tests/DispatcherTest.php create mode 100644 tests/RouteMatchTest.php create mode 100644 tests/RouterMatchRouteTest.php diff --git a/tests/DispatcherTest.php b/tests/DispatcherTest.php new file mode 100644 index 0000000..88247f6 --- /dev/null +++ b/tests/DispatcherTest.php @@ -0,0 +1,273 @@ +http = new Http(new Server($container), 'UTC'); + $this->savedMethod = $_SERVER['REQUEST_METHOD'] ?? null; + $this->savedUri = $_SERVER['REQUEST_URI'] ?? null; + } + + protected function tearDown(): void + { + $_SERVER['REQUEST_METHOD'] = $this->savedMethod; + $_SERVER['REQUEST_URI'] = $this->savedUri; + } + + /** + * @param callable(): void $block + */ + private function capture(callable $block): string + { + \ob_start(); + $block(); + $output = \ob_get_contents() ?: ''; + \ob_end_clean(); + + return $output; + } + + private function runRequest(string $method, string $uri): string + { + $_SERVER['REQUEST_METHOD'] = $method; + $_SERVER['REQUEST_URI'] = $uri; + + return $this->capture(function () { + $this->http->run(new Request(), new Response()); + }); + } + + public function testRegistersRouteAndRouteMatchRequestResources(): void + { + $route = Http::get('/resources-check') + ->inject('route') + ->inject('routeMatch') + ->inject('response') + ->action(function (?Route $route, ?RouteMatch $match, Response $response) { + $payload = \json_encode([ + 'routeClass' => $route === null ? null : $route::class, + 'matchClass' => $match === null ? null : $match::class, + 'matchUrl' => $match?->urlPath, + 'matchKey' => $match?->routeKey, + ]); + $response->send($payload === false ? '' : $payload); + }); + + $output = $this->runRequest('GET', '/resources-check'); + + $decoded = \json_decode($output, true); + $this->assertIsArray($decoded); + $this->assertSame(Route::class, $decoded['routeClass']); + $this->assertSame(RouteMatch::class, $decoded['matchClass']); + $this->assertSame('/resources-check', $decoded['matchUrl']); + $this->assertSame('resources-check', $decoded['matchKey']); + $this->assertInstanceOf(Route::class, $route); + } + + public function testRegistersFreshRequestContextEveryDispatch(): void + { + Http::get('/ctx') + ->inject('context') + ->inject('response') + ->action(function (RequestContext $context, Response $response) { + $response->send((string) ($context->getLabel('seen', 0) + 1)); + $context->label('seen', ($context->getLabel('seen', 0)) + 1); + }); + + // If `context` leaked across requests, the second request would + // observe the override set during the first. + $first = $this->runRequest('GET', '/ctx'); + $second = $this->runRequest('GET', '/ctx'); + + $this->assertSame('1', $first); + $this->assertSame('1', $second); + } + + public function testWildcardRouteIsNeverMutated(): void + { + $wildcard = Http::wildcard() + ->inject('routeMatch') + ->inject('response') + ->action(function (RouteMatch $match, Response $response) { + $response->send($match->urlPath); + }); + + $pathBefore = $wildcard->getPath(); + $groupsBefore = $wildcard->getGroups(); + + $first = $this->runRequest('GET', '/alpha/beta'); + $second = $this->runRequest('GET', '/something/else/entirely'); + + // The dispatcher must not write the request path back onto the + // shared wildcard Route definition — that was the concurrency bug. + $this->assertSame($pathBefore, $wildcard->getPath(), 'Wildcard Route::getPath() must not be mutated by dispatch.'); + $this->assertSame($groupsBefore, $wildcard->getGroups()); + + // But the match exposed to the handler must still reflect the + // current request URL. + $this->assertSame('/alpha/beta', $first); + $this->assertSame('/something/else/entirely', $second); + } + + public function testSequentialRequestsOnSameParameterizedRouteDoNotBleed(): void + { + Http::get('/users/:id') + ->inject('routeMatch') + ->inject('request') + ->inject('response') + ->action(function (RouteMatch $match, Request $request, Response $response) { + $response->send($match->urlPath . '|' . $match->route->getPathValues($request, $match->preparedPath)['id']); + }); + + $a = $this->runRequest('GET', '/users/42'); + $b = $this->runRequest('GET', '/users/99'); + + $this->assertSame('/users/42|42', $a); + $this->assertSame('/users/99|99', $b); + } + + public function testInitAndShutdownHooksFire(): void + { + Http::init()->action(function () { + echo 'init|'; + }); + Http::shutdown()->action(function () { + echo '|shutdown'; + }); + + Http::get('/lifecycle') + ->inject('response') + ->action(function (Response $response) { + echo 'handler'; + $response->send(''); + }); + + $output = $this->runRequest('GET', '/lifecycle'); + + $this->assertSame('init|handler|shutdown', $output); + } + + public function testErrorHookFiresForNotFound(): void + { + Http::error() + ->inject('error') + ->inject('response') + ->action(function (\Throwable $error, Response $response) { + $response->send('err:' . $error->getCode() . ':' . $error->getMessage()); + }); + + $output = $this->runRequest('GET', '/definitely-not-registered'); + + $this->assertSame('err:404:Not Found', $output); + } + + public function testErrorHookReceivesExceptionFromHandler(): void + { + Http::get('/boom') + ->action(function () { + throw new Exception('kaboom', 418); + }); + + Http::error() + ->inject('error') + ->inject('response') + ->action(function (\Throwable $error, Response $response) { + $response->send($error->getCode() . ':' . $error->getMessage()); + }); + + $output = $this->runRequest('GET', '/boom'); + + $this->assertSame('418:kaboom', $output); + } + + public function testHeadRequestResolvesToGetRouteWithPayloadDisabled(): void + { + Http::get('/head-check') + ->inject('response') + ->action(function (Response $response) { + $response->send('body-should-not-appear'); + }); + + $output = $this->runRequest('HEAD', '/head-check'); + + $this->assertStringNotContainsString('body-should-not-appear', $output); + } + + public function testOptionsHookFiresForOptionsMethod(): void + { + Http::get('/opts')->action(function () { + // never called + echo 'GET-HANDLER'; + }); + + Http::options() + ->inject('response') + ->action(function (Response $response) { + $response->send('OPTIONS-HANDLER'); + }); + + $output = $this->runRequest('OPTIONS', '/opts'); + + $this->assertSame('OPTIONS-HANDLER', $output); + $this->assertStringNotContainsString('GET-HANDLER', $output); + } + + public function testContextLabelOverrideDoesNotLeakToRoute(): void + { + $route = Http::get('/lbl') + ->label('router', false) + ->inject('context') + ->inject('response') + ->action(function (RequestContext $context, Response $response) { + $context->label('router', true); + $response->send($context->getLabel('router') ? 'on' : 'off'); + }); + + $this->assertSame('on', $this->runRequest('GET', '/lbl')); + + // The shared Route's label is untouched — a concurrent request + // that doesn't set the override still sees the registered default. + $this->assertFalse($route->getLabel('router', null)); + } + + public function testWildcardRouteMatchCarriesWildcardToken(): void + { + Http::wildcard() + ->inject('routeMatch') + ->inject('response') + ->action(function (RouteMatch $match, Response $response) { + $response->send($match->routeKey); + }); + + $output = $this->runRequest('GET', '/whatever/this/is'); + + $this->assertSame(Router::WILDCARD_TOKEN, $output); + } +} diff --git a/tests/RouteMatchTest.php b/tests/RouteMatchTest.php new file mode 100644 index 0000000..2f93714 --- /dev/null +++ b/tests/RouteMatchTest.php @@ -0,0 +1,54 @@ +assertSame($route, $match->route); + $this->assertSame('/users/42', $match->urlPath); + $this->assertSame('/users/:::', $match->routeKey); + $this->assertSame('/users/:::', $match->preparedPath); + } + + public function testIsReadonlyClass(): void + { + $reflection = new \ReflectionClass(RouteMatch::class); + $this->assertTrue( + $reflection->isReadOnly(), + 'RouteMatch must be a readonly class so per-request match facts cannot be mutated by handler code.', + ); + } + + public function testCannotReassignRouteField(): void + { + $route = new Route('GET', '/x'); + $match = new RouteMatch($route, '/x', '/x', '/x'); + + $this->expectException(\Error::class); + /** @phpstan-ignore-next-line intentional runtime assertion */ + $match->urlPath = '/mutated'; + } + + public function testWildcardTokenRoundTrips(): void + { + $route = new Route('', ''); + $match = new RouteMatch($route, '/anything/at/all', Router::WILDCARD_TOKEN, Router::WILDCARD_TOKEN); + + $this->assertSame(Router::WILDCARD_TOKEN, $match->routeKey); + $this->assertSame('/anything/at/all', $match->urlPath); + } +} diff --git a/tests/RouterMatchRouteTest.php b/tests/RouterMatchRouteTest.php new file mode 100644 index 0000000..aed8e35 --- /dev/null +++ b/tests/RouterMatchRouteTest.php @@ -0,0 +1,138 @@ +assertNull(Router::matchRoute('FROBNICATE', '/anything')); + } + + public function testReturnsNullForUnmatchedPath(): void + { + Router::addRoute(new Route('GET', '/known')); + + $this->assertNull(Router::matchRoute('GET', '/unknown')); + } + + public function testReturnsRouteMatchForExactPath(): void + { + $route = new Route('GET', '/users'); + Router::addRoute($route); + + $match = Router::matchRoute('GET', '/users'); + + $this->assertInstanceOf(RouteMatch::class, $match); + $this->assertSame($route, $match->route); + $this->assertSame('/users', $match->urlPath); + $this->assertSame('users', $match->routeKey); + $this->assertSame('users', $match->preparedPath); + } + + public function testReturnsRouteMatchForParameterizedPath(): void + { + $route = new Route('GET', '/users/:id'); + Router::addRoute($route); + + $match = Router::matchRoute('GET', '/users/42'); + + $this->assertNotNull($match); + $this->assertSame($route, $match->route); + $this->assertSame('/users/42', $match->urlPath); + $this->assertSame('users/:::', $match->routeKey); + } + + public function testReturnsRouteMatchForWildcardRoute(): void + { + $route = new Route('GET', '/files/*'); + Router::addRoute($route); + + $match = Router::matchRoute('GET', '/files/a/b/c'); + + $this->assertNotNull($match); + $this->assertSame($route, $match->route); + $this->assertSame('files/*', $match->routeKey); + $this->assertSame('/files/a/b/c', $match->urlPath); + } + + public function testReturnsRouteMatchForRootWildcard(): void + { + $route = new Route('GET', '/*'); + Router::addRoute($route); + + $match = Router::matchRoute('GET', '/anything'); + + $this->assertNotNull($match); + $this->assertSame(Router::WILDCARD_TOKEN, $match->routeKey); + } + + public function testDoesNotMutateMatchedRoute(): void + { + // The whole reason this PR exists: Router previously wrote + // $match onto the Route via setMatchedPath(), creating a race + // between coroutines. Guard against regression. + $route = new Route('GET', '/users/:id'); + $snapshot = [ + 'method' => $route->getMethod(), + 'path' => $route->getPath(), + 'groups' => $route->getGroups(), + 'hook' => $route->getHook(), + ]; + + Router::addRoute($route); + + Router::matchRoute('GET', '/users/1'); + Router::matchRoute('GET', '/users/99'); + Router::matchRoute('GET', '/users/hello'); + + $this->assertSame($snapshot['method'], $route->getMethod()); + $this->assertSame($snapshot['path'], $route->getPath()); + $this->assertSame($snapshot['groups'], $route->getGroups()); + $this->assertSame($snapshot['hook'], $route->getHook()); + } + + public function testTwoMatchesReturnDistinctRouteMatchInstances(): void + { + $route = new Route('GET', '/users/:id'); + Router::addRoute($route); + + $a = Router::matchRoute('GET', '/users/1'); + $b = Router::matchRoute('GET', '/users/2'); + + $this->assertNotNull($a); + $this->assertNotNull($b); + $this->assertNotSame($a, $b, 'Each call should produce a fresh RouteMatch value so concurrent handlers cannot observe each other.'); + $this->assertSame($route, $a->route); + $this->assertSame($route, $b->route); + $this->assertSame('/users/1', $a->urlPath); + $this->assertSame('/users/2', $b->urlPath); + } + + public function testLegacyShimReturnsRoute(): void + { + $route = new Route('GET', '/shim'); + Router::addRoute($route); + + // The deprecated match() shim must keep returning the Route + // directly so external consumers compile during the deprecation + // window. + $this->assertSame($route, Router::match('GET', '/shim')); + $this->assertNull(Router::match('GET', '/missing')); + } +} From 4f5d41aac517ec43c1319598c00c90adc6d5407c Mon Sep 17 00:00:00 2001 From: loks0n <22452787+loks0n@users.noreply.github.com> Date: Fri, 24 Apr 2026 15:01:56 +0100 Subject: [PATCH 3/7] Remove RequestContext The per-request label-override overlay isn't needed. Route labels should be set at registration time; any per-request mutable state a consumer needs can go in their own DI-container resource. Keeping a dedicated class for this was speculative scope. - Delete src/Http/RequestContext.php and its test file. - Dispatcher no longer constructs a RequestContext or registers a `context` request resource. - Http::execute shim drops the same wiring. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Http/Dispatcher.php | 3 -- src/Http/Http.php | 5 +--- src/Http/RequestContext.php | 47 ----------------------------- tests/DispatcherTest.php | 37 ----------------------- tests/RequestContextTest.php | 58 ------------------------------------ 5 files changed, 1 insertion(+), 149 deletions(-) delete mode 100644 src/Http/RequestContext.php delete mode 100644 tests/RequestContextTest.php diff --git a/src/Http/Dispatcher.php b/src/Http/Dispatcher.php index 0da3ba2..5e7a9bb 100644 --- a/src/Http/Dispatcher.php +++ b/src/Http/Dispatcher.php @@ -40,10 +40,8 @@ public function handle(): void $this->response->setCompressionSupported($this->http->getCompressionSupported()); } - $context = new RequestContext(); $this->http->setRequestResource('request', fn() => $this->request); $this->http->setRequestResource('response', fn() => $this->response); - $this->http->setRequestResource('context', fn() => $context); try { foreach (Http::getRequestHooks() as $hook) { @@ -89,7 +87,6 @@ public function handle(): void $this->match = new RouteMatch($wildcard, $url, Router::WILDCARD_TOKEN, Router::WILDCARD_TOKEN); } - $context->setMatch($this->match); $this->http->setRequestResource('route', fn() => $this->match?->route); $this->http->setRequestResource('routeMatch', fn() => $this->match); diff --git a/src/Http/Http.php b/src/Http/Http.php index e5a4604..8271293 100755 --- a/src/Http/Http.php +++ b/src/Http/Http.php @@ -266,8 +266,7 @@ public static function wildcard(): Route * Returns the registered wildcard route, if any. * * The returned Route is a shared definition and MUST NOT be mutated by - * request-handling code. Per-request state belongs on {@see RouteMatch} - * or {@see RequestContext}. + * request-handling code. Per-request state belongs on {@see RouteMatch}. */ public static function getWildcardRoute(): ?Route { @@ -688,10 +687,8 @@ public function execute(Route $route, Request $request, Response $response): sta $urlPath = \is_string($urlPath) ? ($urlPath === '' ? '/' : $urlPath) : '/'; $match = new RouteMatch($route, $urlPath, $preparedPath, $preparedPath); - $context = new RequestContext($match); $this->setRequestResource('request', fn() => $request); $this->setRequestResource('response', fn() => $response); - $this->setRequestResource('context', fn() => $context); $this->setRequestResource('route', fn() => $route); $this->setRequestResource('routeMatch', fn() => $match); diff --git a/src/Http/RequestContext.php b/src/Http/RequestContext.php deleted file mode 100644 index b2ddf94..0000000 --- a/src/Http/RequestContext.php +++ /dev/null @@ -1,47 +0,0 @@ - */ - private array $labels = []; - - public function __construct(private ?RouteMatch $match = null) {} - - public function setMatch(?RouteMatch $match): void - { - $this->match = $match; - } - - public function getMatch(): ?RouteMatch - { - return $this->match; - } - - public function label(string $key, mixed $value): self - { - $this->labels[$key] = $value; - - return $this; - } - - public function getLabel(string $key, mixed $default = null): mixed - { - if (\array_key_exists($key, $this->labels)) { - return $this->labels[$key]; - } - - return $this->match?->route->getLabel($key, $default) ?? $default; - } -} diff --git a/tests/DispatcherTest.php b/tests/DispatcherTest.php index 88247f6..9b6c9bf 100644 --- a/tests/DispatcherTest.php +++ b/tests/DispatcherTest.php @@ -91,25 +91,6 @@ public function testRegistersRouteAndRouteMatchRequestResources(): void $this->assertInstanceOf(Route::class, $route); } - public function testRegistersFreshRequestContextEveryDispatch(): void - { - Http::get('/ctx') - ->inject('context') - ->inject('response') - ->action(function (RequestContext $context, Response $response) { - $response->send((string) ($context->getLabel('seen', 0) + 1)); - $context->label('seen', ($context->getLabel('seen', 0)) + 1); - }); - - // If `context` leaked across requests, the second request would - // observe the override set during the first. - $first = $this->runRequest('GET', '/ctx'); - $second = $this->runRequest('GET', '/ctx'); - - $this->assertSame('1', $first); - $this->assertSame('1', $second); - } - public function testWildcardRouteIsNeverMutated(): void { $wildcard = Http::wildcard() @@ -239,24 +220,6 @@ public function testOptionsHookFiresForOptionsMethod(): void $this->assertStringNotContainsString('GET-HANDLER', $output); } - public function testContextLabelOverrideDoesNotLeakToRoute(): void - { - $route = Http::get('/lbl') - ->label('router', false) - ->inject('context') - ->inject('response') - ->action(function (RequestContext $context, Response $response) { - $context->label('router', true); - $response->send($context->getLabel('router') ? 'on' : 'off'); - }); - - $this->assertSame('on', $this->runRequest('GET', '/lbl')); - - // The shared Route's label is untouched — a concurrent request - // that doesn't set the override still sees the registered default. - $this->assertFalse($route->getLabel('router', null)); - } - public function testWildcardRouteMatchCarriesWildcardToken(): void { Http::wildcard() diff --git a/tests/RequestContextTest.php b/tests/RequestContextTest.php deleted file mode 100644 index 96a2e51..0000000 --- a/tests/RequestContextTest.php +++ /dev/null @@ -1,58 +0,0 @@ -label('sdk.platform', 'server'); - - $match = new RouteMatch($route, '/things', '/things', '/things'); - $context = new RequestContext($match); - - $this->assertSame('server', $context->getLabel('sdk.platform')); - $this->assertSame('fallback', $context->getLabel('missing', 'fallback')); - } - - public function testOverridesRouteLabelWithoutMutatingRoute(): void - { - $route = new Route('GET', '/things'); - $route->label('router', false); - - $match = new RouteMatch($route, '/things', '/things', '/things'); - $context = new RequestContext($match); - - $context->label('router', true); - - $this->assertTrue($context->getLabel('router')); - // The shared Route definition is untouched — concurrent requests - // that don't override see the original value. - $this->assertFalse($route->getLabel('router', null)); - } - - public function testNullRouteReturnsDefault(): void - { - $context = new RequestContext(); - $this->assertSame('default', $context->getLabel('anything', 'default')); - } - - public function testRouteMatchIsImmutableAtTypeLevel(): void - { - $route = new Route('GET', '/x'); - $match = new RouteMatch($route, '/x', '/x', '/x'); - - $reflection = new \ReflectionClass($match); - $this->assertTrue($reflection->isReadOnly(), 'RouteMatch must be a readonly class'); - } -} From 47f2349b93fa76e610531deeccbb1b6acfc720b5 Mon Sep 17 00:00:00 2001 From: loks0n <22452787+loks0n@users.noreply.github.com> Date: Fri, 24 Apr 2026 15:11:01 +0100 Subject: [PATCH 4/7] Extract Hooks registry out of Http MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hook arrays (init/shutdown/options/error/start/request) lived as protected statics on Http, and the previous refactor added six public getInitHooks()/getShutdownHooks()/etc. accessors purely so Dispatcher could read them. That's a leaky API — internal process-global state promoted to public surface for the sake of one internal collaborator. Move the registries to a dedicated Hooks class (mirrors how Router owns its own static state). Http::init/shutdown/options/error/onStart/ onRequest remain as thin delegators for backwards compat; the six getXxxHooks accessors are gone. Dispatcher reads Hooks::\$init etc. directly. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Http/Dispatcher.php | 24 +++---- src/Http/Hooks.php | 125 +++++++++++++++++++++++++++++++++ src/Http/Http.php | 149 ++++------------------------------------ 3 files changed, 150 insertions(+), 148 deletions(-) create mode 100644 src/Http/Hooks.php diff --git a/src/Http/Dispatcher.php b/src/Http/Dispatcher.php index 5e7a9bb..4aa8d1f 100644 --- a/src/Http/Dispatcher.php +++ b/src/Http/Dispatcher.php @@ -44,14 +44,14 @@ public function handle(): void $this->http->setRequestResource('response', fn() => $this->response); try { - foreach (Http::getRequestHooks() as $hook) { + foreach (Hooks::$request as $hook) { $arguments = $this->http->getArguments($hook, [], []); \call_user_func_array($hook->getAction(), $arguments); } } catch (\Exception $e) { $this->http->setRequestResource('error', fn() => $e); - foreach (Http::getErrorHooks() as $error) { + foreach (Hooks::$errors as $error) { if (\in_array('*', $error->getGroups())) { try { $arguments = $this->http->getArguments($error, [], []); @@ -100,20 +100,20 @@ public function handle(): void if (Http::REQUEST_METHOD_OPTIONS === $method) { try { foreach ($groups as $group) { - foreach (Http::getOptionsHooks() as $option) { + foreach (Hooks::$options as $option) { if (\in_array($group, $option->getGroups())) { \call_user_func_array($option->getAction(), $this->http->getArguments($option, [], $this->request->getParams())); } } } - foreach (Http::getOptionsHooks() as $option) { + foreach (Hooks::$options as $option) { if (\in_array('*', $option->getGroups())) { \call_user_func_array($option->getAction(), $this->http->getArguments($option, [], $this->request->getParams())); } } } catch (\Throwable $e) { - foreach (Http::getErrorHooks() as $error) { + foreach (Hooks::$errors as $error) { if (\in_array('*', $error->getGroups())) { $this->http->setRequestResource('error', fn() => $e); \call_user_func_array($error->getAction(), $this->http->getArguments($error, [], $this->request->getParams())); @@ -130,7 +130,7 @@ public function handle(): void return; } - foreach (Http::getErrorHooks() as $error) { + foreach (Hooks::$errors as $error) { if (\in_array('*', $error->getGroups())) { $this->http->setRequestResource('error', fn() => new Exception('Not Found', 404)); \call_user_func_array($error->getAction(), $this->http->getArguments($error, [], $this->request->getParams())); @@ -147,7 +147,7 @@ public function execute(RouteMatch $match): void try { if ($route->getHook()) { - foreach (Http::getInitHooks() as $hook) { + foreach (Hooks::$init as $hook) { if (\in_array('*', $hook->getGroups())) { \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams)); } @@ -155,7 +155,7 @@ public function execute(RouteMatch $match): void } foreach ($groups as $group) { - foreach (Http::getInitHooks() as $hook) { + foreach (Hooks::$init as $hook) { if (\in_array($group, $hook->getGroups())) { \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams)); } @@ -167,7 +167,7 @@ public function execute(RouteMatch $match): void } foreach ($groups as $group) { - foreach (Http::getShutdownHooks() as $hook) { + foreach (Hooks::$shutdown as $hook) { if (\in_array($group, $hook->getGroups())) { \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams)); } @@ -175,7 +175,7 @@ public function execute(RouteMatch $match): void } if ($route->getHook()) { - foreach (Http::getShutdownHooks() as $hook) { + foreach (Hooks::$shutdown as $hook) { if (\in_array('*', $hook->getGroups())) { \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams)); } @@ -185,7 +185,7 @@ public function execute(RouteMatch $match): void $this->http->setRequestResource('error', fn() => $e); foreach ($groups as $group) { - foreach (Http::getErrorHooks() as $error) { + foreach (Hooks::$errors as $error) { if (\in_array($group, $error->getGroups())) { try { \call_user_func_array($error->getAction(), $this->http->getArguments($error, $pathValues, $requestParams)); @@ -196,7 +196,7 @@ public function execute(RouteMatch $match): void } } - foreach (Http::getErrorHooks() as $error) { + foreach (Hooks::$errors as $error) { if (\in_array('*', $error->getGroups())) { try { \call_user_func_array($error->getAction(), $this->http->getArguments($error, $pathValues, $requestParams)); diff --git a/src/Http/Hooks.php b/src/Http/Hooks.php new file mode 100644 index 0000000..7a57328 --- /dev/null +++ b/src/Http/Hooks.php @@ -0,0 +1,125 @@ +groups(['*']); + self::$init[] = $hook; + + return $hook; + } + + /** + * Register a callback that runs after the matched route action. + */ + public static function shutdown(): Hook + { + $hook = new Hook(); + $hook->groups(['*']); + self::$shutdown[] = $hook; + + return $hook; + } + + /** + * Register a callback for OPTIONS method requests. + */ + public static function options(): Hook + { + $hook = new Hook(); + $hook->groups(['*']); + self::$options[] = $hook; + + return $hook; + } + + /** + * Register an error callback. + */ + public static function error(): Hook + { + $hook = new Hook(); + $hook->groups(['*']); + self::$errors[] = $hook; + + return $hook; + } + + /** + * Register a callback that runs once when the server starts. + */ + public static function onStart(): Hook + { + $hook = new Hook(); + self::$start[] = $hook; + + return $hook; + } + + /** + * Register a callback that runs at the top of every request, before + * route matching. + */ + public static function onRequest(): Hook + { + $hook = new Hook(); + self::$request[] = $hook; + + return $hook; + } + + /** + * Clear every registered hook. Intended for test isolation. + */ + public static function reset(): void + { + self::$init = []; + self::$shutdown = []; + self::$options = []; + self::$errors = []; + self::$start = []; + self::$request = []; + } +} diff --git a/src/Http/Http.php b/src/Http/Http.php index 8271293..048ee57 100755 --- a/src/Http/Http.php +++ b/src/Http/Http.php @@ -53,56 +53,6 @@ class Http */ protected static string $mode = ''; - /** - * Errors - * - * Errors callbacks - * - * @var Hook[] - */ - protected static array $errors = []; - - /** - * Init - * - * A callback function that is initialized on application start - * - * @var Hook[] - */ - protected static array $init = []; - - /** - * Shutdown - * - * A callback function that is initialized on application end - * - * @var Hook[] - */ - protected static array $shutdown = []; - - /** - * Options - * - * A callback function for options method requests - * - * @var Hook[] - */ - protected static array $options = []; - - /** - * Server Start hooks - * - * @var Hook[] - */ - protected static array $startHooks = []; - - /** - * Request hooks - * - * @var Hook[] - */ - protected static array $requestHooks = []; - /** * Wildcard route * If set, this get's executed if no other route is matched @@ -274,99 +224,35 @@ public static function getWildcardRoute(): ?Route } /** - * Init - * - * Set a callback function that will be initialized on application start + * Register a callback that runs before the matched route action. */ public static function init(): Hook { - $hook = new Hook(); - $hook->groups(['*']); - - self::$init[] = $hook; - - return $hook; + return Hooks::init(); } /** - * Shutdown - * - * Set a callback function that will be initialized on application end + * Register a callback that runs after the matched route action. */ public static function shutdown(): Hook { - $hook = new Hook(); - $hook->groups(['*']); - - self::$shutdown[] = $hook; - - return $hook; + return Hooks::shutdown(); } /** - * Options - * - * Set a callback function for all request with options method + * Register a callback for OPTIONS method requests. */ public static function options(): Hook { - $hook = new Hook(); - $hook->groups(['*']); - - self::$options[] = $hook; - - return $hook; + return Hooks::options(); } /** - * Error - * - * An error callback for failed or no matched requests + * Register an error callback for failed or unmatched requests. */ public static function error(): Hook { - $hook = new Hook(); - $hook->groups(['*']); - - self::$errors[] = $hook; - - return $hook; - } - - /** @return Hook[] */ - public static function getInitHooks(): array - { - return self::$init; - } - - /** @return Hook[] */ - public static function getShutdownHooks(): array - { - return self::$shutdown; - } - - /** @return Hook[] */ - public static function getOptionsHooks(): array - { - return self::$options; - } - - /** @return Hook[] */ - public static function getErrorHooks(): array - { - return self::$errors; - } - - /** @return Hook[] */ - public static function getStartHooks(): array - { - return self::$startHooks; - } - - /** @return Hook[] */ - public static function getRequestHooks(): array - { - return self::$requestHooks; + return Hooks::error(); } /** @@ -604,16 +490,12 @@ public function getFileMimeType(string $uri): mixed public static function onStart(): Hook { - $hook = new Hook(); - self::$startHooks[] = $hook; - return $hook; + return Hooks::onStart(); } public static function onRequest(): Hook { - $hook = new Hook(); - self::$requestHooks[] = $hook; - return $hook; + return Hooks::onRequest(); } public function start(): void @@ -625,14 +507,14 @@ public function start(): void $this->server->onStart(function ($server) { $this->setResource('server', fn() => $server); try { - foreach (self::$startHooks as $hook) { + foreach (Hooks::$start as $hook) { $arguments = $this->getArguments($hook, [], []); \call_user_func_array($hook->getAction(), $arguments); } } catch (\Exception $e) { $this->setResource('error', fn() => $e); - foreach (self::$errors as $error) { + foreach (Hooks::$errors as $error) { if (in_array('*', $error->getGroups())) { try { $arguments = $this->getArguments($error, [], []); @@ -809,13 +691,8 @@ protected function validate(string $key, array $param, mixed $value): void public static function reset(): void { Router::reset(); + Hooks::reset(); self::$mode = ''; - self::$errors = []; - self::$init = []; - self::$shutdown = []; - self::$options = []; - self::$startHooks = []; - self::$requestHooks = []; self::$wildcardRoute = null; } } From 07087af8f0e8976a143507e3fd8123ae065cc91a Mon Sep 17 00:00:00 2001 From: loks0n <22452787+loks0n@users.noreply.github.com> Date: Fri, 24 Apr 2026 16:49:39 +0100 Subject: [PATCH 5/7] Fix: Re-read request params at each hook/action call site Dispatcher::execute hoisted \$this->request->getParams() into a local before any init hooks fired, then passed that stale array to every subsequent getArguments() call. That broke the init-hook-mutates-params contract: if an init hook applied filter/rewrite via \$request->setQueryString(), the route action and shutdown hooks would still see the pre-mutation view. The old Http::execute called \$request->getParams() inline at each call site specifically to preserve this ordering. Restore that behaviour. Adds two regression tests: init-hook mutation visible to the route action, and init-hook mutation visible to a shutdown hook's params. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Http/Dispatcher.php | 20 +++++++++------ tests/DispatcherTest.php | 55 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/Http/Dispatcher.php b/src/Http/Dispatcher.php index 4aa8d1f..a79e269 100644 --- a/src/Http/Dispatcher.php +++ b/src/Http/Dispatcher.php @@ -143,13 +143,17 @@ public function execute(RouteMatch $match): void $route = $match->route; $groups = $route->getGroups(); $pathValues = $route->getPathValues($this->request, $match->preparedPath); - $requestParams = $this->request->getParams(); + + // Request params are re-read at each call site: init/request hooks + // may mutate the request (e.g. applying filters), and later hooks + + // the route action must see the updated view. Hoisting this into a + // local would cache stale params across the lifecycle. try { if ($route->getHook()) { foreach (Hooks::$init as $hook) { if (\in_array('*', $hook->getGroups())) { - \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams)); + \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $this->request->getParams())); } } } @@ -157,19 +161,19 @@ public function execute(RouteMatch $match): void foreach ($groups as $group) { foreach (Hooks::$init as $hook) { if (\in_array($group, $hook->getGroups())) { - \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams)); + \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $this->request->getParams())); } } } if (!$this->response->isSent()) { - \call_user_func_array($route->getAction(), $this->http->getArguments($route, $pathValues, $requestParams)); + \call_user_func_array($route->getAction(), $this->http->getArguments($route, $pathValues, $this->request->getParams())); } foreach ($groups as $group) { foreach (Hooks::$shutdown as $hook) { if (\in_array($group, $hook->getGroups())) { - \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams)); + \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $this->request->getParams())); } } } @@ -177,7 +181,7 @@ public function execute(RouteMatch $match): void if ($route->getHook()) { foreach (Hooks::$shutdown as $hook) { if (\in_array('*', $hook->getGroups())) { - \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams)); + \call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $this->request->getParams())); } } } @@ -188,7 +192,7 @@ public function execute(RouteMatch $match): void foreach (Hooks::$errors as $error) { if (\in_array($group, $error->getGroups())) { try { - \call_user_func_array($error->getAction(), $this->http->getArguments($error, $pathValues, $requestParams)); + \call_user_func_array($error->getAction(), $this->http->getArguments($error, $pathValues, $this->request->getParams())); } catch (\Throwable $e) { throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e); } @@ -199,7 +203,7 @@ public function execute(RouteMatch $match): void foreach (Hooks::$errors as $error) { if (\in_array('*', $error->getGroups())) { try { - \call_user_func_array($error->getAction(), $this->http->getArguments($error, $pathValues, $requestParams)); + \call_user_func_array($error->getAction(), $this->http->getArguments($error, $pathValues, $this->request->getParams())); } catch (\Throwable $e) { throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e); } diff --git a/tests/DispatcherTest.php b/tests/DispatcherTest.php index 9b6c9bf..c7320b4 100644 --- a/tests/DispatcherTest.php +++ b/tests/DispatcherTest.php @@ -9,6 +9,7 @@ use Utopia\Http\Adapter\FPM\Request; use Utopia\Http\Adapter\FPM\Response; use Utopia\Http\Adapter\FPM\Server; +use Utopia\Validator\Text; /** * End-to-end coverage for {@see Dispatcher} via Http::run(). @@ -220,6 +221,60 @@ public function testOptionsHookFiresForOptionsMethod(): void $this->assertStringNotContainsString('GET-HANDLER', $output); } + public function testInitHookMutationsToRequestParamsAreVisibleToRouteAction(): void + { + // Regression: Dispatcher::execute must re-read $request->getParams() + // at each hook/action call site. Hoisting the array into a local + // before init hooks fire would cache a pre-hook snapshot, so the + // route action would see stale params despite an init hook having + // mutated them (e.g. to apply auth/filter rewrites). + Http::init() + ->inject('request') + ->action(function (Request $request) { + $request->setQueryString(['x' => 'from-init-hook']); + }); + + Http::get('/filter-me') + ->param('x', 'original', new Text(64), 'x param', true) + ->inject('response') + ->action(function (string $x, Response $response) { + $response->send($x); + }); + + $output = $this->runRequest('GET', '/filter-me'); + + $this->assertSame('from-init-hook', $output); + } + + public function testShutdownHookSeesMutationsFromInitHook(): void + { + // The same guarantee for the init → shutdown path: shutdown hooks + // read getArguments() fresh, so an init-time mutation is visible. + Http::init() + ->inject('request') + ->action(function (Request $request) { + $request->setQueryString(['token' => 'init-token']); + }); + + Http::shutdown() + ->param('token', '', new Text(64), 'token param', true) + ->inject('response') + ->action(function (string $token, Response $response) { + echo '|shutdown:' . $token; + }); + + Http::get('/lifecycle-params') + ->inject('response') + ->action(function (Response $response) { + $response->send('ok'); + }); + + $output = $this->runRequest('GET', '/lifecycle-params'); + + $this->assertStringContainsString('ok', $output); + $this->assertStringContainsString('|shutdown:init-token', $output); + } + public function testWildcardRouteMatchCarriesWildcardToken(): void { Http::wildcard() From c6770aded7c5a8f341118d09cf268da93ca5881d Mon Sep 17 00:00:00 2001 From: loks0n <22452787+loks0n@users.noreply.github.com> Date: Fri, 24 Apr 2026 17:34:06 +0100 Subject: [PATCH 6/7] Add Router::matchRequest convenience entry point Router::matchRoute() takes (method, path) strings, forcing every caller to hand-roll URL parsing and HEAD-to-GET normalisation. The library already does this internally in Dispatcher::handle; the matching boilerplate should not be the caller's problem. Router::matchRequest(Request) wraps matchRoute and handles both. The deprecated Http::match() shim now delegates to it; Dispatcher continues to call matchRoute directly because it needs the parsed URL in scope for the wildcard fallback anyway. Adds three regression tests covering URL extraction, root-URL fallback when the URI has no path, and HEAD normalisation. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Http/Http.php | 15 ++++-------- src/Http/Router.php | 20 +++++++++++++++ tests/RouterMatchRouteTest.php | 45 ++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/src/Http/Http.php b/src/Http/Http.php index 048ee57..114c66f 100755 --- a/src/Http/Http.php +++ b/src/Http/Http.php @@ -531,19 +531,14 @@ public function start(): void } /** - * @deprecated Use {@see Router::matchRoute()} which returns a per-request - * {@see RouteMatch}. This shim discards the `$fresh` argument: the - * previous implementation cached the match on the Http singleton, - * which is not safe under concurrent request handling. + * @deprecated Use {@see Router::matchRequest()} which returns a + * per-request {@see RouteMatch}. This shim discards the `$fresh` + * argument: the previous implementation cached the match on the Http + * singleton, which is not safe under concurrent request handling. */ public function match(Request $request, bool $fresh = true): ?Route { - $url = \parse_url($request->getURI(), PHP_URL_PATH); - $url = \is_string($url) ? ($url === '' ? '/' : $url) : '/'; - $method = $request->getMethod(); - $method = (self::REQUEST_METHOD_HEAD === $method) ? self::REQUEST_METHOD_GET : $method; - - $match = Router::matchRoute($method, $url); + $match = Router::matchRequest($request); if ($match === null) { return null; } diff --git a/src/Http/Router.php b/src/Http/Router.php index d935514..16643cc 100644 --- a/src/Http/Router.php +++ b/src/Http/Router.php @@ -159,6 +159,26 @@ public static function matchRoute(string $method, string $path): ?RouteMatch return null; } + /** + * Match a {@see Request} against the router. + * + * Convenience wrapper around {@see Router::matchRoute()} that extracts + * the path from the request URI and normalises HEAD to GET (the + * HEAD method is served by the GET handler with the response payload + * disabled). Prefer this over hand-rolling the parse + HEAD logic at + * every call site. + */ + public static function matchRequest(Request $request): ?RouteMatch + { + $url = \parse_url($request->getURI(), PHP_URL_PATH); + $url = \is_string($url) ? ($url === '' ? '/' : $url) : '/'; + + $method = $request->getMethod(); + $method = ($method === Http::REQUEST_METHOD_HEAD) ? Http::REQUEST_METHOD_GET : $method; + + return self::matchRoute($method, $url); + } + /** * @deprecated Use {@see Router::matchRoute()} which returns a per-request * {@see RouteMatch}. The old signature returned the shared Route diff --git a/tests/RouterMatchRouteTest.php b/tests/RouterMatchRouteTest.php index aed8e35..75c3306 100644 --- a/tests/RouterMatchRouteTest.php +++ b/tests/RouterMatchRouteTest.php @@ -5,6 +5,7 @@ namespace Utopia\Http; use PHPUnit\Framework\TestCase; +use Utopia\Http\Adapter\FPM\Request; /** * Coverage for {@see Router::matchRoute()} — the coroutine-safe replacement @@ -124,6 +125,50 @@ public function testTwoMatchesReturnDistinctRouteMatchInstances(): void $this->assertSame('/users/2', $b->urlPath); } + public function testMatchRequestExtractsPathFromUri(): void + { + $route = new Route('GET', '/users/:id'); + Router::addRoute($route); + + $_SERVER['REQUEST_METHOD'] = 'GET'; + $_SERVER['REQUEST_URI'] = '/users/42?extra=ignored'; + + $match = Router::matchRequest(new Request()); + + $this->assertNotNull($match); + $this->assertSame($route, $match->route); + $this->assertSame('/users/42', $match->urlPath); + } + + public function testMatchRequestDefaultsEmptyPathToSlash(): void + { + $route = new Route('GET', '/'); + Router::addRoute($route); + + $_SERVER['REQUEST_METHOD'] = 'GET'; + $_SERVER['REQUEST_URI'] = 'https://example.com?x=1'; + + $match = Router::matchRequest(new Request()); + + $this->assertNotNull($match); + $this->assertSame($route, $match->route); + $this->assertSame('/', $match->urlPath); + } + + public function testMatchRequestNormalisesHeadToGet(): void + { + $route = new Route('GET', '/head-target'); + Router::addRoute($route); + + $_SERVER['REQUEST_METHOD'] = 'HEAD'; + $_SERVER['REQUEST_URI'] = '/head-target'; + + $match = Router::matchRequest(new Request()); + + $this->assertNotNull($match); + $this->assertSame($route, $match->route, 'HEAD must resolve against the GET route.'); + } + public function testLegacyShimReturnsRoute(): void { $route = new Route('GET', '/shim'); From 1493a8492e5706e1c2d705464f1a040c35fef5d3 Mon Sep 17 00:00:00 2001 From: loks0n <22452787+loks0n@users.noreply.github.com> Date: Fri, 24 Apr 2026 17:55:22 +0100 Subject: [PATCH 7/7] Collapse Router surface and fold wildcard into Router Two public matching entry points (matchRoute + matchRequest) were a mild smell; the method-agnostic wildcard living as a static singleton on Http was an outright special case leaking into the Dispatcher. Address both at once: - Make Router::matchRoute(string, string) private. Router::matchRequest is now the sole public matching API. External callers that used to hand-roll parse_url + HEAD normalisation to call matchRoute now just pass a Request. - Delete the deprecated Router::match(string, string): ?Route shim. - Move wildcard registration into Router via new Router::setWildcard; delete Http::\$wildcardRoute and Http::getWildcardRoute. Http::wildcard continues to return a Route, just registered on Router. - Dispatcher's match section collapses from the \$url/\$matchMethod/ wildcard-fallback block to a single Router::matchRequest call. - RouterTest/RouterMatchRouteTest migrate to matchRequest via a private test helper that sets \$_SERVER and builds an FPM Request. - Three new tests cover Router::setWildcard semantics (unknown path, unknown method, and method-specific precedence over the wildcard). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Http/Dispatcher.php | 11 +-- src/Http/Http.php | 28 ++---- src/Http/Router.php | 128 ++++++++++++++------------- tests/RouterMatchRouteTest.php | 155 +++++++++++++++++++-------------- tests/RouterTest.php | 88 +++++++++++-------- 5 files changed, 217 insertions(+), 193 deletions(-) diff --git a/src/Http/Dispatcher.php b/src/Http/Dispatcher.php index a79e269..07cb893 100644 --- a/src/Http/Dispatcher.php +++ b/src/Http/Dispatcher.php @@ -76,16 +76,7 @@ public function handle(): void } $method = $this->request->getMethod(); - $url = \parse_url($this->request->getURI(), PHP_URL_PATH); - $url = \is_string($url) ? ($url === '' ? '/' : $url) : '/'; - $matchMethod = (Http::REQUEST_METHOD_HEAD === $method) ? Http::REQUEST_METHOD_GET : $method; - - $this->match = Router::matchRoute($matchMethod, $url); - - if ($this->match === null && Http::getWildcardRoute() !== null) { - $wildcard = Http::getWildcardRoute(); - $this->match = new RouteMatch($wildcard, $url, Router::WILDCARD_TOKEN, Router::WILDCARD_TOKEN); - } + $this->match = Router::matchRequest($this->request); $this->http->setRequestResource('route', fn() => $this->match?->route); $this->http->setRequestResource('routeMatch', fn() => $this->match); diff --git a/src/Http/Http.php b/src/Http/Http.php index 114c66f..da94cff 100755 --- a/src/Http/Http.php +++ b/src/Http/Http.php @@ -53,12 +53,6 @@ class Http */ protected static string $mode = ''; - /** - * Wildcard route - * If set, this get's executed if no other route is matched - */ - protected static ?Route $wildcardRoute = null; - /** * Compression */ @@ -201,26 +195,15 @@ public static function delete(string $url): Route } /** - * Wildcard - * - * Add Wildcard route + * Register a method-agnostic wildcard route. Invoked when no + * method-specific route matches the incoming request. */ public static function wildcard(): Route { - self::$wildcardRoute = new Route('', ''); - - return self::$wildcardRoute; - } + $route = new Route('', ''); + Router::setWildcard($route); - /** - * Returns the registered wildcard route, if any. - * - * The returned Route is a shared definition and MUST NOT be mutated by - * request-handling code. Per-request state belongs on {@see RouteMatch}. - */ - public static function getWildcardRoute(): ?Route - { - return self::$wildcardRoute; + return $route; } /** @@ -688,6 +671,5 @@ public static function reset(): void Router::reset(); Hooks::reset(); self::$mode = ''; - self::$wildcardRoute = null; } } diff --git a/src/Http/Router.php b/src/Http/Router.php index 16643cc..49d9fa5 100644 --- a/src/Http/Router.php +++ b/src/Http/Router.php @@ -32,6 +32,12 @@ class Router */ protected static array $params = []; + /** + * Method-agnostic wildcard route, used as last-resort fallback when no + * method-specific route matches. Registered via {@see self::setWildcard()}. + */ + protected static ?Route $wildcard = null; + /** * Get all registered routes. * @@ -106,67 +112,23 @@ public static function addRouteAlias(string $path, Route $route): void } /** - * Match route against the method and path. - * - * Returns an immutable {@see RouteMatch} carrying the matched route and - * the per-request match facts (URL path, router table key, prepared - * path). The shared {@see Route} definition is never mutated. + * Register a method-agnostic wildcard route. Used as the last-resort + * fallback when no method-specific route matches. At most one wildcard + * can be registered; a subsequent call replaces the previous one. */ - public static function matchRoute(string $method, string $path): ?RouteMatch + public static function setWildcard(Route $route): void { - if (!array_key_exists($method, self::$routes)) { - return null; - } - - $parts = array_values(array_filter(explode('/', $path), fn($segment) => $segment !== '')); - $length = count($parts) - 1; - $filteredParams = array_filter(self::$params, fn($i) => $i <= $length); - - foreach (self::combinations($filteredParams) as $sample) { - $sample = array_filter($sample, fn(int $i) => $i <= $length); - $match = implode( - '/', - array_replace( - $parts, - array_fill_keys($sample, self::PLACEHOLDER_TOKEN), - ), - ); - - if (array_key_exists($match, self::$routes[$method])) { - return new RouteMatch(self::$routes[$method][$match], $path, $match, $match); - } - } - - /** - * Match root wildcard. - */ - $match = self::WILDCARD_TOKEN; - if (array_key_exists($match, self::$routes[$method])) { - return new RouteMatch(self::$routes[$method][$match], $path, $match, $match); - } - - /** - * Match wildcard for path segments. - */ - foreach ($parts as $part) { - $current = ($current ?? '') . "{$part}/"; - $match = $current . self::WILDCARD_TOKEN; - if (array_key_exists($match, self::$routes[$method])) { - return new RouteMatch(self::$routes[$method][$match], $path, $match, $match); - } - } - - return null; + self::$wildcard = $route; } /** * Match a {@see Request} against the router. * - * Convenience wrapper around {@see Router::matchRoute()} that extracts - * the path from the request URI and normalises HEAD to GET (the + * Extracts the path from the request URI, normalises HEAD to GET (the * HEAD method is served by the GET handler with the response payload - * disabled). Prefer this over hand-rolling the parse + HEAD logic at - * every call site. + * disabled), and returns an immutable {@see RouteMatch} carrying the + * matched route and per-request match facts. The shared {@see Route} + * definition is never mutated. */ public static function matchRequest(Request $request): ?RouteMatch { @@ -180,14 +142,61 @@ public static function matchRequest(Request $request): ?RouteMatch } /** - * @deprecated Use {@see Router::matchRoute()} which returns a per-request - * {@see RouteMatch}. The old signature returned the shared Route - * definition; under concurrent request handling (Swoole coroutines) - * mutating that return value is racy. + * Match against a (method, path) pair. Internal — application code should + * call {@see self::matchRequest()} so URL parsing and HEAD normalisation + * are handled consistently. */ - public static function match(string $method, string $path): ?Route + private static function matchRoute(string $method, string $path): ?RouteMatch { - return self::matchRoute($method, $path)?->route; + if (array_key_exists($method, self::$routes)) { + $parts = array_values(array_filter(explode('/', $path), fn($segment) => $segment !== '')); + $length = count($parts) - 1; + $filteredParams = array_filter(self::$params, fn($i) => $i <= $length); + + foreach (self::combinations($filteredParams) as $sample) { + $sample = array_filter($sample, fn(int $i) => $i <= $length); + $match = implode( + '/', + array_replace( + $parts, + array_fill_keys($sample, self::PLACEHOLDER_TOKEN), + ), + ); + + if (array_key_exists($match, self::$routes[$method])) { + return new RouteMatch(self::$routes[$method][$match], $path, $match, $match); + } + } + + /** + * Match root wildcard for this method (e.g. GET /*). + */ + $match = self::WILDCARD_TOKEN; + if (array_key_exists($match, self::$routes[$method])) { + return new RouteMatch(self::$routes[$method][$match], $path, $match, $match); + } + + /** + * Match wildcard for path segments (e.g. GET /foo/*). + */ + foreach ($parts as $part) { + $current = ($current ?? '') . "{$part}/"; + $match = $current . self::WILDCARD_TOKEN; + if (array_key_exists($match, self::$routes[$method])) { + return new RouteMatch(self::$routes[$method][$match], $path, $match, $match); + } + } + } + + /** + * Fall through to the method-agnostic wildcard registered via + * {@see self::setWildcard()}. + */ + if (self::$wildcard !== null) { + return new RouteMatch(self::$wildcard, $path, self::WILDCARD_TOKEN, self::WILDCARD_TOKEN); + } + + return null; } /** @@ -248,6 +257,7 @@ public static function preparePath(string $path): array public static function reset(): void { self::$params = []; + self::$wildcard = null; self::$routes = [ Http::REQUEST_METHOD_GET => [], Http::REQUEST_METHOD_POST => [], diff --git a/tests/RouterMatchRouteTest.php b/tests/RouterMatchRouteTest.php index 75c3306..6c74810 100644 --- a/tests/RouterMatchRouteTest.php +++ b/tests/RouterMatchRouteTest.php @@ -8,10 +8,11 @@ use Utopia\Http\Adapter\FPM\Request; /** - * Coverage for {@see Router::matchRoute()} — the coroutine-safe replacement - * for the legacy Router::match(). These tests focus on the two things that - * change vs. the legacy API: the return type (RouteMatch), and the - * no-mutation guarantee on the shared Route definition. + * Coverage for {@see Router::matchRequest()} — the sole public matching + * entry point after the coroutine-safety refactor. Focus areas: the + * return-type shape ({@see RouteMatch}), the no-mutation guarantee on + * shared Route definitions, and the method-agnostic wildcard fallback + * via {@see Router::setWildcard()}. */ final class RouterMatchRouteTest extends TestCase { @@ -20,16 +21,24 @@ protected function tearDown(): void Router::reset(); } - public function testReturnsNullForUnknownMethod(): void + private function match(string $method, string $uri): ?RouteMatch { - $this->assertNull(Router::matchRoute('FROBNICATE', '/anything')); + $_SERVER['REQUEST_METHOD'] = $method; + $_SERVER['REQUEST_URI'] = $uri; + + return Router::matchRequest(new Request()); + } + + public function testReturnsNullForUnknownMethodWithoutWildcard(): void + { + $this->assertNull($this->match('FROBNICATE', '/anything')); } public function testReturnsNullForUnmatchedPath(): void { Router::addRoute(new Route('GET', '/known')); - $this->assertNull(Router::matchRoute('GET', '/unknown')); + $this->assertNull($this->match('GET', '/unknown')); } public function testReturnsRouteMatchForExactPath(): void @@ -37,7 +46,7 @@ public function testReturnsRouteMatchForExactPath(): void $route = new Route('GET', '/users'); Router::addRoute($route); - $match = Router::matchRoute('GET', '/users'); + $match = $this->match('GET', '/users'); $this->assertInstanceOf(RouteMatch::class, $match); $this->assertSame($route, $match->route); @@ -51,7 +60,7 @@ public function testReturnsRouteMatchForParameterizedPath(): void $route = new Route('GET', '/users/:id'); Router::addRoute($route); - $match = Router::matchRoute('GET', '/users/42'); + $match = $this->match('GET', '/users/42'); $this->assertNotNull($match); $this->assertSame($route, $match->route); @@ -59,12 +68,12 @@ public function testReturnsRouteMatchForParameterizedPath(): void $this->assertSame('users/:::', $match->routeKey); } - public function testReturnsRouteMatchForWildcardRoute(): void + public function testReturnsRouteMatchForPrefixWildcardRoute(): void { $route = new Route('GET', '/files/*'); Router::addRoute($route); - $match = Router::matchRoute('GET', '/files/a/b/c'); + $match = $this->match('GET', '/files/a/b/c'); $this->assertNotNull($match); $this->assertSame($route, $match->route); @@ -72,57 +81,54 @@ public function testReturnsRouteMatchForWildcardRoute(): void $this->assertSame('/files/a/b/c', $match->urlPath); } - public function testReturnsRouteMatchForRootWildcard(): void + public function testReturnsRouteMatchForMethodSpecificRootWildcard(): void { $route = new Route('GET', '/*'); Router::addRoute($route); - $match = Router::matchRoute('GET', '/anything'); + $match = $this->match('GET', '/anything'); $this->assertNotNull($match); $this->assertSame(Router::WILDCARD_TOKEN, $match->routeKey); } - public function testDoesNotMutateMatchedRoute(): void + public function testMethodAgnosticWildcardCatchesUnknownPath(): void { - // The whole reason this PR exists: Router previously wrote - // $match onto the Route via setMatchedPath(), creating a race - // between coroutines. Guard against regression. - $route = new Route('GET', '/users/:id'); - $snapshot = [ - 'method' => $route->getMethod(), - 'path' => $route->getPath(), - 'groups' => $route->getGroups(), - 'hook' => $route->getHook(), - ]; + $wildcard = new Route('', ''); + Router::setWildcard($wildcard); + Router::addRoute(new Route('GET', '/known')); - Router::addRoute($route); + $match = $this->match('GET', '/definitely-unknown'); - Router::matchRoute('GET', '/users/1'); - Router::matchRoute('GET', '/users/99'); - Router::matchRoute('GET', '/users/hello'); + $this->assertNotNull($match); + $this->assertSame($wildcard, $match->route); + $this->assertSame(Router::WILDCARD_TOKEN, $match->routeKey); + $this->assertSame('/definitely-unknown', $match->urlPath); + } - $this->assertSame($snapshot['method'], $route->getMethod()); - $this->assertSame($snapshot['path'], $route->getPath()); - $this->assertSame($snapshot['groups'], $route->getGroups()); - $this->assertSame($snapshot['hook'], $route->getHook()); + public function testMethodAgnosticWildcardCatchesUnknownMethod(): void + { + // Method-agnostic wildcard fires even when the HTTP method isn't + // one of the registered buckets (GET/POST/PUT/PATCH/DELETE). + $wildcard = new Route('', ''); + Router::setWildcard($wildcard); + + $match = $this->match('FROBNICATE', '/anything'); + + $this->assertNotNull($match); + $this->assertSame($wildcard, $match->route); } - public function testTwoMatchesReturnDistinctRouteMatchInstances(): void + public function testMethodSpecificMatchTakesPrecedenceOverWildcard(): void { - $route = new Route('GET', '/users/:id'); - Router::addRoute($route); + $specific = new Route('GET', '/users'); + Router::addRoute($specific); + Router::setWildcard(new Route('', '')); - $a = Router::matchRoute('GET', '/users/1'); - $b = Router::matchRoute('GET', '/users/2'); + $match = $this->match('GET', '/users'); - $this->assertNotNull($a); - $this->assertNotNull($b); - $this->assertNotSame($a, $b, 'Each call should produce a fresh RouteMatch value so concurrent handlers cannot observe each other.'); - $this->assertSame($route, $a->route); - $this->assertSame($route, $b->route); - $this->assertSame('/users/1', $a->urlPath); - $this->assertSame('/users/2', $b->urlPath); + $this->assertNotNull($match); + $this->assertSame($specific, $match->route, 'A method-specific route must win over the wildcard fallback.'); } public function testMatchRequestExtractsPathFromUri(): void @@ -130,10 +136,7 @@ public function testMatchRequestExtractsPathFromUri(): void $route = new Route('GET', '/users/:id'); Router::addRoute($route); - $_SERVER['REQUEST_METHOD'] = 'GET'; - $_SERVER['REQUEST_URI'] = '/users/42?extra=ignored'; - - $match = Router::matchRequest(new Request()); + $match = $this->match('GET', '/users/42?extra=ignored'); $this->assertNotNull($match); $this->assertSame($route, $match->route); @@ -145,10 +148,7 @@ public function testMatchRequestDefaultsEmptyPathToSlash(): void $route = new Route('GET', '/'); Router::addRoute($route); - $_SERVER['REQUEST_METHOD'] = 'GET'; - $_SERVER['REQUEST_URI'] = 'https://example.com?x=1'; - - $match = Router::matchRequest(new Request()); + $match = $this->match('GET', 'https://example.com?x=1'); $this->assertNotNull($match); $this->assertSame($route, $match->route); @@ -160,24 +160,51 @@ public function testMatchRequestNormalisesHeadToGet(): void $route = new Route('GET', '/head-target'); Router::addRoute($route); - $_SERVER['REQUEST_METHOD'] = 'HEAD'; - $_SERVER['REQUEST_URI'] = '/head-target'; - - $match = Router::matchRequest(new Request()); + $match = $this->match('HEAD', '/head-target'); $this->assertNotNull($match); - $this->assertSame($route, $match->route, 'HEAD must resolve against the GET route.'); + $this->assertSame($route, $match->route); + } + + public function testDoesNotMutateMatchedRoute(): void + { + // Regression guard: the router previously wrote matched facts back + // onto the Route via setMatchedPath(), creating a race between + // coroutines. Repeated matches must leave the Route byte-identical. + $route = new Route('GET', '/users/:id'); + $snapshot = [ + 'method' => $route->getMethod(), + 'path' => $route->getPath(), + 'groups' => $route->getGroups(), + 'hook' => $route->getHook(), + ]; + + Router::addRoute($route); + + $this->match('GET', '/users/1'); + $this->match('GET', '/users/99'); + $this->match('GET', '/users/hello'); + + $this->assertSame($snapshot['method'], $route->getMethod()); + $this->assertSame($snapshot['path'], $route->getPath()); + $this->assertSame($snapshot['groups'], $route->getGroups()); + $this->assertSame($snapshot['hook'], $route->getHook()); } - public function testLegacyShimReturnsRoute(): void + public function testTwoMatchesReturnDistinctRouteMatchInstances(): void { - $route = new Route('GET', '/shim'); + $route = new Route('GET', '/users/:id'); Router::addRoute($route); - // The deprecated match() shim must keep returning the Route - // directly so external consumers compile during the deprecation - // window. - $this->assertSame($route, Router::match('GET', '/shim')); - $this->assertNull(Router::match('GET', '/missing')); + $a = $this->match('GET', '/users/1'); + $b = $this->match('GET', '/users/2'); + + $this->assertNotNull($a); + $this->assertNotNull($b); + $this->assertNotSame($a, $b, 'Each call should produce a fresh RouteMatch value so concurrent handlers cannot observe each other.'); + $this->assertSame($route, $a->route); + $this->assertSame($route, $b->route); + $this->assertSame('/users/1', $a->urlPath); + $this->assertSame('/users/2', $b->urlPath); } } diff --git a/tests/RouterTest.php b/tests/RouterTest.php index 4ca0134..0c33636 100644 --- a/tests/RouterTest.php +++ b/tests/RouterTest.php @@ -5,6 +5,7 @@ namespace Utopia\Http; use PHPUnit\Framework\TestCase; +use Utopia\Http\Adapter\FPM\Request; final class RouterTest extends TestCase { @@ -13,6 +14,19 @@ public function tearDown(): void Router::reset(); } + /** + * Test helper: drive Router::matchRequest with a method + path, returning + * the matched Route (or null). Keeps the existing test cases readable + * now that `Router::match(string, string)` is no longer public. + */ + private function match(string $method, string $path): ?Route + { + $_SERVER['REQUEST_METHOD'] = $method; + $_SERVER['REQUEST_URI'] = $path; + + return Router::matchRequest(new Request())?->route; + } + public function testCanMatchUrl(): void { $routeIndex = new Route(Http::REQUEST_METHOD_GET, '/'); @@ -23,9 +37,9 @@ public function testCanMatchUrl(): void Router::addRoute($routeAbout); Router::addRoute($routeAboutMe); - $this->assertEquals($routeIndex, Router::match(Http::REQUEST_METHOD_GET, '/')); - $this->assertEquals($routeAbout, Router::match(Http::REQUEST_METHOD_GET, '/about')); - $this->assertEquals($routeAboutMe, Router::match(Http::REQUEST_METHOD_GET, '/about/me')); + $this->assertEquals($routeIndex, $this->match(Http::REQUEST_METHOD_GET, '/')); + $this->assertEquals($routeAbout, $this->match(Http::REQUEST_METHOD_GET, '/about')); + $this->assertEquals($routeAboutMe, $this->match(Http::REQUEST_METHOD_GET, '/about/me')); } public function testCanMatchUrlWithPlaceholder(): void @@ -44,12 +58,12 @@ public function testCanMatchUrlWithPlaceholder(): void Router::addRoute($routeBlogPostComments); Router::addRoute($routeBlogPostCommentsSingle); - $this->assertEquals($routeBlog, Router::match(Http::REQUEST_METHOD_GET, '/blog')); - $this->assertEquals($routeBlogAuthors, Router::match(Http::REQUEST_METHOD_GET, '/blog/authors')); - $this->assertEquals($routeBlogAuthorsComments, Router::match(Http::REQUEST_METHOD_GET, '/blog/authors/comments')); - $this->assertEquals($routeBlogPost, Router::match(Http::REQUEST_METHOD_GET, '/blog/test')); - $this->assertEquals($routeBlogPostComments, Router::match(Http::REQUEST_METHOD_GET, '/blog/test/comments')); - $this->assertEquals($routeBlogPostCommentsSingle, Router::match(Http::REQUEST_METHOD_GET, '/blog/test/comments/:comment')); + $this->assertEquals($routeBlog, $this->match(Http::REQUEST_METHOD_GET, '/blog')); + $this->assertEquals($routeBlogAuthors, $this->match(Http::REQUEST_METHOD_GET, '/blog/authors')); + $this->assertEquals($routeBlogAuthorsComments, $this->match(Http::REQUEST_METHOD_GET, '/blog/authors/comments')); + $this->assertEquals($routeBlogPost, $this->match(Http::REQUEST_METHOD_GET, '/blog/test')); + $this->assertEquals($routeBlogPostComments, $this->match(Http::REQUEST_METHOD_GET, '/blog/test/comments')); + $this->assertEquals($routeBlogPostCommentsSingle, $this->match(Http::REQUEST_METHOD_GET, '/blog/test/comments/:comment')); } public function testCanMatchUrlWithWildcard(): void @@ -62,11 +76,11 @@ public function testCanMatchUrlWithWildcard(): void Router::addRoute($routeAbout); Router::addRoute($routeAboutWildcard); - $this->assertEquals($routeIndex, Router::match('GET', '/')); - $this->assertEquals($routeAbout, Router::match('GET', '/about')); - $this->assertEquals($routeAboutWildcard, Router::match('GET', '/about/me')); - $this->assertEquals($routeAboutWildcard, Router::match('GET', '/about/you')); - $this->assertEquals($routeAboutWildcard, Router::match('GET', '/about/me/myself/i')); + $this->assertEquals($routeIndex, $this->match('GET', '/')); + $this->assertEquals($routeAbout, $this->match('GET', '/about')); + $this->assertEquals($routeAboutWildcard, $this->match('GET', '/about/me')); + $this->assertEquals($routeAboutWildcard, $this->match('GET', '/about/you')); + $this->assertEquals($routeAboutWildcard, $this->match('GET', '/about/me/myself/i')); } public function testCanMatchHttpMethod(): void @@ -77,11 +91,11 @@ public function testCanMatchHttpMethod(): void Router::addRoute($routeGET); Router::addRoute($routePOST); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/')); - $this->assertEquals($routePOST, Router::match(Http::REQUEST_METHOD_POST, '/')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/')); + $this->assertEquals($routePOST, $this->match(Http::REQUEST_METHOD_POST, '/')); - $this->assertNotEquals($routeGET, Router::match(Http::REQUEST_METHOD_POST, '/')); - $this->assertNotEquals($routePOST, Router::match(Http::REQUEST_METHOD_GET, '/')); + $this->assertNotEquals($routeGET, $this->match(Http::REQUEST_METHOD_POST, '/')); + $this->assertNotEquals($routePOST, $this->match(Http::REQUEST_METHOD_GET, '/')); } public function testCanMatchAlias(): void @@ -93,9 +107,9 @@ public function testCanMatchAlias(): void Router::addRoute($routeGET); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/target')); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/alias')); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/alias2')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/target')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/alias')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/alias2')); } public function testCanMatchMultipleAliases(): void @@ -108,10 +122,10 @@ public function testCanMatchMultipleAliases(): void Router::addRoute($routeGET); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/target')); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/alias1')); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/alias2')); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/alias3')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/target')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/alias1')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/alias2')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/alias3')); } public function testCanMatchMix(): void @@ -127,14 +141,14 @@ public function testCanMatchMix(): void Router::addRoute($routeGET); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/')); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/console')); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/invite')); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/login')); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/recover')); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/console/lorem/ipsum/dolor')); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/auth/lorem/ipsum')); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/register/lorem/ipsum')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/console')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/invite')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/login')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/recover')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/console/lorem/ipsum/dolor')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/auth/lorem/ipsum')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/register/lorem/ipsum')); } public function testCanMatchFilename(): void @@ -142,12 +156,12 @@ public function testCanMatchFilename(): void $routeGET = new Route(Http::REQUEST_METHOD_GET, '/robots.txt'); Router::addRoute($routeGET); - $this->assertEquals($routeGET, Router::match(Http::REQUEST_METHOD_GET, '/robots.txt')); + $this->assertEquals($routeGET, $this->match(Http::REQUEST_METHOD_GET, '/robots.txt')); } public function testCannotFindUnknownRouteByPath(): void { - $this->assertNull(Router::match(Http::REQUEST_METHOD_GET, '/404')); + $this->assertNull($this->match(Http::REQUEST_METHOD_GET, '/404')); } public function testCannotFindUnknownRouteByMethod(): void @@ -156,8 +170,8 @@ public function testCannotFindUnknownRouteByMethod(): void Router::addRoute($route); - $this->assertEquals($route, Router::match(Http::REQUEST_METHOD_GET, '/404')); + $this->assertEquals($route, $this->match(Http::REQUEST_METHOD_GET, '/404')); - $this->assertNull(Router::match(Http::REQUEST_METHOD_POST, '/404')); + $this->assertNull($this->match(Http::REQUEST_METHOD_POST, '/404')); } }