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

Fix for issue #642 regarding the route_to function and post routes #648

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
141 changes: 72 additions & 69 deletions system/Router/RouteCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -497,19 +508,29 @@ 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.
$this->discoverRoutes();

$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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -829,10 +850,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;
}
Expand All @@ -850,10 +868,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;
}
Expand All @@ -871,10 +886,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;
}
Expand All @@ -892,10 +904,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;
}
Expand All @@ -913,10 +922,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;
}
Expand All @@ -934,10 +940,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;
}
Expand All @@ -955,10 +958,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;
}
Expand All @@ -976,10 +976,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;
}
Expand Down Expand Up @@ -1027,38 +1024,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.
Expand Down Expand Up @@ -1115,7 +1118,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 . '/';

Expand Down Expand Up @@ -1192,14 +1195,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'];
}
}

Expand Down
2 changes: 1 addition & 1 deletion system/Router/Router.php
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
5 changes: 2 additions & 3 deletions tests/_support/_bootstrap.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
<?php
// Make sure it recognizes that we're testing.
$_SERVER['CI_ENVIRONMENT'] = 'testing';

// Get our system paths
require 'application/Config/Paths.php';
Expand All @@ -7,9 +9,6 @@
// Path to the front controller (this file)
define('FCPATH', getcwd().'/public'.DIRECTORY_SEPARATOR);

// Make sure it recognizes that we're testing.
$_SERVER['CI_ENVIRONMENT'] = 'testing';

/*
* ---------------------------------------------------------------
* SETUP OUR PATH CONSTANTS
Expand Down
23 changes: 22 additions & 1 deletion tests/system/Router/RouteCollectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,28 @@ public function testNamedRoutesFillInParams()

//--------------------------------------------------------------------

/**
* @see https://github.com/bcit-ci/CodeIgniter4/issues/642
*/
public function testNamedRoutesWithSameURIDifferentMethods()
{
$routes = $this->getCollector();

$routes->get('user/insert', 'myController::goto/$1/$2', ['as' => 'namedRoute1']);
$routes->post('user/insert', function() {}, ['as' => 'namedRoute2']);
$routes->put('user/insert', function() {}, ['as' => 'namedRoute3']);

$match1 = $routes->reverseRoute('namedRoute1');
$match2 = $routes->reverseRoute('namedRoute2');
$match3 = $routes->reverseRoute('namedRoute3');

$this->assertEquals('/user/insert', $match1);
$this->assertEquals('/user/insert', $match2);
$this->assertEquals('/user/insert', $match3);
}

//--------------------------------------------------------------------

public function testAddRedirect()
{
$routes = $this->getCollector();
Expand Down Expand Up @@ -736,5 +758,4 @@ public function testWillDiscoverLocal()
$this->assertTrue(array_key_exists('testing', $match));
$this->assertEquals($match['testing'], '\TestController::index');
}

}