Reversing changes that required a : sigil for named parameters. Also removing ?foo style parameters for querystring args. Having two ways to create querystring args was not sitting well with me.

Tests updated.
This commit is contained in:
mark_story 2010-12-19 23:11:02 -05:00
parent b49b49a5ef
commit e5588f746c
8 changed files with 46 additions and 116 deletions

View file

@ -63,9 +63,8 @@ class PaginatorComponent extends Component {
* - `limit` The initial number of items per page. Defaults to 20.
* - `page` The starting page, defaults to 1.
* - `paramType` What type of parameters you want pagination to use?
* - `named` Use named parameters.
* - `named` Use named parameters / routed parameters.
* - `querystring` Use query string parameters.
* - `route` Use routed parameters, these require you to setup routes that include the pagination params
*
* @var array
*/

View file

@ -73,16 +73,6 @@ class CakeRoute {
*/
protected $_compiledRoute = null;
/**
* Constant for the sigil that indicates a route param is a named parameter.
*/
const SIGIL_NAMED = ':';
/**
* Constant for the sigil that indicates a route param is a query string parameter.
*/
const SIGIL_QUERYSTRING = '?';
/**
* HTTP header shortcut map. Used for evaluating header-based route expressions.
*
@ -314,13 +304,6 @@ class CakeRoute {
continue;
}
// pull out querystring params
if ($key[0] === CakeRoute::SIGIL_QUERYSTRING && ($value !== false && $value !== null)) {
$_query[substr($key, 1)] = $value;
unset($url[$key]);
continue;
}
// pull out named params if named params are greedy or a rule exists.
if (
($greedyNamed || isset($allowedNamedParams[$key])) &&
@ -402,9 +385,6 @@ class CakeRoute {
if (strpos($this->template, '*')) {
$out = str_replace('*', $params['pass'], $out);
}
if (!empty($params['_query'])) {
$out .= $this->queryString($params['_query']);
}
$out = str_replace('//', '/', $out);
return $out;
}

View file

@ -914,8 +914,6 @@ class Router {
$key = $keys[$i];
if (is_numeric($keys[$i])) {
$args[] = $url[$key];
} elseif ($key[0] === CakeRoute::SIGIL_QUERYSTRING) {
$query[substr($key, 1)] = $url[$key];
} else {
$named[$key] = $url[$key];
}
@ -1078,10 +1076,6 @@ class Router {
$params['pass'], $params['named'], $params['paging'], $params['models'], $params['url'], $url['url'],
$params['autoRender'], $params['bare'], $params['requested'], $params['return']
);
foreach ($named as $key => $value) {
$named[CakeRoute::SIGIL_NAMED . $key] = $value;
unset($named[$key]);
}
$params = array_merge($params, $pass, $named);
if (!empty($url)) {
$params['?'] = $url;

View file

@ -119,12 +119,7 @@ class PaginatorHelper extends AppHelper {
* @return void
*/
public function beforeRender($viewFile) {
$named = $this->request->params['named'];
foreach ($named as $key => $val) {
$named[CakeRoute::SIGIL_NAMED . $key] = $val;
unset($named[$key]);
}
$this->options['url'] = array_merge($this->request->params['pass'], $named);
$this->options['url'] = array_merge($this->request->params['pass'], $this->request->params['named']);
parent::beforeRender($viewFile);
}
@ -405,18 +400,15 @@ class PaginatorHelper extends AppHelper {
* @return converted url params.
*/
protected function _convertUrlKeys($url, $type) {
$prefix = '';
switch ($type) {
case 'named':
$prefix = CakeRoute::SIGIL_NAMED;
break;
case 'querystring':
$prefix = CakeRoute::SIGIL_QUERYSTRING;
break;
if ($type == 'named') {
return $url;
}
if (!isset($url['?'])) {
$url['?'] = array();
}
foreach ($this->convertKeys as $key) {
if (isset($url[$key])) {
$url[$prefix . $key] = $url[$key];
$url['?'][$key] = $url[$key];
unset($url[$key]);
}
}

View file

@ -334,7 +334,7 @@ class CakeRouteTestCase extends CakeTestCase {
*/
function testMatchWithNamedParametersAndPassedArgs() {
Router::connectNamed(true);
/*
$route = new CakeRoute('/:controller/:action/*', array('plugin' => null));
$result = $route->match(array('controller' => 'posts', 'action' => 'index', 'plugin' => null, 'page' => 1));
$this->assertEqual($result, '/posts/index/page:1');
@ -350,7 +350,7 @@ class CakeRouteTestCase extends CakeTestCase {
$result = $route->match(array('controller' => 'posts', 'action' => 'view', 'plugin' => null, 5, 'page' => 1, 'limit' => 20, 'order' => 'title'));
$this->assertEqual($result, '/posts/view/5/page:1/limit:20/order:title');
*/
$route = new CakeRoute('/test2/*', array('controller' => 'pages', 'action' => 'display', 2));
$result = $route->match(array('controller' => 'pages', 'action' => 'display', 1));
@ -473,39 +473,4 @@ class CakeRouteTestCase extends CakeTestCase {
$this->assertFalse($result);
}
/**
* test sigil based query string params
*
* @return void
*/
function testQueryStringParams() {
$route = new CakeRoute('/:controller/:action/*');
$result = $route->match(array('controller' => 'posts', 'action' => 'index', '?test' => 'value'));
$expected = '/posts/index/?test=value';
$this->assertEquals($expected, $result);
$result = $route->match(array(
'controller' => 'posts', 'action' => 'index', '?test' => array(1, 2, 3)
));
$expected = '/posts/index/?test%5B0%5D=1&test%5B1%5D=2&test%5B2%5D=3';
$this->assertEquals($expected, $result);
$result = $route->match(array(
'controller' => 'posts', 'action' => 'index', '?test' => 'value', '?other' => 'value'
));
$expected = '/posts/index/?test=value&other=value';
$this->assertEquals($expected, $result);
}
/**
* test that querystring params work with non greedy routes.
*
* @return void
*/
function testQueryStringNonGreedy() {
$route = new CakeRoute('/:controller/:action');
$result = $route->match(array('controller' => 'posts', 'action' => 'index', '?test' => 'value'));
$expected = '/posts/index?test=value';
$this->assertEquals($expected, $result);
}
}

View file

@ -328,7 +328,7 @@ class RouterTest extends CakeTestCase {
Router::connect('short_controller_name/:action/*', array('controller' => 'real_controller_name'));
Router::parse('/');
$result = Router::url(array('controller' => 'real_controller_name', ':page' => '1'));
$result = Router::url(array('controller' => 'real_controller_name', 'page' => '1'));
$expected = '/short_controller_name/index/page:1';
$this->assertEqual($result, $expected);
@ -389,14 +389,14 @@ class RouterTest extends CakeTestCase {
* @return void
*/
function testArrayNamedParameters() {
$result = Router::url(array('controller' => 'tests', ':pages' => array(
$result = Router::url(array('controller' => 'tests', 'pages' => array(
1, 2, 3
)));
$expected = '/tests/index/pages[0]:1/pages[1]:2/pages[2]:3';
$this->assertEqual($result, $expected);
$result = Router::url(array('controller' => 'tests',
':pages' => array(
'pages' => array(
'param1' => array(
'one',
'two'
@ -408,7 +408,7 @@ class RouterTest extends CakeTestCase {
$this->assertEqual($result, $expected);
$result = Router::url(array('controller' => 'tests',
':pages' => array(
'pages' => array(
'param1' => array(
'one' => 1,
'two' => 2
@ -420,7 +420,7 @@ class RouterTest extends CakeTestCase {
$this->assertEqual($result, $expected);
$result = Router::url(array('controller' => 'tests',
':super' => array(
'super' => array(
'nested' => array(
'array' => 'awesome',
'something' => 'else'
@ -431,7 +431,7 @@ class RouterTest extends CakeTestCase {
$expected = '/tests/index/super[nested][array]:awesome/super[nested][something]:else/super[0]:cool';
$this->assertEqual($result, $expected);
$result = Router::url(array('controller' => 'tests', ':namedParam' => array(
$result = Router::url(array('controller' => 'tests', 'namedParam' => array(
'keyed' => 'is an array',
'test'
)));
@ -630,7 +630,7 @@ class RouterTest extends CakeTestCase {
$request->webroot = '/';
Router::setRequestInfo($request);
$result = Router::url(array(':page' => 2));
$result = Router::url(array('page' => 2));
$expected = '/admin/registrations/index/page:2';
$this->assertEqual($result, $expected);
@ -648,7 +648,7 @@ class RouterTest extends CakeTestCase {
Router::parse('/');
$result = Router::url(array(':page' => 3));
$result = Router::url(array('page' => 3));
$expected = '/magazine/admin/subscriptions/index/page:3';
$this->assertEquals($expected, $result);
@ -860,7 +860,7 @@ class RouterTest extends CakeTestCase {
$result = Router::url(array(
'lang' => 'en',
'controller' => 'shows', 'action' => 'index', ':page' => '1',
'controller' => 'shows', 'action' => 'index', 'page' => '1',
));
$expected = '/en/shows/shows/page:1';
$this->assertEqual($result, $expected);
@ -1361,11 +1361,11 @@ class RouterTest extends CakeTestCase {
* @return void
*/
function testNamedArgsUrlGeneration() {
$result = Router::url(array('controller' => 'posts', 'action' => 'index', ':published' => 1, ':deleted' => 1));
$result = Router::url(array('controller' => 'posts', 'action' => 'index', 'published' => 1, 'deleted' => 1));
$expected = '/posts/index/published:1/deleted:1';
$this->assertEqual($result, $expected);
$result = Router::url(array('controller' => 'posts', 'action' => 'index', ':published' => 0, ':deleted' => 0));
$result = Router::url(array('controller' => 'posts', 'action' => 'index', 'published' => 0, 'deleted' => 0));
$expected = '/posts/index/published:0/deleted:0';
$this->assertEqual($result, $expected);
@ -1375,11 +1375,11 @@ class RouterTest extends CakeTestCase {
Router::connect('/', array('controller' => 'graphs', 'action' => 'index'));
Router::connect('/:id/*', array('controller' => 'graphs', 'action' => 'view'), array('id' => $ID));
$result = Router::url(array('controller' => 'graphs', 'action' => 'view', 'id' => 12, ':file' => 'asdf.png'));
$result = Router::url(array('controller' => 'graphs', 'action' => 'view', 'id' => 12, 'file' => 'asdf.png'));
$expected = '/12/file:asdf.png';
$this->assertEqual($result, $expected);
$result = Router::url(array('controller' => 'graphs', 'action' => 'view', 12, ':file' => 'asdf.foo'));
$result = Router::url(array('controller' => 'graphs', 'action' => 'view', 12, 'file' => 'asdf.foo'));
$expected = '/graphs/view/12/file:asdf.foo';
$this->assertEqual($result, $expected);
@ -1398,7 +1398,7 @@ class RouterTest extends CakeTestCase {
);
Router::parse('/');
$result = Router::url(array(':page' => 1, 0 => null, ':sort' => 'controller', ':direction' => 'asc', ':order' => null));
$result = Router::url(array('page' => 1, 0 => null, 'sort' => 'controller', 'direction' => 'asc', 'order' => null));
$expected = "/admin/controller/index/page:1/sort:controller/direction:asc";
$this->assertEqual($result, $expected);
@ -1411,7 +1411,7 @@ class RouterTest extends CakeTestCase {
Router::setRequestInfo($request);
$result = Router::parse('/admin/controller/index/type:whatever');
$result = Router::url(array(':type'=> 'new'));
$result = Router::url(array('type'=> 'new'));
$expected = "/admin/controller/index/type:new";
$this->assertEqual($result, $expected);
}
@ -1546,12 +1546,12 @@ class RouterTest extends CakeTestCase {
$expected = '/protected/others/edit/1';
$this->assertEqual($result, $expected);
$result = Router::url(array('controller' => 'others', 'action' => 'edit', 1, 'protected' => true, ':page' => 1));
$result = Router::url(array('controller' => 'others', 'action' => 'edit', 1, 'protected' => true, 'page' => 1));
$expected = '/protected/others/edit/1/page:1';
$this->assertEqual($result, $expected);
Router::connectNamed(array('random'));
$result = Router::url(array('controller' => 'others', 'action' => 'edit', 1, 'protected' => true, ':random' => 'my-value'));
$result = Router::url(array('controller' => 'others', 'action' => 'edit', 1, 'protected' => true, 'random' => 'my-value'));
$expected = '/protected/others/edit/1/random:my-value';
$this->assertEqual($result, $expected);
}
@ -1610,12 +1610,12 @@ class RouterTest extends CakeTestCase {
$expected = '/protected/others/edit/1';
$this->assertEqual($result, $expected);
$result = Router::url(array('controller' => 'others', 'action' => 'edit', 1, 'protected' => true, ':page' => 1));
$result = Router::url(array('controller' => 'others', 'action' => 'edit', 1, 'protected' => true, 'page' => 1));
$expected = '/protected/others/edit/1/page:1';
$this->assertEqual($result, $expected);
Router::connectNamed(array('random'));
$result = Router::url(array('controller' => 'others', 'action' => 'edit', 1, 'protected' => true, ':random' => 'my-value'));
$result = Router::url(array('controller' => 'others', 'action' => 'edit', 1, 'protected' => true, 'random' => 'my-value'));
$expected = '/protected/others/edit/1/random:my-value';
$this->assertEqual($result, $expected);
}
@ -1721,7 +1721,7 @@ class RouterTest extends CakeTestCase {
$this->assertEqual($result, $expected);
$result = Router::url(array('controller' => 'my_controller', 'action' => 'my_action', 'base' => true));
$expected = '/base/my_controller/my_action';
$expected = '/base/my_controller/my_action/base:1';
$this->assertEqual($result, $expected);
}
@ -1908,7 +1908,7 @@ class RouterTest extends CakeTestCase {
$expected = array('pass' => array(), 'named' => array(), 'prefix' => 'members', 'plugin' => null, 'controller' => 'posts', 'action' => 'index', 'members' => true);
$this->assertEqual($result, $expected);
$result = Router::url(array('members' => true, 'controller' => 'posts', 'action' =>'index', ':page' => 2));
$result = Router::url(array('members' => true, 'controller' => 'posts', 'action' =>'index', 'page' => 2));
$expected = '/base/members/posts/index/page:2';
$this->assertEqual($result, $expected);
@ -2083,7 +2083,7 @@ class RouterTest extends CakeTestCase {
$this->assertEqual($result, $expected);
$result = Router::url(array('action' => 'test_another_action', 'locale' => 'badness'));
$expected = '/test/test_another_action';
$expected = '/test/test_another_action/locale:badness';
$this->assertEqual($result, $expected);
}

View file

@ -477,7 +477,7 @@ class HelperTest extends CakeTestCase {
$result = $this->Helper->url('/controller/action/1?one=1&two=2');
$this->assertEqual($result, '/controller/action/1?one=1&two=2');
$result = $this->Helper->url(array('controller' => 'posts', 'action' => 'index', ':page' => '1" onclick="alert(\'XSS\');"'));
$result = $this->Helper->url(array('controller' => 'posts', 'action' => 'index', 'page' => '1" onclick="alert(\'XSS\');"'));
$this->assertEqual($result, "/posts/index/page:1" onclick="alert('XSS');"");
$result = $this->Helper->url('/controller/action/1/param:this+one+more');
@ -490,12 +490,12 @@ class HelperTest extends CakeTestCase {
$this->assertEqual($result, '/controller/action/1/param:%7Baround%20here%7D%5Bthings%5D%5Bare%5D%24%24');
$result = $this->Helper->url(array(
'controller' => 'posts', 'action' => 'index', ':param' => '%7Baround%20here%7D%5Bthings%5D%5Bare%5D%24%24'
'controller' => 'posts', 'action' => 'index', 'param' => '%7Baround%20here%7D%5Bthings%5D%5Bare%5D%24%24'
));
$this->assertEqual($result, "/posts/index/param:%7Baround%20here%7D%5Bthings%5D%5Bare%5D%24%24");
$result = $this->Helper->url(array(
'controller' => 'posts', 'action' => 'index', ':page' => '1',
'controller' => 'posts', 'action' => 'index', 'page' => '1',
'?' => array('one' => 'value', 'two' => 'value', 'three' => 'purple')
));
$this->assertEqual($result, "/posts/index/page:1?one=value&two=value&three=purple");

View file

@ -674,7 +674,7 @@ class PaginatorHelperTest extends CakeTestCase {
$result = $this->Paginator->sort('title');
$expected = array(
'a' => array('href' => '/articles/index/2/foo:bar/page:1/sort:title/direction:asc'),
'a' => array('href' => '/articles/index/2/page:1/foo:bar/sort:title/direction:asc'),
'Title',
'/a'
);
@ -684,24 +684,24 @@ class PaginatorHelperTest extends CakeTestCase {
$expected = array(
array('span' => array('class' => 'current')), '1', '/span',
' | ',
array('span' => array()), array('a' => array('href' => '/articles/index/2/foo:bar/page:2')), '2', '/a', '/span',
array('span' => array()), array('a' => array('href' => '/articles/index/2/page:2/foo:bar')), '2', '/a', '/span',
' | ',
array('span' => array()), array('a' => array('href' => '/articles/index/2/foo:bar/page:3')), '3', '/a', '/span',
array('span' => array()), array('a' => array('href' => '/articles/index/2/page:3/foo:bar')), '3', '/a', '/span',
' | ',
array('span' => array()), array('a' => array('href' => '/articles/index/2/foo:bar/page:4')), '4', '/a', '/span',
array('span' => array()), array('a' => array('href' => '/articles/index/2/page:4/foo:bar')), '4', '/a', '/span',
' | ',
array('span' => array()), array('a' => array('href' => '/articles/index/2/foo:bar/page:5')), '5', '/a', '/span',
array('span' => array()), array('a' => array('href' => '/articles/index/2/page:5/foo:bar')), '5', '/a', '/span',
' | ',
array('span' => array()), array('a' => array('href' => '/articles/index/2/foo:bar/page:6')), '6', '/a', '/span',
array('span' => array()), array('a' => array('href' => '/articles/index/2/page:6/foo:bar')), '6', '/a', '/span',
' | ',
array('span' => array()), array('a' => array('href' => '/articles/index/2/foo:bar/page:7')), '7', '/a', '/span',
array('span' => array()), array('a' => array('href' => '/articles/index/2/page:7/foo:bar')), '7', '/a', '/span',
);
$this->assertTags($result, $expected);
$result = $this->Paginator->next('Next');
$expected = array(
'<span',
'a' => array('href' => '/articles/index/2/foo:bar/page:2', 'class' => 'next'),
'a' => array('href' => '/articles/index/2/page:2/foo:bar', 'class' => 'next'),
'Next',
'/a',
'/span'
@ -915,10 +915,10 @@ class PaginatorHelperTest extends CakeTestCase {
)
);
$this->Paginator->options(array('url' => array(12, 'page' => 3)));
$result = $this->Paginator->prev('Prev', array('url' => array(':foo' => 'bar')));
$result = $this->Paginator->prev('Prev', array('url' => array('foo' => 'bar')));
$expected = array(
'<span',
'a' => array('href' => '/index/12/foo:bar/page:1/limit:10', 'class' => 'prev'),
'a' => array('href' => '/index/12/page:1/limit:10/foo:bar', 'class' => 'prev'),
'Prev',
'/a',
'/span'
@ -2199,7 +2199,7 @@ class PaginatorHelperTest extends CakeTestCase {
function testQuerystringLinkGeneration() {
$this->Paginator->request->params['paging']['Article']['paramType'] = 'querystring';
$result = $this->Paginator->url(array('page' => '4'));
$expected = '/index?page=4';
$expected = '/?page=4';
$this->assertEquals($expected, $result);
}