Browse Source

Fix route name generation when using braced placeholders.

Earlier reverse routing failed when using placeholder like {controller} or {action}
in URL template since automatic route names are generated based on such special keys
and are used by RouteCollection for matching.

Use new style placeholders in fallback route templates.

Backport changes from #13857 to 3.8
ADmad 6 years ago
parent
commit
4837b0842d

+ 3 - 1
src/Routing/Route/Route.php

@@ -391,7 +391,9 @@ class Route
         ];
         ];
         foreach ($keys as $key => $glue) {
         foreach ($keys as $key => $glue) {
             $value = null;
             $value = null;
-            if (strpos($this->template, ':' . $key) !== false) {
+            if (strpos($this->template, ':' . $key) !== false
+                || strpos($this->template, '{' . $key . '}') !== false
+            ) {
                 $value = '_' . $key;
                 $value = '_' . $key;
             } elseif (isset($this->defaults[$key])) {
             } elseif (isset($this->defaults[$key])) {
                 $value = $this->defaults[$key];
                 $value = $this->defaults[$key];

+ 2 - 2
src/Routing/RouteBuilder.php

@@ -1020,8 +1020,8 @@ class RouteBuilder
     public function fallbacks($routeClass = null)
     public function fallbacks($routeClass = null)
     {
     {
         $routeClass = $routeClass ?: $this->_routeClass;
         $routeClass = $routeClass ?: $this->_routeClass;
-        $this->connect('/:controller', ['action' => 'index'], compact('routeClass'));
-        $this->connect('/:controller/:action/*', [], compact('routeClass'));
+        $this->connect('/{controller}', ['action' => 'index'], compact('routeClass'));
+        $this->connect('/{controller}/{action}/*', [], compact('routeClass'));
     }
     }
 
 
     /**
     /**

+ 3 - 0
tests/TestCase/Routing/Route/RouteTest.php

@@ -1607,6 +1607,9 @@ class RouteTest extends TestCase
         $route = new Route('/:controller/:action');
         $route = new Route('/:controller/:action');
         $this->assertEquals('_controller:_action', $route->getName());
         $this->assertEquals('_controller:_action', $route->getName());
 
 
+        $route = new Route('/{controller}/{action}');
+        $this->assertSame('_controller:_action', $route->getName());
+
         $route = new Route('/articles/:action', ['controller' => 'posts']);
         $route = new Route('/articles/:action', ['controller' => 'posts']);
         $this->assertEquals('posts:_action', $route->getName());
         $this->assertEquals('posts:_action', $route->getName());
 
 

+ 4 - 4
tests/TestCase/Routing/RouteBuilderTest.php

@@ -894,8 +894,8 @@ class RouteBuilderTest extends TestCase
         $routes->fallbacks();
         $routes->fallbacks();
 
 
         $all = $this->collection->routes();
         $all = $this->collection->routes();
-        $this->assertEquals('/api/:controller', $all[0]->template);
-        $this->assertEquals('/api/:controller/:action/*', $all[1]->template);
+        $this->assertSame('/api/{controller}', $all[0]->template);
+        $this->assertSame('/api/{controller}/{action}/*', $all[1]->template);
         $this->assertInstanceOf(Route::class, $all[0]);
         $this->assertInstanceOf(Route::class, $all[0]);
     }
     }
 
 
@@ -910,8 +910,8 @@ class RouteBuilderTest extends TestCase
         $routes->fallbacks('InflectedRoute');
         $routes->fallbacks('InflectedRoute');
 
 
         $all = $this->collection->routes();
         $all = $this->collection->routes();
-        $this->assertEquals('/api/:controller', $all[0]->template);
-        $this->assertEquals('/api/:controller/:action/*', $all[1]->template);
+        $this->assertSame('/api/{controller}', $all[0]->template);
+        $this->assertSame('/api/{controller}/{action}/*', $all[1]->template);
         $this->assertInstanceOf(InflectedRoute::class, $all[0]);
         $this->assertInstanceOf(InflectedRoute::class, $all[0]);
     }
     }
 
 

+ 1 - 1
tests/TestCase/Routing/RouteCollectionTest.php

@@ -582,7 +582,7 @@ class RouteCollectionTest extends TestCase
         ];
         ];
         $routes = new RouteBuilder($this->collection, '/');
         $routes = new RouteBuilder($this->collection, '/');
         $routes->connect('/:action/*', ['controller' => 'Users']);
         $routes->connect('/:action/*', ['controller' => 'Users']);
-        $routes->connect('/admin/:controller', ['prefix' => 'admin', 'action' => 'index']);
+        $routes->connect('/admin/{controller}', ['prefix' => 'admin', 'action' => 'index']);
 
 
         $url = [
         $url = [
             'plugin' => null,
             'plugin' => null,

+ 5 - 5
tests/TestCase/Routing/RouterTest.php

@@ -1582,7 +1582,7 @@ class RouterTest extends TestCase
             'pass' => ['home'],
             'pass' => ['home'],
             'controller' => 'Pages',
             'controller' => 'Pages',
             'action' => 'display',
             'action' => 'display',
-            '_matchedRoute' => '/:controller/:action/*',
+            '_matchedRoute' => '/{controller}/{action}/*',
         ];
         ];
         $this->assertEquals($expected, $result);
         $this->assertEquals($expected, $result);
 
 
@@ -1999,7 +1999,7 @@ class RouterTest extends TestCase
             'action' => 'index',
             'action' => 'index',
             '_ext' => 'rss',
             '_ext' => 'rss',
             'pass' => [],
             'pass' => [],
-            '_matchedRoute' => '/:controller'
+            '_matchedRoute' => '/{controller}'
         ];
         ];
         $this->assertEquals($expected, $result);
         $this->assertEquals($expected, $result);
 
 
@@ -2010,7 +2010,7 @@ class RouterTest extends TestCase
             'action' => 'view',
             'action' => 'view',
             'pass' => ['1'],
             'pass' => ['1'],
             '_ext' => 'rss',
             '_ext' => 'rss',
-            '_matchedRoute' => '/:controller/:action/*'
+            '_matchedRoute' => '/{controller}/{action}/*'
         ];
         ];
         $this->assertEquals($expected, $result);
         $this->assertEquals($expected, $result);
 
 
@@ -2029,7 +2029,7 @@ class RouterTest extends TestCase
             'action' => 'index',
             'action' => 'index',
             '_ext' => 'xml',
             '_ext' => 'xml',
             'pass' => [],
             'pass' => [],
-            '_matchedRoute' => '/:controller'
+            '_matchedRoute' => '/{controller}'
         ];
         ];
         $this->assertEquals($expected, $result);
         $this->assertEquals($expected, $result);
 
 
@@ -2040,7 +2040,7 @@ class RouterTest extends TestCase
             'action' => 'index',
             'action' => 'index',
             'pass' => [],
             'pass' => [],
             '?' => ['hello' => 'goodbye'],
             '?' => ['hello' => 'goodbye'],
-            '_matchedRoute' => '/:controller'
+            '_matchedRoute' => '/{controller}'
         ];
         ];
         $this->assertEquals($expected, $result);
         $this->assertEquals($expected, $result);
 
 

+ 1 - 0
tests/TestCase/View/Helper/HtmlHelperTest.php

@@ -116,6 +116,7 @@ class HtmlHelperTest extends TestCase
      */
      */
     public function testLink()
     public function testLink()
     {
     {
+        Router::reload();
         Router::connect('/:controller/:action/*');
         Router::connect('/:controller/:action/*');
 
 
         $this->View->setRequest($this->View->getRequest()->withAttribute('webroot', ''));
         $this->View->setRequest($this->View->getRequest()->withAttribute('webroot', ''));