Browse Source

Simplify route target parsing.

Using the number of elements doesn't give enough context to determine
whether the route points at a plugin or a prefix. Using named capture
groups lets us handle that situation.

Refs #12051
Mark Story 7 years ago
parent
commit
6429e121d8
2 changed files with 64 additions and 25 deletions
  1. 17 25
      src/Routing/RouteBuilder.php
  2. 47 0
      tests/TestCase/Routing/RouteBuilderTest.php

+ 17 - 25
src/Routing/RouteBuilder.php

@@ -743,32 +743,24 @@ class RouteBuilder
             return $defaults;
         }
 
-        $regex = '/(?:([a-zA-Z0-9\/]*)\.)?([a-zA-Z0-9\/]*?)(?:\/)?([a-zA-Z0-9]*):{2}([a-zA-Z0-9_]*)/i';
+        $regex = '/(?:(?<plugin>[a-zA-Z0-9\/]*)\.)?(?<prefix>[a-zA-Z0-9\/]*?)' .
+            '(?:\/)?(?<controller>[a-zA-Z0-9]*):{2}(?<action>[a-zA-Z0-9_]*)/i';
+
         if (preg_match($regex, $defaults, $matches)) {
-            unset($matches[0]);
-            $matches = array_filter($matches, function ($value) {
-                return $value !== '::';
-            });
-            // Intentionally incomplete switch
-            switch (count($matches)) {
-                case 2:
-                    return [
-                        'controller' => (isset($matches[3]) ? $matches[3] : null),
-                        'action' => (isset($matches[4]) ? $matches[4] : null)
-                    ];
-                case 3:
-                    return [
-                        'prefix' => (isset($matches[2]) ? strtolower($matches[2]) : null),
-                        'controller' => (isset($matches[3]) ? $matches[3] : null),
-                        'action' => (isset($matches[4]) ? $matches[4] : null)
-                    ];
-                case 4:
-                    return [
-                        'plugin' => (isset($matches[1]) ? $matches[1] : null),
-                        'prefix' => (isset($matches[2]) ? strtolower($matches[2]) : null),
-                        'controller' => (isset($matches[3]) ? $matches[3] : null),
-                        'action' => (isset($matches[4]) ? $matches[4] : null)
-                    ];
+            foreach ($matches as $key => $value) {
+                // Remove numeric keys and empty values.
+                if (is_int($key) || $value === '' || $value === '::') {
+                    unset($matches[$key]);
+                }
+            }
+            $length = count($matches);
+
+            if (isset($matches['prefix'])) {
+                $matches['prefix'] = strtolower($matches['prefix']);
+            }
+
+            if ($length >= 2 || $length <= 4) {
+                return $matches;
             }
         }
         throw new RuntimeException("Could not parse `{$defaults}` route destination string.");

+ 47 - 0
tests/TestCase/Routing/RouteBuilderTest.php

@@ -225,6 +225,53 @@ class RouteBuilderTest extends TestCase
      *
      * @return void
      */
+    public function testConnectShortStringPrefix()
+    {
+        $routes = new RouteBuilder($this->collection, '/');
+        $routes->connect('/admin/bookmarks', 'Admin/Bookmarks::index');
+        $expected = [
+            'pass' => [],
+            'plugin' => null,
+            'prefix' => 'admin',
+            'controller' => 'Bookmarks',
+            'action' => 'index',
+            '_matchedRoute' => '/admin/bookmarks'
+        ];
+        $this->assertEquals($expected, $this->collection->parse('/admin/bookmarks'));
+
+        $url = $expected['_matchedRoute'];
+        unset($expected['_matchedRoute']);
+        $this->assertEquals($url, '/' . $this->collection->match($expected, []));
+    }
+
+    /**
+     * Test connect() with short string syntax
+     *
+     * @return void
+     */
+    public function testConnectShortStringPlugin()
+    {
+        $routes = new RouteBuilder($this->collection, '/');
+        $routes->connect('/blog/articles/view', 'Blog.Articles::view');
+        $expected = [
+            'pass' => [],
+            'plugin' => 'Blog',
+            'controller' => 'Articles',
+            'action' => 'view',
+            '_matchedRoute' => '/blog/articles/view'
+        ];
+        $this->assertEquals($expected, $this->collection->parse('/blog/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, '/');