diff --git a/src/Illuminate/Routing/RouteCollection.php b/src/Illuminate/Routing/RouteCollection.php index 9d6a087204c4..defdf55a17ed 100644 --- a/src/Illuminate/Routing/RouteCollection.php +++ b/src/Illuminate/Routing/RouteCollection.php @@ -79,7 +79,7 @@ protected function addLookups($route) // If the route has a name, we will add it to the name look-up table, so that we // will quickly be able to find the route associated with a name and not have // to iterate through every route every time we need to find a named route. - if ($name = $route->getName()) { + if (($name = $route->getName()) && ! $this->inNameLookup($name)) { $this->nameList[$name] = $route; } @@ -88,7 +88,7 @@ protected function addLookups($route) // processing a request and easily generate URLs to the given controllers. $action = $route->getAction(); - if (isset($action['controller'])) { + if (($controller = $action['controller'] ?? null) && ! $this->inActionLookup($controller)) { $this->addToActionList($action, $route); } } @@ -105,6 +105,28 @@ protected function addToActionList($action, $route) $this->actionList[trim($action['controller'], '\\')] = $route; } + /** + * Determine if the given controller is in the action lookup table. + * + * @param string $controller + * @return bool + */ + protected function inActionLookup($controller) + { + return array_key_exists($controller, $this->actionList); + } + + /** + * Determine if the given name is in the name lookup table. + * + * @param string $name + * @return bool + */ + protected function inNameLookup($name) + { + return array_key_exists($name, $this->nameList); + } + /** * Refresh the name look-up table. * @@ -117,8 +139,8 @@ public function refreshNameLookups() $this->nameList = []; foreach ($this->allRoutes as $route) { - if ($route->getName()) { - $this->nameList[$route->getName()] = $route; + if (($name = $route->getName()) && ! $this->inNameLookup($name)) { + $this->nameList[$name] = $route; } } } @@ -135,7 +157,7 @@ public function refreshActionLookups() $this->actionList = []; foreach ($this->allRoutes as $route) { - if (isset($route->getAction()['controller'])) { + if (($controller = $route->getAction()['controller'] ?? null) && ! $this->inActionLookup($controller)) { $this->addToActionList($route->getAction(), $route); } } diff --git a/tests/Integration/Routing/CompiledRouteCollectionTest.php b/tests/Integration/Routing/CompiledRouteCollectionTest.php index fcaa4dad5d4e..35d12753b5ca 100644 --- a/tests/Integration/Routing/CompiledRouteCollectionTest.php +++ b/tests/Integration/Routing/CompiledRouteCollectionTest.php @@ -94,6 +94,38 @@ public function testRouteCollectionCanRetrieveByAction() $this->assertSame($action, Arr::except($route->getAction(), 'as')); } + public function testCompiledAndNonCompiledUrlResolutionHasSamePrecedenceForActions() + { + @unlink(__DIR__.'/Fixtures/cache/routes-v7.php'); + $this->app->useBootstrapPath(__DIR__.'/Fixtures'); + $app = (static function () { + $refresh = true; + + return require __DIR__.'/Fixtures/app.php'; + })(); + $app['router']->get('/foo/{bar}', ['FooController', 'show']); + $app['router']->get('/foo/{bar}/{baz}', ['FooController', 'show']); + $app['router']->getRoutes()->refreshActionLookups(); + + $this->assertSame('foo/{bar}', $app['router']->getRoutes()->getByAction('FooController@show')->uri); + + $this->artisan('route:cache')->assertExitCode(0); + require __DIR__.'/Fixtures/cache/routes-v7.php'; + + $this->assertSame('foo/{bar}', $app['router']->getRoutes()->getByAction('FooController@show')->uri); + + unlink(__DIR__.'/Fixtures/cache/routes-v7.php'); + } + + public function testCompiledAndNonCompiledUrlResolutionHasSamePrecedenceForNames() + { + $this->router->get('/foo/{bar}', ['FooController', 'show'])->name('foo.show'); + $this->router->get('/foo/{bar}/{baz}', ['FooController', 'show'])->name('foo.show'); + $this->router->getRoutes()->refreshNameLookups(); + + $this->assertSame('foo/{bar}', $this->router->getRoutes()->getByName('foo.show')->uri); + } + public function testRouteCollectionCanGetIterator() { $this->routeCollection->add($this->newRoute('GET', 'foo/index', [ diff --git a/tests/Integration/Routing/Fixtures/app.php b/tests/Integration/Routing/Fixtures/app.php new file mode 100644 index 000000000000..f0921fd8b3ae --- /dev/null +++ b/tests/Integration/Routing/Fixtures/app.php @@ -0,0 +1,18 @@ +create(); +} else { + return AppCache::$app ??= Application::configure(basePath: __DIR__)->create(); +} diff --git a/tests/Integration/Routing/Fixtures/bootstrap/cache/.gitignore b/tests/Integration/Routing/Fixtures/bootstrap/cache/.gitignore new file mode 100644 index 000000000000..d6b7ef32c847 --- /dev/null +++ b/tests/Integration/Routing/Fixtures/bootstrap/cache/.gitignore @@ -0,0 +1,2 @@ +* +!.gitignore diff --git a/tests/Integration/Routing/Fixtures/cache/.gitignore b/tests/Integration/Routing/Fixtures/cache/.gitignore new file mode 100644 index 000000000000..d6b7ef32c847 --- /dev/null +++ b/tests/Integration/Routing/Fixtures/cache/.gitignore @@ -0,0 +1,2 @@ +* +!.gitignore diff --git a/tests/Routing/RouteRegistrarTest.php b/tests/Routing/RouteRegistrarTest.php index d92967a758c3..2c0c4cca9350 100644 --- a/tests/Routing/RouteRegistrarTest.php +++ b/tests/Routing/RouteRegistrarTest.php @@ -719,6 +719,8 @@ public function testCanSetShallowOptionOnRegisteredResource() public function testCanSetScopedOptionOnRegisteredResource() { $this->router->resource('users.tasks', RouteRegistrarControllerStub::class)->scoped(); + $this->router->getRoutes()->refreshNameLookups(); + $this->assertSame( ['user' => null], $this->router->getRoutes()->getByName('users.tasks.index')->bindingFields() @@ -731,6 +733,7 @@ public function testCanSetScopedOptionOnRegisteredResource() $this->router->resource('users.tasks', RouteRegistrarControllerStub::class)->scoped([ 'task' => 'slug', ]); + $this->router->getRoutes()->refreshNameLookups(); $this->assertSame( ['user' => null], $this->router->getRoutes()->getByName('users.tasks.index')->bindingFields() @@ -904,6 +907,7 @@ public function testCanSetMiddlewareForSpecifiedMethodsOnRegisteredResource() ->middlewareFor('index', RouteRegistrarMiddlewareStub::class) ->middlewareFor(['create', 'store'], 'one') ->middlewareFor(['edit'], ['one', 'two']); + $this->router->getRoutes()->refreshNameLookups(); $this->assertEquals($this->router->getRoutes()->getByName('users.index')->gatherMiddleware(), ['default', RouteRegistrarMiddlewareStub::class]); $this->assertEquals($this->router->getRoutes()->getByName('users.create')->gatherMiddleware(), ['default', 'one']); @@ -918,6 +922,7 @@ public function testCanSetMiddlewareForSpecifiedMethodsOnRegisteredResource() ->middlewareFor(['create', 'store'], 'one') ->middlewareFor(['edit'], ['one', 'two']) ->middleware('default'); + $this->router->getRoutes()->refreshNameLookups(); $this->assertEquals($this->router->getRoutes()->getByName('users.index')->gatherMiddleware(), [RouteRegistrarMiddlewareStub::class, 'default']); $this->assertEquals($this->router->getRoutes()->getByName('users.create')->gatherMiddleware(), ['one', 'default']); @@ -1448,6 +1453,7 @@ public function testCanSetMiddlewareForSpecifiedMethodsOnRegisteredSingletonReso ->middlewareFor('show', RouteRegistrarMiddlewareStub::class) ->middlewareFor(['create', 'store'], 'one') ->middlewareFor(['edit'], ['one', 'two']); + $this->router->getRoutes()->refreshNameLookups(); $this->assertEquals($this->router->getRoutes()->getByName('users.create')->gatherMiddleware(), ['default', 'one']); $this->assertEquals($this->router->getRoutes()->getByName('users.store')->gatherMiddleware(), ['default', 'one']); @@ -1463,6 +1469,7 @@ public function testCanSetMiddlewareForSpecifiedMethodsOnRegisteredSingletonReso ->middlewareFor(['create', 'store'], 'one') ->middlewareFor(['edit'], ['one', 'two']) ->middleware('default'); + $this->router->getRoutes()->refreshNameLookups(); $this->assertEquals($this->router->getRoutes()->getByName('users.create')->gatherMiddleware(), ['one', 'default']); $this->assertEquals($this->router->getRoutes()->getByName('users.store')->gatherMiddleware(), ['one', 'default']);