From b70a5124705d9da10ed6a8a352f9a9ecc71afff6 Mon Sep 17 00:00:00 2001 From: mark_story Date: Wed, 2 Mar 2011 06:20:45 -0500 Subject: [PATCH 01/12] Moving Router::$named to be protected, and adding an accessor method. This will help with later refactoring. --- cake/libs/route/cake_route.php | 8 ++-- cake/libs/router.php | 57 ++++++++++++++++----------- cake/tests/cases/libs/router.test.php | 3 +- 3 files changed, 42 insertions(+), 26 deletions(-) diff --git a/cake/libs/route/cake_route.php b/cake/libs/route/cake_route.php index ca96428ba..be357d632 100644 --- a/cake/libs/route/cake_route.php +++ b/cake/libs/route/cake_route.php @@ -274,8 +274,9 @@ class CakeRoute { return false; } - $greedyNamed = Router::$named['greedy']; - $allowedNamedParams = Router::$named['rules']; + $namedConfig = Router::namedConfig(); + $greedyNamed = $namedConfig['greedy']; + $allowedNamedParams = $namedConfig['rules']; $named = $pass = $_query = array(); @@ -352,7 +353,8 @@ class CakeRoute { $params['pass'] = implode('/', $params['pass']); } - $separator = Router::$named['separator']; + $namedConfig = Router::namedConfig(); + $separator = $namedConfig['separator']; if (!empty($params['named']) && is_array($params['named'])) { $named = array(); diff --git a/cake/libs/router.php b/cake/libs/router.php index 71f6e7bc5..7ed994ce0 100644 --- a/cake/libs/router.php +++ b/cake/libs/router.php @@ -85,7 +85,7 @@ class Router { const ID = '[0-9]+'; const UUID = '[A-Fa-f0-9]{8}-[A-Fa-f0-9]{4}-[A-Fa-f0-9]{4}-[A-Fa-f0-9]{4}-[A-Fa-f0-9]{12}'; - private static $__named = array( + private static $__namedExpressions = array( 'Action' => Router::ACTION, 'Year' => Router::YEAR, 'Month' => Router::MONTH, @@ -100,7 +100,7 @@ class Router { * @var string * @access public */ - public static $named = array( + protected static $_namedConfig = array( 'default' => array('page', 'fields', 'order', 'limit', 'recursive', 'sort', 'direction', 'step'), 'greedy' => true, 'separator' => ':', @@ -186,10 +186,10 @@ class Router { * Gets the named route elements for use in app/config/routes.php * * @return array Named route elements - * @see Router::$__named + * @see Router::$__namedExpressions */ public static function getNamedExpressions() { - return self::$__named; + return self::$__namedExpressions; } /** @@ -369,7 +369,7 @@ class Router { */ public static function connectNamed($named, $options = array()) { if (isset($options['separator'])) { - self::$named['separator'] = $options['separator']; + self::$_namedConfig['separator'] = $options['separator']; unset($options['separator']); } @@ -380,23 +380,33 @@ class Router { $options = array_merge(array('default' => false, 'reset' => false, 'greedy' => true), $options); } - if ($options['reset'] == true || self::$named['rules'] === false) { - self::$named['rules'] = array(); + if ($options['reset'] == true || self::$_namedConfig['rules'] === false) { + self::$_namedConfig['rules'] = array(); } if ($options['default']) { - $named = array_merge($named, self::$named['default']); + $named = array_merge($named, self::$_namedConfig['default']); } foreach ($named as $key => $val) { if (is_numeric($key)) { - self::$named['rules'][$val] = true; + self::$_namedConfig['rules'][$val] = true; } else { - self::$named['rules'][$key] = $val; + self::$_namedConfig['rules'][$key] = $val; } } - self::$named['greedy'] = $options['greedy']; - return self::$named; + self::$_namedConfig['greedy'] = $options['greedy']; + return self::$_namedConfig; + } + +/** + * Gets the current named parameter configuration values. + * + * @return array + * @see Router::$_namedConfig + */ + public static function namedConfig() { + return self::$_namedConfig; } /** @@ -426,7 +436,10 @@ class Router { * @return array Array of mapped resources */ public static function mapResources($controller, $options = array()) { - $options = array_merge(array('prefix' => '/', 'id' => self::$__named['ID'] . '|' . self::$__named['UUID']), $options); + $options = array_merge(array( + 'prefix' => '/', + 'id' => self::ID . '|' . self::UUID + ), $options); $prefix = $options['prefix']; foreach ((array)$controller as $ctlName) { @@ -621,7 +634,7 @@ class Router { self::connect('/:controller', array('action' => 'index')); self::connect('/:controller/:action/*'); - if (self::$named['rules'] === false) { + if (self::$_namedConfig['rules'] === false) { self::connectNamed(true); } self::$_defaultsMapped = true; @@ -971,10 +984,10 @@ class Router { if (is_array($value)) { $flattend = Set::flatten($value, ']['); foreach ($flattend as $namedKey => $namedValue) { - $output .= '/' . $name . "[$namedKey]" . self::$named['separator'] . $namedValue; + $output .= '/' . $name . "[$namedKey]" . self::$_namedConfig['separator'] . $namedValue; } } else { - $output .= '/' . $name . self::$named['separator'] . $value; + $output .= '/' . $name . self::$_namedConfig['separator'] . $value; } } } @@ -996,8 +1009,8 @@ class Router { $named = array(); foreach ($params as $param => $val) { - if (isset(self::$named['rules'][$param])) { - $rule = self::$named['rules'][$param]; + if (isset(self::$_namedConfig['rules'][$param])) { + $rule = self::$_namedConfig['rules'][$param]; if (Router::matchNamed($param, $val, $rule, compact('controller', 'action'))) { $named[substr($param, 1)] = $val; unset($params[$param]); @@ -1215,12 +1228,12 @@ class Router { $pass = $named = array(); $args = explode('/', $args); - $greedy = isset($options['greedy']) ? $options['greedy'] : self::$named['greedy']; + $greedy = isset($options['greedy']) ? $options['greedy'] : self::$_namedConfig['greedy']; $context = array(); if (isset($options['context'])) { $context = $options['context']; } - $rules = self::$named['rules']; + $rules = self::$_namedConfig['rules']; if (isset($options['named'])) { $greedy = isset($options['greedy']) && $options['greedy'] === true; foreach ((array)$options['named'] as $key => $val) { @@ -1237,9 +1250,9 @@ class Router { continue; } - $separatorIsPresent = strpos($param, self::$named['separator']) !== false; + $separatorIsPresent = strpos($param, self::$_namedConfig['separator']) !== false; if ((!isset($options['named']) || !empty($options['named'])) && $separatorIsPresent) { - list($key, $val) = explode(self::$named['separator'], $param, 2); + list($key, $val) = explode(self::$_namedConfig['separator'], $param, 2); $hasRule = isset($rules[$key]); $passIt = (!$hasRule && !$greedy) || ($hasRule && !self::matchNamed($key, $val, $rules[$key], $context)); if ($passIt) { diff --git a/cake/tests/cases/libs/router.test.php b/cake/tests/cases/libs/router.test.php index d656e46da..f194b4a35 100644 --- a/cake/tests/cases/libs/router.test.php +++ b/cake/tests/cases/libs/router.test.php @@ -1436,7 +1436,8 @@ class RouterTest extends CakeTestCase { Router::reload(); $result = Router::connectNamed(true); - $this->assertEqual(array_keys($result['rules']), Router::$named['default']); + $named = Router::namedConfig(); + $this->assertEqual(array_keys($result['rules']), $named['default']); $this->assertTrue($result['greedy']); Router::reload(); Router::connectNamed(array('param1' => 'not-matching')); From 155db768251d531532f88d4ee0ab05338ab3c2fd Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 3 Mar 2011 05:50:58 -0500 Subject: [PATCH 02/12] Removing junk. --- cake/libs/route/cake_route.php | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/cake/libs/route/cake_route.php b/cake/libs/route/cake_route.php index be357d632..a4a70758a 100644 --- a/cake/libs/route/cake_route.php +++ b/cake/libs/route/cake_route.php @@ -333,7 +333,7 @@ class CakeRoute { } } } - return $this->_writeUrl(array_merge($url, compact('pass', 'named', '_query'))); + return $this->_writeUrl(array_merge($url, compact('pass', 'named'))); } /** @@ -391,20 +391,4 @@ class CakeRoute { return $out; } -/** - * Generates a well-formed querystring from $q - * - * Will compose an array or nested array into a proper querystring. - * - * @param mixed $q An array of parameters to compose into a query string. - * @param bool $escape Whether or not to use escaped & - * @return string - */ - public function queryString($q, $escape = false) { - $join = '&'; - if ($escape === true) { - $join = '&'; - } - return '?' . http_build_query($q, null, $join); - } } \ No newline at end of file From cf26a8e430a8f991ec166979a8ed7ab3d609f9a9 Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 3 Mar 2011 06:11:41 -0500 Subject: [PATCH 03/12] Initial port of Router::getArgs() to CakeRoute. --- cake/libs/route/cake_route.php | 82 ++++++++++++++++++- .../cases/libs/route/cake_route.test.php | 31 ++++++- 2 files changed, 108 insertions(+), 5 deletions(-) diff --git a/cake/libs/route/cake_route.php b/cake/libs/route/cake_route.php index a4a70758a..add9d1e36 100644 --- a/cake/libs/route/cake_route.php +++ b/cake/libs/route/cake_route.php @@ -214,19 +214,93 @@ class CakeRoute { unset($route[$i]); } $route['pass'] = $route['named'] = array(); - $route += $this->defaults; - //move numerically indexed elements from the defaults into pass. - foreach ($route as $key => $value) { + // Assign defaults, set passed args to pass + foreach ($this->defaults as $key => $value) { + if (isset($route[$key])) { + continue; + } if (is_integer($key)) { $route['pass'][] = $value; - unset($route[$key]); + continue; } + $route[$key] = $value; + } + + if (isset($route['_args_'])) { + list($pass, $named) = $this->parseArgs($route['_args_'], $route['controller'], $route['action']); + $route['pass'] = array_merge($route['pass'], $pass); + $route['named'] = $named; + unset($route['_args_']); } return $route; } } +/** + * Parse passed and Named parameters into a list of passed args, and a hash of named parameters. + * The local and global configuration for named parameters will be used. + * + * @param string $args A string with the passed & named params. eg. /1/page:2 + * @return array Array of ($pass, $named) + */ + public function parseArgs($args, $controller = null, $action = null) { + $pass = $named = array(); + $args = explode('/', $args); + + $context = compact('controller', 'action'); + $namedConfig = Router::namedConfig(); + $greedy = isset($this->options['greedy']) ? $this->options['greedy'] : $namedConfig['greedy']; + $rules = $namedConfig['rules']; + if (isset($this->options['named'])) { + $greedy = isset($this->options['greedy']) && $this->options['greedy'] === true; + foreach ((array)$this->options['named'] as $key => $val) { + if (is_numeric($key)) { + $rules[$val] = true; + continue; + } + $rules[$key] = $val; + } + } + + foreach ($args as $param) { + if (empty($param) && $param !== '0' && $param !== 0) { + continue; + } + + $separatorIsPresent = strpos($param, $namedConfig['separator']) !== false; + if ((!isset($options['named']) || !empty($this->options['named'])) && $separatorIsPresent) { + list($key, $val) = explode($namedConfig['separator'], $param, 2); + $hasRule = isset($rules[$key]); + $passIt = (!$hasRule && !$greedy) || ($hasRule && !Router::matchNamed($key, $val, $rules[$key], $context)); + if ($passIt) { + $pass[] = $param; + } else { + if (preg_match_all('/\[([A-Za-z0-9_-]+)?\]/', $key, $matches, PREG_SET_ORDER)) { + $matches = array_reverse($matches); + $parts = explode('[', $key); + $key = array_shift($parts); + $arr = $val; + foreach ($matches as $match) { + if (empty($match[1])) { + $arr = array($arr); + } else { + $arr = array( + $match[1] => $arr + ); + } + } + $val = $arr; + } + $named = array_merge_recursive($named, array($key => $val)); + } + } else { + $pass[] = $param; + } + } + return array($pass, $named); + } + /** * Apply persistent parameters to a url array. Persistant parameters are a special * key used during route creation to force route parameters to persist when omitted from diff --git a/cake/tests/cases/libs/route/cake_route.test.php b/cake/tests/cases/libs/route/cake_route.test.php index c1433451c..bf3eba263 100644 --- a/cake/tests/cases/libs/route/cake_route.test.php +++ b/cake/tests/cases/libs/route/cake_route.test.php @@ -171,12 +171,14 @@ class CakeRouteTestCase extends CakeTestCase { $this->assertPattern($result, '/posts/08/01/2007/title-of-post'); $result = $route->parse('/posts/08/01/2007/title-of-post'); - $this->assertEqual(count($result), 8); + $this->assertEqual(count($result), 7); $this->assertEqual($result['controller'], 'posts'); $this->assertEqual($result['action'], 'view'); $this->assertEqual($result['year'], '2007'); $this->assertEqual($result['month'], '08'); $this->assertEqual($result['day'], '01'); + $this->assertEquals($result['pass'][0], 'title-of-post'); + $route = new CakeRoute( "/:extra/page/:slug/*", @@ -473,4 +475,31 @@ class CakeRouteTestCase extends CakeTestCase { $this->assertFalse($result); } +/** + * test the parseArgs method + * + * @return void + */ + function testPassedArgumentParsing() { + $route = new CakeRoute('/:controller/:action/*'); + $result = $route->parse('/posts/edit/1/2/0'); + $expected = array( + 'controller' => 'posts', + 'action' => 'edit', + 'pass' => array('1', '2', '0'), + 'named' => array() + ); + $this->assertEquals($expected, $result); + + $result = $route->parse('/posts/edit/a-string/page:1'); + $expected = array( + 'controller' => 'posts', + 'action' => 'edit', + 'pass' => array('a-string'), + 'named' => array( + 'page' => 1 + ) + ); + $this->assertEquals($expected, $result); + } } \ No newline at end of file From 636188efeb32a4edffc5639ca11621ac0fc71eac Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 3 Mar 2011 06:22:59 -0500 Subject: [PATCH 04/12] Adding tests for array named params from router test to cakeroute test. --- .../cases/libs/route/cake_route.test.php | 63 ++++++++++++++++++- 1 file changed, 60 insertions(+), 3 deletions(-) diff --git a/cake/tests/cases/libs/route/cake_route.test.php b/cake/tests/cases/libs/route/cake_route.test.php index bf3eba263..ec798d4a4 100644 --- a/cake/tests/cases/libs/route/cake_route.test.php +++ b/cake/tests/cases/libs/route/cake_route.test.php @@ -490,16 +490,73 @@ class CakeRouteTestCase extends CakeTestCase { 'named' => array() ); $this->assertEquals($expected, $result); - - $result = $route->parse('/posts/edit/a-string/page:1'); + + $result = $route->parse('/posts/edit/a-string/page:1/sort:value'); $expected = array( 'controller' => 'posts', 'action' => 'edit', 'pass' => array('a-string'), 'named' => array( - 'page' => 1 + 'page' => 1, + 'sort' => 'value' ) ); $this->assertEquals($expected, $result); } + +/** + * test that parsing array format named parameters works + * + * @return void + */ + function testArrayNamedParameters() { + $route = new CakeRoute('/:controller/:action/*'); + $result = $route->parse('/tests/action/var[]:val1/var[]:val2'); + $expected = array( + 'controller' => 'tests', + 'action' => 'action', + 'named' => array( + 'var' => array( + 'val1', + 'val2' + ) + ), + 'pass' => array(), + ); + $this->assertEqual($result, $expected); + + $result = $route->parse('/tests/action/theanswer[is]:42/var[]:val2/var[]:val3'); + $expected = array( + 'controller' => 'tests', + 'action' => 'action', + 'named' => array( + 'theanswer' => array( + 'is' => 42 + ), + 'var' => array( + 'val2', + 'val3' + ) + ), + 'pass' => array(), + ); + $this->assertEqual($result, $expected); + + $result = $route->parse('/tests/action/theanswer[is][not]:42/theanswer[]:5/theanswer[is]:6'); + $expected = array( + 'controller' => 'tests', + 'action' => 'action', + 'named' => array( + 'theanswer' => array( + 5, + 'is' => array( + 6, + 'not' => 42 + ) + ), + ), + 'pass' => array(), + ); + $this->assertEqual($result, $expected); + } } \ No newline at end of file From 3eef281e1ccf4166e2376e4354608fb90550a258 Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 3 Mar 2011 06:36:02 -0500 Subject: [PATCH 05/12] Adding tests for named parameter rules on the route level. --- cake/libs/route/cake_route.php | 2 + .../cases/libs/route/cake_route.test.php | 54 ++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/cake/libs/route/cake_route.php b/cake/libs/route/cake_route.php index add9d1e36..81ccaf425 100644 --- a/cake/libs/route/cake_route.php +++ b/cake/libs/route/cake_route.php @@ -242,6 +242,8 @@ class CakeRoute { * The local and global configuration for named parameters will be used. * * @param string $args A string with the passed & named params. eg. /1/page:2 + * @param string $controller The current actions controller name, used to match named parameter rules + * @param string $action The current action name, used to match named parameter rules. * @return array Array of ($pass, $named) */ public function parseArgs($args, $controller = null, $action = null) { diff --git a/cake/tests/cases/libs/route/cake_route.test.php b/cake/tests/cases/libs/route/cake_route.test.php index ec798d4a4..a29ebe665 100644 --- a/cake/tests/cases/libs/route/cake_route.test.php +++ b/cake/tests/cases/libs/route/cake_route.test.php @@ -480,7 +480,7 @@ class CakeRouteTestCase extends CakeTestCase { * * @return void */ - function testPassedArgumentParsing() { + function testParsePassedArgument() { $route = new CakeRoute('/:controller/:action/*'); $result = $route->parse('/posts/edit/1/2/0'); $expected = array( @@ -504,12 +504,62 @@ class CakeRouteTestCase extends CakeTestCase { $this->assertEquals($expected, $result); } +/** + * test that only named parameter rules are followed. + * + * @return void + */ + function testParseNamedParametersWithRules() { + $route = new CakeRoute('/:controller/:action/*', array(), array( + 'named' => array( + 'wibble', + 'fish' => array('action' => 'index'), + 'fizz' => array('controller' => 'comments') + ) + )); + $result = $route->parse('/posts/display/wibble:spin/fish:trout/fizz:buzz'); + $expected = array( + 'controller' => 'posts', + 'action' => 'display', + 'pass' => array('fish:trout', 'fizz:buzz'), + 'named' => array( + 'wibble' => 'spin' + ) + ); + $this->assertEquals($expected, $result, 'Fish should not be parsed, as action != index'); + + $result = $route->parse('/posts/index/wibble:spin/fish:trout/fizz:buzz'); + $expected = array( + 'controller' => 'posts', + 'action' => 'index', + 'pass' => array('fizz:buzz'), + 'named' => array( + 'wibble' => 'spin', + 'fish' => 'trout' + ) + ); + $this->assertEquals($expected, $result, 'Fish should be parsed, as action == index'); + + $result = $route->parse('/comments/index/wibble:spin/fish:trout/fizz:buzz'); + $expected = array( + 'controller' => 'comments', + 'action' => 'index', + 'pass' => array(), + 'named' => array( + 'wibble' => 'spin', + 'fish' => 'trout', + 'fizz' => 'buzz' + ) + ); + $this->assertEquals($expected, $result, 'All params should be parsed as conditions were met.'); + } + /** * test that parsing array format named parameters works * * @return void */ - function testArrayNamedParameters() { + function testParseArrayNamedParameters() { $route = new CakeRoute('/:controller/:action/*'); $result = $route->parse('/tests/action/var[]:val1/var[]:val2'); $expected = array( From 97175aac901d11beb8460b6eb32f1f7c302d1557 Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 3 Mar 2011 06:49:11 -0500 Subject: [PATCH 06/12] Making greedy -> greedyNamed its clearer and doesn't consume a possibly easy to use named parameter key. Expanding tests for named parameter rules/conditions. --- cake/libs/route/cake_route.php | 4 +- cake/libs/router.php | 2 +- .../cases/libs/route/cake_route.test.php | 50 +++++++++++++++++-- 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/cake/libs/route/cake_route.php b/cake/libs/route/cake_route.php index 81ccaf425..369c127dd 100644 --- a/cake/libs/route/cake_route.php +++ b/cake/libs/route/cake_route.php @@ -252,10 +252,10 @@ class CakeRoute { $context = compact('controller', 'action'); $namedConfig = Router::namedConfig(); - $greedy = isset($this->options['greedy']) ? $this->options['greedy'] : $namedConfig['greedy']; + $greedy = isset($this->options['greedyNamed']) ? $this->options['greedyNamed'] : $namedConfig['greedy']; $rules = $namedConfig['rules']; if (isset($this->options['named'])) { - $greedy = isset($this->options['greedy']) && $this->options['greedy'] === true; + $greedy = isset($this->options['greedyNamed']) && $this->options['greedyNamed'] === true; foreach ((array)$this->options['named'] as $key => $val) { if (is_numeric($key)) { $rules[$val] = true; diff --git a/cake/libs/router.php b/cake/libs/router.php index 7ed994ce0..0ac9338c2 100644 --- a/cake/libs/router.php +++ b/cake/libs/router.php @@ -395,7 +395,7 @@ class Router { self::$_namedConfig['rules'][$key] = $val; } } - self::$_namedConfig['greedy'] = $options['greedy']; + self::$_namedConfig['greedyNamed'] = $options['greedy']; return self::$_namedConfig; } diff --git a/cake/tests/cases/libs/route/cake_route.test.php b/cake/tests/cases/libs/route/cake_route.test.php index a29ebe665..4a337f02f 100644 --- a/cake/tests/cases/libs/route/cake_route.test.php +++ b/cake/tests/cases/libs/route/cake_route.test.php @@ -514,14 +514,15 @@ class CakeRouteTestCase extends CakeTestCase { 'named' => array( 'wibble', 'fish' => array('action' => 'index'), - 'fizz' => array('controller' => 'comments') + 'fizz' => array('controller' => 'comments'), + 'pattern' => 'val-[\d]+' ) )); - $result = $route->parse('/posts/display/wibble:spin/fish:trout/fizz:buzz'); + $result = $route->parse('/posts/display/wibble:spin/fish:trout/fizz:buzz/unknown:value'); $expected = array( 'controller' => 'posts', 'action' => 'display', - 'pass' => array('fish:trout', 'fizz:buzz'), + 'pass' => array('fish:trout', 'fizz:buzz', 'unknown:value'), 'named' => array( 'wibble' => 'spin' ) @@ -552,6 +553,49 @@ class CakeRouteTestCase extends CakeTestCase { ) ); $this->assertEquals($expected, $result, 'All params should be parsed as conditions were met.'); + + $result = $route->parse('/comments/index/pattern:val--'); + $expected = array( + 'controller' => 'comments', + 'action' => 'index', + 'pass' => array('pattern:val--'), + 'named' => array() + ); + $this->assertEquals($expected, $result, 'Named parameter pattern unmet.'); + + $result = $route->parse('/comments/index/pattern:val-2'); + $expected = array( + 'controller' => 'comments', + 'action' => 'index', + 'pass' => array(), + 'named' => array('pattern' => 'val-2') + ); + $this->assertEquals($expected, $result, 'Named parameter pattern met.'); + } + +/** + * test that greedyNamed ignores rules. + * + * @return void + */ + function testParseGreedyNamed() { + $route = new CakeRoute('/:controller/:action/*', array(), array( + 'named' => array( + 'fizz' => array('controller' => 'comments'), + 'pattern' => 'val-[\d]+', + ), + 'greedyNamed' => true + )); + $result = $route->parse('/posts/display/wibble:spin/fizz:buzz/pattern:ignored'); + $expected = array( + 'controller' => 'posts', + 'action' => 'display', + 'pass' => array('fizz:buzz', 'pattern:ignored'), + 'named' => array( + 'wibble' => 'spin', + ) + ); + $this->assertEquals($expected, $result, 'Greedy named grabs everything, rules are followed'); } /** From 15275eddaceb577a50068e07c43be130a508f66f Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 3 Mar 2011 06:54:46 -0500 Subject: [PATCH 07/12] Moving matchNamed() into CakeRoute. This will allow its removal from Router. --- cake/libs/route/cake_route.php | 34 ++++++++++++++++++- .../cases/libs/route/cake_route.test.php | 2 +- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/cake/libs/route/cake_route.php b/cake/libs/route/cake_route.php index 369c127dd..38a45a30a 100644 --- a/cake/libs/route/cake_route.php +++ b/cake/libs/route/cake_route.php @@ -274,7 +274,7 @@ class CakeRoute { if ((!isset($options['named']) || !empty($this->options['named'])) && $separatorIsPresent) { list($key, $val) = explode($namedConfig['separator'], $param, 2); $hasRule = isset($rules[$key]); - $passIt = (!$hasRule && !$greedy) || ($hasRule && !Router::matchNamed($key, $val, $rules[$key], $context)); + $passIt = (!$hasRule && !$greedy) || ($hasRule && !$this->_matchNamed($key, $val, $rules[$key], $context)); if ($passIt) { $pass[] = $param; } else { @@ -303,6 +303,38 @@ class CakeRoute { return array($pass, $named); } +/** + * Return true if a given named $param's $val matches a given $rule depending on $context. Currently implemented + * rule types are controller, action and match that can be combined with each other. + * + * @param string $param The name of the named parameter + * @param string $val The value of the named parameter + * @param array $rule The rule(s) to apply, can also be a match string + * @param string $context An array with additional context information (controller / action) + * @return boolean + */ + protected function _matchNamed($param, $val, $rule, $context) { + if ($rule === true || $rule === false) { + return $rule; + } + if (is_string($rule)) { + $rule = array('match' => $rule); + } + if (!is_array($rule)) { + return false; + } + + $controllerMatches = !isset($rule['controller'], $context['controller']) || in_array($context['controller'], (array)$rule['controller']); + if (!$controllerMatches) { + return false; + } + $actionMatches = !isset($rule['action'], $context['action']) || in_array($context['action'], (array)$rule['action']); + if (!$actionMatches) { + return false; + } + return (!isset($rule['match']) || preg_match('/' . $rule['match'] . '/', $val)); + } + /** * Apply persistent parameters to a url array. Persistant parameters are a special * key used during route creation to force route parameters to persist when omitted from diff --git a/cake/tests/cases/libs/route/cake_route.test.php b/cake/tests/cases/libs/route/cake_route.test.php index 4a337f02f..b68e05338 100644 --- a/cake/tests/cases/libs/route/cake_route.test.php +++ b/cake/tests/cases/libs/route/cake_route.test.php @@ -514,7 +514,7 @@ class CakeRouteTestCase extends CakeTestCase { 'named' => array( 'wibble', 'fish' => array('action' => 'index'), - 'fizz' => array('controller' => 'comments'), + 'fizz' => array('controller' => array('comments', 'other')), 'pattern' => 'val-[\d]+' ) )); From 171830df3319f24bc76a03686c0bdcb46f4fccf0 Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 3 Mar 2011 07:11:32 -0500 Subject: [PATCH 08/12] Minor cleanup and reformattting. Changing greedy -> greedyNamed for clarity and consistency. --- cake/libs/route/cake_route.php | 115 +++++++++++++++++---------------- cake/libs/router.php | 2 +- 2 files changed, 61 insertions(+), 56 deletions(-) diff --git a/cake/libs/route/cake_route.php b/cake/libs/route/cake_route.php index 38a45a30a..015abb42b 100644 --- a/cake/libs/route/cake_route.php +++ b/cake/libs/route/cake_route.php @@ -185,56 +185,55 @@ class CakeRoute { } if (!preg_match($this->_compiledRoute, $url, $route)) { return false; - } else { - foreach ($this->defaults as $key => $val) { - if ($key[0] === '[' && preg_match('/^\[(\w+)\]$/', $key, $header)) { - if (isset($this->__headerMap[$header[1]])) { - $header = $this->__headerMap[$header[1]]; - } else { - $header = 'http_' . $header[1]; - } - $header = strtoupper($header); - - $val = (array)$val; - $h = false; - - foreach ($val as $v) { - if (env($header) === $v) { - $h = true; - } - } - if (!$h) { - return false; - } - } - } - array_shift($route); - $count = count($this->keys); - for ($i = 0; $i <= $count; $i++) { - unset($route[$i]); - } - $route['pass'] = $route['named'] = array(); - - // Assign defaults, set passed args to pass - foreach ($this->defaults as $key => $value) { - if (isset($route[$key])) { - continue; - } - if (is_integer($key)) { - $route['pass'][] = $value; - continue; - } - $route[$key] = $value; - } - - if (isset($route['_args_'])) { - list($pass, $named) = $this->parseArgs($route['_args_'], $route['controller'], $route['action']); - $route['pass'] = array_merge($route['pass'], $pass); - $route['named'] = $named; - unset($route['_args_']); - } - return $route; } + foreach ($this->defaults as $key => $val) { + if ($key[0] === '[' && preg_match('/^\[(\w+)\]$/', $key, $header)) { + if (isset($this->__headerMap[$header[1]])) { + $header = $this->__headerMap[$header[1]]; + } else { + $header = 'http_' . $header[1]; + } + $header = strtoupper($header); + + $val = (array)$val; + $h = false; + + foreach ($val as $v) { + if (env($header) === $v) { + $h = true; + } + } + if (!$h) { + return false; + } + } + } + array_shift($route); + $count = count($this->keys); + for ($i = 0; $i <= $count; $i++) { + unset($route[$i]); + } + $route['pass'] = $route['named'] = array(); + + // Assign defaults, set passed args to pass + foreach ($this->defaults as $key => $value) { + if (isset($route[$key])) { + continue; + } + if (is_integer($key)) { + $route['pass'][] = $value; + continue; + } + $route[$key] = $value; + } + + if (isset($route['_args_'])) { + list($pass, $named) = $this->parseArgs($route['_args_'], $route['controller'], $route['action']); + $route['pass'] = array_merge($route['pass'], $pass); + $route['named'] = $named; + unset($route['_args_']); + } + return $route; } /** @@ -252,9 +251,9 @@ class CakeRoute { $context = compact('controller', 'action'); $namedConfig = Router::namedConfig(); - $greedy = isset($this->options['greedyNamed']) ? $this->options['greedyNamed'] : $namedConfig['greedy']; + $greedy = $namedConfig['greedyNamed']; $rules = $namedConfig['rules']; - if (isset($this->options['named'])) { + if (!empty($this->options['named'])) { $greedy = isset($this->options['greedyNamed']) && $this->options['greedyNamed'] === true; foreach ((array)$this->options['named'] as $key => $val) { if (is_numeric($key)) { @@ -271,7 +270,7 @@ class CakeRoute { } $separatorIsPresent = strpos($param, $namedConfig['separator']) !== false; - if ((!isset($options['named']) || !empty($this->options['named'])) && $separatorIsPresent) { + if ((!isset($this->options['named']) || !empty($this->options['named'])) && $separatorIsPresent) { list($key, $val) = explode($namedConfig['separator'], $param, 2); $hasRule = isset($rules[$key]); $passIt = (!$hasRule && !$greedy) || ($hasRule && !$this->_matchNamed($key, $val, $rules[$key], $context)); @@ -324,11 +323,17 @@ class CakeRoute { return false; } - $controllerMatches = !isset($rule['controller'], $context['controller']) || in_array($context['controller'], (array)$rule['controller']); + $controllerMatches = ( + !isset($rule['controller'], $context['controller']) || + in_array($context['controller'], (array)$rule['controller']) + ); if (!$controllerMatches) { return false; } - $actionMatches = !isset($rule['action'], $context['action']) || in_array($context['action'], (array)$rule['action']); + $actionMatches = ( + !isset($rule['action'], $context['action']) || + in_array($context['action'], (array)$rule['action']) + ); if (!$actionMatches) { return false; } @@ -383,7 +388,7 @@ class CakeRoute { } $namedConfig = Router::namedConfig(); - $greedyNamed = $namedConfig['greedy']; + $greedyNamed = $namedConfig['greedyNamed']; $allowedNamedParams = $namedConfig['rules']; $named = $pass = $_query = array(); diff --git a/cake/libs/router.php b/cake/libs/router.php index 0ac9338c2..db98384e2 100644 --- a/cake/libs/router.php +++ b/cake/libs/router.php @@ -102,7 +102,7 @@ class Router { */ protected static $_namedConfig = array( 'default' => array('page', 'fields', 'order', 'limit', 'recursive', 'sort', 'direction', 'step'), - 'greedy' => true, + 'greedyNamed' => true, 'separator' => ':', 'rules' => false, ); From 217938a97084655540ea869cbcb4da7c92aedbb9 Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 3 Mar 2011 07:30:22 -0500 Subject: [PATCH 09/12] Moving pass key restructuring into CakeRoute. Adding tests for that and http header matching. --- cake/libs/route/cake_route.php | 10 ++++ .../cases/libs/route/cake_route.test.php | 49 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/cake/libs/route/cake_route.php b/cake/libs/route/cake_route.php index 015abb42b..da29b3e26 100644 --- a/cake/libs/route/cake_route.php +++ b/cake/libs/route/cake_route.php @@ -233,6 +233,16 @@ class CakeRoute { $route['named'] = $named; unset($route['_args_']); } + + // restructure 'pass' key route params + if (isset($this->options['pass'])) { + $j = count($this->options['pass']); + while($j--) { + if (isset($route[$this->options['pass'][$j]])) { + array_unshift($route['pass'], $route[$this->options['pass'][$j]]); + } + } + } return $route; } diff --git a/cake/tests/cases/libs/route/cake_route.test.php b/cake/tests/cases/libs/route/cake_route.test.php index b68e05338..d70eed4e6 100644 --- a/cake/tests/cases/libs/route/cake_route.test.php +++ b/cake/tests/cases/libs/route/cake_route.test.php @@ -450,6 +450,35 @@ class CakeRouteTestCase extends CakeTestCase { $this->assertEqual($result['action'], 'index'); } +/** + * test numerically indexed defaults, get appeneded to pass + * + * @return void + */ + function testParseWithPassDefaults() { + $route = new Cakeroute('/:controller', array('action' => 'display', 'home')); + $result = $route->parse('/posts'); + $expected = array( + 'controller' => 'posts', + 'action' => 'display', + 'pass' => array('home'), + 'named' => array() + ); + $this->assertEquals($expected, $result); + } + +/** + * test that http header conditions can cause route failures. + * + * @return void + */ + function testParseWithHttpHeaderConditions() { + $_SERVER['REQUEST_METHOD'] = 'GET'; + $route = new CakeRoute('/sample', array('controller' => 'posts', 'action' => 'index', '[method]' => 'POST')); + + $this->assertFalse($route->parse('/sample')); + } + /** * test that patterns work for :action * @@ -653,4 +682,24 @@ class CakeRouteTestCase extends CakeTestCase { ); $this->assertEqual($result, $expected); } + +/** + * test restructuring args with pass key + * + * @return void + */ + function testPassArgRestructure() { + $route = new CakeRoute('/:controller/:action/:slug', array(), array( + 'pass' => array('slug') + )); + $result = $route->parse('/posts/view/my-title'); + $expected = array( + 'controller' => 'posts', + 'action' => 'view', + 'slug' => 'my-title', + 'pass' => array('my-title'), + 'named' => array() + ); + $this->assertEquals($expected, $result, 'Slug should have moved'); + } } \ No newline at end of file From 9cf940853b712b1bcd2b012b44807309b6b5ee95 Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 3 Mar 2011 07:32:06 -0500 Subject: [PATCH 10/12] Updating router tests to use/expect greedyNamed. Removing code that has been moved to CakeRoute or is no longer needed. --- cake/libs/router.php | 148 -------------------------- cake/tests/cases/libs/router.test.php | 13 ++- 2 files changed, 8 insertions(+), 153 deletions(-) diff --git a/cake/libs/router.php b/cake/libs/router.php index db98384e2..08372f7c7 100644 --- a/cake/libs/router.php +++ b/cake/libs/router.php @@ -499,36 +499,7 @@ class Router { if (($r = $route->parse($url)) !== false) { self::$_currentRoute[] =& $route; - - $params = $route->options; - $argOptions = array(); - - if (array_key_exists('named', $params)) { - $argOptions['named'] = $params['named']; - unset($params['named']); - } - if (array_key_exists('greedy', $params)) { - $argOptions['greedy'] = $params['greedy']; - unset($params['greedy']); - } $out = $r; - - if (isset($out['_args_'])) { - $argOptions['context'] = array('action' => $out['action'], 'controller' => $out['controller']); - $parsedArgs = self::getArgs($out['_args_'], $argOptions); - $out['pass'] = array_merge($out['pass'], $parsedArgs['pass']); - $out['named'] = $parsedArgs['named']; - unset($out['_args_']); - } - - if (isset($params['pass'])) { - $j = count($params['pass']); - while($j--) { - if (isset($out[$params['pass'][$j]])) { - array_unshift($out['pass'], $out[$params['pass'][$j]]); - } - } - } break; } } @@ -997,61 +968,6 @@ class Router { return $output; } -/** - * Takes an array of URL parameters and separates the ones that can be used as named arguments - * - * @param array $params Associative array of URL parameters. - * @param string $controller Name of controller being routed. Used in scoping. - * @param string $action Name of action being routed. Used in scoping. - * @return array - */ - public static function getNamedElements($params, $controller = null, $action = null) { - $named = array(); - - foreach ($params as $param => $val) { - if (isset(self::$_namedConfig['rules'][$param])) { - $rule = self::$_namedConfig['rules'][$param]; - if (Router::matchNamed($param, $val, $rule, compact('controller', 'action'))) { - $named[substr($param, 1)] = $val; - unset($params[$param]); - } - } - } - return array($named, $params); - } - -/** - * Return true if a given named $param's $val matches a given $rule depending on $context. Currently implemented - * rule types are controller, action and match that can be combined with each other. - * - * @param string $param The name of the named parameter - * @param string $val The value of the named parameter - * @param array $rule The rule(s) to apply, can also be a match string - * @param string $context An array with additional context information (controller / action) - * @return boolean - */ - public static function matchNamed($param, $val, $rule, $context = array()) { - if ($rule === true || $rule === false) { - return $rule; - } - if (is_string($rule)) { - $rule = array('match' => $rule); - } - if (!is_array($rule)) { - return false; - } - - $controllerMatches = !isset($rule['controller'], $context['controller']) || in_array($context['controller'], (array)$rule['controller']); - if (!$controllerMatches) { - return false; - } - $actionMatches = !isset($rule['action'], $context['action']) || in_array($context['action'], (array)$rule['action']); - if (!$actionMatches) { - return false; - } - return (!isset($rule['match']) || preg_match('/' . $rule['match'] . '/', $val)); - } - /** * Generates a well-formed querystring from $q * @@ -1218,70 +1134,6 @@ class Router { return self::$_validExtensions; } -/** - * Takes a passed params and converts it to args - * - * @param array $params - * @return array Array containing passed and named parameters - */ - public static function getArgs($args, $options = array()) { - $pass = $named = array(); - $args = explode('/', $args); - - $greedy = isset($options['greedy']) ? $options['greedy'] : self::$_namedConfig['greedy']; - $context = array(); - if (isset($options['context'])) { - $context = $options['context']; - } - $rules = self::$_namedConfig['rules']; - if (isset($options['named'])) { - $greedy = isset($options['greedy']) && $options['greedy'] === true; - foreach ((array)$options['named'] as $key => $val) { - if (is_numeric($key)) { - $rules[$val] = true; - continue; - } - $rules[$key] = $val; - } - } - - foreach ($args as $param) { - if (empty($param) && $param !== '0' && $param !== 0) { - continue; - } - - $separatorIsPresent = strpos($param, self::$_namedConfig['separator']) !== false; - if ((!isset($options['named']) || !empty($options['named'])) && $separatorIsPresent) { - list($key, $val) = explode(self::$_namedConfig['separator'], $param, 2); - $hasRule = isset($rules[$key]); - $passIt = (!$hasRule && !$greedy) || ($hasRule && !self::matchNamed($key, $val, $rules[$key], $context)); - if ($passIt) { - $pass[] = $param; - } else { - if (preg_match_all('/\[([A-Za-z0-9_-]+)?\]/', $key, $matches, PREG_SET_ORDER)) { - $matches = array_reverse($matches); - $parts = explode('[', $key); - $key = array_shift($parts); - $arr = $val; - foreach ($matches as $match) { - if (empty($match[1])) { - $arr = array($arr); - } else { - $arr = array( - $match[1] => $arr - ); - } - } - $val = $arr; - } - $named = array_merge_recursive($named, array($key => $val)); - } - } else { - $pass[] = $param; - } - } - return compact('pass', 'named'); - } } //Save the initial state diff --git a/cake/tests/cases/libs/router.test.php b/cake/tests/cases/libs/router.test.php index f194b4a35..9a4ce30d7 100644 --- a/cake/tests/cases/libs/router.test.php +++ b/cake/tests/cases/libs/router.test.php @@ -436,6 +436,7 @@ class RouterTest extends CakeTestCase { $expected = '/tests/index/namedParam[keyed]:is an array/namedParam[0]:test'; $this->assertEqual($result, $expected); + //@todo Delete from here down, tests are in CakeRoute now. $result = Router::parse('/tests/action/var[]:val1/var[]:val2'); $expected = array( 'controller' => 'tests', @@ -1017,7 +1018,7 @@ class RouterTest extends CakeTestCase { $this->assertEqual($result, $expected); Router::reload(); - Router::connect('/posts/view/*', array('controller' => 'posts', 'action' => 'view'), array('named' => array('foo', 'answer'), 'greedy' => true)); + Router::connect('/posts/view/*', array('controller' => 'posts', 'action' => 'view'), array('named' => array('foo', 'answer'), 'greedyNamed' => true)); $result = Router::parse('/posts/view/foo:bar/routing:fun/answer:42'); $expected = array('pass' => array(), 'named' => array('foo' => 'bar', 'routing' => 'fun', 'answer' => '42'), 'plugin' => null, 'controller' => 'posts', 'action' => 'view'); $this->assertEqual($result, $expected); @@ -1328,7 +1329,7 @@ class RouterTest extends CakeTestCase { */ function testConnectNamed() { $named = Router::connectNamed(false, array('default' => true)); - $this->assertFalse($named['greedy']); + $this->assertFalse($named['greedyNamed']); $this->assertEqual(array_keys($named['rules']), $named['default']); Router::reload(); @@ -1429,7 +1430,7 @@ class RouterTest extends CakeTestCase { Router::reload(); $result = Router::connectNamed(false); $this->assertEqual(array_keys($result['rules']), array()); - $this->assertFalse($result['greedy']); + $this->assertFalse($result['greedyNamed']); $result = Router::parse('/controller/action/param1:value1:1/param2:value2:3/param:value'); $expected = array('pass' => array('param1:value1:1', 'param2:value2:3', 'param:value'), 'named' => array(), 'controller' => 'controller', 'action' => 'action', 'plugin' => null); $this->assertEqual($result, $expected); @@ -1438,15 +1439,16 @@ class RouterTest extends CakeTestCase { $result = Router::connectNamed(true); $named = Router::namedConfig(); $this->assertEqual(array_keys($result['rules']), $named['default']); - $this->assertTrue($result['greedy']); + $this->assertTrue($result['greedyNamed']); Router::reload(); Router::connectNamed(array('param1' => 'not-matching')); $result = Router::parse('/controller/action/param1:value1:1/param2:value2:3/param:value'); $expected = array('pass' => array('param1:value1:1'), 'named' => array('param2' => 'value2:3', 'param' => 'value'), 'controller' => 'controller', 'action' => 'action', 'plugin' => null); $this->assertEqual($result, $expected); + //@todo delete this test. Router::reload(); - Router::connect('/foo/:action/*', array('controller' => 'bar'), array('named' => array('param1' => array('action' => 'index')), 'greedy' => true)); + Router::connect('/foo/:action/*', array('controller' => 'bar'), array('named' => array('param1' => array('action' => 'index')), 'greedyNamed' => true)); $result = Router::parse('/foo/index/param1:value1:1/param2:value2:3/param:value'); $expected = array('pass' => array(), 'named' => array('param1' => 'value1:1', 'param2' => 'value2:3', 'param' => 'value'), 'controller' => 'bar', 'action' => 'index', 'plugin' => null); $this->assertEqual($result, $expected); @@ -1473,6 +1475,7 @@ class RouterTest extends CakeTestCase { $expected = array('pass' => array('param2:value2:3', 'param3:value'), 'named' => array('param1' => 'value1:1'), 'controller' => 'controller', 'action' => 'action', 'plugin' => null); $this->assertEqual($result, $expected); + //@todo delete this test. Router::reload(); Router::connect('/foo/*', array('controller' => 'bar', 'action' => 'fubar'), array('named' => array('param1' => 'value[\d]:[\d]'))); Router::connectNamed(array(), array('greedy' => false)); From b8aa78ce9f8f0534bb414d8ee1ff623da8a6b6e1 Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 3 Mar 2011 21:16:43 -0500 Subject: [PATCH 11/12] Making a method protected, it doesn't need to public really. --- cake/libs/route/cake_route.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cake/libs/route/cake_route.php b/cake/libs/route/cake_route.php index da29b3e26..e260f122b 100644 --- a/cake/libs/route/cake_route.php +++ b/cake/libs/route/cake_route.php @@ -228,7 +228,7 @@ class CakeRoute { } if (isset($route['_args_'])) { - list($pass, $named) = $this->parseArgs($route['_args_'], $route['controller'], $route['action']); + list($pass, $named) = $this->_parseArgs($route['_args_'], $route['controller'], $route['action']); $route['pass'] = array_merge($route['pass'], $pass); $route['named'] = $named; unset($route['_args_']); @@ -255,7 +255,7 @@ class CakeRoute { * @param string $action The current action name, used to match named parameter rules. * @return array Array of ($pass, $named) */ - public function parseArgs($args, $controller = null, $action = null) { + protected function _parseArgs($args, $controller = null, $action = null) { $pass = $named = array(); $args = explode('/', $args); From 0d3288bab0d183b3d707968ac942f51519532f33 Mon Sep 17 00:00:00 2001 From: mark_story Date: Thu, 3 Mar 2011 21:18:35 -0500 Subject: [PATCH 12/12] Refactoring protected methods. --- cake/libs/route/cake_route.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/cake/libs/route/cake_route.php b/cake/libs/route/cake_route.php index e260f122b..c40c84e18 100644 --- a/cake/libs/route/cake_route.php +++ b/cake/libs/route/cake_route.php @@ -228,7 +228,7 @@ class CakeRoute { } if (isset($route['_args_'])) { - list($pass, $named) = $this->_parseArgs($route['_args_'], $route['controller'], $route['action']); + list($pass, $named) = $this->_parseArgs($route['_args_'], $route); $route['pass'] = array_merge($route['pass'], $pass); $route['named'] = $named; unset($route['_args_']); @@ -251,15 +251,13 @@ class CakeRoute { * The local and global configuration for named parameters will be used. * * @param string $args A string with the passed & named params. eg. /1/page:2 - * @param string $controller The current actions controller name, used to match named parameter rules - * @param string $action The current action name, used to match named parameter rules. + * @param string $context The current route context, which should contain controller/action keys. * @return array Array of ($pass, $named) */ - protected function _parseArgs($args, $controller = null, $action = null) { + protected function _parseArgs($args, $context) { $pass = $named = array(); $args = explode('/', $args); - $context = compact('controller', 'action'); $namedConfig = Router::namedConfig(); $greedy = $namedConfig['greedyNamed']; $rules = $namedConfig['rules'];