Skip to content

Commit fed2ebb

Browse files
papacpapac
committed
Refactoring routing
1 parent b7ed438 commit fed2ebb

File tree

2 files changed

+84
-36
lines changed

2 files changed

+84
-36
lines changed

‎src/Router/Route.php‎

Lines changed: 74 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public function __construct(string $path, mixed $callback)
8484
{
8585
$this->config = Loader::getInstance();
8686
$this->callback = $callback;
87-
$this->path = str_replace('.', '\\.', $path);
87+
$this->path = $path;
8888
$this->match = [];
8989
}
9090

@@ -159,7 +159,8 @@ public function call(): mixed
159159
{
160160
// Association of parameters at the request
161161
foreach ($this->keys as $key => $value) {
162-
if (!isset($this->match[$key])) {
162+
if (!isset($this->match[$key]) || $this->match[$key] === null) {
163+
$this->params[$value] = null;
163164
continue;
164165
}
165166

@@ -173,7 +174,10 @@ public function call(): mixed
173174
$this->match[$key] = $tmp;
174175
}
175176

176-
return Compass::getInstance()->call($this->callback, $this->match);
177+
// Filter out null values before passing to Compass
178+
$args = array_filter($this->match, fn($v) => $v !== null);
179+
180+
return Compass::getInstance()->call($this->callback, $args);
177181
}
178182

179183
/**
@@ -246,13 +250,32 @@ public function getParameter(string $key): ?string
246250
*/
247251
public function match(string $uri, ?string $host = null): bool
248252
{
249-
// If a domain constraint is set, check the host
253+
// If a domain constraint is set, check the host and capture params
250254
if ($this->domain !== null && $host !== null) {
251-
// Convert domain pattern to regex (support wildcards like *.example.com)
252-
$pattern = str_replace(['.', '*'], ['\\.', '.*'], $this->domain);
253-
if (!preg_match('/^' . $pattern . '$/i', $host)) {
255+
$domain_param_names = [];
256+
$domain_pattern = $this->domain;
257+
// Build regex for domain with parameter capture (supports :param and <param>)
258+
$domain_pattern = preg_replace_callback(
259+
'/(:([a-zA-Z0-9_]+)|<([a-zA-Z0-9_]+)>)/',
260+
function ($m) use (&$domain_param_names) {
261+
$name = $m[2] !== '' ? $m[2] : $m[3];
262+
$domain_param_names[] = $name;
263+
return '([^.]+)';
264+
},
265+
$domain_pattern
266+
);
267+
// Escape dots and handle wildcards
268+
$domain_pattern = str_replace(['.', '*'], ['\\.', '[^.]+'], $domain_pattern);
269+
if (!preg_match('~^' . $domain_pattern . '$~i', $host, $domain_matches)) {
254270
return false;
255271
}
272+
// Store domain params
273+
array_shift($domain_matches);
274+
foreach ($domain_param_names as $i => $name) {
275+
if (isset($domain_matches[$i])) {
276+
$this->params[$name] = $domain_matches[$i];
277+
}
278+
}
256279
}
257280

258281
// Normalization of the url of the navigator.
@@ -270,34 +293,59 @@ public function match(string $uri, ?string $host = null): bool
270293
return true;
271294
}
272295

273-
// We check the length of the path defined by the programmer
274-
// with that of the current url in the user's browser.
275-
$path = implode('', preg_split('/(\/:[a-z0-9-_]+\?)/', $this->path));
276-
277-
if (count(explode('/', $path)) != count(explode('/', $uri))) {
278-
if (count(explode('/', $this->path)) != count(explode('/', $uri))) {
279-
return false;
296+
// Check segment count (accounting for optional params)
297+
$route_segments = explode('/', trim($this->path, '/'));
298+
$uri_segments = explode('/', trim($uri, '/'));
299+
$optional_count = 0;
300+
foreach ($route_segments as $seg) {
301+
if (preg_match('/^(:[a-zA-Z0-9_]+\?|<[a-zA-Z0-9_]+\?>)$/', $seg)) {
302+
$optional_count++;
280303
}
281304
}
305+
$route_required = count($route_segments) - $optional_count;
306+
$uri_count = count($uri_segments);
307+
if ($uri_count < $route_required || $uri_count > count($route_segments)) {
308+
return false;
309+
}
282310

283-
// Copied of url
284-
$path = $uri;
285-
286-
// In case the developer did not add of constraint on captured variables
311+
// Robust regex builder for path parameters (supports :param, <param>, optional, required)
287312
if (empty($this->with)) {
288-
$path = preg_replace('~:\w+(\?)?~', '([^\s]+)$1', $this->path);
289-
290-
preg_match_all('~:([a-z-0-9_-]+?)\?~', $this->path, $this->keys);
291-
292-
$this->keys = end($this->keys);
293-
294-
return $this->checkRequestUri($path, $uri);
313+
$param_names = [];
314+
$regex_parts = [];
315+
foreach ($route_segments as $seg) {
316+
/** Optional :param? or <param?> */
317+
if (preg_match('/^:([a-zA-Z0-9_]+)\?$/', $seg, $m) || preg_match('/^<([a-zA-Z0-9_]+)\?>$/', $seg, $m)) {
318+
$param_names[] = $m[1];
319+
$regex_parts[] = '(?:/([^/]+))?';
320+
}
321+
// Required :param or <param>
322+
elseif (preg_match('/^:([a-zA-Z0-9_]+)$/', $seg, $m) || preg_match('/^<([a-zA-Z0-9_]+)>$/', $seg, $m)) {
323+
$param_names[] = $m[1];
324+
$regex_parts[] = '/([^/]+)';
325+
}
326+
// Static segment
327+
else {
328+
$regex_parts[] = '/' . preg_quote($seg, '~');
329+
}
330+
}
331+
$regex = '~^' . implode('', $regex_parts) . '$~';
332+
$this->keys = $param_names;
333+
// Build URI with leading slash for matching
334+
$normalized_uri = '/' . implode('/', $uri_segments);
335+
if (!preg_match($regex, $normalized_uri, $matches)) {
336+
return false;
337+
}
338+
array_shift($matches);
339+
// Pad missing optionals with null
340+
$matches = array_pad($matches, count($this->keys), null);
341+
$this->match = $matches;
342+
return true;
295343
}
296344

297345
// In case the developer has added constraints
298346
// on the captured variables
299347
if (!preg_match_all('~:([\w]+)?~', $this->path, $match)) {
300-
return $this->checkRequestUri($path, $uri);
348+
return $this->checkRequestUri($this->path, $uri);
301349
}
302350

303351
$tmp_path = $this->path;

‎tests/Routing/RouteTest.php‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,21 +94,21 @@ public function test_uri_with_optionnal_parameter()
9494
}
9595

9696

97-
public function testRouteMatchesDomainAndPath()
97+
public function test_route_matches_domain_and_path()
9898
{
9999
$route = (new Route('/foo/bar', fn() => 'ok'))
100100
->withDomain('sub.example.com');
101101
$this->assertTrue($route->match('/foo/bar', 'sub.example.com'));
102102
}
103103

104-
public function testRouteDoesNotMatchWrongDomain()
104+
public function test_route_does_not_match_wrong_domain()
105105
{
106106
$route = (new Route('/foo/bar', fn() => 'ok'))
107107
->withDomain('sub.example.com');
108108
$this->assertFalse($route->match('/foo/bar', 'other.example.com'));
109109
}
110110

111-
public function testRouteMatchesWildcardDomain()
111+
public function test_route_matches_wildcard_domain()
112112
{
113113
$route = (new Route('/foo/bar', fn() => 'ok'))
114114
->withDomain('*.example.com');
@@ -117,28 +117,28 @@ public function testRouteMatchesWildcardDomain()
117117
$this->assertFalse($route->match('/foo/bar', 'example.com'));
118118
}
119119

120-
public function testRouteMatchesWithoutDomainConstraint()
120+
public function test_route_matches_without_domain_constraint()
121121
{
122122
$route = new Route('/foo/bar', fn() => 'ok');
123123
$this->assertTrue($route->match('/foo/bar', 'any.domain.com'));
124124
}
125125

126-
public function testRouteDoesNotMatchIfPathWrongEvenIfDomainMatches()
126+
public function test_route_does_not_match_if_path_wrong_even_if_domain_matches()
127127
{
128128
$route = (new Route('/foo/bar', fn() => 'ok'))
129129
->withDomain('sub.example.com');
130130
$this->assertFalse($route->match('/foo/other', 'sub.example.com'));
131131
}
132132

133-
public function testRouteCapturesSubdomainParameter()
133+
public function test_route_captures_subdomain_parameter()
134134
{
135135
$route = (new Route('/foo/bar', fn() => 'ok'))
136136
->withDomain(':sub.example.com');
137137
$this->assertTrue($route->match('/foo/bar', 'app.example.com'));
138138
$this->assertEquals('app', $route->getParameter('sub'));
139139
}
140140

141-
public function testRouteCapturesMultipleDomainParameters()
141+
public function test_route_captures_multiple_domain_parameters()
142142
{
143143
$route = (new Route('/foo/bar', fn() => 'ok'))
144144
->withDomain(':sub.:env.example.com');
@@ -147,15 +147,15 @@ public function testRouteCapturesMultipleDomainParameters()
147147
$this->assertEquals('dev', $route->getParameter('env'));
148148
}
149149

150-
public function testRouteDoesNotMatchIfDomainParameterWrong()
150+
public function test_route_does_not_match_if_domain_parameter_wrong()
151151
{
152152
$route = (new Route('/foo/bar', fn() => 'ok'))
153153
->withDomain(':sub.example.com');
154154
$this->assertFalse($route->match('/foo/bar', 'example.com'));
155155
$this->assertNull($route->getParameter('sub'));
156156
}
157157

158-
public function testRouteDomainParameterWithWildcard()
158+
public function test_route_domain_parameter_with_wildcard()
159159
{
160160
$route = (new Route('/foo/bar', fn() => 'ok'))
161161
->withDomain(':sub.*.example.com');
@@ -184,7 +184,7 @@ public function test_angle_bracket_multiple_params_in_path()
184184

185185
public function test_angle_bracket_optional_param_in_path()
186186
{
187-
$route = new Route('/foo/<bar>?', function ($bar = null) {
187+
$route = new Route('/foo/<bar?>', function ($bar = null) {
188188
return $bar ?? 'none';
189189
});
190190
$this->assertTrue($route->match('/foo'));

0 commit comments

Comments
 (0)