diff --git a/src/Illuminate/Routing/CompiledRouteCollection.php b/src/Illuminate/Routing/CompiledRouteCollection.php index 45791b7f4c22..b271bce53ab8 100644 --- a/src/Illuminate/Routing/CompiledRouteCollection.php +++ b/src/Illuminate/Routing/CompiledRouteCollection.php @@ -5,6 +5,8 @@ use Illuminate\Container\Container; use Illuminate\Http\Request; use Illuminate\Support\Collection; +use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\Routing\Exception\MethodNotAllowedException; use Symfony\Component\Routing\Exception\ResourceNotFoundException; use Symfony\Component\Routing\Matcher\CompiledUrlMatcher; @@ -26,6 +28,13 @@ class CompiledRouteCollection extends AbstractRouteCollection */ protected $attributes = []; + /** + * An array of the routes that were added after loading the compiled routes. + * + * @var \Illuminate\Routing\RouteCollection|null + */ + protected $routes; + /** * The router instance used by the route. * @@ -51,6 +60,7 @@ public function __construct(array $compiled, array $attributes) { $this->compiled = $compiled; $this->attributes = $attributes; + $this->routes = new RouteCollection; } /** @@ -61,21 +71,7 @@ public function __construct(array $compiled, array $attributes) */ public function add(Route $route) { - $name = $route->getName() ?: $this->generateRouteName(); - - $this->attributes[$name] = [ - 'methods' => $route->methods(), - 'uri' => $route->uri(), - 'action' => $route->getAction() + ['as' => $name], - 'fallback' => $route->isFallback, - 'defaults' => $route->defaults, - 'wheres' => $route->wheres, - 'bindingFields' => $route->bindingFields(), - ]; - - $this->compiled = []; - - return $route; + return $this->routes->add($route); } /** @@ -108,41 +104,32 @@ public function refreshActionLookups() * @param \Illuminate\Http\Request $request * @return \Illuminate\Routing\Route * + * @throws \Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException */ public function match(Request $request) { - if (empty($this->compiled) && $this->attributes) { - $this->recompileRoutes(); - } - - $route = null; - $matcher = new CompiledUrlMatcher( $this->compiled, (new RequestContext)->fromRequest($request) ); + $route = null; + try { if ($result = $matcher->matchRequest($request)) { $route = $this->getByName($result['_route']); } } catch (ResourceNotFoundException | MethodNotAllowedException $e) { - // + try { + return $this->routes->match($request); + } catch (NotFoundHttpException | MethodNotAllowedHttpException $e) { + // + } } return $this->handleMatchedRoute($request, $route); } - /** - * Recompile the routes from the attributes array. - * - * @return void - */ - protected function recompileRoutes() - { - $this->compiled = $this->dumper()->getCompiledRoutes(); - } - /** * Get routes from the collection by method. * @@ -162,7 +149,7 @@ public function get($method = null) */ public function hasNamedRoute($name) { - return isset($this->attributes[$name]); + return isset($this->attributes[$name]) || $this->routes->hasNamedRoute($name); } /** @@ -173,7 +160,11 @@ public function hasNamedRoute($name) */ public function getByName($name) { - return isset($this->attributes[$name]) ? $this->newRoute($this->attributes[$name]) : null; + if (isset($this->attributes[$name])) { + return $this->newRoute($this->attributes[$name]); + } + + return $this->routes->getByName($name); } /** @@ -192,7 +183,11 @@ public function getByAction($action) return $attributes['action']['uses'] === $action; }); - return $attributes ? $this->newRoute($attributes) : null; + if ($attributes) { + return $this->newRoute($attributes); + } + + return $this->routes->getByAction($action); } /** @@ -202,7 +197,13 @@ public function getByAction($action) */ public function getRoutes() { - return $this->mapAttributesToRoutes()->values()->all(); + return collect($this->attributes) + ->map(function (array $attributes) { + return $this->newRoute($attributes); + }) + ->merge($this->routes->getRoutes()) + ->values() + ->all(); } /** @@ -212,7 +213,7 @@ public function getRoutes() */ public function getRoutesByMethod() { - return $this->mapAttributesToRoutes() + return collect($this->getRoutes()) ->groupBy(function (Route $route) { return $route->methods(); }) @@ -231,21 +232,11 @@ public function getRoutesByMethod() */ public function getRoutesByName() { - return $this->mapAttributesToRoutes()->keyBy(function (Route $route) { - return $route->getName(); - })->all(); - } - - /** - * Get all of the routes in the collection. - * - * @return \Illuminate\Support\Collection - */ - public function mapAttributesToRoutes() - { - return collect($this->attributes)->map(function (array $attributes) { - return $this->newRoute($attributes); - }); + return collect($this->getRoutes()) + ->keyBy(function (Route $route) { + return $route->getName(); + }) + ->all(); } /** diff --git a/src/Illuminate/Routing/RouteCollection.php b/src/Illuminate/Routing/RouteCollection.php index 8d0a3b279ddd..7e6f98bca055 100644 --- a/src/Illuminate/Routing/RouteCollection.php +++ b/src/Illuminate/Routing/RouteCollection.php @@ -2,6 +2,7 @@ namespace Illuminate\Routing; +use Illuminate\Container\Container; use Illuminate\Http\Request; use Illuminate\Support\Arr; @@ -146,6 +147,7 @@ public function refreshActionLookups() * @param \Illuminate\Http\Request $request * @return \Illuminate\Routing\Route * + * @throws \Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException */ public function match(Request $request) @@ -247,4 +249,20 @@ public function toSymfonyRouteCollection() return $symfonyRoutes; } + + /** + * Convert the collection to a CompiledRouteCollection instance. + * + * @param \Illuminate\Routing\Router $router + * @param \Illuminate\Container\Container $container + * @return \Illuminate\Routing\CompiledRouteCollection + */ + public function toCompiledRouteCollection(Router $router, Container $container) + { + ['compiled' => $compiled, 'attributes' => $attributes] = $this->compile(); + + return (new CompiledRouteCollection($compiled, $attributes)) + ->setRouter($router) + ->setContainer($container); + } } diff --git a/src/Illuminate/Routing/RouteCollectionInterface.php b/src/Illuminate/Routing/RouteCollectionInterface.php index 6a6ba95f34b3..8e25d0fa1056 100644 --- a/src/Illuminate/Routing/RouteCollectionInterface.php +++ b/src/Illuminate/Routing/RouteCollectionInterface.php @@ -38,6 +38,7 @@ public function refreshActionLookups(); * @param \Illuminate\Http\Request $request * @return \Illuminate\Routing\Route * + * @throws \Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException */ public function match(Request $request); diff --git a/tests/Integration/Routing/CompiledRouteCollectionTest.php b/tests/Integration/Routing/CompiledRouteCollectionTest.php index 05e79afeb8f4..7226d3dbb988 100644 --- a/tests/Integration/Routing/CompiledRouteCollectionTest.php +++ b/tests/Integration/Routing/CompiledRouteCollectionTest.php @@ -4,8 +4,8 @@ use ArrayIterator; use Illuminate\Http\Request; -use Illuminate\Routing\CompiledRouteCollection; use Illuminate\Routing\Route; +use Illuminate\Routing\RouteCollection; use Illuminate\Support\Arr; use Illuminate\Tests\Integration\IntegrationTest; use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException; @@ -14,7 +14,7 @@ class CompiledRouteCollectionTest extends IntegrationTest { /** - * @var \Illuminate\Routing\CompiledRouteCollection + * @var \Illuminate\Routing\RouteCollection */ protected $routeCollection; @@ -29,9 +29,11 @@ protected function setUp(): void $this->router = $this->app['router']; - $this->routeCollection = (new CompiledRouteCollection([], [])) - ->setRouter($this->router) - ->setContainer($this->app); + $this->routeCollection = new RouteCollection; + + // $this->routeCollection = (new CompiledRouteCollection([], [])) + // ->setRouter($this->router) + // ->setContainer($this->app); } protected function tearDown(): void @@ -42,21 +44,31 @@ protected function tearDown(): void unset($this->router); } + /** + * @return \Illuminate\Routing\CompiledRouteCollection + */ + protected function collection() + { + return $this->routeCollection->toCompiledRouteCollection($this->router, $this->app); + } + public function testRouteCollectionCanAddRoute() { $this->routeCollection->add($this->newRoute('GET', 'foo', [ 'uses' => 'FooController@index', 'as' => 'foo_index', ])); - $this->assertCount(1, $this->routeCollection); + + $this->assertCount(1, $this->collection()); } public function testRouteCollectionAddReturnsTheRoute() { - $outputRoute = $this->routeCollection->add($inputRoute = $this->newRoute('GET', 'foo', [ + $outputRoute = $this->collection()->add($inputRoute = $this->newRoute('GET', 'foo', [ 'uses' => 'FooController@index', 'as' => 'foo_index', ])); + $this->assertInstanceOf(Route::class, $outputRoute); $this->assertEquals($inputRoute, $outputRoute); } @@ -68,9 +80,11 @@ public function testRouteCollectionCanRetrieveByName() 'as' => 'route_name', ])); + $routes = $this->collection(); + $this->assertSame('route_name', $routeIndex->getName()); - $this->assertSame('route_name', $this->routeCollection->getByName('route_name')->getName()); - $this->assertEquals($routeIndex, $this->routeCollection->getByName('route_name')); + $this->assertSame('route_name', $routes->getByName('route_name')->getName()); + $this->assertEquals($routeIndex, $routes->getByName('route_name')); } public function testRouteCollectionCanRetrieveByAction() @@ -79,7 +93,7 @@ public function testRouteCollectionCanRetrieveByAction() 'uses' => 'FooController@index', ])); - $route = $this->routeCollection->getByAction('FooController@index'); + $route = $this->collection()->getByAction('FooController@index'); $this->assertSame($action, Arr::except($routeIndex->getAction(), 'as')); $this->assertSame($action, Arr::except($route->getAction(), 'as')); @@ -91,52 +105,68 @@ public function testRouteCollectionCanGetIterator() 'uses' => 'FooController@index', 'as' => 'foo_index', ])); - $this->assertInstanceOf(ArrayIterator::class, $this->routeCollection->getIterator()); + + $this->assertInstanceOf(ArrayIterator::class, $this->collection()->getIterator()); } public function testRouteCollectionCanGetIteratorWhenEmpty() { - $this->assertCount(0, $this->routeCollection); - $this->assertInstanceOf(ArrayIterator::class, $this->routeCollection->getIterator()); + $routes = $this->collection(); + + $this->assertCount(0, $routes); + $this->assertInstanceOf(ArrayIterator::class, $routes->getIterator()); } - public function testRouteCollectionCanGetIteratorWhenRouteAreAdded() + public function testRouteCollectionCanGetIteratorWhenRoutesAreAdded() { $this->routeCollection->add($routeIndex = $this->newRoute('GET', 'foo/index', [ 'uses' => 'FooController@index', 'as' => 'foo_index', ])); - $this->assertCount(1, $this->routeCollection); + + $routes = $this->collection(); + + $this->assertCount(1, $routes); $this->routeCollection->add($routeShow = $this->newRoute('GET', 'bar/show', [ 'uses' => 'BarController@show', 'as' => 'bar_show', ])); - $this->assertCount(2, $this->routeCollection); - $this->assertInstanceOf(ArrayIterator::class, $this->routeCollection->getIterator()); + $routes = $this->collection(); + + $this->assertCount(2, $routes); + + $this->assertInstanceOf(ArrayIterator::class, $routes->getIterator()); } public function testRouteCollectionCanHandleSameRoute() { - $routeIndex = $this->newRoute('GET', 'foo/index', [ + $this->routeCollection->add($routeIndex = $this->newRoute('GET', 'foo/index', [ 'uses' => 'FooController@index', 'as' => 'foo_index', - ]); + ])); - $this->routeCollection->add($routeIndex); - $this->assertCount(1, $this->routeCollection); + $routes = $this->collection(); + + $this->assertCount(1, $routes); // Add exactly the same route $this->routeCollection->add($routeIndex); - $this->assertCount(1, $this->routeCollection); + + $routes = $this->collection(); + + $this->assertCount(1, $routes); // Add a non-existing route $this->routeCollection->add($this->newRoute('GET', 'bar/show', [ 'uses' => 'BarController@show', 'as' => 'bar_show', ])); - $this->assertCount(2, $this->routeCollection); + + $routes = $this->collection(); + + $this->assertCount(2, $routes); } public function testRouteCollectionCanGetAllRoutes() @@ -145,12 +175,10 @@ public function testRouteCollectionCanGetAllRoutes() 'uses' => 'FooController@index', 'as' => 'foo_index', ])); - $this->routeCollection->add($routeShow = $this->newRoute('GET', 'foo/show', [ 'uses' => 'FooController@show', 'as' => 'foo_show', ])); - $this->routeCollection->add($routeNew = $this->newRoute('POST', 'bar', [ 'uses' => 'BarController@create', 'as' => 'bar_create', @@ -161,7 +189,7 @@ public function testRouteCollectionCanGetAllRoutes() $routeShow, $routeNew, ]; - $this->assertEquals($allRoutes, $this->routeCollection->getRoutes()); + $this->assertEquals($allRoutes, $this->collection()->getRoutes()); } public function testRouteCollectionCanGetRoutesByName() @@ -185,7 +213,7 @@ public function testRouteCollectionCanGetRoutesByName() $this->routeCollection->add($routesByName['foo_show']); $this->routeCollection->add($routesByName['bar_create']); - $this->assertEquals($routesByName, $this->routeCollection->getRoutesByName()); + $this->assertEquals($routesByName, $this->collection()->getRoutesByName()); } public function testRouteCollectionCanGetRoutesByMethod() @@ -221,7 +249,7 @@ public function testRouteCollectionCanGetRoutesByMethod() 'POST' => [ 'bar' => $routes['bar_create'], ], - ], $this->routeCollection->getRoutesByMethod()); + ], $this->collection()->getRoutesByMethod()); } public function testRouteCollectionCleansUpOverwrittenRoutes() @@ -239,9 +267,14 @@ public function testRouteCollectionCleansUpOverwrittenRoutes() $this->assertEquals($routeB, $this->routeCollection->getByName('overwrittenRouteA')); $this->assertEquals($routeB, $this->routeCollection->getByAction('OverwrittenView@view')); + $routes = $this->collection(); + + // The lookups of $routeA should not be there anymore, because they are no longer valid. + $this->assertNull($routes->getByName('routeA')); + $this->assertNull($routes->getByAction('View@view')); // The lookups of $routeB are still there. - $this->assertEquals($routeB, $this->routeCollection->getByName('overwrittenRouteA')); - $this->assertEquals($routeB, $this->routeCollection->getByAction('OverwrittenView@view')); + $this->assertEquals($routeB, $routes->getByName('overwrittenRouteA')); + $this->assertEquals($routeB, $routes->getByAction('OverwrittenView@view')); } public function testMatchingThrowsNotFoundExceptionWhenRouteIsNotFound() @@ -250,23 +283,64 @@ public function testMatchingThrowsNotFoundExceptionWhenRouteIsNotFound() $this->expectException(NotFoundHttpException::class); - $this->routeCollection->match(Request::create('/foo')); + $this->collection()->match(Request::create('/foo')); } public function testMatchingThrowsMethodNotAllowedHttpExceptionWhenMethodIsNotAllowed() { - $this->routeCollection->add($this->newRoute('POST', '/foo', ['uses' => 'FooController@index'])); + $this->routeCollection->add($this->newRoute('GET', '/foo', ['uses' => 'FooController@index'])); $this->expectException(MethodNotAllowedHttpException::class); - $this->routeCollection->match(Request::create('/foo')); + $this->collection()->match(Request::create('/foo', 'POST')); + } + + public function testMatchingRouteWithSameDynamicallyAddedRouteAlwaysMatchesCachedOneFirst() + { + $this->routeCollection->add( + $route = $this->newRoute('GET', '/', ['uses' => 'FooController@index', 'as' => 'foo']) + ); + + $routes = $this->collection(); + + $routes->add($this->newRoute('GET', '/', ['uses' => 'FooController@index', 'as' => 'bar'])); + + $this->assertEquals('foo', $routes->match(Request::create('/', 'GET'))->getName()); + } + + public function testMatchingFindsRouteWithDifferentMethodDynamically() + { + $this->routeCollection->add($this->newRoute('GET', '/foo', ['uses' => 'FooController@index'])); + + $routes = $this->collection(); + + $routes->add($route = $this->newRoute('POST', '/foo', ['uses' => 'FooController@index'])); + + $this->assertSame($route, $routes->match(Request::create('/foo', 'POST'))); + } + + public function testMatchingWildcardFromCompiledRoutesAlwaysTakesPrecedent() + { + $this->routeCollection->add( + $route = $this->newRoute('GET', '{wildcard}', ['uses' => 'FooController@index', 'as' => 'foo']) + ->where('wildcard', '.*') + ); + + $routes = $this->collection(); + + $routes->add( + $this->newRoute('GET', '{wildcard}', ['uses' => 'FooController@index', 'as' => 'bar']) + ->where('wildcard', '.*') + ); + + $this->assertSame('foo', $routes->match(Request::create('/foo', 'GET'))->getName()); } public function testSlashPrefixIsProperly() { $this->routeCollection->add($this->newRoute('GET', 'foo/bar', ['uses' => 'FooController@index', 'prefix' => '/'])); - $route = $this->routeCollection->getByAction('FooController@index'); + $route = $this->collection()->getByAction('FooController@index'); $this->assertEquals('foo/bar', $route->uri()); } @@ -279,7 +353,7 @@ public function testRouteBindingsAreProperlySaved() 'as' => 'foo', ])); - $route = $this->routeCollection->getByName('foo'); + $route = $this->collection()->getByName('foo'); $this->assertEquals('profile/{user}/posts/{post}/show', $route->uri()); $this->assertSame(['user' => 'username', 'post' => 'slug'], $route->bindingFields());