Skip to content

Commit dcb3855

Browse files
Elias-BlackElias-Black
committed
fix: routes keys must not be its names
1 parent c377646 commit dcb3855

File tree

5 files changed

+133
-9
lines changed

5 files changed

+133
-9
lines changed

‎system/Commands/Utilities/Routes.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public function run(array $params)
112112
$sampleUri = $uriGenerator->get($route['route']);
113113
$filters = $filterCollector->get($route['method'], $sampleUri);
114114

115-
$routeName = ($route['route'] === $route['name']) ? '»' : $route['name'];
115+
$routeName = $route['name'] ?? '»';
116116

117117
$tbody[] = [
118118
strtoupper($route['method']),

‎system/Router/DefinedRouteCollector.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public function collect(): Generator
4949
$handler = $view ? '(View) ' . $view : '(Closure)';
5050
}
5151

52-
$routeName = $this->routeCollection->getRoutesOptions($route, $method)['as'] ?? $route;
52+
$routeName = $this->routeCollection->getRoutesOptions($route, $method)['as'] ?? null;
5353

5454
yield [
5555
'method' => $method,

‎system/Router/RouteCollection.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1541,7 +1541,8 @@ protected function create(string $verb, string $from, $to, ?array $options = nul
15411541
// this works only because discovered routes are added just prior
15421542
// to attempting to route the request.
15431543
$routeKeyExists = isset($this->routes[$verb][$routeKey]);
1544-
if ((isset($this->routesNames[$verb][$name]) || $routeKeyExists) && ! $overwrite) {
1544+
$routeNameExists = isset($this->routesNames[$verb][$routeKey]);
1545+
if (($routeNameExists || $routeKeyExists) && ! $overwrite) {
15451546
return;
15461547
}
15471548

‎tests/system/Router/DefinedRouteCollectorTest.php

Lines changed: 102 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,37 +67,37 @@ public function testCollect(): void
6767
[
6868
'method' => 'GET',
6969
'route' => 'journals',
70-
'name' => 'journals',
70+
'name' => null,
7171
'handler' => '\App\Controllers\Blogs',
7272
],
7373
[
7474
'method' => 'GET',
7575
'route' => '100',
76-
'name' => '100',
76+
'name' => null,
7777
'handler' => '\App\Controllers\Home::index',
7878
],
7979
[
8080
'method' => 'GET',
8181
'route' => 'product/([0-9]+)',
82-
'name' => 'product/([0-9]+)',
82+
'name' => null,
8383
'handler' => '\App\Controllers\Catalog::productLookupByID/$1',
8484
],
8585
[
8686
'method' => 'GET',
8787
'route' => 'feed',
88-
'name' => 'feed',
88+
'name' => null,
8989
'handler' => '(Closure)',
9090
],
9191
[
9292
'method' => 'GET',
9393
'route' => '200',
94-
'name' => '200',
94+
'name' => null,
9595
'handler' => '(Closure)',
9696
],
9797
[
9898
'method' => 'GET',
9999
'route' => 'about',
100-
'name' => 'about',
100+
'name' => null,
101101
'handler' => '(View) pages/about',
102102
],
103103
];
@@ -144,4 +144,100 @@ public function testCollectSameFromWithDifferentVerb(): void
144144
];
145145
$this->assertSame($expected, $definedRoutes);
146146
}
147+
148+
/**
149+
* @see https://github.com/codeigniter4/CodeIgniter4/pull/9499
150+
*/
151+
public function testCollectSameNameWithDifferentFrom(): void
152+
{
153+
$routes = $this->createRouteCollection();
154+
$routes->get('/', 'HomeController::index');
155+
$routes->post('login', 'AuthController::login', ['as' => 'login']);
156+
$routes->get('logout', 'AuthController::logout', ['as' => 'logout']);
157+
158+
$collector = new DefinedRouteCollector($routes);
159+
160+
$definedRoutes = [];
161+
162+
foreach ($collector->collect() as $route) {
163+
$definedRoutes[] = $route;
164+
}
165+
166+
$expected = [
167+
[
168+
'method' => 'GET',
169+
'route' => '/',
170+
'name' => null,
171+
'handler' => '\\App\\Controllers\\HomeController::index',
172+
],
173+
[
174+
'method' => 'GET',
175+
'route' => 'logout',
176+
'name' => 'logout',
177+
'handler' => '\\App\\Controllers\\AuthController::logout',
178+
],
179+
[
180+
'method' => 'POST',
181+
'route' => 'login',
182+
'name' => 'login',
183+
'handler' => '\\App\\Controllers\\AuthController::login',
184+
],
185+
];
186+
$this->assertSame($expected, $definedRoutes);
187+
}
188+
189+
/**
190+
* @see https://github.com/codeigniter4/CodeIgniter4/pull/9499
191+
*/
192+
public function testCollectSameNames(): void
193+
{
194+
$routes = $this->createRouteCollection();
195+
$routes->get('/', 'HomeController::index');
196+
$routes->post('login', 'AuthController::login', ['as' => 'login']);
197+
$routes->post('user/login', 'UserController::login', ['as' => 'login']);
198+
$routes->get('logout', 'AuthController::logout', ['as' => 'logout']);
199+
$routes->get('user/logout', 'UserController::logout', ['as' => 'logout']);
200+
201+
$collector = new DefinedRouteCollector($routes);
202+
203+
$definedRoutes = [];
204+
205+
foreach ($collector->collect() as $route) {
206+
$definedRoutes[] = $route;
207+
}
208+
209+
$expected = [
210+
[
211+
'method' => 'GET',
212+
'route' => '/',
213+
'name' => null,
214+
'handler' => '\\App\\Controllers\\HomeController::index',
215+
],
216+
[
217+
'method' => 'GET',
218+
'route' => 'logout',
219+
'name' => 'logout',
220+
'handler' => '\\App\\Controllers\\AuthController::logout',
221+
],
222+
[
223+
'method' => 'GET',
224+
'route' => 'user/logout',
225+
'name' => 'logout',
226+
'handler' => '\\App\\Controllers\\UserController::logout',
227+
],
228+
[
229+
'method' => 'POST',
230+
'route' => 'login',
231+
'name' => 'login',
232+
'handler' => '\\App\\Controllers\\AuthController::login',
233+
],
234+
[
235+
'method' => 'POST',
236+
'route' => 'user/login',
237+
'name' => 'login',
238+
'handler' => '\\App\\Controllers\\UserController::login',
239+
],
240+
];
241+
$this->assertSame($expected, $definedRoutes);
242+
}
147243
}

‎tests/system/Router/RouteCollectionTest.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,6 +1105,33 @@ public function testNamedRoutesWithPipesInRegex(): void
11051105
$this->assertSame('/system/that', $routes->reverseRoute('pipedRoute', 'that'));
11061106
}
11071107

1108+
public function testNamedRouteAsPath(): void
1109+
{
1110+
service('request')->setMethod(Method::GET);
1111+
$routes = $this->getCollector();
1112+
1113+
$expected = [
1114+
'here' => '\there',
1115+
'from' => '\to',
1116+
];
1117+
1118+
$routes->get('here', 'there');
1119+
$routes->get('from', 'to', ['as' => 'here']);
1120+
1121+
$this->assertSame($expected, $routes->getRoutes());
1122+
}
1123+
1124+
public function testSameNamedRoutes(): void
1125+
{
1126+
service('request')->setMethod(Method::GET);
1127+
$routes = $this->getCollector();
1128+
1129+
$routes->get('from1', 'to1', ['as' => 'namedRoute']);
1130+
$routes->get('from2', 'to2', ['as' => 'namedRoute']);
1131+
1132+
$this->assertSame('/from2', $routes->reverseRoute('namedRoute'));
1133+
}
1134+
11081135
public function testAddRedirect(): void
11091136
{
11101137
$routes = $this->getCollector();

0 commit comments

Comments
 (0)