8000 [5.5] Route: simplify RouteCollection. by lucasmichot · Pull Request #18417 · laravel/framework · GitHub
[go: up one dir, main page]
Skip to content

[5.5] Route: simplify RouteCollection. #18417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[5.5] Route: simplify RouteCollection. #18417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions src/Illuminate/Foundation/Support/Providers/RouteServiceProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ public function boot()
$this->loadCachedRoutes();
} else {
$this->loadRoutes();

$this->app->booted(function () {
$this->app['router']->getRoutes()->refreshNameLookups();
});
}
}

Expand Down
135 changes: 44 additions & 91 deletions src/Illuminate/Routing/RouteCollection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -49,83 +38,25 @@ 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;
}

/**
* Refresh the name look-up table.
*
* 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;
}
}
//
}

/**
Expand Down Expand Up @@ -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();
}

/**
Expand All @@ -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;
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this much slower than looking it up in a map? People may have many route(...) calls in their views.

}

/**
Expand All @@ -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;
});
}

/**
Expand All @@ -284,7 +221,7 @@ public function getByAction($action)
*/
public function getRoutes()
{
return array_values($this->allRoutes);
return $this->getUniqueRoutes()->values()->toArray();
}

/**
Expand All @@ -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]];
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this only return for the first method the route supports?

})->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();
});
}

/**
Expand All @@ -304,7 +257,7 @@ public function getRoutesByMethod()
*/
public function getIterator()
{
return new ArrayIterator($this->getRoutes());
return $this->getUniqueRoutes()->getIterator();
}

/**
Expand All @@ -314,6 +267,6 @@ public function getIterator()
*/
public function count()
{
return count($this->getRoutes());
return $this->getUniqueRoutes()->count();
}
}
8 changes: 1 addition & 7 deletions tests/Routing/RouteCollectionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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'));
}

Expand Down
1 change: 0 additions & 1 deletion tests/Routing/RoutingUrlGeneratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}
Expand Down
0