From cb4e856e6ede67b839036154bbbdc99285f5157c Mon Sep 17 00:00:00 2001 From: Giuseppe Mazzapica Date: Fri, 19 Feb 2016 15:57:30 +0100 Subject: [PATCH] Better handling of exact route match no need to use regex matching if route path and url path are identical --- src/Cortex/Router/Router.php | 86 +++++++++++++------- tests/src/Functional/CortexTest.php | 1 + tests/src/Unit/Router/RouterTest.php | 116 ++++++++++++++++++++------- 3 files changed, 146 insertions(+), 57 deletions(-) diff --git a/src/Cortex/Router/Router.php b/src/Cortex/Router/Router.php index ccae18d..161c6b1 100644 --- a/src/Cortex/Router/Router.php +++ b/src/Cortex/Router/Router.php @@ -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); @@ -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); @@ -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); } @@ -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'])) { diff --git a/tests/src/Functional/CortexTest.php b/tests/src/Functional/CortexTest.php index 2a57cd4..412a0d3 100644 --- a/tests/src/Functional/CortexTest.php +++ b/tests/src/Functional/CortexTest.php @@ -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; }); diff --git a/tests/src/Unit/Router/RouterTest.php b/tests/src/Unit/Router/RouterTest.php index 2b4bb69..497d8e4 100644 --- a/tests/src/Unit/Router/RouterTest.php +++ b/tests/src/Unit/Router/RouterTest.php @@ -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'); @@ -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(); @@ -196,7 +198,7 @@ public function testMatchNotMatching() assertSame($expected, $data); } - public function testMatchMatching() + public function testMatchMatchingExactMatch() { $handler = function () { return func_get_args(); @@ -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']); @@ -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, @@ -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(); @@ -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']); @@ -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, @@ -324,7 +386,7 @@ public function testMatchMatchingNoQueryVars() assertSame($expected, $data); } - public function testMatchMatchingCallableVars() + public function testMatchMatchingExactMatchCallableVars() { $handler = function () { return func_get_args(); @@ -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(); @@ -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']); @@ -373,7 +433,7 @@ public function testMatchMatchingCallableVars() $expected = [ 'route' => 'r1', 'path' => '/foo', - 'vars' => ['a', 'b', 'c'], + 'vars' => ['c'], 'handler' => $handler, 'before' => null, 'after' => null,