Re-build routing prefixes.

* Switch prefixes away from method prefixes to namespace prefixes. This has two benefits. One it keeps classes smaller, and more focused. Two prefix actions involved a lot of hackery that is now gone. * Update tests throughout the Routing package for the change. * Add tests for plugin + app prefix dispatching. * Fix a number of issues with generated route names that were disguised by the old prefix system.
cakephp · Jul 4, 2012 · 752e039 · 752e039
1 parent 8ba87b7
commit 752e039
Show file tree

Hide file tree

Showing 10 changed files with 288 additions and 205 deletions.
diff --git a/lib/Cake/Config/routes.php b/lib/Cake/Config/routes.php
@@ -56,10 +56,14 @@
 	}
 	$pluginPattern = implode('|', $plugins);
 	$match = array('plugin' => $pluginPattern);
-	$shortParams = array('routeClass' => 'Cake\Routing\Route\PluginShortRoute', 'plugin' => $pluginPattern);
+	$shortParams = array(
+		'routeClass' => 'Cake\Routing\Route\PluginShortRoute',
+		'plugin' => $pluginPattern,
+		'_name' => '_plugin._controller:index',
+	);
 
 	foreach ($prefixes as $prefix) {
-		$params = array('prefix' => $prefix, $prefix => true);
+		$params = array('prefix' => $prefix);
 		$indexParams = $params + array('action' => 'index');
 		Router::connect("/{$prefix}/:plugin", $indexParams, $shortParams);
 		Router::connect("/{$prefix}/:plugin/:controller", $indexParams, $match);

diff --git a/lib/Cake/Routing/Dispatcher.php b/lib/Cake/Routing/Dispatcher.php
@@ -252,18 +252,19 @@ protected function _getController($request, $response) {
  */
 	protected function _loadController($request) {
 		$pluginName = $pluginPath = $controller = null;
+		$namespace = 'Controller';
 		if (!empty($request->params['plugin'])) {
-			$pluginName = $controller = Inflector::camelize($request->params['plugin']);
+			$pluginName = Inflector::camelize($request->params['plugin']);
 			$pluginPath = $pluginName . '.';
 		}
 		if (!empty($request->params['controller'])) {
 			$controller = Inflector::camelize($request->params['controller']);
 		}
+		if (!empty($request->params['prefix'])) {
+			$namespace .= '/' . Inflector::camelize($request->params['prefix']);
+		}
 		if ($pluginPath . $controller) {
-			$controller = App::classname($pluginPath . $controller, 'Controller', 'Controller');
-			if ($controller) {
-				return $controller;
-			}
+			return App::classname($pluginPath . $controller, $namespace, 'Controller');
 		}
 		return false;
 	}

diff --git a/lib/Cake/Routing/Route/Route.php b/lib/Cake/Routing/Route/Route.php
@@ -222,16 +222,18 @@ public function getName() {
 		if (isset($this->defaults['plugin'])) {
 			$name = $this->defaults['plugin'] . '.';
 		}
+		if (strpos($this->template, ':plugin') !== false) {
+			$name = '_plugin.';
+		}
 		foreach (array('controller', 'action') as $key) {
 			if ($key === 'action') {
 				$name .= ':';
 			}
-			if (isset($this->defaults[$key])) {
-				$name .= $this->defaults[$key];
-			}
 			$var = ':' . $key;
 			if (strpos($this->template, $var) !== false) {
 				$name .= '_' . $key;
+			} elseif (isset($this->defaults[$key])) {
+				$name .= $this->defaults[$key];
 			}
 		}
 		return $this->_name = strtolower($name);
@@ -421,10 +423,6 @@ public function match($url, $context = array()) {
 			unset($url['_ext']);
 		}
 
-		if (isset($defaults['prefix'])) {
-			$url['prefix'] = $defaults['prefix'];
-		}
-
 		// Missing defaults is a fail.
 		if (array_diff_key($defaults, $url) !== array()) {
 			return false;
@@ -497,14 +495,6 @@ public function match($url, $context = array()) {
  * @return string Composed route string.
  */
 	protected function _writeUrl($params, $pass = array(), $query = array()) {
-		if (isset($params['prefix'])) {
-			$prefixed = $params['prefix'] . '_';
-		}
-		if (isset($prefixed, $params['action']) && strpos($params['action'], $prefixed) === 0) {
-			$params['action'] = substr($params['action'], strlen($prefixed) * -1);
-			unset($params['prefix']);
-		}
-
 		$pass = implode('/', array_map('rawurlencode', $pass));
 		$out = $this->template;
 

diff --git a/lib/Cake/Routing/RouteCollection.php b/lib/Cake/Routing/RouteCollection.php
@@ -113,13 +113,18 @@ protected function _getNames($url) {
 		$fallbacks = array(
 			'%2$s:%3$s',
 			'%2$s:_action',
+			'_controller:%3$s',
 			'_controller:_action'
 		);
 		if ($plugin) {
 			$fallbacks = array(
 				'%1$s.%2$s:%3$s',
 				'%1$s.%2$s:_action',
-				'_controller:_action'
+				'%1$s._controller:%3$s',
+				'%1$s._controller:_action',
+				'_plugin._controller:%3$s',
+				'_plugin._controller:_action',
+				'_controller:_action',
 			);
 		}
 		foreach ($fallbacks as $i => $template) {

diff --git a/lib/Cake/Routing/Router.php b/lib/Cake/Routing/Router.php
@@ -266,20 +266,14 @@ public static function resourceMap($resourceMap = null) {
  * @throws RouterException
  */
 	public static function connect($route, $defaults = array(), $options = array()) {
-		foreach (static::$_prefixes as $prefix) {
-			if (isset($defaults[$prefix])) {
-				if ($defaults[$prefix]) {
-					$defaults['prefix'] = $prefix;
-				} else {
-					unset($defaults[$prefix]);
-				}
-				break;
-			}
-		}
-		if (isset($defaults['prefix'])) {
+		if (!empty($defaults['prefix'])) {
 			static::$_prefixes[] = $defaults['prefix'];
 			static::$_prefixes = array_keys(array_flip(static::$_prefixes));
 		}
+		if (empty($defaults['prefix'])) {
+			unset($defaults['prefix']);
+		}
+
 		$defaults += array('plugin' => null);
 		if (empty($options['action'])) {
 			$defaults += array('action' => 'index');
@@ -416,13 +410,7 @@ public static function parse($url) {
 			$url = substr($url, 0, strpos($url, '?'));
 		}
 
-		$out = static::$_routes->parse($url);
-
-		if (isset($out['prefix'])) {
-			$out['action'] = $out['prefix'] . '_' . $out['action'];
-		}
-
-		return $out;
+		return static::$_routes->parse($url);
 	}
 
 /**
@@ -689,29 +677,25 @@ public static function url($url = null, $options = array()) {
 			}
 
 			// Copy the current action if the controller is the current one.
-			if (empty($url['action'])) {
-				if (empty($url['controller']) || $params['controller'] === $url['controller']) {
-					$url['action'] = $params['action'];
-				} else {
-					$url['action'] = 'index';
-				}
+			if (
+				empty($url['action']) &&
+				(empty($url['controller']) || $params['controller'] === $url['controller'])
+			) {
+				$url['action'] = $params['action'];
 			}
 
-			$prefixExists = (array_intersect_key($url, array_flip(static::$_prefixes)));
-			foreach (static::$_prefixes as $prefix) {
-				if (!empty($params[$prefix]) && !$prefixExists) {
-					$url[$prefix] = true;
-				} elseif (isset($url[$prefix]) && !$url[$prefix]) {
-					unset($url[$prefix]);
-				}
-				if (isset($url[$prefix]) && strpos($url['action'], $prefix . '_') === 0) {
-					$url['action'] = substr($url['action'], strlen($prefix) + 1);
-				}
+			// Keep the current prefix around, or remove it if its falsey
+			if (!empty($params['prefix']) && !isset($url['prefix'])) {
+				$url['prefix'] = $params['prefix'];
+			}
+			if (empty($url['prefix'])) {
+				unset($url['prefix']);
 			}
 
 			$url += array(
 				'controller' => $params['controller'],
-				'plugin' => $params['plugin']
+				'plugin' => $params['plugin'],
+				'action' => 'index'
 			);
 			$url = static::_applyUrlFilters($url);
 			$output = static::$_routes->match($url);

diff --git a/lib/Cake/Test/TestApp/Controller/Admin/PostsController.php b/lib/Cake/Test/TestApp/Controller/Admin/PostsController.php
@@ -0,0 +1,45 @@
+<?php
+/**
+ * CakePHP(tm) : Rapid Development Framework (http://cakephp.org)
+ * Copyright 2005-2012, Cake Software Foundation, Inc. (http://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright 2005-2012, Cake Software Foundation, Inc. (http://cakefoundation.org)
+ * @link          http://cakephp.org CakePHP(tm) Project
+ * @since         CakePHP(tm) v 3.0.0
+ * @license       MIT License (http://www.opensource.org/licenses/mit-license.php)
+ */
+namespace TestApp\Controller\Admin;
+
+use Cake\Controller\Controller;
+
+/**
+ * Posts Controller class.
+ *
+ * For testing prefix routing.
+ */
+class PostsController extends Controller {
+/**
+ * components
+ *
+ * @var array
+ */
+	public $components = array();
+
+/**
+ * components
+ *
+ * @var array
+ */
+	public $uses = array();
+
+/**
+ * index action
+ *
+ * @return void
+ */
+	public function index() {
+	}
+}
diff --git a/lib/Cake/Test/TestApp/Plugin/TestPlugin/Controller/Admin/CommentsController.php b/lib/Cake/Test/TestApp/Plugin/TestPlugin/Controller/Admin/CommentsController.php
@@ -0,0 +1,45 @@
+<?php
+/**
+ * CakePHP(tm) : Rapid Development Framework (http://cakephp.org)
+ * Copyright 2005-2012, Cake Software Foundation, Inc. (http://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright 2005-2012, Cake Software Foundation, Inc. (http://cakefoundation.org)
+ * @link          http://cakephp.org CakePHP(tm) Project
+ * @since         CakePHP(tm) v 3.0.0
+ * @license       MIT License (http://www.opensource.org/licenses/mit-license.php)
+ */
+namespace TestApp\Controller\Admin;
+
+use Cake\Controller\Controller;
+
+/**
+ * Comments Controller class.
+ *
+ * For testing prefix routing.
+ */
+class CommentsController extends Controller {
+/**
+ * components
+ *
+ * @var array
+ */
+	public $components = array();
+
+/**
+ * components
+ *
+ * @var array
+ */
+	public $uses = array();
+
+/**
+ * index action
+ *
+ * @return void
+ */
+	public function index() {
+	}
+}
diff --git a/lib/Cake/Test/TestCase/Routing/DispatcherTest.php b/lib/Cake/Test/TestCase/Routing/DispatcherTest.php
@@ -472,27 +472,64 @@ public function testDispatchActionReturnsResponse() {
 	}
 
 /**
- * testAdminDispatch method
+ * testPrefixDispatch method
  *
  * @return void
  */
-	public function testAdminDispatch() {
+	public function testPrefixDispatch() {
 		$Dispatcher = new TestDispatcher();
 		Configure::write('Routing.prefixes', array('admin'));
-		$url = new Request('admin/test_dispatch_pages/index');
+		$request = new Request('admin/posts/index');
 		$response = $this->getMock('Cake\Network\Response');
 
 		Router::reload();
 		require CAKE . 'Config' . DS . 'routes.php';
 
-		$Dispatcher->dispatch($url, $response, array('return' => 1));
+		$Dispatcher->dispatch($request, $response, array('return' => 1));
 
-		$this->assertEquals('TestDispatchPages', $Dispatcher->controller->name);
+		$this->assertInstanceOf(
+			'TestApp\Controller\Admin\PostsController',
+			$Dispatcher->controller
+		);
+		$this->assertEquals('admin', $request->params['prefix']);
+		$this->assertEquals('posts', $request->params['controller']);
+		$this->assertEquals('index', $request->params['action']);
 
-		$this->assertTrue($Dispatcher->controller->params['admin']);
+		$expected = '/admin/posts/index';
+		$this->assertSame($expected, $request->here);
+	}
+
+/**
+ * test prefix dispatching in a plugin.
+ *
+ * @return void
+ */
+	public function testPrefixDispatchPlugin() {
+		App::build(array(
+			'Plugin' => array(CAKE . 'Test' . DS . 'TestApp' . DS . 'Plugin' . DS)
+		), App::RESET);
+		Configure::write('Routing.prefixes', array('admin'));
+		Plugin::load('TestPlugin');
+
+		$request = new Request('admin/posts/index');
+		$response = $this->getMock('Cake\Network\Response');
+
+		Router::reload();
+		require CAKE . 'Config' . DS . 'routes.php';
+
+		$Dispatcher = new TestDispatcher();
+		$Dispatcher->dispatch($request, $response, array('return' => 1));
+
+		$this->assertInstanceOf(
+			'TestApp\Controller\Admin\PostsController',
+			$Dispatcher->controller
+		);
+		$this->assertEquals('admin', $request->params['prefix']);
+		$this->assertEquals('posts', $request->params['controller']);
+		$this->assertEquals('index', $request->params['action']);
 
-		$expected = '/admin/test_dispatch_pages/index';
-		$this->assertSame($expected, $Dispatcher->controller->here);
+		$expected = '/admin/posts/index';
+		$this->assertSame($expected, $request->here);
 	}
 
 /**

diff --git a/lib/Cake/Test/TestCase/Routing/Route/RouteTest.php b/lib/Cake/Test/TestCase/Routing/Route/RouteTest.php
@@ -325,17 +325,17 @@ public function testMatchBasic() {
 		$this->assertFalse($result);
 
 		$route = new Route('/admin/subscriptions/:action/*', array(
-			'controller' => 'subscribe', 'admin' => true, 'prefix' => 'admin'
+			'controller' => 'subscribe', 'prefix' => 'admin'
 		));
 
-		$url = array('controller' => 'subscribe', 'admin' => true, 'action' => 'edit', 1);
+		$url = array('controller' => 'subscribe', 'prefix' =>  'admin', 'action' => 'edit', 1);
 		$result = $route->match($url);
 		$expected = '/admin/subscriptions/edit/1';
 		$this->assertEquals($expected, $result);
 
 		$url = array(
 			'controller' => 'subscribe',
-			'admin' => true,
+			'prefix' => 'admin',
 			'action' => 'edit_admin_e',
 			1
 		);
@@ -756,6 +756,9 @@ public function testGetName() {
 
 		$route = new Route('/articles/list', array('controller' => 'posts', 'action' => 'index'));
 		$this->assertEquals('posts:index', $route->getName());
+
+		$route = new Route('/:controller/:action', array('action' => 'index'));
+		$this->assertEquals('_controller:_action', $route->getName());
 	}
 
 /**