Better handling of exact route match

gmazzap · gmazzap · commit cb4e856e6ede · 2016-02-19T15:57:30.000+01:00
no need to use regex matching if route path and url path are identical
diff --git 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
@@ -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
@@ -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,
