Skip to content

Commit 1493a84

Browse files
loks0nclaude
loks0n
and
claude
committed
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) <noreply@anthropic.com>
1 parent c6770ad commit 1493a84

5 files changed

Lines changed: 217 additions & 193 deletions

File tree

‎src/Http/Dispatcher.php‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,16 +76,7 @@ public function handle(): void
7676
}
7777

7878
$method = $this->request->getMethod();
79-
$url = \parse_url($this->request->getURI(), PHP_URL_PATH);
80-
$url = \is_string($url) ? ($url === '' ? '/' : $url) : '/';
81-
$matchMethod = (Http::REQUEST_METHOD_HEAD === $method) ? Http::REQUEST_METHOD_GET : $method;
82-
83-
$this->match = Router::matchRoute($matchMethod, $url);
84-
85-
if ($this->match === null && Http::getWildcardRoute() !== null) {
86-
$wildcard = Http::getWildcardRoute();
87-
$this->match = new RouteMatch($wildcard, $url, Router::WILDCARD_TOKEN, Router::WILDCARD_TOKEN);
88-
}
79+
$this->match = Router::matchRequest($this->request);
8980

9081
$this->http->setRequestResource('route', fn() => $this->match?->route);
9182
$this->http->setRequestResource('routeMatch', fn() => $this->match);

‎src/Http/Http.php‎

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,6 @@ class Http
5353
*/
5454
protected static string $mode = '';
5555

56-
/**
57-
* Wildcard route
58-
* If set, this get's executed if no other route is matched
59-
*/
60-
protected static ?Route $wildcardRoute = null;
61-
6256
/**
6357
* Compression
6458
*/
@@ -201,26 +195,15 @@ public static function delete(string $url): Route
201195
}
202196

203197
/**
204-
* Wildcard
205-
*
206-
* Add Wildcard route
198+
* Register a method-agnostic wildcard route. Invoked when no
199+
* method-specific route matches the incoming request.
207200
*/
208201
public static function wildcard(): Route
209202
{
210-
self::$wildcardRoute = new Route('', '');
211-
212-
return self::$wildcardRoute;
213-
}
203+
$route = new Route('', '');
204+
Router::setWildcard($route);
214205

215-
/**
216-
* Returns the registered wildcard route, if any.
217-
*
218-
* The returned Route is a shared definition and MUST NOT be mutated by
219-
* request-handling code. Per-request state belongs on {@see RouteMatch}.
220-
*/
221-
public static function getWildcardRoute(): ?Route
222-
{
223-
return self::$wildcardRoute;
206+
return $route;
224207
}
225208

226209
/**
@@ -688,6 +671,5 @@ public static function reset(): void
688671
Router::reset();
689672
Hooks::reset();
690673
self::$mode = '';
691-
self::$wildcardRoute = null;
692674
}
693675
}

‎src/Http/Router.php‎

Lines changed: 69 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ class Router
3232
*/
3333
protected static array $params = [];
3434

35+
/**
36+
* Method-agnostic wildcard route, used as last-resort fallback when no
37+
* method-specific route matches. Registered via {@see self::setWildcard()}.
38+
*/
39+
protected static ?Route $wildcard = null;
40+
3541
/**
3642
* Get all registered routes.
3743
*
@@ -106,67 +112,23 @@ public static function addRouteAlias(string $path, Route $route): void
106112
}
107113

108114
/**
109-
* Match route against the method and path.
110-
*
111-
* Returns an immutable {@see RouteMatch} carrying the matched route and
112-
* the per-request match facts (URL path, router table key, prepared
113-
* path). The shared {@see Route} definition is never mutated.
115+
* Register a method-agnostic wildcard route. Used as the last-resort
116+
* fallback when no method-specific route matches. At most one wildcard
117+
* can be registered; a subsequent call replaces the previous one.
114118
*/
115-
public static function matchRoute(string $method, string $path): ?RouteMatch
119+
public static function setWildcard(Route $route): void
116120
{
117-
if (!array_key_exists($method, self::$routes)) {
118-
return null;
119-
}
120-
121-
$parts = array_values(array_filter(explode('/', $path), fn($segment) => $segment !== ''));
122-
$length = count($parts) - 1;
123-
$filteredParams = array_filter(self::$params, fn($i) => $i <= $length);
124-
125-
foreach (self::combinations($filteredParams) as $sample) {
126-
$sample = array_filter($sample, fn(int $i) => $i <= $length);
127-
$match = implode(
128-
'/',
129-
array_replace(
130-
$parts,
131-
array_fill_keys($sample, self::PLACEHOLDER_TOKEN),
132-
),
133-
);
134-
135-
if (array_key_exists($match, self::$routes[$method])) {
136-
return new RouteMatch(self::$routes[$method][$match], $path, $match, $match);
137-
}
138-
}
139-
140-
/**
141-
* Match root wildcard.
142-
*/
143-
$match = self::WILDCARD_TOKEN;
144-
if (array_key_exists($match, self::$routes[$method])) {
145-
return new RouteMatch(self::$routes[$method][$match], $path, $match, $match);
146-
}
147-
148-
/**
149-
* Match wildcard for path segments.
150-
*/
151-
foreach ($parts as $part) {
152-
$current = ($current ?? '') . "{$part}/";
153-
$match = $current . self::WILDCARD_TOKEN;
154-
if (array_key_exists($match, self::$routes[$method])) {
155-
return new RouteMatch(self::$routes[$method][$match], $path, $match, $match);
156-
}
157-
}
158-
159-
return null;
121+
self::$wildcard = $route;
160122
}
161123

162124
/**
163125
* Match a {@see Request} against the router.
164126
*
165-
* Convenience wrapper around {@see Router::matchRoute()} that extracts
166-
* the path from the request URI and normalises HEAD to GET (the
127+
* Extracts the path from the request URI, normalises HEAD to GET (the
167128
* HEAD method is served by the GET handler with the response payload
168-
* disabled). Prefer this over hand-rolling the parse + HEAD logic at
169-
* every call site.
129+
* disabled), and returns an immutable {@see RouteMatch} carrying the
130+
* matched route and per-request match facts. The shared {@see Route}
131+
* definition is never mutated.
170132
*/
171133
public static function matchRequest(Request $request): ?RouteMatch
172134
{
@@ -180,14 +142,61 @@ public static function matchRequest(Request $request): ?RouteMatch
180142
}
181143

182144
/**
183-
* @deprecated Use {@see Router::matchRoute()} which returns a per-request
184-
* {@see RouteMatch}. The old signature returned the shared Route
185-
* definition; under concurrent request handling (Swoole coroutines)
186-
* mutating that return value is racy.
145+
* Match against a (method, path) pair. Internal — application code should
146+
* call {@see self::matchRequest()} so URL parsing and HEAD normalisation
147+
* are handled consistently.
187148
*/
188-
public static function match(string $method, string $path): ?Route
149+
private static function matchRoute(string $method, string $path): ?RouteMatch
189150
{
190-
return self::matchRoute($method, $path)?->route;
151+
if (array_key_exists($method, self::$routes)) {
152+
$parts = array_values(array_filter(explode('/', $path), fn($segment) => $segment !== ''));
153+
$length = count($parts) - 1;
154+
$filteredParams = array_filter(self::$params, fn($i) => $i <= $length);
155+
156+
foreach (self::combinations($filteredParams) as $sample) {
157+
$sample = array_filter($sample, fn(int $i) => $i <= $length);
158+
$match = implode(
159+
'/',
160+
array_replace(
161+
$parts,
162+
array_fill_keys($sample, self::PLACEHOLDER_TOKEN),
163+
),
164+
);
165+
166+
if (array_key_exists($match, self::$routes[$method])) {
167+
return new RouteMatch(self::$routes[$method][$match], $path, $match, $match);
168+
}
169+
}
170+
171+
/**
172+
* Match root wildcard for this method (e.g. GET /*).
173+
*/
174+
$match = self::WILDCARD_TOKEN;
175+
if (array_key_exists($match, self::$routes[$method])) {
176+
return new RouteMatch(self::$routes[$method][$match], $path, $match, $match);
177+
}
178+
179+
/**
180+
* Match wildcard for path segments (e.g. GET /foo/*).
181+
*/
182+
foreach ($parts as $part) {
183+
$current = ($current ?? '') . "{$part}/";
184+
$match = $current . self::WILDCARD_TOKEN;
185+
if (array_key_exists($match, self::$routes[$method])) {
186+
return new RouteMatch(self::$routes[$method][$match], $path, $match, $match);
187+
}
188+
}
189+
}
190+
191+
/**
192+
* Fall through to the method-agnostic wildcard registered via
193+
* {@see self::setWildcard()}.
194+
*/
195+
if (self::$wildcard !== null) {
196+
return new RouteMatch(self::$wildcard, $path, self::WILDCARD_TOKEN, self::WILDCARD_TOKEN);
197+
}
198+
199+
return null;
191200
}
192201

193202
/**
@@ -248,6 +257,7 @@ public static function preparePath(string $path): array
248257
public static function reset(): void
249258
{
250259
self::$params = [];
260+
self::$wildcard = null;
251261
self::$routes = [
252262
Http::REQUEST_METHOD_GET => [],
253263
Http::REQUEST_METHOD_POST => [],

0 commit comments

Comments
 (0)