Skip to content

Commit

Permalink
Simplify route target parsing.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
markstory committed May 7, 2018
1 parent 3cf4efb commit 6429e12
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 25 deletions.
42 changes: 17 additions & 25 deletions src/Routing/RouteBuilder.php
Expand Up @@ -743,32 +743,24 @@ protected static function parseDefaults($defaults)
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.");
Expand Down
47 changes: 47 additions & 0 deletions tests/TestCase/Routing/RouteBuilderTest.php
Expand Up @@ -220,6 +220,53 @@ public function testConnectShortString()
$this->assertEquals($url, '/' . $this->collection->match($expected, []));
}

/**
* Test connect() with short string syntax
*
* @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
*
Expand Down

0 comments on commit 6429e12

Please sign in to comment.