From 585fbc0b7df5daa9aa61ab39f8ad520c15e30789 Mon Sep 17 00:00:00 2001 From: David McReynolds Date: Wed, 2 Aug 2017 16:58:10 -0700 Subject: [PATCH] Fix for issue #642 regarding using the route_to function with routes other then post --- system/Router/RouteCollection.php | 141 ++++++++++---------- system/Router/Router.php | 2 +- tests/system/Router/RouteCollectionTest.php | 16 ++- 3 files changed, 88 insertions(+), 71 deletions(-) diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index f4312fe2589c..1e8309ba0935 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -130,7 +130,18 @@ class RouteCollection implements RouteCollectionInterface * * @var array */ - protected $routes = []; + protected $routes = [ + '*' => [], + 'options' => [], + 'get' => [], + 'head' => [], + 'post' => [], + 'put' => [], + 'delete' => [], + 'trace' => [], + 'connect' => [], + 'cli' => [], + ]; /** * The current method that the script is being called by. @@ -497,8 +508,13 @@ public function shouldAutoRoute(): bool * * @return array */ - public function getRoutes(): array + public function getRoutes($verb = null): array { + if (empty($verb)) + { + $verb = $this->getHTTPVerb(); + } + // Since this is the entry point for the Router, // take a moment to do any route discovery // we might need to do. @@ -506,10 +522,15 @@ public function getRoutes(): array $routes = []; - foreach ($this->routes as $r) + if (isset($this->routes[$verb])) { - $key = key($r['route']); - $routes[$key] = $r['route'][$key]; + $collection = array_merge($this->routes['*'], $this->routes[$verb]); + + foreach ($collection as $r) + { + $key = key($r['route']); + $routes[$key] = $r['route'][$key]; + } } return $routes; @@ -565,7 +586,7 @@ public function map(array $routes = [], array $options = null): RouteCollectionI */ public function add(string $from, $to, array $options = null): RouteCollectionInterface { - $this->create($from, $to, $options); + $this->create('*', $from, $to, $options); return $this; } @@ -586,12 +607,12 @@ public function add(string $from, $to, array $options = null): RouteCollectionIn 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)) + if (array_key_exists($to, $this->routes['*'])) { - $to = $this->routes[$to]['route']; + $to = $this->routes['*'][$to]['route']; } - $this->create($from, $to, ['redirect' => $status]); + $this->create('*', $from, $to, ['redirect' => $status]); return $this; } @@ -607,7 +628,7 @@ public function addRedirect(string $from, string $to, int $status = 302) */ public function isRedirect(string $from): bool { - foreach ($this->routes as $name => $route) + foreach ($this->routes['*'] as $name => $route) { // Named route? if ($name == $from || key($route['route']) == $from) @@ -630,7 +651,7 @@ public function isRedirect(string $from): bool */ public function getRedirectCode(string $from): int { - foreach ($this->routes as $name => $route) + foreach ($this->routes['*'] as $name => $route) { // Named route? if ($name == $from || key($route['route']) == $from) @@ -822,10 +843,7 @@ public function match(array $verbs = [], string $from, $to, array $options = nul */ public function get(string $from, $to, array $options = null): RouteCollectionInterface { - if ($this->HTTPVerb == 'get') - { - $this->create($from, $to, $options); - } + $this->create('get', $from, $to, $options); return $this; } @@ -843,10 +861,7 @@ public function get(string $from, $to, array $options = null): RouteCollectionIn */ public function post(string $from, $to, array $options = null): RouteCollectionInterface { - if ($this->HTTPVerb == 'post') - { - $this->create($from, $to, $options); - } + $this->create('post', $from, $to, $options); return $this; } @@ -864,10 +879,7 @@ public function post(string $from, $to, array $options = null): RouteCollectionI */ public function put(string $from, $to, array $options = null): RouteCollectionInterface { - if ($this->HTTPVerb == 'put') - { - $this->create($from, $to, $options); - } + $this->create('put', $from, $to, $options); return $this; } @@ -885,10 +897,7 @@ public function put(string $from, $to, array $options = null): RouteCollectionIn */ public function delete(string $from, $to, array $options = null): RouteCollectionInterface { - if ($this->HTTPVerb == 'delete') - { - $this->create($from, $to, $options); - } + $this->create('delete', $from, $to, $options); return $this; } @@ -906,10 +915,7 @@ public function delete(string $from, $to, array $options = null): RouteCollectio */ public function head(string $from, $to, array $options = null): RouteCollectionInterface { - if ($this->HTTPVerb == 'head') - { - $this->create($from, $to, $options); - } + $this->create('head', $from, $to, $options); return $this; } @@ -927,10 +933,7 @@ public function head(string $from, $to, array $options = null): RouteCollectionI */ public function patch(string $from, $to, array $options = null): RouteCollectionInterface { - if ($this->HTTPVerb == 'patch') - { - $this->create($from, $to, $options); - } + $this->create('patch', $from, $to, $options); return $this; } @@ -948,10 +951,7 @@ public function patch(string $from, $to, array $options = null): RouteCollection */ public function options(string $from, $to, array $options = null): RouteCollectionInterface { - if ($this->HTTPVerb == 'options') - { - $this->create($from, $to, $options); - } + $this->create('options', $from, $to, $options); return $this; } @@ -969,10 +969,7 @@ public function options(string $from, $to, array $options = null): RouteCollecti */ public function cli(string $from, $to, array $options = null): RouteCollectionInterface { - if ($this->HTTPVerb == 'cli') - { - $this->create($from, $to, $options); - } + $this->create('cli', $from, $to, $options); return $this; } @@ -1020,38 +1017,44 @@ public function environment(string $env, \Closure $callback): RouteCollectionInt public function reverseRoute(string $search, ...$params) { // Named routes get higher priority. - if (array_key_exists($search, $this->routes)) + foreach ($this->routes as $verb => $collection) { - return $this->fillRouteParams(key($this->routes[$search]['route']), $params); + if (array_key_exists($search, $collection)) + { + return $this->fillRouteParams(key($collection[$search]['route']), $params); + } } // If it's not a named route, then loop over // all routes to find a match. - foreach ($this->routes as $route) + foreach ($this->routes as $verb => $collection) { - $from = key($route['route']); - $to = $route['route'][$from]; - - // Lose any namespace slash at beginning of strings - // to ensure more consistent match. - $to = ltrim($to, '\\'); - $search = ltrim($search, '\\'); - - // If there's any chance of a match, then it will - // be with $search at the beginning of the $to string. - if (strpos($to, $search) !== 0) + foreach ($collection as $route) { - continue; - } + $from = key($route['route']); + $to = $route['route'][$from]; + + // Lose any namespace slash at beginning of strings + // to ensure more consistent match. + $to = ltrim($to, '\\'); + $search = ltrim($search, '\\'); + + // If there's any chance of a match, then it will + // be with $search at the beginning of the $to string. + if (strpos($to, $search) !== 0) + { + continue; + } - // Ensure that the number of $params given here - // matches the number of back-references in the route - if (substr_count($to, '$') != count($params)) - { - continue; - } + // Ensure that the number of $params given here + // matches the number of back-references in the route + if (substr_count($to, '$') != count($params)) + { + continue; + } - return $this->fillRouteParams($from, $params); + return $this->fillRouteParams($from, $params); + } } // If we're still here, then we did not find a match. @@ -1108,7 +1111,7 @@ protected function fillRouteParams(string $from, array $params = null): string * @param array|string $to * @param array $options */ - protected function create(string $from, $to, array $options = null) + protected function create(string $verb, string $from, $to, array $options = null) { $prefix = is_null($this->group) ? '' : $this->group . '/'; @@ -1185,14 +1188,14 @@ protected function create(string $from, $to, array $options = null) $name = $options['as'] ?? $from; - $this->routes[$name] = [ + $this->routes[$verb][$name] = [ 'route' => [$from => $to] ]; // Is this a redirect? if (isset($options['redirect']) && is_numeric($options['redirect'])) { - $this->routes[$name]['redirect'] = $options['redirect']; + $this->routes['*'][$name]['redirect'] = $options['redirect']; } } diff --git a/system/Router/Router.php b/system/Router/Router.php index bcbb6899974b..d90393f03743 100644 --- a/system/Router/Router.php +++ b/system/Router/Router.php @@ -339,7 +339,7 @@ public function getLocale() */ protected function checkRoutes(string $uri): bool { - $routes = $this->collection->getRoutes(); + $routes = $this->collection->getRoutes($this->collection->getHTTPVerb()); // Don't waste any time if (empty($routes)) diff --git a/tests/system/Router/RouteCollectionTest.php b/tests/system/Router/RouteCollectionTest.php index 22f42c8761a7..453d82b7104c 100644 --- a/tests/system/Router/RouteCollectionTest.php +++ b/tests/system/Router/RouteCollectionTest.php @@ -653,6 +653,21 @@ public function testNamedRoutesFillInParams() //-------------------------------------------------------------------- + /** + * @see https://github.com/bcit-ci/CodeIgniter4/issues/642 + */ + public function testReverseRoutesWithPostRoutes() + { + $routes = $this->getCollector(); + + $routes->post('user/insert', function() {}, ['as' => 'namedRoute']); + $match = $routes->reverseRoute('namedRoute'); + + $this->assertEquals('/user/insert', $match); + } + + //-------------------------------------------------------------------- + public function testAddRedirect() { $routes = $this->getCollector(); @@ -720,5 +735,4 @@ public function testWillDiscoverLocal() $this->assertTrue(array_key_exists('testing', $match)); $this->assertEquals($match['testing'], '\TestController::index'); } - }