Skip to content

Commit

Permalink
Better handling of exact route match
Browse files Browse the repository at this point in the history 
no need to use regex matching if route path and url path are identical
  • Loading branch information
@gmazzap
gmazzap committed Feb 19, 2016
1 parent cf5da01 commit cb4e856
Show file tree
Hide file tree
Showing 3 changed files with 146 additions and 57 deletions.
86 changes: 57 additions & 29 deletions src/Cortex/Router/Router.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,17 @@ public function match(UriInterface $uri, $httpMethod)
return $this->results;
}

if (! count($this->routes) || ! $this->parseRoutes($uri, $httpMethod)) {
if (! count($this->routes) || !$this->parseRoutes($uri, $httpMethod)) {
$this->results = new MatchingResult(['route' => null]);

return $this->results;
}

// in case of exact match, no need to go further
if ($this->results instanceof MatchingResult) {
return $this->results;
}

$dispatcher = $this->buildDispatcher($this->collector->getData());
unset($this->collector);

Expand Down Expand Up @@ -121,21 +126,25 @@ private function parseRoutes(UriInterface $uri, $httpMethod)
{
$iterator = new RouteFilterIterator($this->routes, $uri);
$parsed = 0;
$iterator->rewind();
while ($iterator->valid()) {
/** @var \Brain\Cortex\Route\RouteInterface $route */
$route = $this->groups->mergeGroup($iterator->current());
if (empty($route['method']) || strtolower($route['method']) === 'any') {
$route['method'] = $httpMethod;
/** @var \Brain\Cortex\Route\RouteInterface $route */
foreach ($iterator as $route) {
$route = $this->sanitizeRouteMethod($this->groups->mergeGroup($route), $httpMethod);
if (!$this->validateRoute($route, $uri, $httpMethod)) {
continue;
}
if ($route instanceof RouteInterface && $this->validate($route)) {
$id = $route->id();
$this->parsedRoutes[$id] = $route;
$path = '/'.trim($route['path'], '/');
$this->collector->addRoute(strtoupper($route['method']), $path, $id);
$parsed++;

$parsed++;
$id = $route->id();
$this->parsedRoutes[$id] = $route;
$path = '/'.trim($route['path'], '/');
// exact match
if ($path === '/'.trim($uri->path(), '/')) {
$this->results = $this->finalizeRoute($route, [], $uri);
unset($this->parsedRoutes, $this->collector);
break;
}
$iterator->next();

$this->collector->addRoute(strtoupper($route['method']), $path, $id);
}

unset($this->routes, $this->groups);
Expand All @@ -145,30 +154,49 @@ private function parseRoutes(UriInterface $uri, $httpMethod)

/**
* @param \Brain\Cortex\Route\RouteInterface $route
* @param string $httpMethod
* @return \Brain\Cortex\Route\RouteInterface
*/
private function sanitizeRouteMethod(RouteInterface $route, $httpMethod)
{
if (empty($route['method']) || !(is_string($route['method']) || is_array($route['method']))) {
$route['method'] = $httpMethod;
}

if (is_array($route['method'])) {
$route['method'] = array_map('strtoupper', array_filter($route['method'], 'is_string'));

return $route;
}

(strtolower($route['method']) === 'any') and $route['method'] = $httpMethod;

$route['method'] = strtoupper($route['method']);

return $route;
}

/**
* @param \Brain\Cortex\Route\RouteInterface $route
* @param \Brain\Cortex\Uri\UriInterface $uri
* @param $httpMethod
* @return bool
*/
private function validate(RouteInterface $route)
private function validateRoute(RouteInterface $route, UriInterface $uri, $httpMethod)
{
$id = $route->id();
$path = $route['path'];
$method = $route['method'];
$path = trim($route['path'], '/');
if (count($uri->chunks()) !== (substr_count($path, '/') + 1)) {
return false;
}
$method = (array) $route['method'];
$handler = $route['handler'];
$methods = [
'GET',
'POST',
'PUT',
'OPTIONS',
'HEAD',
'DELETE',
'TRACE',
'CONNECT',
];

return
is_string($id)
&& $id
&& filter_var($path, FILTER_SANITIZE_URL) === $path
&& in_array(strtoupper((string) $method), $methods, true)
&& in_array($httpMethod, $method, true)
&& (is_callable($handler) || $handler instanceof ControllerInterface);
}

Expand Down Expand Up @@ -203,7 +231,7 @@ private function finalizeRoute(RouteInterface $route, array $vars, UriInterface
$result = null;
if (is_callable($route['vars'])) {
$cb = $route['vars'];
$routeVars = $cb($vars);
$routeVars = $cb($vars, $uri);
is_array($routeVars) and $vars = $routeVars;
$routeVars instanceof MatchingResult and $result = $routeVars;
} elseif (is_array($route['vars'])) {
Expand Down
1 change: 1 addition & 0 deletions tests/src/Functional/CortexTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ public function testCortexNotMatchBecauseUrlChangedViaFilter()
$uri->shouldReceive('host')->andReturn('example.com');
$uri->shouldReceive('vars')->andReturn([]);
$uri->shouldReceive('path')->andReturn('meh/meh/meh'); // this does not match
$uri->shouldReceive('chunks')->andReturn(['meh', 'meh', 'meh']);

return $uri;
});
Expand Down
116 changes: 88 additions & 28 deletions tests/src/Unit/Router/RouterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ public function testMatchNothingIfNoValidatingRoutes()
$uri = \Mockery::mock(UriInterface::class);
$uri->shouldReceive('scheme')->andReturn('http');
$uri->shouldReceive('host')->andReturn('example.com');
$uri->shouldReceive('chunks')->andReturn([]);

$router = new Router($routes, $groups);
$result = $router->match($uri, 'GET');
Expand Down Expand Up @@ -161,6 +162,7 @@ public function testMatchNotMatching()
$uri->shouldReceive('scheme')->andReturn('http');
$uri->shouldReceive('host')->andReturn('example.com');
$uri->shouldReceive('path')->andReturn('bar');
$uri->shouldReceive('chunks')->andReturn(['bar']);

$collector = \Mockery::mock(RouteCollector::class);
$collector->shouldReceive('addRoute')->once()->with('POST', '/foo', 'r1')->andReturnNull();
Expand Down Expand Up @@ -196,7 +198,7 @@ public function testMatchNotMatching()
assertSame($expected, $data);
}

public function testMatchMatching()
public function testMatchMatchingExactMatch()
{
$handler = function () {
return func_get_args();
Expand All @@ -221,17 +223,13 @@ public function testMatchMatching()
$uri->shouldReceive('host')->andReturn('example.com');
$uri->shouldReceive('path')->andReturn('foo');
$uri->shouldReceive('vars')->once()->andReturn(['c' => 'C']);
$uri->shouldReceive('chunks')->andReturn(['foo']);

$collector = \Mockery::mock(RouteCollector::class);
$collector->shouldReceive('addRoute')->once()->with('POST', '/foo', 'r1')->andReturnNull();
$collector->shouldReceive('getData')->once()->andReturn(['foo' => 'bar']);
$collector->shouldReceive('addRoute')->never();

$dispatcher = \Mockery::mock(Dispatcher::class);
$dispatcher->shouldReceive('dispatch')->with('POST', '/foo')->andReturn([
Dispatcher::FOUND,
'r1',
['a' => 'A', 'b' => 'B'],
]);
$dispatcher->shouldReceive('dispatch')->never();

$factory = function (array $args) use ($dispatcher) {
assertSame($args, ['foo' => 'bar']);
Expand All @@ -245,7 +243,7 @@ public function testMatchMatching()
$expected = [
'route' => 'r1',
'path' => '/foo',
'vars' => ['a' => 'A', 'b' => 'B', 'c' => 'C', 'd' => 'D'],
'vars' => ['c' => 'C', 'd' => 'D'],
'handler' => $handler,
'before' => null,
'after' => null,
Expand All @@ -260,7 +258,75 @@ public function testMatchMatching()
assertSame($expected, $data);
}

public function testMatchMatchingNoQueryVars()
public function testMatchDynamicMatch()
{
$handler = function () {
return func_get_args();
};

$route = new Route([
'id' => 'r1',
'path' => '/foo/{bar}',
'handler' => $handler,
'vars' => ['d' => 'D'],
'method' => 'POST',
]);
$routes = new PriorityRouteCollection();

$routes->addRoute($route);

$groups = \Mockery::mock(GroupCollectionInterface::class);
$groups->shouldReceive('mergeGroup')->once()->with($route)->andReturn($route);

$uri = \Mockery::mock(UriInterface::class);
$uri->shouldReceive('scheme')->andReturn('http');
$uri->shouldReceive('host')->andReturn('example.com');
$uri->shouldReceive('path')->andReturn('foo/i-am-bar');
$uri->shouldReceive('vars')->once()->andReturn(['c' => 'C']);
$uri->shouldReceive('chunks')->andReturn(['foo', 'meh']);

$collector = \Mockery::mock(RouteCollector::class);
$collector->shouldReceive('addRoute')->once()->with('POST', '/foo/{bar}', 'r1');
$collector->shouldReceive('getData')->once()->andReturn(['foo' => 'bar']);

$dispatcher = \Mockery::mock(Dispatcher::class);
$dispatcher->shouldReceive('dispatch')
->once()
->with('POST', '/foo/i-am-bar')
->andReturn([
Dispatcher::FOUND,
'r1',
['bar' => 'i-am-bar'],
]);

$factory = function (array $args) use ($dispatcher) {
assertSame($args, ['foo' => 'bar']);

return $dispatcher;
};

$router = new Router($routes, $groups, $collector, $factory);
$result = $router->match($uri, 'POST');

$expected = [
'route' => 'r1',
'path' => '/foo/{bar}',
'vars' => ['bar' => 'i-am-bar', 'c' => 'C', 'd' => 'D'],
'handler' => $handler,
'before' => null,
'after' => null,
'template' => null,
];

$proxy = new Proxy($result);
/** @noinspection PhpUndefinedFieldInspection */
$data = $proxy->data;

assertTrue($result->matched());
assertSame($expected, $data);
}

public function testMatchMatchingExactMatchNoQueryVars()
{
$handler = function () {
return func_get_args();
Expand All @@ -285,17 +351,13 @@ public function testMatchMatchingNoQueryVars()
$uri->shouldReceive('scheme')->andReturn('http');
$uri->shouldReceive('host')->andReturn('example.com');
$uri->shouldReceive('path')->andReturn('foo');
$uri->shouldReceive('chunks')->andReturn(['foo']);

$collector = \Mockery::mock(RouteCollector::class);
$collector->shouldReceive('addRoute')->once()->with('POST', '/foo', 'r1')->andReturnNull();
$collector->shouldReceive('getData')->once()->andReturn(['foo' => 'bar']);
$collector->shouldReceive('addRoute')->never();

$dispatcher = \Mockery::mock(Dispatcher::class);
$dispatcher->shouldReceive('dispatch')->with('POST', '/foo')->andReturn([
Dispatcher::FOUND,
'r1',
['a' => 'A', 'b' => 'B'],
]);
$dispatcher->shouldReceive('dispatch')->never();

$factory = function (array $args) use ($dispatcher) {
assertSame($args, ['foo' => 'bar']);
Expand All @@ -309,7 +371,7 @@ public function testMatchMatchingNoQueryVars()
$expected = [
'route' => 'r1',
'path' => '/foo',
'vars' => ['a' => 'A', 'b' => 'B', 'd' => 'D'],
'vars' => ['d' => 'D'],
'handler' => $handler,
'before' => null,
'after' => null,
Expand All @@ -324,7 +386,7 @@ public function testMatchMatchingNoQueryVars()
assertSame($expected, $data);
}

public function testMatchMatchingCallableVars()
public function testMatchMatchingExactMatchCallableVars()
{
$handler = function () {
return func_get_args();
Expand All @@ -334,7 +396,9 @@ public function testMatchMatchingCallableVars()
'id' => 'r1',
'path' => '/foo',
'handler' => $handler,
'vars' => 'array_keys',
'vars' => function (array $vars) {
return array_keys($vars);
},
'method' => 'POST',
]);
$routes = new PriorityRouteCollection();
Expand All @@ -349,17 +413,13 @@ public function testMatchMatchingCallableVars()
$uri->shouldReceive('host')->andReturn('example.com');
$uri->shouldReceive('path')->andReturn('foo');
$uri->shouldReceive('vars')->once()->andReturn(['c' => 'C']);
$uri->shouldReceive('chunks')->andReturn(['foo']);

$collector = \Mockery::mock(RouteCollector::class);
$collector->shouldReceive('addRoute')->once()->with('POST', '/foo', 'r1')->andReturnNull();
$collector->shouldReceive('getData')->once()->andReturn(['foo' => 'bar']);
$collector->shouldReceive('addRoute')->never();

$dispatcher = \Mockery::mock(Dispatcher::class);
$dispatcher->shouldReceive('dispatch')->with('POST', '/foo')->andReturn([
Dispatcher::FOUND,
'r1',
['a' => 'A', 'b' => 'B'],
]);
$dispatcher->shouldReceive('dispatch')->never();

$factory = function (array $args) use ($dispatcher) {
assertSame($args, ['foo' => 'bar']);
Expand All @@ -373,7 +433,7 @@ public function testMatchMatchingCallableVars()
$expected = [
'route' => 'r1',
'path' => '/foo',
'vars' => ['a', 'b', 'c'],
'vars' => ['c'],
'handler' => $handler,
'before' => null,
'after' => null,
Expand Down

0 comments on commit cb4e856

Please sign in to comment.