Fixing incorrect keying for ext routing parameter. It was

nested under params[url][ext]. This makes it unlike all other routing parameters. Having the nested value also makes reversing requests harder, and generating urls more difficult. Adding a test for Router::reverse() and extensions. Fixes #1923, fixes #1928
cakephp · Aug 23, 2011 · 6acf024 · 6acf024
1 parent 68b2d67
commit 6acf024
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 13 deletions.
diff --git a/lib/Cake/Controller/Component/RequestHandlerComponent.php b/lib/Cake/Controller/Component/RequestHandlerComponent.php
@@ -109,8 +109,8 @@ function __construct(ComponentCollection $collection, $settings = array()) {
 	public function initialize($controller, $settings = array()) {
 		$this->request = $controller->request;
 		$this->response = $controller->response;
-		if (isset($this->request->params['url']['ext'])) {
-			$this->ext = $this->request->params['url']['ext'];
+		if (isset($this->request->params['ext'])) {
+			$this->ext = $this->request->params['ext'];
 		}
 		if (empty($this->ext) || $this->ext == 'html') {
 			$accepts = $this->request->accepts();

diff --git a/lib/Cake/Routing/Router.php b/lib/Cake/Routing/Router.php
@@ -476,8 +476,8 @@ public static function parse($url) {
 			$out['action'] = $out['prefix'] . '_' . $out['action'];
 		}
 
-		if (!empty($ext) && !isset($out['url']['ext'])) {
-			$out['url']['ext'] = $ext;
+		if (!empty($ext) && !isset($out['ext'])) {
+			$out['ext'] = $ext;
 		}
 		return $out;
 	}

diff --git a/lib/Cake/Test/Case/Routing/RouterTest.php b/lib/Cake/Test/Case/Routing/RouterTest.php
@@ -577,7 +577,7 @@ public function testUrlGenerationWithAdminPrefix() {
 		$request->addParams(array(
 			'controller' => 'registrations', 'action' => 'admin_index',
 			'plugin' => null, 'prefix' => 'admin', 'admin' => true,
-			'url' => array('ext' => 'html', 'url' => 'admin/registrations/index')
+			'ext' => 'html'
 		));
 		$request->base = '';
 		$request->here = '/admin/registrations/index';
@@ -1210,18 +1210,18 @@ public function testExtensionParsing() {
 		require CAKE . 'Config' . DS . 'routes.php';
 
 		$result = Router::parse('/posts.rss');
-		$expected = array('plugin' => null, 'controller' => 'posts', 'action' => 'index', 'url' => array('ext' => 'rss'), 'pass'=> array(), 'named' => array());
+		$expected = array('plugin' => null, 'controller' => 'posts', 'action' => 'index', 'ext' => 'rss', 'pass'=> array(), 'named' => array());
 		$this->assertEqual($expected, $result);
 
 		$result = Router::parse('/posts/view/1.rss');
-		$expected = array('plugin' => null, 'controller' => 'posts', 'action' => 'view', 'pass' => array('1'), 'named' => array(), 'url' => array('ext' => 'rss'), 'named' => array());
+		$expected = array('plugin' => null, 'controller' => 'posts', 'action' => 'view', 'pass' => array('1'), 'named' => array(), 'ext' => 'rss', 'named' => array());
 		$this->assertEqual($expected, $result);
 
 		$result = Router::parse('/posts/view/1.rss?query=test');
 		$this->assertEqual($expected, $result);
 
 		$result = Router::parse('/posts/view/1.atom');
-		$expected['url'] = array('ext' => 'atom');
+		$expected['ext'] = 'atom';
 		$this->assertEqual($expected, $result);
 
 		Router::reload();
@@ -1230,24 +1230,24 @@ public function testExtensionParsing() {
 		Router::parseExtensions('rss', 'xml');
 
 		$result = Router::parse('/posts.xml');
-		$expected = array('plugin' => null, 'controller' => 'posts', 'action' => 'index', 'url' => array('ext' => 'xml'), 'pass'=> array(), 'named' => array());
+		$expected = array('plugin' => null, 'controller' => 'posts', 'action' => 'index', 'ext' => 'xml', 'pass'=> array(), 'named' => array());
 		$this->assertEqual($expected, $result);
 
 		$result = Router::parse('/posts.atom?hello=goodbye');
 		$expected = array('plugin' => null, 'controller' => 'posts.atom', 'action' => 'index', 'pass' => array(), 'named' => array());
 		$this->assertEqual($expected, $result);
 
 		Router::reload();
-		Router::connect('/controller/action', array('controller' => 'controller', 'action' => 'action', 'url' => array('ext' => 'rss')));
+		Router::connect('/controller/action', array('controller' => 'controller', 'action' => 'action', 'ext' => 'rss'));
 		$result = Router::parse('/controller/action');
-		$expected = array('controller' => 'controller', 'action' => 'action', 'plugin' => null, 'url' => array('ext' => 'rss'), 'named' => array(), 'pass' => array());
+		$expected = array('controller' => 'controller', 'action' => 'action', 'plugin' => null, 'ext' => 'rss', 'named' => array(), 'pass' => array());
 		$this->assertEqual($expected, $result);
 
 		Router::reload();
 		Router::parseExtensions('rss');
-		Router::connect('/controller/action', array('controller' => 'controller', 'action' => 'action', 'url' => array('ext' => 'rss')));
+		Router::connect('/controller/action', array('controller' => 'controller', 'action' => 'action', 'ext' => 'rss'));
 		$result = Router::parse('/controller/action');
-		$expected = array('controller' => 'controller', 'action' => 'action', 'plugin' => null, 'url' => array('ext' => 'rss'), 'named' => array(), 'pass' => array());
+		$expected = array('controller' => 'controller', 'action' => 'action', 'plugin' => null, 'ext' => 'rss', 'named' => array(), 'pass' => array());
 		$this->assertEqual($expected, $result);
 	}
 
@@ -2273,6 +2273,27 @@ public function testRouterReverse() {
 		$this->assertPattern('/^http(s)?:\/\//', $result);
 	}
 
+/**
+ * Test that extensions work with Router::reverse()
+ *
+ * @return void
+ */
+	public function testReverseWithExtension() {
+		Router::parseExtensions('json');
+
+		$request = new CakeRequest('/posts/view/1.json');
+		$request->addParams(array(
+			'controller' => 'posts',
+			'action' => 'view',
+			'pass' => array(1),
+			'named' => array(),
+			'ext' => 'json',
+		));
+		$result = Router::reverse($request);
+		$expected = '/posts/view/1.json';
+		$this->assertEquals($expected, $result);
+	}
+
 /**
  * test that setRequestInfo can accept arrays and turn that into a CakeRequest object.
  *