From 40c897afe69ed59582b30cb05f437e544ce46bdf Mon Sep 17 00:00:00 2001 From: Andreas Lutro Date: Sat, 12 Apr 2014 20:22:43 +0200 Subject: [PATCH 01/14] Delegate autoloading to composer However, use of composer is not required. You can still use src/bootstrap.php as the autoloader. --- composer.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 6c378f1..62aad22 100644 --- a/composer.json +++ b/composer.json @@ -13,6 +13,9 @@ "php": ">=5.4.0" }, "autoload": { - "files": ["src/bootstrap.php"] + "psr-4": { + "FastRoute\\": "src/" + }, + "files": ["src/functions.php"] } } From 1c549b589d763656977f46c7b96e79f667009a1f Mon Sep 17 00:00:00 2001 From: Danack Date: Sun, 27 Apr 2014 20:03:48 +0100 Subject: [PATCH 02/14] Update README.md You probably didn't mean method twice. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b836e8f..9f571f8 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ appropriately) is your job - this library is not bound to the PHP web SAPIs. The `dispatch()` method returns an array those first element contains a status code. It is one of `Dispatcher::NOT_FOUND`, `Dispatcher::METHOD_NOT_ALLOWED` and `Dispatcher::FOUND`. For the method not allowed status the second array element contains a list of HTTP methods allowed for -this method. For example: +the supplied URI. For example: [FastRoute\Dispatcher::METHOD_NOT_ALLOWED, ['GET', 'POST']] From 4206d03260569e351ddb5eaa55c910bee5cde03a Mon Sep 17 00:00:00 2001 From: Rowan Lewis Date: Thu, 8 May 2014 18:08:34 +0300 Subject: [PATCH 03/14] A new approach involving lots of tab characters. --- src/DataGenerator/CharCountBased.php | 50 +++++++++++++++++++++ src/Dispatcher/CharCountBased.php | 65 ++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+) create mode 100644 src/DataGenerator/CharCountBased.php create mode 100644 src/Dispatcher/CharCountBased.php diff --git a/src/DataGenerator/CharCountBased.php b/src/DataGenerator/CharCountBased.php new file mode 100644 index 0000000..2e36ba7 --- /dev/null +++ b/src/DataGenerator/CharCountBased.php @@ -0,0 +1,50 @@ +regexToRoutesMap)) { + return [$this->staticRoutes, []]; + } + + return [$this->staticRoutes, $this->generateVariableRouteData()]; + } + + private function generateVariableRouteData() { + $chunkSize = $this->computeChunkSize(count($this->regexToRoutesMap)); + $chunks = array_chunk($this->regexToRoutesMap, $chunkSize, true); + return array_map([$this, 'processChunk'], $chunks); + } + + private function computeChunkSize($count) { + $numParts = max(1, round($count / self::APPROX_CHUNK_SIZE)); + return ceil($count / $numParts); + } + + private function processChunk($regexToRoutesMap) { + $routeMap = []; + $regexes = []; + $index = 0; + $tabs = ''; + + foreach ($regexToRoutesMap as $regex => $routes) { + $index++; + $tabs .= "\t"; + + foreach ($routes as $route) { + $routeMap[$tabs][$route->httpMethod] = [ + $route->handler, $route->variables + ]; + } + + $regexes[] = '(?:(' . $regex . ')(?[\t]{' . $index . '}))'; + } + + $regex = '~^(?|' . implode('|', $regexes) . ')~'; + + return ['regex' => $regex, 'tabs' => $tabs, 'routeMap' => $routeMap]; + } +} \ No newline at end of file diff --git a/src/Dispatcher/CharCountBased.php b/src/Dispatcher/CharCountBased.php new file mode 100644 index 0000000..b1489c3 --- /dev/null +++ b/src/Dispatcher/CharCountBased.php @@ -0,0 +1,65 @@ +staticRouteMap, $this->variableRouteData) = $data; + } + + public function dispatch($httpMethod, $uri) { + if (isset($this->staticRouteMap[$uri])) { + return $this->dispatchStaticRoute($httpMethod, $uri); + } else { + return $this->dispatchVariableRoute($httpMethod, $uri); + } + } + + private 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)]; + } + } + + private function dispatchVariableRoute($httpMethod, $uri) { + foreach ($this->variableRouteData as $data) { + if (!preg_match($data['regex'], $uri . $data['tabs'], $matches)) { + continue; + } + + $routes = $data['routeMap'][$matches['id']]; + + if (false === isset($routes[$httpMethod])) { + if ($httpMethod === 'HEAD' && isset($routes['GET'])) { + $httpMethod = 'GET'; + } + + else { + return [self::METHOD_NOT_ALLOWED, array_keys($routes)]; + } + } + + list($handler, $varNames) = $routes[$httpMethod]; + $vars = []; $i = 2; + + foreach ($varNames as $varName) { + $vars[$varName] = $matches[$i++]; + } + + return [self::FOUND, $handler, $vars]; + } + + return [self::NOT_FOUND]; + } +} From 299ed796eddd4b70ebf31c80691eceefb8188bf5 Mon Sep 17 00:00:00 2001 From: nikic Date: Sat, 17 May 2014 12:18:01 +0200 Subject: [PATCH 04/14] Fix CharCountBased implementation --- src/DataGenerator/CharCountBased.php | 18 +++++++++--------- src/Dispatcher/CharCountBased.php | 13 ++++++------- test/Dispatcher/CharCountBasedTest.php | 13 +++++++++++++ 3 files changed, 28 insertions(+), 16 deletions(-) create mode 100644 test/Dispatcher/CharCountBasedTest.php diff --git a/src/DataGenerator/CharCountBased.php b/src/DataGenerator/CharCountBased.php index 2e36ba7..9a94108 100644 --- a/src/DataGenerator/CharCountBased.php +++ b/src/DataGenerator/CharCountBased.php @@ -27,24 +27,24 @@ private function computeChunkSize($count) { private function processChunk($regexToRoutesMap) { $routeMap = []; $regexes = []; - $index = 0; - $tabs = ''; + $suffixLen = 0; + $suffix = ''; + $count = count($regexToRoutesMap); foreach ($regexToRoutesMap as $regex => $routes) { - $index++; - $tabs .= "\t"; + $suffixLen++; + $suffix .= "\t"; foreach ($routes as $route) { - $routeMap[$tabs][$route->httpMethod] = [ + $routeMap[$suffix][$route->httpMethod] = [ $route->handler, $route->variables ]; } - $regexes[] = '(?:(' . $regex . ')(?[\t]{' . $index . '}))'; + $regexes[] = '(?:' . $regex . '(\t{' . $suffixLen . '})\t{' . ($count - $suffixLen) . '})'; } - $regex = '~^(?|' . implode('|', $regexes) . ')~'; - - return ['regex' => $regex, 'tabs' => $tabs, 'routeMap' => $routeMap]; + $regex = '~^(?|' . implode('|', $regexes) . ')$~'; + return ['regex' => $regex, 'suffix' => $suffix, 'routeMap' => $routeMap]; } } \ No newline at end of file diff --git a/src/Dispatcher/CharCountBased.php b/src/Dispatcher/CharCountBased.php index b1489c3..641d276 100644 --- a/src/Dispatcher/CharCountBased.php +++ b/src/Dispatcher/CharCountBased.php @@ -34,27 +34,26 @@ private function dispatchStaticRoute($httpMethod, $uri) { private function dispatchVariableRoute($httpMethod, $uri) { foreach ($this->variableRouteData as $data) { - if (!preg_match($data['regex'], $uri . $data['tabs'], $matches)) { + if (!preg_match($data['regex'], $uri . $data['suffix'], $matches)) { continue; } - $routes = $data['routeMap'][$matches['id']]; + $routes = $data['routeMap'][end($matches)]; if (false === isset($routes[$httpMethod])) { if ($httpMethod === 'HEAD' && isset($routes['GET'])) { $httpMethod = 'GET'; - } - - else { + } else { return [self::METHOD_NOT_ALLOWED, array_keys($routes)]; } } list($handler, $varNames) = $routes[$httpMethod]; - $vars = []; $i = 2; + $vars = []; + $i = 0; foreach ($varNames as $varName) { - $vars[$varName] = $matches[$i++]; + $vars[$varName] = $matches[++$i]; } return [self::FOUND, $handler, $vars]; diff --git a/test/Dispatcher/CharCountBasedTest.php b/test/Dispatcher/CharCountBasedTest.php new file mode 100644 index 0000000..8168498 --- /dev/null +++ b/test/Dispatcher/CharCountBasedTest.php @@ -0,0 +1,13 @@ + Date: Sat, 17 May 2014 12:41:13 +0200 Subject: [PATCH 05/14] Improve CharCountBased performance The extra / avoids ([^/]+) placeholders trying to eat up the tab characters, thus avoiding unnecessary backtracking. --- src/DataGenerator/CharCountBased.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/DataGenerator/CharCountBased.php b/src/DataGenerator/CharCountBased.php index 9a94108..c9e3e8e 100644 --- a/src/DataGenerator/CharCountBased.php +++ b/src/DataGenerator/CharCountBased.php @@ -41,10 +41,10 @@ private function processChunk($regexToRoutesMap) { ]; } - $regexes[] = '(?:' . $regex . '(\t{' . $suffixLen . '})\t{' . ($count - $suffixLen) . '})'; + $regexes[] = '(?:' . $regex . '/(\t{' . $suffixLen . '})\t{' . ($count - $suffixLen) . '})'; } $regex = '~^(?|' . implode('|', $regexes) . ')$~'; - return ['regex' => $regex, 'suffix' => $suffix, 'routeMap' => $routeMap]; + return ['regex' => $regex, 'suffix' => '/' . $suffix, 'routeMap' => $routeMap]; } } \ No newline at end of file From 8d0443b49c8347b64afb7afa91db29a6fb18f5f2 Mon Sep 17 00:00:00 2001 From: nikic Date: Sat, 17 May 2014 13:25:10 +0200 Subject: [PATCH 06/14] Add MARK based dispatcher (PHP 5.6) $ /c/php-5.6.0beta3-nts/php bench.php 1 placeholders | GPB-10C | GCB-10C | CCB-30C | MB-30C ------------------------------------------------------ first route | 0.088 s | 0.094 s | 0.132 s | 0.111 s last route | 0.494 s | 0.534 s | 0.340 s | 0.272 s unknown route | 0.422 s | 0.457 s | 0.272 s | 0.230 s 9 placeholders | GPB-10C | GCB-10C | CCB-30C | MB-30C ------------------------------------------------------ first route | 0.193 s | 0.185 s | 0.269 s | 0.258 s last route | 1.045 s | 0.828 s | 0.616 s | 0.554 s unknown route | 0.664 s | 0.664 s | 0.496 s | 0.444 s --- src/DataGenerator/MarkBased.php | 46 +++++++++++++++++++++++ src/Dispatcher/MarkBased.php | 62 +++++++++++++++++++++++++++++++ test/Dispatcher/MarkBasedTest.php | 19 ++++++++++ 3 files changed, 127 insertions(+) create mode 100644 src/DataGenerator/MarkBased.php create mode 100644 src/Dispatcher/MarkBased.php create mode 100644 test/Dispatcher/MarkBasedTest.php diff --git a/src/DataGenerator/MarkBased.php b/src/DataGenerator/MarkBased.php new file mode 100644 index 0000000..2043a8a --- /dev/null +++ b/src/DataGenerator/MarkBased.php @@ -0,0 +1,46 @@ +regexToRoutesMap)) { + return [$this->staticRoutes, []]; + } + + return [$this->staticRoutes, $this->generateVariableRouteData()]; + } + + private function generateVariableRouteData() { + $chunkSize = $this->computeChunkSize(count($this->regexToRoutesMap)); + $chunks = array_chunk($this->regexToRoutesMap, $chunkSize, true); + return array_map([$this, 'processChunk'], $chunks); + } + + private function computeChunkSize($count) { + $numParts = max(1, round($count / self::APPROX_CHUNK_SIZE)); + return ceil($count / $numParts); + } + + private function processChunk($regexToRoutesMap) { + $routeMap = []; + $regexes = []; + $markName = 'a'; + foreach ($regexToRoutesMap as $regex => $routes) { + $regexes[] = $regex . '(*MARK:' . $markName . ')'; + + foreach ($routes as $route) { + $routeMap[$markName][$route->httpMethod] + = [$route->handler, $route->variables]; + } + + ++$markName; + } + + $regex = '~^(?|' . implode('|', $regexes) . ')$~'; + return ['regex' => $regex, 'routeMap' => $routeMap]; + } +} + diff --git a/src/Dispatcher/MarkBased.php b/src/Dispatcher/MarkBased.php new file mode 100644 index 0000000..e533e40 --- /dev/null +++ b/src/Dispatcher/MarkBased.php @@ -0,0 +1,62 @@ +staticRouteMap, $this->variableRouteData) = $data; + } + + public function dispatch($httpMethod, $uri) { + if (isset($this->staticRouteMap[$uri])) { + return $this->dispatchStaticRoute($httpMethod, $uri); + } else { + return $this->dispatchVariableRoute($httpMethod, $uri); + } + } + + private 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)]; + } + } + + private function dispatchVariableRoute($httpMethod, $uri) { + foreach ($this->variableRouteData as $data) { + if (!preg_match($data['regex'], $uri, $matches)) { + continue; + } + + $routes = $data['routeMap'][$matches['MARK']]; + if (!isset($routes[$httpMethod])) { + if ($httpMethod === 'HEAD' && isset($routes['GET'])) { + $httpMethod = 'GET'; + } else { + return [self::METHOD_NOT_ALLOWED, array_keys($routes)]; + } + } + + list($handler, $varNames) = $routes[$httpMethod]; + + $vars = []; + $i = 0; + foreach ($varNames as $varName) { + $vars[$varName] = $matches[++$i]; + } + return [self::FOUND, $handler, $vars]; + } + + return [self::NOT_FOUND]; + } +} diff --git a/test/Dispatcher/MarkBasedTest.php b/test/Dispatcher/MarkBasedTest.php new file mode 100644 index 0000000..d7f4157 --- /dev/null +++ b/test/Dispatcher/MarkBasedTest.php @@ -0,0 +1,19 @@ +markTestSkipped('PHP 5.6 required for MARK support'); + } + } + + protected function getDispatcherClass() { + return 'FastRoute\\Dispatcher\\MarkBased'; + } + + protected function getDataGeneratorClass() { + return 'FastRoute\\DataGenerator\\MarkBased'; + } +} From 7dc9a43cb856debc22fe158a1144018e16f7a637 Mon Sep 17 00:00:00 2001 From: nikic Date: Sat, 17 May 2014 13:33:04 +0200 Subject: [PATCH 07/14] Avoid duplication in data generators --- src/DataGenerator/CharCountBased.php | 23 +++-------------------- src/DataGenerator/GroupCountBased.php | 23 +++-------------------- src/DataGenerator/GroupPosBased.php | 23 +++-------------------- src/DataGenerator/MarkBased.php | 23 +++-------------------- src/DataGenerator/RegexBasedAbstract.php | 22 ++++++++++++++++++++++ 5 files changed, 34 insertions(+), 80 deletions(-) diff --git a/src/DataGenerator/CharCountBased.php b/src/DataGenerator/CharCountBased.php index c9e3e8e..b733380 100644 --- a/src/DataGenerator/CharCountBased.php +++ b/src/DataGenerator/CharCountBased.php @@ -3,28 +3,11 @@ namespace FastRoute\DataGenerator; class CharCountBased extends RegexBasedAbstract { - const APPROX_CHUNK_SIZE = 30; - - public function getData() { - if (empty($this->regexToRoutesMap)) { - return [$this->staticRoutes, []]; - } - - return [$this->staticRoutes, $this->generateVariableRouteData()]; - } - - private function generateVariableRouteData() { - $chunkSize = $this->computeChunkSize(count($this->regexToRoutesMap)); - $chunks = array_chunk($this->regexToRoutesMap, $chunkSize, true); - return array_map([$this, 'processChunk'], $chunks); - } - - private function computeChunkSize($count) { - $numParts = max(1, round($count / self::APPROX_CHUNK_SIZE)); - return ceil($count / $numParts); + protected function getApproxChunkSize() { + return 30; } - private function processChunk($regexToRoutesMap) { + protected function processChunk($regexToRoutesMap) { $routeMap = []; $regexes = []; diff --git a/src/DataGenerator/GroupCountBased.php b/src/DataGenerator/GroupCountBased.php index 486a97e..16ed4c5 100644 --- a/src/DataGenerator/GroupCountBased.php +++ b/src/DataGenerator/GroupCountBased.php @@ -3,28 +3,11 @@ namespace FastRoute\DataGenerator; class GroupCountBased extends RegexBasedAbstract { - const APPROX_CHUNK_SIZE = 10; - - public function getData() { - if (empty($this->regexToRoutesMap)) { - return [$this->staticRoutes, []]; - } - - return [$this->staticRoutes, $this->generateVariableRouteData()]; - } - - private function generateVariableRouteData() { - $chunkSize = $this->computeChunkSize(count($this->regexToRoutesMap)); - $chunks = array_chunk($this->regexToRoutesMap, $chunkSize, true); - return array_map([$this, 'processChunk'], $chunks); - } - - private function computeChunkSize($count) { - $numParts = max(1, round($count / self::APPROX_CHUNK_SIZE)); - return ceil($count / $numParts); + protected function getApproxChunkSize() { + return 10; } - private function processChunk($regexToRoutesMap) { + protected function processChunk($regexToRoutesMap) { $routeMap = []; $regexes = []; $numGroups = 0; diff --git a/src/DataGenerator/GroupPosBased.php b/src/DataGenerator/GroupPosBased.php index 5ea5419..136cdf3 100644 --- a/src/DataGenerator/GroupPosBased.php +++ b/src/DataGenerator/GroupPosBased.php @@ -3,28 +3,11 @@ namespace FastRoute\DataGenerator; class GroupPosBased extends RegexBasedAbstract { - const APPROX_CHUNK_SIZE = 10; - - public function getData() { - if (empty($this->regexToRoutesMap)) { - return [$this->staticRoutes, []]; - } - - return [$this->staticRoutes, $this->generateVariableRouteData()]; - } - - private function generateVariableRouteData() { - $chunkSize = $this->computeChunkSize(count($this->regexToRoutesMap)); - $chunks = array_chunk($this->regexToRoutesMap, $chunkSize, true); - return array_map([$this, 'processChunk'], $chunks); - } - - private function computeChunkSize($count) { - $numParts = max(1, round($count / self::APPROX_CHUNK_SIZE)); - return ceil($count / $numParts); + protected function getApproxChunkSize() { + return 10; } - private function processChunk($regexToRoutesMap) { + protected function processChunk($regexToRoutesMap) { $routeMap = []; $regexes = []; $offset = 1; diff --git a/src/DataGenerator/MarkBased.php b/src/DataGenerator/MarkBased.php index 2043a8a..1c9db55 100644 --- a/src/DataGenerator/MarkBased.php +++ b/src/DataGenerator/MarkBased.php @@ -3,28 +3,11 @@ namespace FastRoute\DataGenerator; class MarkBased extends RegexBasedAbstract { - const APPROX_CHUNK_SIZE = 30; - - public function getData() { - if (empty($this->regexToRoutesMap)) { - return [$this->staticRoutes, []]; - } - - return [$this->staticRoutes, $this->generateVariableRouteData()]; - } - - private function generateVariableRouteData() { - $chunkSize = $this->computeChunkSize(count($this->regexToRoutesMap)); - $chunks = array_chunk($this->regexToRoutesMap, $chunkSize, true); - return array_map([$this, 'processChunk'], $chunks); - } - - private function computeChunkSize($count) { - $numParts = max(1, round($count / self::APPROX_CHUNK_SIZE)); - return ceil($count / $numParts); + protected function getApproxChunkSize() { + return 30; } - private function processChunk($regexToRoutesMap) { + protected function processChunk($regexToRoutesMap) { $routeMap = []; $regexes = []; $markName = 'a'; diff --git a/src/DataGenerator/RegexBasedAbstract.php b/src/DataGenerator/RegexBasedAbstract.php index 1318f45..0d431a8 100644 --- a/src/DataGenerator/RegexBasedAbstract.php +++ b/src/DataGenerator/RegexBasedAbstract.php @@ -10,6 +10,9 @@ abstract class RegexBasedAbstract implements DataGenerator { protected $staticRoutes = []; protected $regexToRoutesMap = []; + protected abstract function getApproxChunkSize(); + protected abstract function processChunk($regexToRoutesMap); + public function addRoute($httpMethod, $routeData, $handler) { if ($this->isStaticRoute($routeData)) { $this->addStaticRoute($httpMethod, $routeData, $handler); @@ -18,6 +21,25 @@ public function addRoute($httpMethod, $routeData, $handler) { } } + public function getData() { + if (empty($this->regexToRoutesMap)) { + return [$this->staticRoutes, []]; + } + + return [$this->staticRoutes, $this->generateVariableRouteData()]; + } + + private function generateVariableRouteData() { + $chunkSize = $this->computeChunkSize(count($this->regexToRoutesMap)); + $chunks = array_chunk($this->regexToRoutesMap, $chunkSize, true); + return array_map([$this, 'processChunk'], $chunks); + } + + private function computeChunkSize($count) { + $numParts = max(1, round($count / $this->getApproxChunkSize())); + return ceil($count / $numParts); + } + private function isStaticRoute($routeData) { return count($routeData) == 1 && is_string($routeData[0]); } From b3960cf3e83b017bea0a9fdda63de3f204b5a6f2 Mon Sep 17 00:00:00 2001 From: nikic Date: Tue, 30 Sep 2014 18:50:49 +0200 Subject: [PATCH 08/14] Make test for MARK support HHVM compatible --- test/Dispatcher/MarkBasedTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/Dispatcher/MarkBasedTest.php b/test/Dispatcher/MarkBasedTest.php index d7f4157..04b0af9 100644 --- a/test/Dispatcher/MarkBasedTest.php +++ b/test/Dispatcher/MarkBasedTest.php @@ -4,7 +4,8 @@ class MarkBasedTest extends DispatcherTest { public function setUp() { - if (version_compare(PHP_VERSION, '5.6.0-beta1', '<')) { + preg_match('/(*MARK:A)a/', 'a', $matches); + if (!isset($matches['MARK'])) { $this->markTestSkipped('PHP 5.6 required for MARK support'); } } From 88ac2888aa29d888cf3f447899753a4a1e599759 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Tue, 30 Sep 2014 07:07:30 +0200 Subject: [PATCH 09/14] Added HHVM to travis.yml --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index a0f906c..4f0a7bc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,4 +4,5 @@ php: - 5.4 - 5.5 - 5.6 + - hhvm From 5b1672381e6150e62f8803ce8854f71038d08f76 Mon Sep 17 00:00:00 2001 From: nikic Date: Wed, 26 Nov 2014 18:17:00 +0100 Subject: [PATCH 10/14] Fix issue #25 Instead of first matching against the route and then checking the allowed methods, the dispatch now happens in reverse: Only the routes for a particular method are considered initially. If there is no route for the specified method, the URI will be matched against all other methods as well in order to determine the allowed methods list (or conclude that no route exists at all). The performance impact of this change is twofold: On the one hand fewer matches are required in case a route is found (because only the method we're actually interested is tested). On the other hand there is more logic and potentially also more matches necessary in cases where a method-not-allowed or not-found status occurs. --- src/DataGenerator/CharCountBased.php | 11 ++-- src/DataGenerator/GroupCountBased.php | 10 ++-- src/DataGenerator/GroupPosBased.php | 11 ++-- src/DataGenerator/MarkBased.php | 8 +-- src/DataGenerator/RegexBasedAbstract.php | 37 +++++++------- src/Dispatcher/CharCountBased.php | 44 ++-------------- src/Dispatcher/GroupCountBased.php | 42 ++-------------- src/Dispatcher/GroupPosBased.php | 42 ++-------------- src/Dispatcher/MarkBased.php | 42 ++-------------- src/Dispatcher/RegexBasedAbstract.php | 64 ++++++++++++++++++++++++ test/Dispatcher/DispatcherTest.php | 43 ++++++++++++++++ 11 files changed, 155 insertions(+), 199 deletions(-) create mode 100644 src/Dispatcher/RegexBasedAbstract.php diff --git a/src/DataGenerator/CharCountBased.php b/src/DataGenerator/CharCountBased.php index b733380..5c38b89 100644 --- a/src/DataGenerator/CharCountBased.php +++ b/src/DataGenerator/CharCountBased.php @@ -14,20 +14,15 @@ protected function processChunk($regexToRoutesMap) { $suffixLen = 0; $suffix = ''; $count = count($regexToRoutesMap); - foreach ($regexToRoutesMap as $regex => $routes) { + foreach ($regexToRoutesMap as $regex => $route) { $suffixLen++; $suffix .= "\t"; - foreach ($routes as $route) { - $routeMap[$suffix][$route->httpMethod] = [ - $route->handler, $route->variables - ]; - } - $regexes[] = '(?:' . $regex . '/(\t{' . $suffixLen . '})\t{' . ($count - $suffixLen) . '})'; + $routeMap[$suffix] = [$route->handler, $route->variables]; } $regex = '~^(?|' . implode('|', $regexes) . ')$~'; return ['regex' => $regex, 'suffix' => '/' . $suffix, 'routeMap' => $routeMap]; } -} \ No newline at end of file +} diff --git a/src/DataGenerator/GroupCountBased.php b/src/DataGenerator/GroupCountBased.php index 16ed4c5..d51807f 100644 --- a/src/DataGenerator/GroupCountBased.php +++ b/src/DataGenerator/GroupCountBased.php @@ -11,16 +11,12 @@ protected function processChunk($regexToRoutesMap) { $routeMap = []; $regexes = []; $numGroups = 0; - foreach ($regexToRoutesMap as $regex => $routes) { - $numVariables = count(reset($routes)->variables); + foreach ($regexToRoutesMap as $regex => $route) { + $numVariables = count($route->variables); $numGroups = max($numGroups, $numVariables); $regexes[] = $regex . str_repeat('()', $numGroups - $numVariables); - - foreach ($routes as $route) { - $routeMap[$numGroups + 1][$route->httpMethod] - = [$route->handler, $route->variables]; - } + $routeMap[$numGroups + 1] = [$route->handler, $route->variables]; ++$numGroups; } diff --git a/src/DataGenerator/GroupPosBased.php b/src/DataGenerator/GroupPosBased.php index 136cdf3..4152f7a 100644 --- a/src/DataGenerator/GroupPosBased.php +++ b/src/DataGenerator/GroupPosBased.php @@ -11,14 +11,11 @@ protected function processChunk($regexToRoutesMap) { $routeMap = []; $regexes = []; $offset = 1; - foreach ($regexToRoutesMap as $regex => $routes) { - foreach ($routes as $route) { - $routeMap[$offset][$route->httpMethod] - = [$route->handler, $route->variables]; - } - + foreach ($regexToRoutesMap as $regex => $route) { $regexes[] = $regex; - $offset += count(reset($routes)->variables); + $routeMap[$offset] = [$route->handler, $route->variables]; + + $offset += count($route->variables); } $regex = '~^(?:' . implode('|', $regexes) . ')$~'; diff --git a/src/DataGenerator/MarkBased.php b/src/DataGenerator/MarkBased.php index 1c9db55..61359f5 100644 --- a/src/DataGenerator/MarkBased.php +++ b/src/DataGenerator/MarkBased.php @@ -11,13 +11,9 @@ protected function processChunk($regexToRoutesMap) { $routeMap = []; $regexes = []; $markName = 'a'; - foreach ($regexToRoutesMap as $regex => $routes) { + foreach ($regexToRoutesMap as $regex => $route) { $regexes[] = $regex . '(*MARK:' . $markName . ')'; - - foreach ($routes as $route) { - $routeMap[$markName][$route->httpMethod] - = [$route->handler, $route->variables]; - } + $routeMap[$markName] = [$route->handler, $route->variables]; ++$markName; } diff --git a/src/DataGenerator/RegexBasedAbstract.php b/src/DataGenerator/RegexBasedAbstract.php index 0d431a8..6c1e0c5 100644 --- a/src/DataGenerator/RegexBasedAbstract.php +++ b/src/DataGenerator/RegexBasedAbstract.php @@ -8,7 +8,7 @@ abstract class RegexBasedAbstract implements DataGenerator { protected $staticRoutes = []; - protected $regexToRoutesMap = []; + protected $methodToRegexToRoutesMap = []; protected abstract function getApproxChunkSize(); protected abstract function processChunk($regexToRoutesMap); @@ -22,7 +22,7 @@ public function addRoute($httpMethod, $routeData, $handler) { } public function getData() { - if (empty($this->regexToRoutesMap)) { + if (empty($this->methodToRegexToRoutesMap)) { return [$this->staticRoutes, []]; } @@ -30,9 +30,13 @@ public function getData() { } private function generateVariableRouteData() { - $chunkSize = $this->computeChunkSize(count($this->regexToRoutesMap)); - $chunks = array_chunk($this->regexToRoutesMap, $chunkSize, true); - return array_map([$this, 'processChunk'], $chunks); + $data = []; + foreach ($this->methodToRegexToRoutesMap as $method => $regexToRoutesMap) { + $chunkSize = $this->computeChunkSize(count($regexToRoutesMap)); + $chunks = array_chunk($regexToRoutesMap, $chunkSize, true); + $data[$method] = array_map([$this, 'processChunk'], $chunks); + } + return $data; } private function computeChunkSize($count) { @@ -54,15 +58,14 @@ private function addStaticRoute($httpMethod, $routeData, $handler) { )); } - foreach ($this->regexToRoutesMap as $routes) { - if (!isset($routes[$httpMethod])) continue; - - $route = $routes[$httpMethod]; - if ($route->matches($routeStr)) { - throw new BadRouteException(sprintf( - 'Static route "%s" is shadowed by previously defined variable route "%s" for method "%s"', - $routeStr, $route->regex, $httpMethod - )); + if (isset($this->methodToRegexToRoutesMap[$httpMethod])) { + foreach ($this->methodToRegexToRoutesMap[$httpMethod] as $route) { + if ($route->matches($routeStr)) { + throw new BadRouteException(sprintf( + 'Static route "%s" is shadowed by previously defined variable route "%s" for method "%s"', + $routeStr, $route->regex, $httpMethod + )); + } } } @@ -72,14 +75,14 @@ private function addStaticRoute($httpMethod, $routeData, $handler) { private function addVariableRoute($httpMethod, $routeData, $handler) { list($regex, $variables) = $this->buildRegexForRoute($routeData); - if (isset($this->regexToRoutesMap[$regex][$httpMethod])) { + if (isset($this->methodToRegexToRoutesMap[$httpMethod][$regex])) { throw new BadRouteException(sprintf( 'Cannot register two routes matching "%s" for method "%s"', $regex, $httpMethod )); } - $this->regexToRoutesMap[$regex][$httpMethod] = new Route( + $this->methodToRegexToRoutesMap[$httpMethod][$regex] = new Route( $httpMethod, $handler, $regex, $variables ); } @@ -107,4 +110,4 @@ private function buildRegexForRoute($routeData) { return [$regex, $variables]; } -} \ No newline at end of file +} diff --git a/src/Dispatcher/CharCountBased.php b/src/Dispatcher/CharCountBased.php index 641d276..22ba240 100644 --- a/src/Dispatcher/CharCountBased.php +++ b/src/Dispatcher/CharCountBased.php @@ -2,60 +2,24 @@ namespace FastRoute\Dispatcher; -use FastRoute\Dispatcher; - -class CharCountBased implements Dispatcher { - private $staticRouteMap; - private $variableRouteData; - +class CharCountBased extends RegexBasedAbstract { public function __construct($data) { list($this->staticRouteMap, $this->variableRouteData) = $data; } - public function dispatch($httpMethod, $uri) { - if (isset($this->staticRouteMap[$uri])) { - return $this->dispatchStaticRoute($httpMethod, $uri); - } else { - return $this->dispatchVariableRoute($httpMethod, $uri); - } - } - - private 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)]; - } - } - - private function dispatchVariableRoute($httpMethod, $uri) { - foreach ($this->variableRouteData as $data) { + protected function dispatchVariableRoute($routeData, $uri) { + foreach ($routeData as $data) { if (!preg_match($data['regex'], $uri . $data['suffix'], $matches)) { continue; } - $routes = $data['routeMap'][end($matches)]; - - if (false === isset($routes[$httpMethod])) { - if ($httpMethod === 'HEAD' && isset($routes['GET'])) { - $httpMethod = 'GET'; - } else { - return [self::METHOD_NOT_ALLOWED, array_keys($routes)]; - } - } - - list($handler, $varNames) = $routes[$httpMethod]; + list($handler, $varNames) = $data['routeMap'][end($matches)]; $vars = []; $i = 0; foreach ($varNames as $varName) { $vars[$varName] = $matches[++$i]; } - return [self::FOUND, $handler, $vars]; } diff --git a/src/Dispatcher/GroupCountBased.php b/src/Dispatcher/GroupCountBased.php index 5532faf..0abd322 100644 --- a/src/Dispatcher/GroupCountBased.php +++ b/src/Dispatcher/GroupCountBased.php @@ -2,52 +2,18 @@ namespace FastRoute\Dispatcher; -use FastRoute\Dispatcher; - -class GroupCountBased implements Dispatcher { - private $staticRouteMap; - private $variableRouteData; - +class GroupCountBased extends RegexBasedAbstract { public function __construct($data) { list($this->staticRouteMap, $this->variableRouteData) = $data; } - public function dispatch($httpMethod, $uri) { - if (isset($this->staticRouteMap[$uri])) { - return $this->dispatchStaticRoute($httpMethod, $uri); - } else { - return $this->dispatchVariableRoute($httpMethod, $uri); - } - } - - private 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)]; - } - } - - private function dispatchVariableRoute($httpMethod, $uri) { - foreach ($this->variableRouteData as $data) { + protected function dispatchVariableRoute($routeData, $uri) { + foreach ($routeData as $data) { if (!preg_match($data['regex'], $uri, $matches)) { continue; } - $routes = $data['routeMap'][count($matches)]; - if (!isset($routes[$httpMethod])) { - if ($httpMethod === 'HEAD' && isset($routes['GET'])) { - $httpMethod = 'GET'; - } else { - return [self::METHOD_NOT_ALLOWED, array_keys($routes)]; - } - } - - list($handler, $varNames) = $routes[$httpMethod]; + list($handler, $varNames) = $data['routeMap'][count($matches)]; $vars = []; $i = 0; diff --git a/src/Dispatcher/GroupPosBased.php b/src/Dispatcher/GroupPosBased.php index 3fc7cd2..32227d4 100644 --- a/src/Dispatcher/GroupPosBased.php +++ b/src/Dispatcher/GroupPosBased.php @@ -2,38 +2,13 @@ namespace FastRoute\Dispatcher; -use FastRoute\Dispatcher; - -class GroupPosBased implements Dispatcher { - private $staticRouteMap; - private $variableRouteData; - +class GroupPosBased extends RegexBasedAbstract { public function __construct($data) { list($this->staticRouteMap, $this->variableRouteData) = $data; } - public function dispatch($httpMethod, $uri) { - if (isset($this->staticRouteMap[$uri])) { - return $this->dispatchStaticRoute($httpMethod, $uri); - } else { - return $this->dispatchVariableRoute($httpMethod, $uri); - } - } - - private 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)]; - } - } - - private function dispatchVariableRoute($httpMethod, $uri) { - foreach ($this->variableRouteData as $data) { + protected function dispatchVariableRoute($routeData, $uri) { + foreach ($routeData as $data) { if (!preg_match($data['regex'], $uri, $matches)) { continue; } @@ -41,16 +16,7 @@ private function dispatchVariableRoute($httpMethod, $uri) { // find first non-empty match for ($i = 1; '' === $matches[$i]; ++$i); - $routes = $data['routeMap'][$i]; - if (!isset($routes[$httpMethod])) { - if ($httpMethod === 'HEAD' && isset($routes['GET'])) { - $httpMethod = 'GET'; - } else { - return [self::METHOD_NOT_ALLOWED, array_keys($routes)]; - } - } - - list($handler, $varNames) = $routes[$httpMethod]; + list($handler, $varNames) = $data['routeMap'][$i]; $vars = []; foreach ($varNames as $varName) { diff --git a/src/Dispatcher/MarkBased.php b/src/Dispatcher/MarkBased.php index e533e40..fefa711 100644 --- a/src/Dispatcher/MarkBased.php +++ b/src/Dispatcher/MarkBased.php @@ -2,52 +2,18 @@ namespace FastRoute\Dispatcher; -use FastRoute\Dispatcher; - -class MarkBased implements Dispatcher { - private $staticRouteMap; - private $variableRouteData; - +class MarkBased extends RegexBasedAbstract { public function __construct($data) { list($this->staticRouteMap, $this->variableRouteData) = $data; } - public function dispatch($httpMethod, $uri) { - if (isset($this->staticRouteMap[$uri])) { - return $this->dispatchStaticRoute($httpMethod, $uri); - } else { - return $this->dispatchVariableRoute($httpMethod, $uri); - } - } - - private 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)]; - } - } - - private function dispatchVariableRoute($httpMethod, $uri) { - foreach ($this->variableRouteData as $data) { + protected function dispatchVariableRoute($routeData, $uri) { + foreach ($routeData as $data) { if (!preg_match($data['regex'], $uri, $matches)) { continue; } - $routes = $data['routeMap'][$matches['MARK']]; - if (!isset($routes[$httpMethod])) { - if ($httpMethod === 'HEAD' && isset($routes['GET'])) { - $httpMethod = 'GET'; - } else { - return [self::METHOD_NOT_ALLOWED, array_keys($routes)]; - } - } - - list($handler, $varNames) = $routes[$httpMethod]; + list($handler, $varNames) = $data['routeMap'][$matches['MARK']]; $vars = []; $i = 0; diff --git a/src/Dispatcher/RegexBasedAbstract.php b/src/Dispatcher/RegexBasedAbstract.php new file mode 100644 index 0000000..5e86907 --- /dev/null +++ b/src/Dispatcher/RegexBasedAbstract.php @@ -0,0 +1,64 @@ +staticRouteMap[$uri])) { + return $this->dispatchStaticRoute($httpMethod, $uri); + } + + $varRouteData = $this->variableRouteData; + if (isset($varRouteData[$httpMethod])) { + $result = $this->dispatchVariableRoute($varRouteData[$httpMethod], $uri); + if ($result[0] === self::FOUND) { + return $result; + } + } else if ($httpMethod === 'HEAD' && isset($varRouteData['GET'])) { + $result = $this->dispatchVariableRoute($varRouteData['GET'], $uri); + if ($result[0] === self::FOUND) { + return $result; + } + } + + // Find allowed methods for this URI by matching against all other + // HTTP methods as well + $allowedMethods = []; + foreach ($varRouteData as $method => $routeData) { + if ($method === $httpMethod) { + continue; + } + + $result = $this->dispatchVariableRoute($routeData, $uri); + if ($result[0] === self::FOUND) { + $allowedMethods[] = $method; + } + } + + // If there are no allowed methods the route simply does not exist + if ($allowedMethods) { + return [self::METHOD_NOT_ALLOWED, $allowedMethods]; + } else { + 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 index 3d49e30..92a1ffe 100644 --- a/test/Dispatcher/DispatcherTest.php +++ b/test/Dispatcher/DispatcherTest.php @@ -252,6 +252,35 @@ public function provideFoundDispatchCases() { $cases[] = [$method, $uri, $callback, $handler, $argDict]; + // 11 ---- More specified routes are not shadowed by less specific of another method ------> + + $callback = function(RouteCollector $r) { + $r->addRoute('GET', '/user/{name}', 'handler0'); + $r->addRoute('POST', '/user/{name:[a-z]+}', 'handler1'); + }; + + $method = 'POST'; + $uri = '/user/rdlowrey'; + $handler = 'handler1'; + $argDict = ['name' => 'rdlowrey']; + + $cases[] = [$method, $uri, $callback, $handler, $argDict]; + + // 12 ---- Handler of more specific routes is used, if it occurs first --------------------> + + $callback = function(RouteCollector $r) { + $r->addRoute('GET', '/user/{name}', 'handler0'); + $r->addRoute('POST', '/user/{name:[a-z]+}', 'handler1'); + $r->addRoute('POST', '/user/{name}', 'handler2'); + }; + + $method = 'POST'; + $uri = '/user/rdlowrey'; + $handler = 'handler1'; + $argDict = ['name' => 'rdlowrey']; + + $cases[] = [$method, $uri, $callback, $handler, $argDict]; + // x --------------------------------------------------------------------------------------> return $cases; @@ -370,6 +399,20 @@ public function provideMethodNotAllowedDispatchCases() { $cases[] = [$method, $uri, $callback, $allowedMethods]; + // 3 --------------------------------------------------------------------------------------> + + $callback = function(RouteCollector $r) { + $r->addRoute('POST', '/user/{name}', 'handler1'); + $r->addRoute('PUT', '/user/{name:[a-z]+}', 'handler2'); + $r->addRoute('PATCH', '/user/{name:[a-z]+}', 'handler3'); + }; + + $method = 'GET'; + $uri = '/user/rdlowrey'; + $allowedMethods = ['POST', 'PUT', 'PATCH']; + + $cases[] = [$method, $uri, $callback, $allowedMethods]; + // x --------------------------------------------------------------------------------------> return $cases; From 37299fccaeae2928277b3f155b7a1ce41ccd48a3 Mon Sep 17 00:00:00 2001 From: nikic Date: Wed, 26 Nov 2014 19:35:23 +0100 Subject: [PATCH 11/14] Add test for route with constant suffix --- test/Dispatcher/DispatcherTest.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/Dispatcher/DispatcherTest.php b/test/Dispatcher/DispatcherTest.php index 92a1ffe..3c9a1d8 100644 --- a/test/Dispatcher/DispatcherTest.php +++ b/test/Dispatcher/DispatcherTest.php @@ -281,6 +281,20 @@ public function provideFoundDispatchCases() { $cases[] = [$method, $uri, $callback, $handler, $argDict]; + // 13 ---- Route with constant suffix -----------------------------------------------------> + + $callback = function(RouteCollector $r) { + $r->addRoute('GET', '/user/{name}', 'handler0'); + $r->addRoute('GET', '/user/{name}/edit', 'handler1'); + }; + + $method = 'GET'; + $uri = '/user/rdlowrey/edit'; + $handler = 'handler1'; + $argDict = ['name' => 'rdlowrey']; + + $cases[] = [$method, $uri, $callback, $handler, $argDict]; + // x --------------------------------------------------------------------------------------> return $cases; From 57a2aec2d597f166c4139e44953bc42b7eaf9e54 Mon Sep 17 00:00:00 2001 From: nikic Date: Wed, 26 Nov 2014 19:36:25 +0100 Subject: [PATCH 12/14] Use require_once in test bootstrap Fixes issue #23, though I don't really understand when this would be necessary. --- test/bootstrap.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/bootstrap.php b/test/bootstrap.php index eabc959..27e6d4c 100644 --- a/test/bootstrap.php +++ b/test/bootstrap.php @@ -1,6 +1,6 @@ Date: Thu, 27 Nov 2014 10:47:55 +0300 Subject: [PATCH 13/14] typo in README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9f571f8..257342e 100644 --- a/README.md +++ b/README.md @@ -130,7 +130,7 @@ array has a certain structure, best understood using an example: '/user/', ['name', '[^/]+'], '/', - ['id', [0-9]+'], + ['id', '[0-9]+'], ] This array can then be passed to the `addRoute()` method of a data generator. After all routes have From 1e82e889ed3ea67ee80923796c48f409fe46b420 Mon Sep 17 00:00:00 2001 From: Daniel Bruce Date: Fri, 28 Nov 2014 09:50:30 -0500 Subject: [PATCH 14/14] Added test case for HEAD request not found. --- test/Dispatcher/DispatcherTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/Dispatcher/DispatcherTest.php b/test/Dispatcher/DispatcherTest.php index 3c9a1d8..aa6dadd 100644 --- a/test/Dispatcher/DispatcherTest.php +++ b/test/Dispatcher/DispatcherTest.php @@ -364,6 +364,13 @@ public function provideNotFoundDispatchCases() { $cases[] = [$method, $uri, $callback]; + // 6 --------------------------------------------------------------------------------------> + + // reuse callback from #5 + $method = 'HEAD'; + + $cases[] = array($method, $uri, $callback); + // x --------------------------------------------------------------------------------------> return $cases;