Fix failing tests around prefixes.

There were commented out tests for prefixes + controller routes. Using a prefix should create more specific route names that match earlier preventing names like `users:_action` from matching. * Fix up name generation for plugin/prefixes with 0 as a name. * Remove bad Controller test for prefix. * Remove duplicate route prefix test.
cakephp · Jul 7, 2014 · 12c5f5b · 12c5f5b
1 parent 0dcfaff
commit 12c5f5b
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 47 deletions.
diff --git a/src/Routing/Route/Route.php b/src/Routing/Route/Route.php
@@ -227,22 +227,27 @@ public function getName() {
 			return $this->_name;
 		}
 		$name = '';
-		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 .= ':';
-			}
-			$var = ':' . $key;
-			if (strpos($this->template, $var) !== false) {
-				$name .= '_' . $key;
+		$keys = [
+			'prefix' => ':',
+			'plugin' => '.',
+			'controller' => ':',
+			'action' => ''
+		];
+		foreach ($keys as $key => $glue) {
+			$value = null;
+			if (strpos($this->template, ':' . $key) !== false) {
+				$value = '_' . $key;
 			} elseif (isset($this->defaults[$key])) {
-				$name .= $this->defaults[$key];
+				$value = $this->defaults[$key];
+			}
+
+			if ($value === null) {
+				continue;
+			}
+			if (is_bool($value)) {
+				$value = $value ? '1' : '0';
 			}
+			$name .= $value . $glue;
 		}
 		return $this->_name = strtolower($name);
 	}

diff --git a/src/Routing/RouteCollection.php b/src/Routing/RouteCollection.php
@@ -120,31 +120,76 @@ public function parse($url) {
  */
 	protected function _getNames($url) {
 		$plugin = false;
-		if (isset($url['plugin'])) {
+		if (isset($url['plugin']) && $url['plugin'] !== false) {
 			$plugin = strtolower($url['plugin']);
 		}
+		$prefix = false;
+		if (isset($url['prefix']) && $url['prefix'] !== false) {
+			$prefix = strtolower($url['prefix']);
+		}
 		$controller = strtolower($url['controller']);
 		$action = strtolower($url['action']);
 
-		$fallbacks = [
+		$names = [
 			"${controller}:${action}",
 			"${controller}:_action",
 			"_controller:${action}",
 			"_controller:_action"
 		];
-		if ($plugin) {
-			$fallbacks = [
+
+		// No prefix, no plugin
+		if ($prefix === false && $plugin === false) {
+			return $names;
+		}
+
+		// Only a plugin
+		if ($prefix === false) {
+			return [
 				"${plugin}.${controller}:${action}",
 				"${plugin}.${controller}:_action",
 				"${plugin}._controller:${action}",
 				"${plugin}._controller:_action",
 				"_plugin.${controller}:${action}",
+				"_plugin.${controller}:_action",
 				"_plugin._controller:${action}",
 				"_plugin._controller:_action",
-				"_controller:_action"
 			];
 		}
-		return $fallbacks;
+
+		// Only a prefix
+		if ($plugin === false) {
+			return [
+				"${prefix}:${controller}:${action}",
+				"${prefix}:${controller}:_action",
+				"${prefix}:_controller:${action}",
+				"${prefix}:_controller:_action",
+				"_prefix:${controller}:${action}",
+				"_prefix:${controller}:_action",
+				"_prefix:_controller:${action}",
+				"_prefix:_controller:_action"
+			];
+		}
+
+		// Prefix and plugin has the most options
+		// as there are 4 factors.
+		return [
+			"${prefix}:${plugin}.${controller}:${action}",
+			"${prefix}:${plugin}.${controller}:_action",
+			"${prefix}:${plugin}._controller:${action}",
+			"${prefix}:${plugin}._controller:_action",
+			"${prefix}:_plugin.${controller}:${action}",
+			"${prefix}:_plugin.${controller}:_action",
+			"${prefix}:_plugin._controller:${action}",
+			"${prefix}:_plugin._controller:_action",
+			"_prefix:${plugin}.${controller}:${action}",
+			"_prefix:${plugin}.${controller}:_action",
+			"_prefix:${plugin}._controller:${action}",
+			"_prefix:${plugin}._controller:_action",
+			"_prefix:_plugin.${controller}:${action}",
+			"_prefix:_plugin.${controller}:_action",
+			"_prefix:_plugin._controller:${action}",
+			"_prefix:_plugin._controller:_action",
+		];
 	}
 
 /**

diff --git a/tests/TestCase/Controller/ControllerTest.php b/tests/TestCase/Controller/ControllerTest.php
@@ -800,26 +800,6 @@ public function testInvokeActionBaseMethods() {
 		$Controller->invokeAction();
 	}
 
-/**
- * test invoking controller methods.
- *
- * @expectedException \Cake\Controller\Error\PrivateActionException
- * @expectedExceptionMessage Private Action TestController::admin_add() is not directly accessible.
- * @return void
- */
-	public function testInvokeActionPrefixProtection() {
-		Configure::write('Routing.prefixes', array('admin'));
-		Router::reload();
-		Router::connect('/admin/:controller/:action/*', array('prefix' => 'admin'));
-
-		$url = new Request('test/admin_add/');
-		$url->addParams(array('controller' => 'test_controller', 'action' => 'admin_add'));
-		$response = $this->getMock('Cake\Network\Response');
-
-		$Controller = new TestController($url, $response);
-		$Controller->invokeAction();
-	}
-
 /**
  * test invoking controller methods.
  *

diff --git a/tests/TestCase/Routing/Route/RouteTest.php b/tests/TestCase/Routing/Route/RouteTest.php
@@ -22,7 +22,6 @@
 
 /**
  * Test case for Route
- *
  */
 class RouteTest extends TestCase {
 
@@ -880,6 +879,37 @@ public function testGetNamePlugins() {
 		$this->assertEquals('asset.assets:get', $route->getName());
 	}
 
+/**
+ * Test getName() with prefixes.
+ *
+ * @return void
+ */
+	public function testGetNamePrefix() {
+		$route = new Route(
+			'/admin/:controller/:action',
+			array('prefix' => 'admin')
+		);
+		$this->assertEquals('admin:_controller:_action', $route->getName());
+
+		$route = new Route(
+			'/:prefix/assets/:action',
+			array('controller' => 'assets')
+		);
+		$this->assertEquals('_prefix:assets:_action', $route->getName());
+
+		$route = new Route(
+			'/admin/assets/get',
+			array('prefix' => 'admin', 'plugin' => 'asset', 'controller' => 'assets', 'action' => 'get')
+		);
+		$this->assertEquals('admin:asset.assets:get', $route->getName());
+
+		$route = new Route(
+			'/:prefix/:plugin/:controller/:action/*',
+			[]
+		);
+		$this->assertEquals('_prefix:_plugin._controller:_action', $route->getName());
+	}
+
 /**
  * test that utf-8 patterns work for :section
  *

diff --git a/tests/TestCase/Routing/RouteBuilderTest.php b/tests/TestCase/Routing/RouteBuilderTest.php
@@ -280,10 +280,10 @@ public function testResourcesNested() {
  * @return void
  */
 	public function testFallbacks() {
-		$routes = new ScopedRouteCollection('/api', ['prefix' => 'api']);
+		$routes = new RouteBuilder($this->collection, '/api', ['prefix' => 'api']);
 		$routes->fallbacks();
 
-		$all = $routes->routes();
+		$all = $this->collection->routes();
 		$this->assertEquals('/api/:controller', $all[0]->template);
 		$this->assertEquals('/api/:controller/:action/*', $all[1]->template);
 	}

diff --git a/tests/TestCase/Routing/RouterTest.php b/tests/TestCase/Routing/RouterTest.php
@@ -2055,10 +2055,6 @@ public function testUrlWritingWithPrefixes() {
 		$expected = '/company/users/login';
 		$this->assertEquals($expected, $result);
 
-		$result = Router::url(array('controller' => 'users', 'action' => 'login', 'prefix' => 'company'));
-		$expected = '/company/users/login';
-		$this->assertEquals($expected, $result);
-
 		$request = new Request();
 		Router::setRequestInfo(
 			$request->addParams(array(