Move string target parsing into RouteBuilder.

Having it in RouteBuilder makes it accessible to the HTTP method
helpers, as well as connect() on the scoped builder.

Prefix names need to be lower cased, so that has been updated as well.

I've added tests for nested plugins and nested prefixes as well.

src/Routing/RouteBuilder.php

@@ -586,6 +586,7 @@ class RouteBuilder
             'routeClass' => $this->_routeClass,
         ];
 
+        $target = $this->parseDefaults($target);
         $target['_method'] = $method;
 
         $route = $this->_makeRoute($template, $target, $options);
@@ -703,12 +704,9 @@ class RouteBuilder
      * @throws \InvalidArgumentException
      * @throws \BadMethodCallException
      */
-    public function connect($route, array $defaults = [], array $options = [])
+    public function connect($route, $defaults = [], array $options = [])
     {
-        if ($defaults === null) {
-            $defaults = [];
-        }
-
+        $defaults = $this->parseDefaults($defaults);
         if (!isset($options['action']) && !isset($defaults['action'])) {
             $defaults['action'] = 'index';
         }
@@ -734,6 +732,50 @@ class RouteBuilder
     }
 
     /**
+     * Parse the defaults if they're a string
+     *
+     * @param string|array $defaults Defaults array from the connect() method.
+     * @return string|array
+     */
+    protected static function parseDefaults($defaults)
+    {
+        if (!is_string($defaults)) {
+            return $defaults;
+        }
+
+        $regex = '/(?:([a-zA-Z0-9\/]*)\.)?([a-zA-Z0-9\/]*?)(?:\/)?([a-zA-Z0-9]*):{2}([a-zA-Z0-9_]*)/i';
+        if (preg_match($regex, $defaults, $matches)) {
+            unset($matches[0]);
+            $matches = array_filter($matches, function ($value) {
+                return $value !== '' && $value !== '::';
+            });
+
+            // Intentionally incomplete switch
+            switch (count($matches)) {
+                case 2:
+                    return [
+                        'controller' => $matches[3],
+                        'action' => $matches[4]
+                    ];
+                case 3:
+                    return [
+                        'prefix' => strtolower($matches[2]),
+                        'controller' => $matches[3],
+                        'action' => $matches[4]
+                    ];
+                case 4:
+                    return [
+                        'plugin' => $matches[1],
+                        'prefix' => strtolower($matches[2]),
+                        'controller' => $matches[3],
+                        'action' => $matches[4]
+                    ];
+            }
+        }
+        throw new RuntimeException("Could not parse `{$defaults}` route destination string.");
+    }
+
+    /**
      * Create a route object, or return the provided object.
      *
      * @param string|\Cake\Routing\Route\Route $route The route template or route object.

src/Routing/Router.php

@@ -205,8 +205,6 @@ class Router
      */
     public static function connect($route, $defaults = [], $options = [])
     {
-        $defaults = static::parseDefaults($defaults);
-
         static::$initialized = true;
         static::scope('/', function ($routes) use ($route, $defaults, $options) {
             $routes->connect($route, $defaults, $options);
@@ -214,51 +212,6 @@ class Router
     }
 
     /**
-     * Parse the defaults if they're a string
-     *
-     * @param string|array Defaults array from the connect() method.
-     * @return string|array
-     */
-    protected static function parseDefaults($defaults)
-    {
-        if (!is_string($defaults)) {
-            return $defaults;
-        }
-
-        $regex = '/(?:([a-zA-Z0-9]*)\.)?([a-zA-Z0-9]*)(?:\\\\)?([a-zA-Z0-9]*):{2}([a-zA-Z0-9]*)/i';
-
-        if (preg_match($regex, $defaults, $matches)) {
-            unset($matches[0]);
-            $matches = array_filter($matches, function ($value) {
-                return $value !== '' && $value !== '::';
-            });
-
-            switch (count($matches)) {
-                case 2:
-                    return [
-                        'controller' => $matches[2],
-                        'view' => $matches[4]
-                    ];
-                case 3:
-                    return [
-                        'prefix' => $matches[2],
-                        'controller' => $matches[3],
-                        'view' => $matches[4]
-                    ];
-                case 4:
-                    return [
-                        'plugin' => $matches[1],
-                        'prefix' => $matches[2],
-                        'controller' => $matches[3],
-                        'view' => $matches[4]
-                    ];
-                default:
-                    throw new RuntimeException('Could not parse the string syntax for Router::connect() defaults.');
-            }
-        }
-    }
-
-    /**
      * Connects a new redirection Route in the router.
      *
      * Compatibility proxy to \Cake\Routing\RouteBuilder::redirect() in the `/` scope.

tests/TestCase/Routing/RouteBuilderTest.php

@@ -22,6 +22,7 @@ use Cake\Routing\Route\InflectedRoute;
 use Cake\Routing\Route\RedirectRoute;
 use Cake\Routing\Route\Route;
 use Cake\TestSuite\TestCase;
+use RuntimeException;
 
 /**
  * RouteBuilder test case
@@ -185,6 +186,65 @@ class RouteBuilderTest extends TestCase
     }
 
     /**
+     * Test connect() with short string syntax
+     *
+     * @return void
+     */
+    public function testConnectShortStringInvalid()
+    {
+        $this->expectException(RuntimeException::class);
+        $routes = new RouteBuilder($this->collection, '/');
+        $routes->connect('/my-articles/view', 'Articles:no');
+    }
+
+    /**
+     * Test connect() with short string syntax
+     *
+     * @return void
+     */
+    public function testConnectShortString()
+    {
+        $routes = new RouteBuilder($this->collection, '/');
+        $routes->connect('/my-articles/view', 'Articles::view');
+        $expected = [
+            'pass' => [],
+            'controller' => 'Articles',
+            'action' => 'view',
+            'plugin' => null,
+            '_matchedRoute' => '/my-articles/view'
+        ];
+        $this->assertEquals($expected, $this->collection->parse('/my-articles/view'));
+
+        $url = $expected['_matchedRoute'];
+        unset($expected['_matchedRoute']);
+        $this->assertEquals($url, '/' . $this->collection->match($expected, []));
+    }
+
+    /**
+     * Test connect() with short string syntax
+     *
+     * @return void
+     */
+    public function testConnectShortStringPluginPrefix()
+    {
+        $routes = new RouteBuilder($this->collection, '/');
+        $routes->connect('/admin/blog/articles/view', 'Vendor/Blog.Management/Admin/Articles::view');
+        $expected = [
+            'pass' => [],
+            'plugin' => 'Vendor/Blog',
+            'prefix' => 'management/admin',
+            'controller' => 'Articles',
+            'action' => 'view',
+            '_matchedRoute' => '/admin/blog/articles/view'
+        ];
+        $this->assertEquals($expected, $this->collection->parse('/admin/blog/articles/view'));
+
+        $url = $expected['_matchedRoute'];
+        unset($expected['_matchedRoute']);
+        $this->assertEquals($url, '/' . $this->collection->match($expected, []));
+    }
+
+    /**
      * Test if a route name already exist
      *
      * @return void
@@ -192,11 +252,9 @@ class RouteBuilderTest extends TestCase
     public function testNameExists()
     {
         $routes = new RouteBuilder($this->collection, '/l', ['prefix' => 'api']);
-
         $this->assertFalse($routes->nameExists('myRouteName'));
 
         $routes->connect('myRouteUrl', ['action' => 'index'], ['_name' => 'myRouteName']);
-
         $this->assertTrue($routes->nameExists('myRouteName'));
     }
 
@@ -1035,6 +1093,30 @@ class RouteBuilderTest extends TestCase
     }
 
     /**
+     * Test that the HTTP method helpers create the right kind of routes.
+     *
+     * @dataProvider httpMethodProvider
+     * @return void
+     */
+    public function testHttpMethodsStringTarget($method)
+    {
+        $routes = new RouteBuilder($this->collection, '/', [], ['namePrefix' => 'app:']);
+        $route = $routes->{strtolower($method)}(
+            '/bookmarks/:id',
+            'Bookmarks::view',
+            'route-name'
+        );
+        $this->assertInstanceOf(Route::class, $route, 'Should return a route');
+        $this->assertSame($method, $route->defaults['_method']);
+        $this->assertSame('app:route-name', $route->options['_name']);
+        $this->assertSame('/bookmarks/:id', $route->template);
+        $this->assertEquals(
+            ['plugin' => null, 'controller' => 'Bookmarks', 'action' => 'view', '_method' => $method],
+            $route->defaults
+        );
+    }
+
+    /**
      * Integration test for http method helpers and route fluent method
      *
      * @return void

tests/TestCase/Routing/RouterTest.php

@@ -3423,50 +3423,24 @@ class RouterTest extends TestCase
     }
 
     /**
-     * testShortStringSyntax
+     * test connect() with short string syntax
      *
      * @return void
      */
-    public function testShortStringSyntax()
+    public function testConnectShortStringSyntax()
     {
-        Router::connect('/admin/articles/view', 'Admin\Articles::view');
+        Router::connect('/admin/articles/view', 'Admin/Articles::view');
         $result = Router::parseRequest($this->makeRequest('/admin/articles/view', 'GET'));
         $expected = [
             'pass' => [],
-            'prefix' => 'Admin',
+            'prefix' => 'admin',
             'controller' => 'Articles',
-            'view' => 'view',
-            'action' => 'index',
+            'action' => 'view',
             'plugin' => null,
             '_matchedRoute' => '/admin/articles/view'
 
         ];
         $this->assertEquals($result, $expected);
-
-        Router::connect('/admin/blog/articles/view', 'Blog.Admin\Articles::view');
-        $result = Router::parseRequest($this->makeRequest('/admin/blog/articles/view', 'GET'));
-        $expected = [
-            'pass' => [],
-            'plugin' => 'Blog',
-            'prefix' => 'Admin',
-            'controller' => 'Articles',
-            'view' => 'view',
-            'action' => 'index',
-            '_matchedRoute' => '/admin/blog/articles/view'
-        ];
-        $this->assertEquals($result, $expected);
-
-        Router::connect('/my-articles/view', 'Articles::view');
-        $result = Router::parseRequest($this->makeRequest('/my-articles/view', 'GET'));
-        $expected = [
-            'pass' => [],
-            'controller' => 'Articles',
-            'view' => 'view',
-            'action' => 'index',
-            'plugin' => null,
-            '_matchedRoute' => '/my-articles/view'
-        ];
-        $this->assertEquals($result, $expected);
     }
 
     /**