From 3ded24d3dac5c96635ce8edd994e6dd7a5266d5e Mon Sep 17 00:00:00 2001 From: Lucas Michot Date: Fri, 27 Jan 2017 10:54:52 +0100 Subject: [PATCH] Simplify RouteCollection. --- .../Providers/RouteServiceProvider.php | 4 - src/Illuminate/Routing/RouteCollection.php | 135 ++++++------------ tests/Routing/RouteCollectionTest.php | 8 +- tests/Routing/RoutingUrlGeneratorTest.php | 1 - 4 files changed, 45 insertions(+), 103 deletions(-) diff --git a/src/Illuminate/Foundation/Support/Providers/RouteServiceProvider.php b/src/Illuminate/Foundation/Support/Providers/RouteServiceProvider.php index 564705a62080..f6fa63926198 100644 --- a/src/Illuminate/Foundation/Support/Providers/RouteServiceProvider.php +++ b/src/Illuminate/Foundation/Support/Providers/RouteServiceProvider.php @@ -28,10 +28,6 @@ public function boot() $this->loadCachedRoutes(); } else { $this->loadRoutes(); - - $this->app->booted(function () { - $this->app['router']->getRoutes()->refreshNameLookups(); - }); } } diff --git a/src/Illuminate/Routing/RouteCollection.php b/src/Illuminate/Routing/RouteCollection.php index b6317def6dbf..319dec611cba 100644 --- a/src/Illuminate/Routing/RouteCollection.php +++ b/src/Illuminate/Routing/RouteCollection.php @@ -3,43 +3,32 @@ namespace Illuminate\Routing; use Countable; -use ArrayIterator; use IteratorAggregate; use Illuminate\Support\Arr; use Illuminate\Http\Request; use Illuminate\Http\Response; +use Illuminate\Support\Collection; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException; class RouteCollection implements Countable, IteratorAggregate { /** - * An array of the routes keyed by method. + * The Route collection. * - * @var array + * @var \Illuminate\Support\Collection */ - protected $routes = []; + protected $collection; /** - * An flattened array of all of the routes. + * Create a new RouteCollection instance. * - * @var array - */ - protected $allRoutes = []; - - /** - * A look-up table of routes by their names. - * - * @var array - */ - protected $nameList = []; - - /** - * A look-up table of routes by controller action. - * - * @var array + * @return void */ - protected $actionList = []; + public function __construct() + { + $this->collection = new Collection; + } /** * Add a Route instance to the collection. @@ -49,65 +38,11 @@ class RouteCollection implements Countable, IteratorAggregate */ public function add(Route $route) { - $this->addToCollections($route); - - $this->addLookups($route); - - return $route; - } - - /** - * Add the given route to the arrays of routes. - * - * @param \Illuminate\Routing\Route $route - * @return void - */ - protected function addToCollections($route) - { - $domainAndUri = $route->domain().$route->uri(); - foreach ($route->methods() as $method) { - $this->routes[$method][$domainAndUri] = $route; - } - - $this->allRoutes[$method.$domainAndUri] = $route; - } - - /** - * Add the route to any look-up tables if necessary. - * - * @param \Illuminate\Routing\Route $route - * @return void - */ - 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 any route associate with a name and not have - // to iterate through every route every time we need to perform a look-up. - $action = $route->getAction(); - - if (isset($action['as'])) { - $this->nameList[$action['as']] = $route; - } - - // When the route is routing to a controller we will also store the action that - // is used by the route. This will let us reverse route to controllers while - // processing a request and easily generate URLs to the given controllers. - if (isset($action['controller'])) { - $this->addToActionList($action, $route); + $this->collection->put($method.$route->domain().$route->uri(), $route); } - } - /** - * Add a route to the controller action dictionary. - * - * @param array $action - * @param \Illuminate\Routing\Route $route - * @return void - */ - protected function addToActionList($action, $route) - { - $this->actionList[trim($action['controller'], '\\')] = $route; + return $route; } /** @@ -115,17 +50,13 @@ protected function addToActionList($action, $route) * * This is done in case any names are fluently defined. * + * @deprecated since version 5.5. + * * @return void */ public function refreshNameLookups() { - $this->nameList = []; - - foreach ($this->allRoutes as $route) { - if ($route->getName()) { - $this->nameList[$route->getName()] = $route; - } - } + // } /** @@ -241,7 +172,9 @@ protected function methodNotAllowed(array $others) */ public function get($method = null) { - return is_null($method) ? $this->getRoutes() : Arr::get($this->routes, $method, []); + return is_null($method) ? $this->getRoutes() : $this->collection->filter(function (Route $route) use ($method) { + return in_array($method, $route->methods()); + })->toArray(); } /** @@ -263,7 +196,9 @@ public function hasNamedRoute($name) */ public function getByName($name) { - return isset($this->nameList[$name]) ? $this->nameList[$name] : null; + return $this->collection->first(function (Route $route) use ($name) { + return Arr::get($route->getAction(), 'as') === $name; + }); } /** @@ -274,7 +209,9 @@ public function getByName($name) */ public function getByAction($action) { - return isset($this->actionList[$action]) ? $this->actionList[$action] : null; + return $this->collection->first(function (Route $route) use ($action) { + return trim(Arr::get($route->getAction(), 'controller'), '\\') === $action; + }); } /** @@ -284,7 +221,7 @@ public function getByAction($action) */ public function getRoutes() { - return array_values($this->allRoutes); + return $this->getUniqueRoutes()->values()->toArray(); } /** @@ -294,7 +231,23 @@ public function getRoutes() */ public function getRoutesByMethod() { - return $this->routes; + return $this->collection->mapWithKeys(function (Route $route) { + foreach ($route->methods() as $method) { + return [$method => [$route->domain().$route->uri() => $route]]; + } + })->toArray(); + } + + /** + * Get the unique routes. + * + * @return \Illuminate\Support\Collection + */ + protected function getUniqueRoutes() + { + return $this->collection->unique(function (Route $route) { + return Arr::last($route->methods()).$route->domain().$route->uri(); + }); } /** @@ -304,7 +257,7 @@ public function getRoutesByMethod() */ public function getIterator() { - return new ArrayIterator($this->getRoutes()); + return $this->getUniqueRoutes()->getIterator(); } /** @@ -314,6 +267,6 @@ public function getIterator() */ public function count() { - return count($this->getRoutes()); + return $this->getUniqueRoutes()->count(); } } diff --git a/tests/Routing/RouteCollectionTest.php b/tests/Routing/RouteCollectionTest.php index b0119cd42d83..979a3aa5e83e 100644 --- a/tests/Routing/RouteCollectionTest.php +++ b/tests/Routing/RouteCollectionTest.php @@ -121,7 +121,7 @@ public function testRouteCollectionCanHandleSameRoute() $this->assertCount(2, $this->routeCollection); } - public function testRouteCollectionCanRefreshNameLookups() + public function testRouteCollectionDoesNotNeedToRefreshNameLookups() { $routeIndex = new Route('GET', 'foo/index', [ 'uses' => 'FooController@index', @@ -132,12 +132,6 @@ public function testRouteCollectionCanRefreshNameLookups() // The route name is set by calling \Illuminate\Routing\Route::name() $this->routeCollection->add($routeIndex)->name('route_name'); - - // No route is found. This is normal, as no refresh as been done. - $this->assertNull($this->routeCollection->getByName('route_name')); - - // After the refresh, the name will be properly set to the route. - $this->routeCollection->refreshNameLookups(); $this->assertEquals($routeIndex, $this->routeCollection->getByName('route_name')); } diff --git a/tests/Routing/RoutingUrlGeneratorTest.php b/tests/Routing/RoutingUrlGeneratorTest.php index 92960e0957bc..469356dd579b 100755 --- a/tests/Routing/RoutingUrlGeneratorTest.php +++ b/tests/Routing/RoutingUrlGeneratorTest.php @@ -170,7 +170,6 @@ public function testFluentRouteNameDefinitions() $route = new Route(['GET'], 'foo/bar', []); $route->name('foo'); $routes->add($route); - $routes->refreshNameLookups(); $this->assertEquals('http://www.foo.com/foo/bar', $url->route('foo')); }