Skip to content
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.

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

perf: RouteCollection $routes optimization #7175

Merged
merged 11 commits into from
Jun 28, 2023
166 changes: 112 additions & 54 deletions system/Router/RouteCollection.php
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
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
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
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