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.
cakephp · Dec 20, 2010 · e5588f7 · e5588f7
1 parent b49b49a
commit e5588f7
Show file tree

Hide file tree

Showing 8 changed files with 46 additions and 116 deletions.
diff --git a/cake/libs/controller/components/paginator.php b/cake/libs/controller/components/paginator.php
@@ -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
  */

diff --git a/cake/libs/route/cake_route.php b/cake/libs/route/cake_route.php
@@ -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 @@ public function match($url) {
 				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 @@ protected function _writeUrl($params) {
 		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;
 	}

diff --git a/cake/libs/router.php b/cake/libs/router.php
@@ -914,8 +914,6 @@ protected static function _handleNoRoute($url) {
 			$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 @@ public static function reverse($params) {
 			$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;

diff --git a/cake/libs/view/helpers/paginator.php b/cake/libs/view/helpers/paginator.php
@@ -119,12 +119,7 @@ function __construct(View $View, $settings = array()) {
  * @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 @@ public function url($options = array(), $asArray = false, $model = null) {
  * @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]);
 			}
 		}

diff --git a/cake/tests/cases/libs/route/cake_route.test.php b/cake/tests/cases/libs/route/cake_route.test.php
@@ -334,7 +334,7 @@ function testMatchWithFalseyValues() {
  */
 	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 @@ function testMatchWithNamedParametersAndPassedArgs() {
 
 		$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 @@ function testPatternOnAction() {
 		$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);
-	}
 }
diff --git a/cake/tests/cases/libs/router.test.php b/cake/tests/cases/libs/router.test.php
@@ -328,7 +328,7 @@ function testUrlGenerationBasic() {
 		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 @@ function testUrlGenerationBasic() {
  * @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 @@ function testArrayNamedParameters() {
 		$this->assertEqual($result, $expected);
 
 		$result = Router::url(array('controller' => 'tests',
-			':pages' => array(
+			'pages' => array(
 				'param1' => array(
 					'one' => 1,
 					'two' => 2
@@ -420,7 +420,7 @@ function testArrayNamedParameters() {
 		$this->assertEqual($result, $expected);
 
 		$result = Router::url(array('controller' => 'tests',
-			':super' => array(
+			'super' => array(
 				'nested' => array(
 					'array' => 'awesome',
 					'something' => 'else'
@@ -431,7 +431,7 @@ function testArrayNamedParameters() {
 		$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 @@ function testUrlGenerationWithAdminPrefix() {
 		$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 @@ function testUrlGenerationWithAdminPrefix() {
 
 		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 @@ function testUrlGenerationPlugins() {
 
 		$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 @@ function testConnectNamed() {
  * @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 @@ function testNamedArgsUrlGeneration() {
 		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 @@ function testNamedArgsUrlGeneration() {
 		);
 		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 @@ function testNamedArgsUrlGeneration() {
 		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 @@ function testUrlGenerationWithLegacyPrefixes() {
 		$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 @@ function testUrlGenerationWithAutoPrefixes() {
 		$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 @@ function testRemoveBase() {
 		$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 @@ function testParsingWithPrefixes() {
 		$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 @@ function testRegexRouteMatching() {
 		$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);
 	}
 

diff --git a/cake/tests/cases/libs/view/helper.test.php b/cake/tests/cases/libs/view/helper.test.php
@@ -477,7 +477,7 @@ function testUrlConversion() {
 		$result = $this->Helper->url('/controller/action/1?one=1&two=2');
 		$this->assertEqual($result, '/controller/action/1?one=1&amp;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&quot; onclick=&quot;alert(&#039;XSS&#039;);&quot;");
 
 		$result = $this->Helper->url('/controller/action/1/param:this+one+more');
@@ -490,12 +490,12 @@ function testUrlConversion() {
 		$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&amp;two=value&amp;three=purple");