Skip to content

Commit

Permalink
Only sort the keys once per request instead of on each match.
Browse files Browse the repository at this point in the history
Sorting the keys property by value sorts keys with the same prefix for
free. This does change the order of the keys, but I don't think that is
actually a large issue as it is just a list.

Refs #2991
  • Loading branch information
markstory committed Mar 11, 2014
1 parent 0c207db commit c0ac611
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 16 deletions.
16 changes: 6 additions & 10 deletions lib/Cake/Routing/Route/CakeRoute.php
Expand Up @@ -173,6 +173,10 @@ protected function _writeRoute() {
foreach ($this->keys as $key) {
unset($this->defaults[$key]);
}

$keys = $this->keys;
sort($keys);
$this->keys = array_reverse($keys);
}

/**
Expand Down Expand Up @@ -420,7 +424,6 @@ public function match($url) {
$named = $pass = array();

foreach ($url as $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) {
Expand Down Expand Up @@ -517,16 +520,10 @@ protected function _writeUrl($params) {
}
$out = $this->template;

if ($this->keys !== array()) {

if (!empty($this->keys)) {
$search = $replace = array();

$lengths = array_map('strlen', $this->keys);
$flipped = array_combine($this->keys, $lengths);
arsort($flipped);
$keys = array_keys($flipped);

foreach ($keys as $key) {
foreach ($this->keys as $key) {
$string = null;
if (isset($params[$key])) {
$string = $params[$key];
Expand All @@ -537,7 +534,6 @@ protected function _writeUrl($params) {
$replace[] = $string;
}
$out = str_replace($search, $replace, $out);

}

if (strpos($this->template, '*')) {
Expand Down
12 changes: 6 additions & 6 deletions lib/Cake/Test/Case/Routing/Route/CakeRouteTest.php
Expand Up @@ -118,7 +118,7 @@ public function testRouteCompilingWithParamPatterns() {
$this->assertRegExp($result, '/posts/view/518098');
$this->assertNotRegExp($result, '/posts/edit/name-of-post');
$this->assertNotRegExp($result, '/posts/edit/4/other:param');
$this->assertEquals(array('controller', 'action', 'id'), $route->keys);
$this->assertEquals(array('id', 'controller', 'action'), $route->keys);

$route = new CakeRoute(
'/:lang/:controller/:action/:id',
Expand All @@ -130,7 +130,7 @@ public function testRouteCompilingWithParamPatterns() {
$this->assertRegExp($result, '/cze/articles/view/1');
$this->assertNotRegExp($result, '/language/articles/view/2');
$this->assertNotRegExp($result, '/eng/articles/view/name-of-article');
$this->assertEquals(array('lang', 'controller', 'action', 'id'), $route->keys);
$this->assertEquals(array('lang', 'id', 'controller', 'action'), $route->keys);

foreach (array(':', '@', ';', '$', '-') as $delim) {
$route = new CakeRoute('/posts/:id' . $delim . ':title');
Expand All @@ -141,7 +141,7 @@ public function testRouteCompilingWithParamPatterns() {
$this->assertNotRegExp($result, '/posts/11!nameofarticle');
$this->assertNotRegExp($result, '/posts/11');

$this->assertEquals(array('id', 'title'), $route->keys);
$this->assertEquals(array('title', 'id'), $route->keys);
}

$route = new CakeRoute(
Expand All @@ -155,7 +155,7 @@ public function testRouteCompilingWithParamPatterns() {
$this->assertNotRegExp($result, '/posts/hey_now:nameofarticle');
$this->assertNotRegExp($result, '/posts/:nameofarticle/2009');
$this->assertNotRegExp($result, '/posts/:nameofarticle/01');
$this->assertEquals(array('id', 'title', 'year'), $route->keys);
$this->assertEquals(array('year', 'title', 'id'), $route->keys);

$route = new CakeRoute(
'/posts/:url_title-(uuid::id)',
Expand Down Expand Up @@ -204,7 +204,7 @@ public function testComplexRouteCompilingAndParsing() {

$this->assertRegExp($result, '/some_extra/page/this_is_the_slug');
$this->assertRegExp($result, '/page/this_is_the_slug');
$this->assertEquals(array('extra', 'slug'), $route->keys);
$this->assertEquals(array('slug', 'extra'), $route->keys);
$this->assertEquals(array('extra' => '[a-z1-9_]*', 'slug' => '[a-z1-9_]+', 'action' => 'view'), $route->options);
$expected = array(
'controller' => 'pages',
Expand Down Expand Up @@ -864,8 +864,8 @@ public function testMatchSimilarParameters() {
$route = new CakeRoute('/:thisParam/:thisParamIsLonger');

$url = array(
'thisParamIsLonger' => 'bar',
'thisParam' => 'foo',
'thisParamIsLonger' => 'bar'
);

$result = $route->match($url);
Expand Down

0 comments on commit c0ac611

Please sign in to comment.