Skip to content

Commit

Permalink
Remove _handleNoRoute.
Browse files Browse the repository at this point in the history 
Routes should always correctly reverse match.  If not an
exception should be raised. (No exceptions just yet though).
  • Loading branch information
@markstory
markstory committed Jul 4, 2012
1 parent f977d71 commit 34cd212
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 93 deletions.
5 changes: 4 additions & 1 deletion lib/Cake/Routing/RouteCollection.php
View file
Expand Up @@ -50,7 +50,10 @@ public function match($url, $requestContext = null) {
$names = $this->_getNames($url);
foreach ($names as $name) {
if (isset($this->_routeTable[$name])) {
return $this->_matchRoutes($this->_routeTable[$name], $url, $requestContext);
$output = $this->_matchRoutes($this->_routeTable[$name], $url, $requestContext);
if ($output) {
return $output;
}
}
}
return $this->_matchRoutes($this->_routes, $url, $requestContext);
Expand Down
64 changes: 0 additions & 64 deletions lib/Cake/Routing/Router.php
View file
Expand Up @@ -692,12 +692,7 @@ public static function url($url = null, $full = false) {
}

$url += array('controller' => $params['controller'], 'plugin' => $params['plugin']);

$output = self::$_routes->match($url, $params);

if ($output === false) {
$output = static::_handleNoRoute($url);
}
} else {
if (
(strpos($url, '://') !== false ||
Expand Down Expand Up @@ -736,65 +731,6 @@ public static function url($url = null, $full = false) {
return $output . $extension . static::queryString($q, array(), $escape) . $frag;
}

/**
* A special fallback method that handles url arrays that cannot match
* any defined routes.
*
* @param array $url A url that didn't match any routes
* @return string A generated url for the array
* @see Router::url()
*/
protected static function _handleNoRoute($url) {
$args = array();
$skip = array_merge(
array('bare', 'action', 'controller', 'plugin', 'prefix'),
static::$_prefixes
);

$keys = array_values(array_diff(array_keys($url), $skip));
$count = count($keys);

// Remove this once parsed URL parameters can be inserted into 'pass'
for ($i = 0; $i < $count; $i++) {
$key = $keys[$i];
if (is_numeric($keys[$i])) {
$args[] = $url[$key];
}
}

$args = Hash::filter($args);
foreach (static::$_prefixes as $prefix) {
$prefixed = $prefix . '_';
if (!empty($url[$prefix]) && strpos($url['action'], $prefixed) === 0) {
$url['action'] = substr($url['action'], strlen($prefixed) * -1);
break;
}
}

if (empty($args) && (!isset($url['action']) || $url['action'] === 'index')) {
$url['action'] = null;
}

$urlOut = array_filter(array($url['controller'], $url['action']));

if (isset($url['plugin'])) {
array_unshift($urlOut, $url['plugin']);
}

foreach (static::$_prefixes as $prefix) {
if (isset($url[$prefix])) {
array_unshift($urlOut, $prefix);
break;
}
}
$output = implode('/', $urlOut);

if (!empty($args)) {
$output .= '/' . implode('/', array_map('rawurlencode', $args));
}
return $output;
}

/**
* Generates a well-formed querystring from $q
*
Expand Down
71 changes: 43 additions & 28 deletions lib/Cake/Test/TestCase/Routing/RouterTest.php
View file
Expand Up @@ -253,6 +253,8 @@ public function testGenerateUrlResourceRoute() {
* @return void
*/
public function testUrlNormalization() {
Router::connect('/:controller/:action');

$expected = '/users/logout';

$result = Router::normalize('/users/logout/');
Expand Down Expand Up @@ -377,17 +379,14 @@ public function testUrlGenerationBasic() {
$expected = '/posts/1';
$this->assertEquals($expected, $result);

$result = Router::url(array('controller' => 'posts', 'action' => 'index', '0'));
$expected = '/posts/index/0';
$this->assertEquals($expected, $result);

Router::connect('/view/*', array('controller' => 'posts', 'action' => 'view'));
Router::promote();
$result = Router::url(array('controller' => 'posts', 'action' => 'view', '1'));
$expected = '/view/1';
$this->assertEquals($expected, $result);

Router::reload();
Router::connect('/:controller/:action');
Router::parse('/');
$request = new Request();
$request->addParams(array(
Expand All @@ -402,14 +401,6 @@ public function testUrlGenerationBasic() {
$expected = '/users/login';
$this->assertEquals($expected, $result);

Router::reload();
Router::connect('/page/*', array('plugin' => null, 'controller' => 'pages', 'action' => 'view'));
Router::parse('/');

$result = Router::url(array('plugin' => 'my_plugin', 'controller' => 'pages', 'action' => 'view', 'my-page'));
$expected = '/my_plugin/pages/view/my-page';
$this->assertEquals($expected, $result);

Router::reload();
Router::connect('/contact/:action', array('plugin' => 'contact', 'controller' => 'contact'));
Router::parse('/');
Expand All @@ -420,6 +411,7 @@ public function testUrlGenerationBasic() {
$this->assertEquals($expected, $result);

Router::reload();
Router::connect('/:controller', array('action' => 'index'));
$request = new Request();
$request->addParams(array(
'action' => 'index', 'plugin' => 'myplugin', 'controller' => 'mycontroller', 'admin' => false
Expand All @@ -438,8 +430,10 @@ public function testUrlGenerationBasic() {
* Test generation of routes with query string parameters.
*
* @return void
**/
*/
public function testUrlGenerationWithQueryStrings() {
Router::connect('/:controller/:action/*');

$result = Router::url(array('controller' => 'posts', 'action' => 'index', '0', '?' => 'var=test&var2=test2'));
$expected = '/posts/index/0?var=test&var2=test2';
$this->assertEquals($expected, $result);
Expand Down Expand Up @@ -553,6 +547,7 @@ public function testUrlGenerationWithAdminPrefix() {
Router::connect('/pages/*', array('controller' => 'pages', 'action' => 'display'));
Router::connect('/reset/*', array('admin' => true, 'controller' => 'users', 'action' => 'reset'));
Router::connect('/tests', array('controller' => 'tests', 'action' => 'index'));
Router::connect('/admin/:controller/:action/*', array('admin' => true, 'prefix' => 'admin'));
Router::parseExtensions('rss');

$request = new Request();
Expand All @@ -575,6 +570,7 @@ public function testUrlGenerationWithAdminPrefix() {

Router::reload();
Router::connect('/admin/subscriptions/:action/*', array('controller' => 'subscribe', 'admin' => true, 'prefix' => 'admin'));
Router::connect('/admin/:controller/:action/*', array('admin' => true, 'prefix' => 'admin'));
Router::parse('/');

$request = new Request();
Expand Down Expand Up @@ -620,6 +616,7 @@ public function testUrlGenerationWithAdminPrefix() {
$this->assertEquals($expected, $result);

Router::reload();
Router::connect('/admin/:controller/:action/*', array('admin' => true, 'prefix' => 'admin'));

$request = new Request();
$request->addParams(array(
Expand All @@ -637,6 +634,7 @@ public function testUrlGenerationWithAdminPrefix() {
$this->assertEquals($expected, $result);

Router::reload();
Router::connect('/admin/:controller/:action/*', array('admin' => true, 'prefix' => 'admin'));
Router::parse('/');
$request = new Request();
$request->addParams(array(
Expand Down Expand Up @@ -671,6 +669,7 @@ public function testUrlGenerationWithAdminPrefix() {
$this->assertEquals($expected, $result);

Router::reload();
Router::connect('/admin/:controller/:action/*', array('admin' => true, 'prefix' => 'admin'));
Router::parse('/');

$request = new Request();
Expand All @@ -688,6 +687,7 @@ public function testUrlGenerationWithAdminPrefix() {
$this->assertEquals($expected, $result);

Router::reload();
Router::connect('/admin/:controller/:action/*', array('admin' => true, 'prefix' => 'admin'));
Router::parse('/');

$request = new Request();
Expand Down Expand Up @@ -727,7 +727,10 @@ public function testUrlGenerationWithAdminPrefix() {
* @return void
*/
public function testUrlGenerationWithExtensions() {
Router::connect('/:controller', array('action' => 'index'));
Router::connect('/:controller/:action');
Router::parse('/');

$result = Router::url(array('plugin' => null, 'controller' => 'articles', 'action' => 'add', 'id' => null, 'ext' => 'json'));
$expected = '/articles/add.json';
$this->assertEquals($expected, $result);
Expand Down Expand Up @@ -783,11 +786,12 @@ public function testUrlGenerationPlugins() {
* @return void
*/
public function testCanLeavePlugin() {
Router::reload();
Router::connect('/admin/:controller', array('action' => 'index', 'admin' => true, 'prefix' => 'admin'));
Router::connect('/admin/:controller/:action/*', array('admin' => true, 'prefix' => 'admin'));
Router::connect(
'/admin/other/:controller/:action/*',
array(
'admin' => 1,
'admin' => true,
'plugin' => 'aliased',
'prefix' => 'admin'
)
Expand All @@ -811,10 +815,6 @@ public function testCanLeavePlugin() {
$result = Router::url(array('plugin' => null, 'controller' => 'posts', 'action' => 'index'));
$this->assertEquals('/admin/posts', $result);

$result = Router::url(array('controller' => 'posts', 'action' => 'index'));
$this->assertEquals('/admin/this/posts', $result);

$result = Router::url(array('plugin' => 'aliased', 'controller' => 'posts', 'action' => 'index'));
$this->assertEquals('/admin/other/posts/index', $result);
}

Expand Down Expand Up @@ -932,6 +932,8 @@ public function testUrlParsing() {
*/
public function testPersistentParameters() {
Router::reload();
Router::connect('/:controller', array('action' => 'index'));
Router::connect('/:controller/:action/*');
Router::connect(
'/:lang/:color/posts/view/*',
array('controller' => 'posts', 'action' => 'view'),
Expand All @@ -944,6 +946,7 @@ public function testPersistentParameters() {
));
Router::connect('/:lang/:color/posts/edit/*', array('controller' => 'posts', 'action' => 'edit'));
Router::connect('/about', array('controller' => 'pages', 'action' => 'view', 'about'));

Router::parse('/en/red/posts/view/5');

$request = new Request();
Expand All @@ -961,6 +964,7 @@ public function testPersistentParameters() {
'webroot' => '/',
))
);

$expected = '/en/red/posts/view/6';
$result = Router::url(array('controller' => 'posts', 'action' => 'view', 6));
$this->assertEquals($expected, $result);
Expand Down Expand Up @@ -1228,6 +1232,8 @@ public function testExtensionParsing() {
* @return void
*/
public function testQuerystringGeneration() {
Router::connect('/:controller/:action/*');

$result = Router::url(array('controller' => 'posts', 'action' => 'index', '0', '?' => 'var=test&var2=test2'));
$expected = '/posts/index/0?var=test&var2=test2';
$this->assertEquals($expected, $result);
Expand Down Expand Up @@ -1257,8 +1263,11 @@ public function testQuerystringGeneration() {
* @return void
*/
public function testUrlGenerationWithAutoPrefixes() {
Configure::write('Routing.prefixes', array('protected'));
Router::reload();
Router::connect('/protected/:controller/:action/*', array('prefix' => 'protected', 'protected' => true));
Router::connect('/admin/:controller/:action/*', array('prefix' => 'admin', 'admin' => true));
Router::connect('/:controller/:action/*');

Router::parse('/');

$request = new Request();
Expand Down Expand Up @@ -1320,8 +1329,9 @@ public function testUrlGenerationWithAutoPrefixes() {
* @return void
*/
public function testAutoPrefixRoutePersistence() {
Configure::write('Routing.prefixes', array('protected'));
Router::reload();
Router::connect('/protected/:controller/:action', array('prefix' => 'protected', 'protected' => true));
Router::connect('/:controller/:action');
Router::parse('/');

$request = new Request();
Expand Down Expand Up @@ -1351,8 +1361,9 @@ public function testAutoPrefixRoutePersistence() {
* @return void
*/
public function testPrefixOverride() {
Configure::write('Routing.prefixes', array('protected', 'admin'));
Router::reload();
Router::connect('/admin/:controller/:action', array('prefix' => 'admin', 'admin' => true));
Router::connect('/protected/:controller/:action', array('prefix' => 'protected', 'protected' => true));
Router::parse('/');

$request = new Request();
Expand Down Expand Up @@ -1405,9 +1416,6 @@ public function testPrefixFalseIgnored() {
$url = Router::url(array('admin' => false, 'controller' => 'asset_compress', 'action' => 'get', 'test'));
$expected = '/cache_css/test';
$this->assertEquals($expected, $url);

$url = Router::url(array('admin' => true, 'controller' => 'asset_compress', 'action' => 'get', 'test'));
$this->assertEquals('/admin/asset_compress/get/test', $url);
}

/**
Expand All @@ -1416,6 +1424,8 @@ public function testPrefixFalseIgnored() {
* @return void
*/
public function testRemoveBase() {
Router::connect('/:controller/:action');

$request = new Request();
Router::setRequestInfo(
$request->addParams(array(
Expand Down Expand Up @@ -1528,7 +1538,7 @@ public function testParsingWithPatternOnAction() {
$this->assertEquals(array(), $result);

$result = Router::url(array('controller' => 'blog_posts', 'action' => 'foo'));
$this->assertEquals('/blog_posts/foo', $result);
$this->assertEquals('/', $result);

$result = Router::url(array('controller' => 'blog_posts', 'action' => 'actions'));
$this->assertEquals('/blog/actions', $result);
Expand Down Expand Up @@ -1606,7 +1616,7 @@ public function testParsingWithPrefixes() {
*/
public function testUrlWritingWithPrefixes() {
Router::connect('/company/:controller/:action/*', array('prefix' => 'company', 'company' => true));
Router::connect('/login', array('controller' => 'users', 'action' => 'login'));
Router::connect('/:action', array('controller' => 'users'));

$result = Router::url(array('controller' => 'users', 'action' => 'login', 'company' => true));
$expected = '/company/users/login';
Expand Down Expand Up @@ -1751,7 +1761,7 @@ public function testRegexRouteMatching() {
);

$result = Router::url(array('action' => 'test_another_action'));
$expected = '/test/test_another_action';
$expected = '/';
$this->assertEquals($expected, $result);

$result = Router::url(array('action' => 'test_another_action', 'locale' => 'eng'));
Expand Down Expand Up @@ -1938,6 +1948,7 @@ public function testCustomRouteException() {
* @return void
*/
public function testRouterReverse() {
Router::connect('/:controller/:action/*');
$params = array(
'controller' => 'posts',
'action' => 'view',
Expand Down Expand Up @@ -2004,6 +2015,7 @@ public function testRouterReverse() {
* @return void
*/
public function testReverseWithExtension() {
Router::connect('/:controller/:action/*');
Router::parseExtensions('json');

$request = new Request('/posts/view/1.json');
Expand Down Expand Up @@ -2048,6 +2060,9 @@ public function testSetRequestInfoLegacy() {
* Test that Router::url() uses the first request
*/
public function testUrlWithRequestAction() {
Router::connect('/:controller', array('action' => 'index'));
Router::connect('/:controller/:action');

$firstRequest = new Request('/posts/index');
$firstRequest->addParams(array(
'plugin' => null,
Expand Down

0 comments on commit 34cd212

Please sign in to comment.