Fix issue #50

If the static route doesn't exist for the given method, try matching against dynamic ones as well. Allowed methods are now collected from both static and dynamic routes. This changes the route data format, so cache is no longer valid -- maybe include a version identifier in the cache file?
nikic · May 15, 2015 · ffb3b68 · ffb3b68
1 parent cec21d8
commit ffb3b68
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 22 deletions.
diff --git a/src/DataGenerator/RegexBasedAbstract.php b/src/DataGenerator/RegexBasedAbstract.php
@@ -51,7 +51,7 @@ private function isStaticRoute($routeData) {
     private function addStaticRoute($httpMethod, $routeData, $handler) {
         $routeStr = $routeData[0];
 
-        if (isset($this->staticRoutes[$routeStr][$httpMethod])) {
+        if (isset($this->staticRoutes[$httpMethod][$routeStr])) {
             throw new BadRouteException(sprintf(
                 'Cannot register two routes matching "%s" for method "%s"',
                 $routeStr, $httpMethod
@@ -69,7 +69,7 @@ private function addStaticRoute($httpMethod, $routeData, $handler) {
             }
         }
 
-        $this->staticRoutes[$routeStr][$httpMethod] = $handler;
+        $this->staticRoutes[$httpMethod][$routeStr] = $handler;
     }
 
     private function addVariableRoute($httpMethod, $routeData, $handler) {

diff --git a/src/Dispatcher/RegexBasedAbstract.php b/src/Dispatcher/RegexBasedAbstract.php
@@ -11,8 +11,12 @@ abstract class RegexBasedAbstract implements Dispatcher {
     protected abstract function dispatchVariableRoute($routeData, $uri);
 
     public function dispatch($httpMethod, $uri) {
-        if (isset($this->staticRouteMap[$uri])) {
-            return $this->dispatchStaticRoute($httpMethod, $uri);
+        if (isset($this->staticRouteMap[$httpMethod][$uri])) {
+            $handler = $this->staticRouteMap[$httpMethod][$uri];
+            return [self::FOUND, $handler, []];
+        } else if ($httpMethod === 'HEAD' && isset($this->staticRouteMap['GET'][$uri])) {
+            $handler = $this->staticRouteMap['GET'][$uri];
+            return [self::FOUND, $handler, []];
         }
 
         $varRouteData = $this->variableRouteData;
@@ -28,9 +32,15 @@ public function dispatch($httpMethod, $uri) {
             }
         }
 
-        // Find allowed methods for this URI by matching against all other
-        // HTTP methods as well
+        // Find allowed methods for this URI by matching against all other HTTP methods as well
         $allowedMethods = [];
+
+        foreach ($this->staticRouteMap as $method => $uriMap) {
+            if ($method !== $httpMethod && isset($uriMap[$uri])) {
+                $allowedMethods[] = $method;
+            }
+        }
+
         foreach ($varRouteData as $method => $routeData) {
             if ($method === $httpMethod) {
                 continue;
@@ -49,16 +59,4 @@ public function dispatch($httpMethod, $uri) {
             return [self::NOT_FOUND];
         }
     }
-
-    protected function dispatchStaticRoute($httpMethod, $uri) {
-        $routes = $this->staticRouteMap[$uri];
-
-        if (isset($routes[$httpMethod])) {
-            return [self::FOUND, $routes[$httpMethod], []];
-        } elseif ($httpMethod === 'HEAD' && isset($routes['GET'])) {
-            return [self::FOUND, $routes['GET'], []];
-        } else {
-            return [self::METHOD_NOT_ALLOWED, array_keys($routes)];
-        }
-    }
 }
diff --git a/test/Dispatcher/DispatcherTest.php b/test/Dispatcher/DispatcherTest.php
@@ -31,10 +31,10 @@ private function generateDispatcherOptions() {
      */
     public function testFoundDispatches($method, $uri, $callback, $handler, $argDict) {
         $dispatcher = \FastRoute\simpleDispatcher($callback, $this->generateDispatcherOptions());
-        list($routedStatus, $routedTo, $routedArgs) = $dispatcher->dispatch($method, $uri);
-        $this->assertSame($dispatcher::FOUND, $routedStatus);
-        $this->assertSame($handler, $routedTo);
-        $this->assertSame($argDict, $routedArgs);
+        $info = $dispatcher->dispatch($method, $uri);
+        $this->assertSame($dispatcher::FOUND, $info[0]);
+        $this->assertSame($handler, $info[1]);
+        $this->assertSame($argDict, $info[2]);
     }
 
     /**
@@ -318,6 +318,15 @@ public function provideFoundDispatchCases() {
         $cases[] = ['POST', '/user', $callback, 'handlerGetPost', $argDict];
         $cases[] = ['DELETE', '/user', $callback, 'handlerDelete', $argDict];
 
+        // 15 ----
+
+        $callback = function(RouteCollector $r) {
+            $r->addRoute('POST', '/user.json', 'handler0');
+            $r->addRoute('GET', '/{entity}.json', 'handler1');
+        };
+
+        $cases[] = ['GET', '/user.json', $callback, 'handler1', ['entity' => 'user']];
+
 
         // x -------------------------------------------------------------------------------------->
 
@@ -468,6 +477,15 @@ public function provideMethodNotAllowedDispatchCases() {
 
         $cases[] = ['PUT', '/user', $callback, ['GET', 'POST', 'DELETE']];
 
+        // 5
+
+        $callback = function(RouteCollector $r) {
+            $r->addRoute('POST', '/user.json', 'handler0');
+            $r->addRoute('GET', '/{entity}.json', 'handler1');
+        };
+
+        $cases[] = ['PUT', '/user.json', $callback, ['POST', 'GET']];
+
         // x -------------------------------------------------------------------------------------->
 
         return $cases;