From a95676570dcf747f2fa82c9da56da0a1893b64dd Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 25 Jan 2023 09:56:29 +0900 Subject: [PATCH 01/11] docs: fix PHPDoc I found another type data in the $route array. --- system/Router/RouteCollection.php | 1 + 1 file changed, 1 insertion(+) diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index f535d392817c..fe3f2e99fa82 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -119,6 +119,7 @@ class RouteCollection implements RouteCollectionInterface * routeName => [ * 'route' => [ * routeKey(regex) => handler, + * or routeKey(regex)(from) => [routeKey(regex)(to) => handler], // redirect * ], * 'redirect' => statusCode, * ] From e0864515fdb8930c92f4909c97c2022f5d4e7346 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 25 Jan 2023 10:42:38 +0900 Subject: [PATCH 02/11] refactor: stop reassignment --- system/Router/RouteCollection.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index fe3f2e99fa82..c111804e0ea4 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -640,12 +640,14 @@ 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']; + $redirectTo = $this->routes['*'][$to]['route']; } elseif (array_key_exists($to, $this->routes['get'])) { - $to = $this->routes['get'][$to]['route']; + $redirectTo = $this->routes['get'][$to]['route']; + } else { + $redirectTo = $to; } - $this->create('*', $from, $to, ['redirect' => $status]); + $this->create('*', $from, $redirectTo, ['redirect' => $status]); return $this; } From aa569317d786b6fc9f173121a6b674c54362aaf7 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 25 Jan 2023 11:07:00 +0900 Subject: [PATCH 03/11] refactor: add variable To distinguish between from and routeKey. --- system/Router/RouteCollection.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index c111804e0ea4..08997a7bd6e6 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -1388,10 +1388,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 @@ -1406,7 +1407,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'); @@ -1415,16 +1416,16 @@ 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; + $fromExists = dot_array_search('*.route.' . $routeKey, $this->routes[$verb] ?? []) !== null; if ((isset($this->routes[$verb][$name]) || $fromExists) && ! $overwrite) { return; } $this->routes[$verb][$name] = [ - 'route' => [$from => $to], + 'route' => [$routeKey => $to], ]; - $this->routesOptions[$verb][$from] = $options; + $this->routesOptions[$verb][$routeKey] = $options; // Is this a redirect? if (isset($options['redirect']) && is_numeric($options['redirect'])) { From 2af286417e271576ea6d192aa73735d2e12a045e Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 25 Jan 2023 11:28:12 +0900 Subject: [PATCH 04/11] refactor: rename parameter name --- system/Router/RouteCollection.php | 12 ++++++++---- system/Router/RouteCollectionInterface.php | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index 08997a7bd6e6..6c3a7bc776e3 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -654,12 +654,14 @@ public function addRedirect(string $from, string $to, int $status = 302) /** * 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) { + if ($name === $routeKey || key($route['route']) === $routeKey) { return isset($route['redirect']) && is_numeric($route['redirect']); } } @@ -669,12 +671,14 @@ public function isRedirect(string $from): bool /** * 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) { + if ($name === $routeKey || key($route['route']) === $routeKey) { return $route['redirect'] ?? 0; } } diff --git a/system/Router/RouteCollectionInterface.php b/system/Router/RouteCollectionInterface.php index 701e893dcd16..0ca43523a89b 100644 --- a/system/Router/RouteCollectionInterface.php +++ b/system/Router/RouteCollectionInterface.php @@ -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 From 26be1f2c138eb1ee38cf0f1c8c2557f31a61488e Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 25 Jan 2023 12:09:45 +0900 Subject: [PATCH 05/11] refactor: rename parameter and variable name --- system/Router/RouteCollection.php | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index 6c3a7bc776e3..5ed06fb0eb26 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -1114,8 +1114,8 @@ public function reverseRoute(string $search, ...$params) // all routes to find a match. foreach ($this->routes as $collection) { foreach ($collection as $route) { - $from = key($route['route']); - $to = $route['route'][$from]; + $routeKey = key($route['route']); + $to = $route['route'][$routeKey]; // ignore closures if (! is_string($to)) { @@ -1139,7 +1139,7 @@ public function reverseRoute(string $search, ...$params) continue; } - return $this->buildReverseRoute($from, $params); + return $this->buildReverseRoute($routeKey, $params); } } @@ -1254,21 +1254,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? @@ -1286,13 +1286,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, '/'); } /** From 7c125b90cf2b2614ae24d1c93b2686c398dcb9fc Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 25 Jan 2023 14:59:58 +0900 Subject: [PATCH 06/11] perf: change RouteCollection::$routes structure --- system/Router/RouteCollection.php | 125 ++++++++++++++++++++---------- 1 file changed, 85 insertions(+), 40 deletions(-) diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index 5ed06fb0eb26..b38844bbcdd8 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -116,13 +116,16 @@ class RouteCollection implements RouteCollectionInterface * * [ * verb => [ - * routeName => [ - * 'route' => [ - * routeKey(regex) => handler, - * or routeKey(regex)(from) => [routeKey(regex)(to) => handler], // redirect - * ], + * routeKey(regex) => [ + * 'name' => routeName + * 'handler' => handler, + * ], + * // redirect route + * or routeKey(regex)(from) => [ + * 'name' => routeName + * 'handler' => [routeKey(regex)(to) => handler], * 'redirect' => statusCode, - * ] + * ], * ], * ] */ @@ -139,6 +142,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 * @@ -538,9 +565,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']; } } @@ -639,11 +665,16 @@ 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['*'])) { - $redirectTo = $this->routes['*'][$to]['route']; - } elseif (array_key_exists($to, $this->routes['get'])) { - $redirectTo = $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; } @@ -659,11 +690,16 @@ public function addRedirect(string $from, string $to, int $status = 302) */ public function isRedirect(string $routeKey): bool { - foreach ($this->routes['*'] as $name => $route) { - // Named route? - if ($name === $routeKey || key($route['route']) === $routeKey) { - 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; @@ -676,11 +712,16 @@ public function isRedirect(string $routeKey): bool */ public function getRedirectCode(string $routeKey): int { - foreach ($this->routes['*'] as $name => $route) { - // Named route? - if ($name === $routeKey || key($route['route']) === $routeKey) { - 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; @@ -1095,9 +1136,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); } } @@ -1113,9 +1156,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) { - $routeKey = key($route['route']); - $to = $route['route'][$routeKey]; + foreach ($collection as $routeKey => $route) { + $to = $route['handler']; // ignore closures if (! is_string($to)) { @@ -1340,7 +1382,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); } @@ -1420,20 +1462,21 @@ 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.' . $routeKey, $this->routes[$verb] ?? []) !== null; - if ((isset($this->routes[$verb][$name]) || $fromExists) && ! $overwrite) { + $fromExists = isset($this->routes[$verb][$routeKey]); + if ((isset($this->routesNames[$verb][$name]) || $fromExists) && ! $overwrite) { return; } - $this->routes[$verb][$name] = [ - 'route' => [$routeKey => $to], + $this->routes[$verb][$routeKey] = [ + 'name' => $name, + 'handler' => $to, ]; - $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']; } } @@ -1574,12 +1617,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; } @@ -1645,9 +1691,8 @@ 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]); + foreach ($this->routes[$tmpVerb] as $routeKey => $route) { + $controller = $this->getControllerName($route['handler']); if ($controller !== null) { $controllers[] = $controller; } From 579de5477c53987141c324fc36bc00e99c99b5ab Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 25 Jan 2023 17:37:20 +0900 Subject: [PATCH 07/11] refactor: remove unused variable --- system/Router/RouteCollection.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index b38844bbcdd8..03078395b4d2 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -1691,7 +1691,7 @@ public function getRegisteredControllers(?string $verb = '*'): array if ($verb === '*') { foreach ($this->defaultHTTPMethods as $tmpVerb) { - foreach ($this->routes[$tmpVerb] as $routeKey => $route) { + foreach ($this->routes[$tmpVerb] as $route) { $controller = $this->getControllerName($route['handler']); if ($controller !== null) { $controllers[] = $controller; From fea7e9a1ee0d690dae81536de69aafd616867ca1 Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 26 Jan 2023 08:04:09 +0900 Subject: [PATCH 08/11] refactor: rename variable name --- system/Router/RouteCollection.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index 03078395b4d2..d194d474a599 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -1462,8 +1462,8 @@ 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 = isset($this->routes[$verb][$routeKey]); - if ((isset($this->routesNames[$verb][$name]) || $fromExists) && ! $overwrite) { + $routeKeyExists = isset($this->routes[$verb][$routeKey]); + if ((isset($this->routesNames[$verb][$name]) || $routeKeyExists) && ! $overwrite) { return; } From 3851f414918ef282742cefd54c4256b5048a02ac Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 30 Apr 2023 12:08:43 +0900 Subject: [PATCH 09/11] feat: add $from for future use --- system/Router/RouteCollection.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index d194d474a599..865c8aa696f7 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -119,12 +119,14 @@ class RouteCollection implements RouteCollectionInterface * routeKey(regex) => [ * 'name' => routeName * 'handler' => handler, + * 'from' => from, * ], * // redirect route * or routeKey(regex)(from) => [ * 'name' => routeName * 'handler' => [routeKey(regex)(to) => handler], * 'redirect' => statusCode, + * 'from' => from, * ], * ], * ] @@ -1470,6 +1472,7 @@ protected function create(string $verb, string $from, $to, ?array $options = nul $this->routes[$verb][$routeKey] = [ 'name' => $name, 'handler' => $to, + 'from' => $from, ]; $this->routesOptions[$verb][$routeKey] = $options; $this->routesNames[$verb][$name] = $routeKey; From 0c7a2bf48a01f16bdc8da8c9b350093f4eda9d14 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 30 Apr 2023 12:09:07 +0900 Subject: [PATCH 10/11] docs: update doc comment --- system/Router/RouteCollection.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index 865c8aa696f7..030edf8f9ca1 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -117,16 +117,18 @@ class RouteCollection implements RouteCollectionInterface * [ * verb => [ * routeKey(regex) => [ - * 'name' => routeName - * 'handler' => handler, - * 'from' => from, + * 'name' => routeName + * 'handler' => handler, + * 'from' => from, * ], - * // redirect route - * or routeKey(regex)(from) => [ + * ], + * // redirect route + * '*' => [ + * routeKey(regex)(from) => [ * 'name' => routeName * 'handler' => [routeKey(regex)(to) => handler], + * 'from' => from, * 'redirect' => statusCode, - * 'from' => from, * ], * ], * ] From df8556ee1bc8d980e0cbdcf121c12c213cd2f6ca Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 27 Jun 2023 09:40:25 +0900 Subject: [PATCH 11/11] docs: add changelog and upgrade note --- user_guide_src/source/changelogs/v4.4.0.rst | 2 ++ user_guide_src/source/installation/upgrade_440.rst | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/user_guide_src/source/changelogs/v4.4.0.rst b/user_guide_src/source/changelogs/v4.4.0.rst index 2d8cbf5c73e1..0e5db5e1fc23 100644 --- a/user_guide_src/source/changelogs/v4.4.0.rst +++ b/user_guide_src/source/changelogs/v4.4.0.rst @@ -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 ************ diff --git a/user_guide_src/source/installation/upgrade_440.rst b/user_guide_src/source/installation/upgrade_440.rst index 9c765f24434b..a0c34f67e77b 100644 --- a/user_guide_src/source/installation/upgrade_440.rst +++ b/user_guide_src/source/installation/upgrade_440.rst @@ -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 **********************