From 8e0343e8deb096fb985021018fa246fe3bc23021 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 6 Jun 2018 14:52:39 -0700 Subject: [PATCH 1/8] Try swapping in fastroute --- src/Dispatcher.php | 45 ++++++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/src/Dispatcher.php b/src/Dispatcher.php index 9aa3311..f412ba8 100644 --- a/src/Dispatcher.php +++ b/src/Dispatcher.php @@ -6,6 +6,7 @@ use BadMethodCallException; use DomainException; +use FastRoute; use Firehed\API\Interfaces\ErrorHandlerInterface; use Firehed\API\Interfaces\HandlesOwnErrorsInterface; use Firehed\Common\ClassMapper; @@ -246,20 +247,38 @@ private function parseInput(): ParsedInput */ private function getEndpoint(): Interfaces\EndpointInterface { - list($class, $uri_data) = (new ClassMapper($this->endpoint_list)) - ->filter(strtoupper($this->request->getMethod())) - ->search($this->request->getUri()->getPath()); - if (!$class) { - throw new OutOfBoundsException('Endpoint not found', 404); - } - // Conceivably, we could use reflection to ensure the found class - // adheres to the interface; in practice, the built route is already - // doing the filtering so this should be redundant. - $this->setUriData(new ParsedInput($uri_data)); - if ($this->container && $this->container->has($class)) { - return $this->container->get($class); + $dispatcher = FastRoute\simpleDispatcher(function (FastRoute\RouteCollector $rc) { + $data = json_decode(file_get_contents($this->endpoint_list), true); + unset($data['@gener'.'ated']); + $pattern = '#\(\?P?<(\w+)>(.*)\)#U'; + foreach ($data as $method => $routes) { + // var_dump($routes); + foreach ($routes as $regex => $fqcn) { + $frUri = preg_replace($pattern, '{\1:\2}', $regex); + // var_dump($frUri); + $rc->addRoute($method, $frUri, $fqcn); + } + } + }); + $info = $dispatcher->dispatch( + $this->request->getMethod(), + $this->request->GetUri()->getPath() + ); + switch ($info[0]) { + case FastRoute\Dispatcher::NOT_FOUND: + throw new OutOfBoundsException('Endpoint not found', 404); + case FastRoute\Dispatcher::METHOD_NOT_ALLOWED: + // allowed methods = $info[1] + throw new OutOfBoundsException('Method not allowed', 405); + case FastRoute\Dispatcher::FOUND: + $fqcn = $info[1]; + $uriData = $info[2]; + $this->setUriData(new ParsedInput($uriData)); + if ($this->container && $this->container->has($fqcn)) { + return $this->container->get($fqcn); + } + return new $class; } - return new $class; } private function setUriData(ParsedInput $uri_data): self From 36d3ac0d1fb08d8ebedee9549475df2797a8bcf5 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 6 Jun 2018 14:53:23 -0700 Subject: [PATCH 2/8] Include fastroute --- composer.json | 1 + 1 file changed, 1 insertion(+) diff --git a/composer.json b/composer.json index 058adaa..80b57d3 100644 --- a/composer.json +++ b/composer.json @@ -4,6 +4,7 @@ "php": "^7.0", "firehed/common": "^1.0", "firehed/input": "^2.0", + "nikic/fast-route": "^1.3", "psr/container": "^1.0", "psr/http-message": "^1.0", "psr/log": "^1.0", From 76614b714b147053483a2fa6564ac9ee19119323 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 6 Jun 2018 14:56:45 -0700 Subject: [PATCH 3/8] Handle direct array injection --- src/Dispatcher.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Dispatcher.php b/src/Dispatcher.php index f412ba8..6239fce 100644 --- a/src/Dispatcher.php +++ b/src/Dispatcher.php @@ -248,7 +248,11 @@ private function parseInput(): ParsedInput private function getEndpoint(): Interfaces\EndpointInterface { $dispatcher = FastRoute\simpleDispatcher(function (FastRoute\RouteCollector $rc) { - $data = json_decode(file_get_contents($this->endpoint_list), true); + if (is_array($this->endpoint_list)) { + $data = $this->endpoint_list; + } else { + $data = json_decode(file_get_contents($this->endpoint_list), true); + } unset($data['@gener'.'ated']); $pattern = '#\(\?P?<(\w+)>(.*)\)#U'; foreach ($data as $method => $routes) { From c8fb9beb022607ed133e016890c71b76a087e0d7 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Wed, 6 Jun 2018 14:58:53 -0700 Subject: [PATCH 4/8] Fix var --- src/Dispatcher.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Dispatcher.php b/src/Dispatcher.php index 6239fce..cdf7cf7 100644 --- a/src/Dispatcher.php +++ b/src/Dispatcher.php @@ -281,7 +281,7 @@ private function getEndpoint(): Interfaces\EndpointInterface if ($this->container && $this->container->has($fqcn)) { return $this->container->get($fqcn); } - return new $class; + return new $fqcn; } } From 6268416bfe882f4dc849d891c4afe6438ffb7803 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Mon, 10 Sep 2018 14:13:16 -0700 Subject: [PATCH 5/8] Test for HTTP 405 response --- src/Dispatcher.php | 2 -- tests/DispatcherTest.php | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/Dispatcher.php b/src/Dispatcher.php index 2f19727..ca00a95 100644 --- a/src/Dispatcher.php +++ b/src/Dispatcher.php @@ -289,10 +289,8 @@ private function getEndpoint(ServerRequestInterface $request): Interfaces\Endpoi unset($data['@gener'.'ated']); $pattern = '#\(\?P?<(\w+)>(.*)\)#U'; foreach ($data as $method => $routes) { - // var_dump($routes); foreach ($routes as $regex => $fqcn) { $frUri = preg_replace($pattern, '{\1:\2}', $regex); - // var_dump($frUri); $rc->addRoute($method, $frUri, $fqcn); } } diff --git a/tests/DispatcherTest.php b/tests/DispatcherTest.php index 99f15f5..585a7cd 100644 --- a/tests/DispatcherTest.php +++ b/tests/DispatcherTest.php @@ -360,6 +360,23 @@ public function testUnsupportedContentTypeCanReachErrorHandlers() } } + /** @covers ::dispatch */ + public function testMethodNotAllowed() + { + $req = $this->getMockRequestWithUriPath('/user/1', 'PUT'); + try { + $response = (new Dispatcher()) + ->setEndpointList($this->getEndpointListForFixture()) + ->setParserList($this->getDefaultParserList()) + ->setRequest($req) + ->dispatch(); + $this->fail('An exception should have been thrown'); + } catch (Throwable $e) { + $this->assertInstanceOf(RuntimeException::class, $e); + $this->assertSame(405, $e->getCode()); + } + } + /** * @covers ::dispatch */ From 0f9c71aa71ddc79178fdb3fb186f98be55732b16 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Mon, 10 Sep 2018 14:17:46 -0700 Subject: [PATCH 6/8] Future-proof switch statement --- src/Dispatcher.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Dispatcher.php b/src/Dispatcher.php index ca00a95..ef26b00 100644 --- a/src/Dispatcher.php +++ b/src/Dispatcher.php @@ -313,6 +313,10 @@ private function getEndpoint(ServerRequestInterface $request): Interfaces\Endpoi return $this->container->get($fqcn); } return new $fqcn; + default: + // @codeCoverageIgnoreStart + throw new \DomainException('Unexpected Dispatcher route info'); + // @codeCoverageIgnoreEnd } } From 6ef6f3806a93c4ec03bd336f5c5aafe9d6d27b59 Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Tue, 11 Sep 2018 15:05:31 -0700 Subject: [PATCH 7/8] closer implementation --- src/Console/CompileAll.php | 25 +++++++++++++++++++++++-- src/Dispatcher.php | 24 +++++++++--------------- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/src/Console/CompileAll.php b/src/Console/CompileAll.php index 0e9e977..8de4778 100644 --- a/src/Console/CompileAll.php +++ b/src/Console/CompileAll.php @@ -3,6 +3,7 @@ namespace Firehed\API\Console; +use function FastRoute\cachedDispatcher; use Firehed\API\Config; use Firehed\API\Dispatcher; use Firehed\API\Interfaces\EndpointInterface; @@ -38,14 +39,34 @@ protected function execute(InputInterface $input, OutputInterface $output) $logger->debug('Current directory: {cwd}', ['cwd' => getcwd()]); $logger->debug('Building classmap'); // Build out the endpoint map - (new ClassMapGenerator()) + $endpoints = (new ClassMapGenerator()) ->setPath(getcwd().'/'.$this->config->get('source')) ->setInterface(EndpointInterface::class) ->addCategory('getMethod') ->setMethod('getURI') ->setNamespace($this->config->get(Config::KEY_NAMESPACE)) - ->setOutputFile(Dispatcher::ENDPOINT_LIST) + // ->setOutputFile(Dispatcher::ENDPOINT_LIST) ->generate(); + unset($endpoints['@gener'.'ated']); + // Regex-parsing regex: grab named captures + $pattern = '#\(\?P?<(\w+)>(.*)\)#'; + + if (file_exists(Dispatcher::ENDPOINT_LIST)) { + unlink(Dispatcher::ENDPOINT_LIST); + } + $dispatcher = cachedDispatcher(function ($routeCollector) use ($endpoints, $pattern) { + foreach ($endpoints as $method => $routes) { + foreach ($routes as $regex => $fqcn) { + $frUri = preg_replace($pattern, '{\1:\2}', $regex); + $stripped = strtr($frUri, ['(' => '', ')' => '']); + echo "$regex => $frUri => $stripped\n"; + $routeCollector->addRoute($method, $stripped, $fqcn); + // $routeCollector->addRoute($method, $regex, $fqcn); + } + } + }, [ + 'cacheFile' => Dispatcher::ENDPOINT_LIST, + ]); $output->writeln(sprintf( 'Wrote endpoint map to %s', diff --git a/src/Dispatcher.php b/src/Dispatcher.php index ef26b00..4e0a779 100644 --- a/src/Dispatcher.php +++ b/src/Dispatcher.php @@ -280,21 +280,7 @@ private function parseInput(ServerRequestInterface $request): ParsedInput */ private function getEndpoint(ServerRequestInterface $request): Interfaces\EndpointInterface { - $dispatcher = FastRoute\simpleDispatcher(function (FastRoute\RouteCollector $rc) { - if (is_array($this->endpointList)) { - $data = $this->endpointList; - } else { - $data = include $this->endpointList; - } - unset($data['@gener'.'ated']); - $pattern = '#\(\?P?<(\w+)>(.*)\)#U'; - foreach ($data as $method => $routes) { - foreach ($routes as $regex => $fqcn) { - $frUri = preg_replace($pattern, '{\1:\2}', $regex); - $rc->addRoute($method, $frUri, $fqcn); - } - } - }); + $dispatcher = new \FastRoute\Dispatcher\GroupCountBased($this->getRouteData()); $info = $dispatcher->dispatch( $request->getMethod(), $request->getUri()->getPath() @@ -320,6 +306,14 @@ private function getEndpoint(ServerRequestInterface $request): Interfaces\Endpoi } } + private function getRouteData(): array + { + if (is_string($this->endpointList)) { + return include $this->endpointList; + } + // TODO: handle explicit routing table provided + } + private function setUriData(ParsedInput $uri_data): self { $this->uri_data = $uri_data; From 3b135bcb053414be10c2745475bb06528cd5a6ac Mon Sep 17 00:00:00 2001 From: Eric Stern Date: Tue, 11 Sep 2018 15:49:07 -0700 Subject: [PATCH 8/8] Separate routing logic --- src/Console/CompileAll.php | 26 +++-------- src/Dispatcher.php | 45 +++++-------------- src/Router.php | 91 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 55 deletions(-) create mode 100644 src/Router.php diff --git a/src/Console/CompileAll.php b/src/Console/CompileAll.php index 8de4778..a279e4a 100644 --- a/src/Console/CompileAll.php +++ b/src/Console/CompileAll.php @@ -3,10 +3,10 @@ namespace Firehed\API\Console; -use function FastRoute\cachedDispatcher; use Firehed\API\Config; use Firehed\API\Dispatcher; use Firehed\API\Interfaces\EndpointInterface; +use Firehed\API\Router; use Firehed\Input\Interfaces\ParserInterface; use Firehed\Common\ClassMapGenerator; use Symfony\Component\Console\Command\Command; @@ -48,29 +48,13 @@ protected function execute(InputInterface $input, OutputInterface $output) // ->setOutputFile(Dispatcher::ENDPOINT_LIST) ->generate(); unset($endpoints['@gener'.'ated']); - // Regex-parsing regex: grab named captures - $pattern = '#\(\?P?<(\w+)>(.*)\)#'; - if (file_exists(Dispatcher::ENDPOINT_LIST)) { - unlink(Dispatcher::ENDPOINT_LIST); - } - $dispatcher = cachedDispatcher(function ($routeCollector) use ($endpoints, $pattern) { - foreach ($endpoints as $method => $routes) { - foreach ($routes as $regex => $fqcn) { - $frUri = preg_replace($pattern, '{\1:\2}', $regex); - $stripped = strtr($frUri, ['(' => '', ')' => '']); - echo "$regex => $frUri => $stripped\n"; - $routeCollector->addRoute($method, $stripped, $fqcn); - // $routeCollector->addRoute($method, $regex, $fqcn); - } - } - }, [ - 'cacheFile' => Dispatcher::ENDPOINT_LIST, - ]); + $router = new Router(); + $router->setData($endpoints); + $router->writeCache(); $output->writeln(sprintf( - 'Wrote endpoint map to %s', - Dispatcher::ENDPOINT_LIST + 'Wrote endpoint map' )); $logger->debug('Building parser map'); diff --git a/src/Dispatcher.php b/src/Dispatcher.php index 4e0a779..55726bc 100644 --- a/src/Dispatcher.php +++ b/src/Dispatcher.php @@ -22,13 +22,12 @@ class Dispatcher implements RequestHandlerInterface { - const ENDPOINT_LIST = '__endpoint_list__.php'; const PARSER_LIST = '__parser_list__.php'; private $authenticationProvider; private $authorizationProvider; private $container; - private $endpointList = self::ENDPOINT_LIST; + private $endpointList; private $error_handler; private $parserList = self::PARSER_LIST; private $psrMiddleware = []; @@ -150,10 +149,10 @@ public function setParserList($parserList): self * * @internal Overrides the standard endpoint list. Used primarily for unit * testing. - * @param array|string $endpointList The endpoint list or its path + * @param array $endpointList The endpoint list * @return self */ - public function setEndpointList($endpointList): self + public function setEndpointList(array $endpointList): self { $this->endpointList = $endpointList; return $this; @@ -280,38 +279,16 @@ private function parseInput(ServerRequestInterface $request): ParsedInput */ private function getEndpoint(ServerRequestInterface $request): Interfaces\EndpointInterface { - $dispatcher = new \FastRoute\Dispatcher\GroupCountBased($this->getRouteData()); - $info = $dispatcher->dispatch( - $request->getMethod(), - $request->getUri()->getPath() - ); - switch ($info[0]) { - case FastRoute\Dispatcher::NOT_FOUND: - throw new OutOfBoundsException('Endpoint not found', 404); - case FastRoute\Dispatcher::METHOD_NOT_ALLOWED: - // allowed methods = $info[1] - throw new OutOfBoundsException('Method not allowed', 405); - case FastRoute\Dispatcher::FOUND: - $fqcn = $info[1]; - $uriData = $info[2]; - $this->setUriData(new ParsedInput($uriData)); - if ($this->container && $this->container->has($fqcn)) { - return $this->container->get($fqcn); - } - return new $fqcn; - default: - // @codeCoverageIgnoreStart - throw new \DomainException('Unexpected Dispatcher route info'); - // @codeCoverageIgnoreEnd + $router = new Router(); + if ($this->endpointList !== null) { + $router->setData($this->endpointList); } - } - - private function getRouteData(): array - { - if (is_string($this->endpointList)) { - return include $this->endpointList; + list($fqcn, $uriData) = $router->route($request); + $this->setUriData($uriData); + if ($this->container && $this->container->has($fqcn)) { + return $this->container->get($fqcn); } - // TODO: handle explicit routing table provided + return new $fqcn; } private function setUriData(ParsedInput $uri_data): self diff --git a/src/Router.php b/src/Router.php new file mode 100644 index 0000000..c84c3d8 --- /dev/null +++ b/src/Router.php @@ -0,0 +1,91 @@ + [ + * 'SOME REGEX' => 'FQCN', + * ], + * ] + * @param array $routeMap the route map + */ + public function setData(array $routeMap) + { + $this->routeMap = $routeMap; + } + + public function writeCache() + { + $rd = $this->getRouteData(); + file_put_contents(self::CACHE_FILE, sprintf('getRouteData()); + $info = $dispatcher->dispatch( + $request->getMethod(), + $request->getUri()->getPath() + ); + switch ($info[0]) { + case FastRoute\Dispatcher::NOT_FOUND: + throw new OutOfBoundsException('Endpoint not found', 404); + case FastRoute\Dispatcher::METHOD_NOT_ALLOWED: + // allowed methods = $info[1] + throw new OutOfBoundsException('Method not allowed', 405); + case FastRoute\Dispatcher::FOUND: + return [$info[1], new ParsedInput($info[2])]; + default: + // @codeCoverageIgnoreStart + throw new \DomainException('Unexpected Dispatcher route info'); + // @codeCoverageIgnoreEnd + } + } + + private function getRouteData(): array + { + if ($this->routeMap === null) { + if (!file_exists(self::CACHE_FILE)) { + throw new \Exception('Route file missing. Run bin/app compile:all'); + } + return include self::CACHE_FILE; + } + $rc = new FastRoute\RouteCollector( + new FastRoute\RouteParser\Std(), + new FastRoute\DataGenerator\GroupCountBased() + ); + + // Regex-parsing regex: grab named captures + $pattern = '#\(\?P?<(\w+)>(.*)\)#'; + foreach ($this->routeMap as $method => $routes) { + foreach ($routes as $regex => $fqcn) { + $frUri = preg_replace($pattern, '{\1:\2}', $regex); + $stripped = strtr($frUri, ['(' => '', ')' => '']); + // echo "$regex => $frUri => $stripped\n"; + $rc ->addRoute($method, $stripped, $fqcn); + } + } + + return $rc->getData(); + } +}