Skip to content

Commit

Permalink
Merge pull request #7175 from kenjis/perf-router
Browse files Browse the repository at this point in the history 
perf: RouteCollection $routes optimization
  • Loading branch information
@kenjis
kenjis authored Jun 28, 2023
2 parents d97ee0e + df8556e commit c02bae9
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 56 deletions.
166 changes: 112 additions & 54 deletions system/Router/RouteCollection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,20 @@ class RouteCollection implements RouteCollectionInterface
*
* [
* verb => [
* routeName => [
* 'route' => [
* routeKey(regex) => handler,
* ],
* routeKey(regex) => [
* 'name' => routeName
* 'handler' => handler,
* 'from' => from,
* ],
* ],
* // redirect route
* '*' => [
* routeKey(regex)(from) => [
* 'name' => routeName
* 'handler' => [routeKey(regex)(to) => handler],
* 'from' => from,
* 'redirect' => statusCode,
* ]
* ],
* ],
* ]
*/
Expand All @@ -138,6 +146,30 @@ class RouteCollection implements RouteCollectionInterface
'cli' => [],
];

/**
* Array of routes names
*
* @var array
*
* [
* verb => [
* routeName => routeKey(regex)
* ],
* ]
*/
protected $routesNames = [
'*' => [],
'options' => [],
'get' => [],
'head' => [],
'post' => [],
'put' => [],
'delete' => [],
'trace' => [],
'connect' => [],
'cli' => [],
];

/**
* Array of routes options
*
Expand Down Expand Up @@ -537,9 +569,8 @@ public function getRoutes(?string $verb = null): array
// before any of the generic, "add" routes.
$collection = $this->routes[$verb] + ($this->routes['*'] ?? []);

foreach ($collection as $r) {
$key = key($r['route']);
$routes[$key] = $r['route'][$key];
foreach ($collection as $routeKey => $r) {
$routes[$routeKey] = $r['handler'];
}
}

Expand Down Expand Up @@ -638,42 +669,63 @@ public function add(string $from, $to, ?array $options = null): RouteCollectionI
public function addRedirect(string $from, string $to, int $status = 302)
{
// Use the named route's pattern if this is a named route.
if (array_key_exists($to, $this->routes['*'])) {
$to = $this->routes['*'][$to]['route'];
} elseif (array_key_exists($to, $this->routes['get'])) {
$to = $this->routes['get'][$to]['route'];
if (array_key_exists($to, $this->routesNames['*'])) {
$routeName = $to;
$routeKey = $this->routesNames['*'][$routeName];
$redirectTo = [$routeKey => $this->routes['*'][$routeKey]['handler']];
} elseif (array_key_exists($to, $this->routesNames['get'])) {
$routeName = $to;
$routeKey = $this->routesNames['get'][$routeName];
$redirectTo = [$routeKey => $this->routes['get'][$routeKey]['handler']];
} else {
// The named route is not found.
$redirectTo = $to;
}

$this->create('*', $from, $to, ['redirect' => $status]);
$this->create('*', $from, $redirectTo, ['redirect' => $status]);

return $this;
}

/**
* Determines if the route is a redirecting route.
*
* @param string $routeKey routeKey or route name
*/
public function isRedirect(string $from): bool
public function isRedirect(string $routeKey): bool
{
foreach ($this->routes['*'] as $name => $route) {
// Named route?
if ($name === $from || key($route['route']) === $from) {
return isset($route['redirect']) && is_numeric($route['redirect']);
}
if (isset($this->routes['*'][$routeKey]['redirect'])) {
return true;
}

// This logic is not used. Should be deprecated?
$routeName = $this->routes['*'][$routeKey]['name'] ?? null;
if ($routeName === $routeKey) {
$routeKey = $this->routesNames['*'][$routeName];

return isset($this->routes['*'][$routeKey]['redirect']);
}

return false;
}

/**
* Grabs the HTTP status code from a redirecting Route.
*
* @param string $routeKey routeKey or route name
*/
public function getRedirectCode(string $from): int
public function getRedirectCode(string $routeKey): int
{
foreach ($this->routes['*'] as $name => $route) {
// Named route?
if ($name === $from || key($route['route']) === $from) {
return $route['redirect'] ?? 0;
}
if (isset($this->routes['*'][$routeKey]['redirect'])) {
return $this->routes['*'][$routeKey]['redirect'];
}

// This logic is not used. Should be deprecated?
$routeName = $this->routes['*'][$routeKey]['name'] ?? null;
if ($routeName === $routeKey) {
$routeKey = $this->routesNames['*'][$routeName];

return $this->routes['*'][$routeKey]['redirect'];
}

return 0;
Expand Down Expand Up @@ -1088,9 +1140,11 @@ public function environment(string $env, Closure $callback): RouteCollectionInte
public function reverseRoute(string $search, ...$params)
{
// Named routes get higher priority.
foreach ($this->routes as $collection) {
foreach ($this->routesNames as $collection) {
if (array_key_exists($search, $collection)) {
return $this->buildReverseRoute(key($collection[$search]['route']), $params);
$routeKey = $collection[$search];

return $this->buildReverseRoute($routeKey, $params);
}
}

Expand All @@ -1106,9 +1160,8 @@ public function reverseRoute(string $search, ...$params)
// If it's not a named route, then loop over
// all routes to find a match.
foreach ($this->routes as $collection) {
foreach ($collection as $route) {
$from = key($route['route']);
$to = $route['route'][$from];
foreach ($collection as $routeKey => $route) {
$to = $route['handler'];

// ignore closures
if (! is_string($to)) {
Expand All @@ -1132,7 +1185,7 @@ public function reverseRoute(string $search, ...$params)
continue;
}

return $this->buildReverseRoute($from, $params);
return $this->buildReverseRoute($routeKey, $params);
}
}

Expand Down Expand Up @@ -1247,21 +1300,21 @@ protected function fillRouteParams(string $from, ?array $params = null): string
* @param array $params One or more parameters to be passed to the route.
* The last parameter allows you to set the locale.
*/
protected function buildReverseRoute(string $from, array $params): string
protected function buildReverseRoute(string $routeKey, array $params): string
{
$locale = null;

// Find all of our back-references in the original route
preg_match_all('/\(([^)]+)\)/', $from, $matches);
preg_match_all('/\(([^)]+)\)/', $routeKey, $matches);

if (empty($matches[0])) {
if (strpos($from, '{locale}') !== false) {
if (strpos($routeKey, '{locale}') !== false) {
$locale = $params[0] ?? null;
}

$from = $this->replaceLocale($from, $locale);
$routeKey = $this->replaceLocale($routeKey, $locale);

return '/' . ltrim($from, '/');
return '/' . ltrim($routeKey, '/');
}

// Locale is passed?
Expand All @@ -1279,13 +1332,13 @@ protected function buildReverseRoute(string $from, array $params): string

// Ensure that the param we're inserting matches
// the expected param type.
$pos = strpos($from, $pattern);
$from = substr_replace($from, $params[$index], $pos, strlen($pattern));
$pos = strpos($routeKey, $pattern);
$routeKey = substr_replace($routeKey, $params[$index], $pos, strlen($pattern));
}

$from = $this->replaceLocale($from, $locale);
$routeKey = $this->replaceLocale($routeKey, $locale);

return '/' . ltrim($from, '/');
return '/' . ltrim($routeKey, '/');
}

/**
Expand Down Expand Up @@ -1333,7 +1386,7 @@ protected function create(string $verb, string $from, $to, ?array $options = nul
}

// When redirecting to named route, $to is an array like `['zombies' => '\Zombies::index']`.
if (is_array($to) && count($to) === 2) {
if (is_array($to) && isset($to[0])) {
$to = $this->processArrayCallableSyntax($from, $to);
}

Expand Down Expand Up @@ -1385,10 +1438,11 @@ protected function create(string $verb, string $from, $to, ?array $options = nul
}
}

$routeKey = $from;
// Replace our regex pattern placeholders with the actual thing
// so that the Router doesn't need to know about any of this.
foreach ($this->placeholders as $tag => $pattern) {
$from = str_ireplace(':' . $tag, $pattern, $from);
$routeKey = str_ireplace(':' . $tag, $pattern, $routeKey);
}

// If is redirect, No processing
Expand All @@ -1403,7 +1457,7 @@ protected function create(string $verb, string $from, $to, ?array $options = nul
$to = '\\' . ltrim($to, '\\');
}

$name = $options['as'] ?? $from;
$name = $options['as'] ?? $routeKey;

helper('array');

Expand All @@ -1412,20 +1466,22 @@ protected function create(string $verb, string $from, $to, ?array $options = nul
// routes should always be the "source of truth".
// this works only because discovered routes are added just prior
// to attempting to route the request.
$fromExists = dot_array_search('*.route.' . $from, $this->routes[$verb] ?? []) !== null;
if ((isset($this->routes[$verb][$name]) || $fromExists) && ! $overwrite) {
$routeKeyExists = isset($this->routes[$verb][$routeKey]);
if ((isset($this->routesNames[$verb][$name]) || $routeKeyExists) && ! $overwrite) {
return;
}

$this->routes[$verb][$name] = [
'route' => [$from => $to],
$this->routes[$verb][$routeKey] = [
'name' => $name,
'handler' => $to,
'from' => $from,
];

$this->routesOptions[$verb][$from] = $options;
$this->routesOptions[$verb][$routeKey] = $options;
$this->routesNames[$verb][$name] = $routeKey;

// Is this a redirect?
if (isset($options['redirect']) && is_numeric($options['redirect'])) {
$this->routes['*'][$name]['redirect'] = $options['redirect'];
$this->routes['*'][$routeKey]['redirect'] = $options['redirect'];
}
}

Expand Down Expand Up @@ -1566,12 +1622,15 @@ private function determineCurrentSubdomain()
*/
public function resetRoutes()
{
$this->routes = ['*' => []];
$this->routes = $this->routesNames = ['*' => []];

foreach ($this->defaultHTTPMethods as $verb) {
$this->routes[$verb] = [];
$this->routes[$verb] = [];
$this->routesNames[$verb] = [];
}

$this->routesOptions = [];

$this->prioritizeDetected = false;
$this->didDiscover = false;
}
Expand Down Expand Up @@ -1638,8 +1697,7 @@ public function getRegisteredControllers(?string $verb = '*'): array
if ($verb === '*') {
foreach ($this->defaultHTTPMethods as $tmpVerb) {
foreach ($this->routes[$tmpVerb] as $route) {
$routeKey = key($route['route']);
$controller = $this->getControllerName($route['route'][$routeKey]);
$controller = $this->getControllerName($route['handler']);
if ($controller !== null) {
$controllers[] = $controller;
}
Expand Down
4 changes: 2 additions & 2 deletions system/Router/RouteCollectionInterface.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,12 @@ public function reverseRoute(string $search, ...$params);
/**
* Determines if the route is a redirecting route.
*/
public function isRedirect(string $from): bool;
public function isRedirect(string $routeKey): bool;

/**
* Grabs the HTTP status code from a redirecting Route.
*/
public function getRedirectCode(string $from): int;
public function getRedirectCode(string $routeKey): int;

/**
* Get the flag that limit or not the routes with {locale} placeholder to App::$supportedLocales
Expand Down
2 changes: 2 additions & 0 deletions user_guide_src/source/changelogs/v4.4.0.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ Changes
So if you installed CodeIgniter under the folder that contains the special
characters like ``(``, ``)``, etc., CodeIgniter didn't work. Since v4.4.0,
this restriction has been removed.
- **RouteCollection:** The array structure of the protected property ``$routes``
has been modified for performance.

Deprecations
************
Expand Down
9 changes: 9 additions & 0 deletions user_guide_src/source/installation/upgrade_440.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@ Interface Changes
Some interface changes have been made. Classes that implement them should update
their APIs to reflect the changes. See :ref:`v440-interface-changes` for details.

RouteCollection::$routes
========================

The array structure of the protected property ``$routes`` has been modified for
performance.

If you extend ``RouteCollection`` and use the ``$routes``, update your code to
match the new array structure.

Mandatory File Changes
**********************

Expand Down

0 comments on commit c02bae9

Please sign in to comment.