Removing named parameter sigils.

cakephp · Dec 20, 2010 · b49b49a · b49b49a
1 parent d3fc29c
commit b49b49a
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 25 deletions.
diff --git a/cake/libs/route/cake_route.php b/cake/libs/route/cake_route.php
@@ -284,27 +284,19 @@ public function match($url) {
 			return false;
 		}
 
+		$greedyNamed = Router::$named['greedy'];
+		$allowedNamedParams = Router::$named['rules'];
+
 		$named = $pass = $_query = array();
 
 		foreach ($url as $key => $value) {
-			// pull out named params so comparisons later on are faster.
-			if ($key[0] === CakeRoute::SIGIL_NAMED && ($value !== false && $value !== null)) {
-				$named[substr($key, 1)] = $value;
-				unset($url[$key]);
-				continue;
-			}
-
-			// pull out querystring params
-			if ($key[0] === CakeRoute::SIGIL_QUERYSTRING && ($value !== false && $value !== null)) {
-				$_query[substr($key, 1)] = $value;
-				unset($url[$key]);
-				continue;
-			}
 
-			// keys that exist in the defaults and have different values cause match failures.
-			$keyExists = array_key_exists($key, $defaults);
-			if ($keyExists && $defaults[$key] != $value) {
+			// keys that exist in the defaults and have different values is a match failure.
+			$defaultExists = array_key_exists($key, $defaults);
+			if ($defaultExists && $defaults[$key] != $value) {
 				return false;
+			} elseif ($defaultExists) {
+				continue;
 			}
 
 			// If the key is a routed key, its not different yet.
@@ -322,8 +314,24 @@ 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])) &&
+				($value !== false && $value !== null)
+			) {
+				$named[$key] = $value;
+				continue;
+			}
+
 			// keys that don't exist are different.
-			if (!$keyExists && !empty($value)) {
+			if (!$defaultExists && !empty($value)) {
 				return false;
 			}
 		}

diff --git a/cake/libs/router.php b/cake/libs/router.php
@@ -914,10 +914,10 @@ protected static function _handleNoRoute($url) {
 			$key = $keys[$i];
 			if (is_numeric($keys[$i])) {
 				$args[] = $url[$key];
-			} elseif ($key[0] === CakeRoute::SIGIL_NAMED) {
-				$named[substr($key, 1)] = $url[$key];
 			} elseif ($key[0] === CakeRoute::SIGIL_QUERYSTRING) {
 				$query[substr($key, 1)] = $url[$key];
+			} else {
+				$named[$key] = $url[$key];
 			}
 		}
 

diff --git a/cake/tests/cases/libs/route/cake_route.test.php b/cake/tests/cases/libs/route/cake_route.test.php
@@ -310,7 +310,7 @@ function testGreedyRouteFailurePassedArg() {
  */
 	function testGreedyRouteFailureNamedParam() {
 		$route = new CakeRoute('/:controller/:action', array('plugin' => null));
-		$result = $route->match(array('controller' => 'posts', 'action' => 'view', ':page' => 1));
+		$result = $route->match(array('controller' => 'posts', 'action' => 'view', 'page' => 1));
 		$this->assertFalse($result);
 	}
 
@@ -334,9 +334,9 @@ 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));
+		$result = $route->match(array('controller' => 'posts', 'action' => 'index', 'plugin' => null, 'page' => 1));
 		$this->assertEqual($result, '/posts/index/page:1');
 
 		$result = $route->match(array('controller' => 'posts', 'action' => 'view', 'plugin' => null, 5));
@@ -348,9 +348,9 @@ function testMatchWithNamedParametersAndPassedArgs() {
 		$result = $route->match(array('controller' => 'posts', 'action' => 'view', 'plugin' => null, '0'));
 		$this->assertEqual($result, '/posts/view/0');
 
-		$result = $route->match(array('controller' => 'posts', 'action' => 'view', 'plugin' => null, 5, ':page' => 1, ':limit' => 20, ':order' => 'title'));
+		$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));
@@ -370,7 +370,7 @@ function testMatchWithNamedParametersAndPassedArgs() {
  */
 	function testNamedParamsWithNullFalse() {
 		$route = new CakeRoute('/:controller/:action/*');
-		$result = $route->match(array('controller' => 'posts', 'action' => 'index', ':page' => null, 'sort' => false));
+		$result = $route->match(array('controller' => 'posts', 'action' => 'index', 'page' => null, 'sort' => false));
 		$this->assertEquals('/posts/index/', $result);
 	}