Skip to content

Commit

Permalink
Fixing issues in router where plugin => null would not always exit a …
Browse files Browse the repository at this point in the history
…plugin route. Test cases added.
  • Loading branch information
markstory committed Dec 9, 2009
1 parent efa36ab commit 4421fe6
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 20 deletions.
19 changes: 10 additions & 9 deletions cake/libs/router.php
Expand Up @@ -228,13 +228,13 @@ function getNamedExpressions() {
* Examples: * Examples:
* *
* `Router::connect('/:controller/:action/*');` * `Router::connect('/:controller/:action/*');`
* *
* The first parameter will be used as a controller name while the second is used as the action name. * The first parameter will be used as a controller name while the second is used as the action name.
* the '/*' syntax makes this route greedy in that it will match requests like `/posts/index` as well as requests * the '/*' syntax makes this route greedy in that it will match requests like `/posts/index` as well as requests
* like `/posts/edit/1/foo/bar`. * like `/posts/edit/1/foo/bar`.
* *
* `Router::connect('/home-page', array('controller' => 'pages', 'action' => 'display', 'home'));` * `Router::connect('/home-page', array('controller' => 'pages', 'action' => 'display', 'home'));`
* *
* The above shows the use of route parameter defaults. And providing routing parameters for a static route. * The above shows the use of route parameter defaults. And providing routing parameters for a static route.
* *
* {{{ * {{{
Expand All @@ -251,8 +251,8 @@ function getNamedExpressions() {
* @param string $route A string describing the template of the route * @param string $route A string describing the template of the route
* @param array $defaults An array describing the default route parameters. These parameters will be used by default * @param array $defaults An array describing the default route parameters. These parameters will be used by default
* and can supply routing parameters that are not dynamic. See above. * and can supply routing parameters that are not dynamic. See above.
* @param array $options An array matching the named elements in the route to regular expressions which that * @param array $options An array matching the named elements in the route to regular expressions which that
* element should match. Also contains additional parameters such as which routed parameters should be * element should match. Also contains additional parameters such as which routed parameters should be
* shifted into the passed arguments. As well as supplying patterns for routing parameters. * shifted into the passed arguments. As well as supplying patterns for routing parameters.
* @see routes * @see routes
* @return array Array of routes * @return array Array of routes
Expand Down Expand Up @@ -1374,6 +1374,7 @@ function match($url) {


$diffUnfiltered = Set::diff($url, $defaults); $diffUnfiltered = Set::diff($url, $defaults);
$diff = array(); $diff = array();

foreach ($diffUnfiltered as $key => $var) { foreach ($diffUnfiltered as $key => $var) {
if ($var === 0 || $var === '0' || !empty($var)) { if ($var === 0 || $var === '0' || !empty($var)) {
$diff[$key] = $var; $diff[$key] = $var;
Expand All @@ -1386,14 +1387,13 @@ function match($url) {
} }


//remove defaults that are also keys. They can cause match failures //remove defaults that are also keys. They can cause match failures
$count = count($this->keys); foreach ($this->keys as $key) {
while ($count--) { unset($defaults[$key]);
unset($defaults[$this->keys[$count]]);
} }
$filteredDefaults = array_filter($defaults); $filteredDefaults = array_filter($defaults);


//if the difference between the url diff and defaults contains keys from defaults its not a match //if the difference between the url diff and defaults contains keys from defaults its not a match
if (array_intersect_key($filteredDefaults, $diff) !== array()) { if (array_intersect_key($filteredDefaults, $diffUnfiltered) !== array()) {
return false; return false;
} }


Expand All @@ -1405,6 +1405,7 @@ function match($url) {
} }
$i++; $i++;
} }

$passedArgsAndParams = array_diff_key($diff, $filteredDefaults, $keyNames); $passedArgsAndParams = array_diff_key($diff, $filteredDefaults, $keyNames);
list($named, $params) = Router::getNamedElements($passedArgsAndParams, $url['controller'], $url['action']); list($named, $params) = Router::getNamedElements($passedArgsAndParams, $url['controller'], $url['action']);


Expand Down
64 changes: 53 additions & 11 deletions cake/tests/cases/libs/router.test.php
Expand Up @@ -391,7 +391,7 @@ function testUrlGenerationBasic() {
Router::reload(); Router::reload();
Router::connect('/:controller/:action/:id', array(), array('id' => $ID)); Router::connect('/:controller/:action/:id', array(), array('id' => $ID));
Router::parse('/'); Router::parse('/');

$result = Router::url(array('controller' => 'posts', 'action' => 'view', 'id' => '1')); $result = Router::url(array('controller' => 'posts', 'action' => 'view', 'id' => '1'));
$expected = '/posts/view/1'; $expected = '/posts/view/1';
$this->assertEqual($result, $expected); $this->assertEqual($result, $expected);
Expand Down Expand Up @@ -770,7 +770,7 @@ function testUrlGenerationWithExtensions() {
* @access public * @access public
* @return void * @return void
*/ */
function testPluginUrlGeneration() { function testUrlGenerationPlugins() {
Router::setRequestInfo(array( Router::setRequestInfo(array(
array( array(
'controller' => 'controller', 'action' => 'index', 'form' => array(), 'controller' => 'controller', 'action' => 'index', 'form' => array(),
Expand All @@ -785,17 +785,17 @@ function testPluginUrlGeneration() {
$this->assertEqual(Router::url('read/1'), '/base/test/controller/read/1'); $this->assertEqual(Router::url('read/1'), '/base/test/controller/read/1');


Router::reload(); Router::reload();

Router::connect('/:lang/:plugin/:controller/*', array('action' => 'index')); Router::connect('/:lang/:plugin/:controller/*', array('action' => 'index'));


Router::setRequestInfo(array( Router::setRequestInfo(array(
array( array(
'lang' => 'en', 'lang' => 'en',
'plugin' => 'shows', 'controller' => 'shows', 'action' => 'index', 'pass' => 'plugin' => 'shows', 'controller' => 'shows', 'action' => 'index', 'pass' =>
array(), 'form' => array(), 'url' => array(), 'form' => array(), 'url' =>
array('url' => 'en/shows/')), array('url' => 'en/shows/')),
array('plugin' => NULL, 'controller' => NULL, 'action' => NULL, 'base' => '', array('plugin' => NULL, 'controller' => NULL, 'action' => NULL, 'base' => '',
'here' => '/en/shows/', 'webroot' => '/'))); 'here' => '/en/shows/', 'webroot' => '/')
));


Router::parse('/en/shows/'); Router::parse('/en/shows/');


Expand All @@ -807,6 +807,48 @@ function testPluginUrlGeneration() {
$this->assertEqual($result, $expected); $this->assertEqual($result, $expected);
} }


/**
* test that you can leave active plugin routes with plugin = null
*
* @return void
*/
function testCanLeavePlugin() {
Router::reload();
Router::connect(
'/admin/other/:controller/:action/*',
array(
'admin' => 1,
'plugin' => 'aliased',
'prefix' => 'admin'
)
);
Router::setRequestInfo(array(
array(
'pass' => array(),
'admin' => true,
'prefix' => 'admin',
'plugin' => 'this',
'action' => 'admin_index',
'controller' => 'interesting',
'url' => array('url' => 'admin/this/interesting/index'),
),
array(
'base' => '',
'here' => '/admin/this/interesting/index',
'webroot' => '/',
'passedArgs' => array(),
)
));
$result = Router::url(array('plugin' => null, 'controller' => 'posts', 'action' => 'index'));
$this->assertEqual($result, '/admin/posts');

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

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

/** /**
* testUrlParsing method * testUrlParsing method
* *
Expand Down Expand Up @@ -1147,7 +1189,7 @@ function testPrefixRoutingAndPlugins() {
$result = Router::url(array('plugin' => 'test_plugin', 'controller' => 'test_plugin', 'action' => 'index')); $result = Router::url(array('plugin' => 'test_plugin', 'controller' => 'test_plugin', 'action' => 'index'));
$expected = '/admin/test_plugin'; $expected = '/admin/test_plugin';
$this->assertEqual($result, $expected); $this->assertEqual($result, $expected);

Router::reload(); Router::reload();
Router::parse('/'); Router::parse('/');
Router::setRequestInfo(array( Router::setRequestInfo(array(
Expand Down

0 comments on commit 4421fe6

Please sign in to comment.