From 6b0b7ec31d9df6cfc6a07a8415f2db109785d4f8 Mon Sep 17 00:00:00 2001 From: Geert Eltink Date: Wed, 22 Mar 2017 19:15:57 +0100 Subject: [PATCH 01/13] Use the standard route parser from FastRoute Instead of using a custom regex, nikic suggests to use parsed results: https://github.com/nikic/FastRoute/issues/66#issuecomment-130395124 This also throws errors for missing mandatory parameters (#30). --- src/FastRouteRouter.php | 67 ++++++++++++++++-------------------- test/FastRouteRouterTest.php | 15 ++++++++ 2 files changed, 45 insertions(+), 37 deletions(-) diff --git a/src/FastRouteRouter.php b/src/FastRouteRouter.php index b2db23d..6be3d4c 100644 --- a/src/FastRouteRouter.php +++ b/src/FastRouteRouter.php @@ -15,6 +15,7 @@ use Psr\Http\Message\ServerRequestInterface as Request; use Zend\Expressive\Router\Exception; use Zend\Stdlib\ArrayUtils; +use LogicException; /** * Router implementation bridging nikic/fast-route. @@ -70,23 +71,6 @@ class FastRouteRouter implements RouterInterface RequestMethod::METHOD_TRACE, ]; - /** - * Regular expression pattern for identifying a variable subsititution. - * - * This is an sprintf pattern; the variable name will be substituted for - * the final pattern. - * - * @see \FastRoute\RouteParser\Std for mirror, generic representation. - */ - const VARIABLE_REGEX = <<< 'REGEX' -\{ - \s* %s \s* - (?: - : \s* ([^{}]*(?:\{(?-1)\}[^{}]*)*) - )? -\} -REGEX; - /** * Cache generated route data? * @@ -277,41 +261,50 @@ public function generateUri($name, array $substitutions = [], array $options = [ } $route = $this->routes[$name]; - $path = $route->getPath(); $options = ArrayUtils::merge($route->getOptions(), $options); if (! empty($options['defaults'])) { $substitutions = array_merge($options['defaults'], $substitutions); } - foreach ($substitutions as $key => $value) { - $pattern = sprintf( - '~%s~x', - sprintf(self::VARIABLE_REGEX, preg_quote($key)) - ); - $path = preg_replace($pattern, $value, $path); - } + $routeParser = new RouteParser(); + $routes = $routeParser->parse($route->getPath()); + + // One route pattern can correspond to multiple routes if it has optional parts + foreach ($routes as $r) { + $path = ''; + $paramIdx = 0; + foreach ($r as $part) { + // Fixed segment in the route + if (is_string($part)) { + $path .= $part; + continue; + } + + // Placeholder in the route + if ($paramIdx === count($substitutions)) { + throw new LogicException('Not enough parameters given'); + } - // 1. remove optional segments' ending delimiters - // and remove leftover regex char classes like `:[` and `:prefix-[` (issue #18) - // 2. split path into an array of optional segments and remove those - // containing unsubstituted parameters starting from the last segment - $path = preg_replace('/\]|:.*\[/', '', $path); - $segs = array_reverse(explode('[', $path)); - foreach ($segs as $n => $seg) { - if (strpos($seg, '{') !== false) { - if (isset($segs[$n - 1])) { + if (! isset($substitutions[$part[0]])) { throw new Exception\InvalidArgumentException( 'Optional segments with unsubstituted parameters cannot ' . 'contain segments with substituted parameters when using FastRoute' ); } - unset($segs[$n]); + + $path .= $substitutions[$part[0]]; + $paramIdx++; + } + + // If number of params in route matches with number of params given, use that route. + // Otherwise try to find a route that has more params + if ($paramIdx === count($substitutions)) { + return $path; } } - $path = implode('', array_reverse($segs)); - return $path; + throw new LogicException('Too many parameters given'); } /** diff --git a/test/FastRouteRouterTest.php b/test/FastRouteRouterTest.php index edf61e9..df195a2 100644 --- a/test/FastRouteRouterTest.php +++ b/test/FastRouteRouterTest.php @@ -631,4 +631,19 @@ public function testFastRouteCache() unlink($cache_file); } + + /** + * Test for issue #30 + */ + public function testGenerateUriRaisesExceptionForMissingMandatoryParameters() + { + $router = new FastRouteRouter(); + $route = new Route('/foo/{id}', 'foo', ['GET'], 'foo'); + $router->addRoute($route); + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('Not enough parameters given'); + + $router->generateUri('foo'); + } } From 73e330b1ad7c60d5b9e7d86b72d0b5cf4039db4e Mon Sep 17 00:00:00 2001 From: Geert Eltink Date: Thu, 23 Mar 2017 18:28:21 +0100 Subject: [PATCH 02/13] Rework of integrating the standard route parser from FastRoute --- src/FastRouteRouter.php | 122 ++++++++++++++++++++++------------- test/FastRouteRouterTest.php | 16 +---- 2 files changed, 78 insertions(+), 60 deletions(-) diff --git a/src/FastRouteRouter.php b/src/FastRouteRouter.php index 6be3d4c..8ce25de 100644 --- a/src/FastRouteRouter.php +++ b/src/FastRouteRouter.php @@ -7,15 +7,14 @@ namespace Zend\Expressive\Router; -use Fig\Http\Message\RequestMethodInterface as RequestMethod; use FastRoute\DataGenerator\GroupCountBased as RouteGenerator; use FastRoute\Dispatcher\GroupCountBased as Dispatcher; use FastRoute\RouteCollector; use FastRoute\RouteParser\Std as RouteParser; +use Fig\Http\Message\RequestMethodInterface as RequestMethod; use Psr\Http\Message\ServerRequestInterface as Request; use Zend\Expressive\Router\Exception; use Zend\Stdlib\ArrayUtils; -use LogicException; /** * Router implementation bridging nikic/fast-route. @@ -29,17 +28,14 @@ class FastRouteRouter implements RouterInterface createRouter(); } - $this->router = $router; + $this->router = $router; $this->dispatcherCallback = $dispatcherFactory; $this->loadConfig($config); @@ -163,6 +157,7 @@ public function __construct( * Load configuration parameters * * @param array $config Array of custom configuration options. + * * @return void */ private function loadConfig(array $config = null) @@ -200,6 +195,7 @@ public function addRoute(Route $route) /** * @param Request $request + * * @return RouteResult */ public function match(Request $request) @@ -238,12 +234,13 @@ public function match(Request $request) * It does *not* use the pattern to validate that the substitution value is * valid beforehand, however. * - * @param string $name Route name. - * @param array $substitutions Key/value pairs to substitute into the route - * pattern. - * @param array $options Key/value option pairs to pass to the router for - * purposes of generating a URI; takes precedence over options present - * in route used to generate URI. + * @param string $name Route name. + * @param array $substitutions Key/value pairs to substitute into the route + * pattern. + * @param array $options Key/value option pairs to pass to the router for + * purposes of generating a URI; takes precedence over options present + * in route used to generate URI. + * * @return string URI path generated. * @throws Exception\InvalidArgumentException if the route name is not * known. @@ -267,44 +264,69 @@ public function generateUri($name, array $substitutions = [], array $options = [ $substitutions = array_merge($options['defaults'], $substitutions); } - $routeParser = new RouteParser(); - $routes = $routeParser->parse($route->getPath()); + $routeParser = new RouteParser(); + $routes = array_reverse($routeParser->parse($route->getPath())); + $requiredParameters = []; // One route pattern can correspond to multiple routes if it has optional parts - foreach ($routes as $r) { + foreach ($routes as $parts) { + // Check if all parameters can be substituted + $requiredParameters = $this->hasRequiredParameters($parts, $substitutions); + + // If not all parameters can be substituted, try the next route + if ($requiredParameters !== true) { + continue; + } + $path = ''; - $paramIdx = 0; - foreach ($r as $part) { - // Fixed segment in the route + foreach ($parts as $part) { if (is_string($part)) { $path .= $part; continue; } - // Placeholder in the route - if ($paramIdx === count($substitutions)) { - throw new LogicException('Not enough parameters given'); - } - - if (! isset($substitutions[$part[0]])) { - throw new Exception\InvalidArgumentException( - 'Optional segments with unsubstituted parameters cannot ' - . 'contain segments with substituted parameters when using FastRoute' - ); + // Check substitute value with regex + $regex = '~^' . str_replace('/', '\/', $part[1]) . '$~'; + if (!preg_match($regex, $substitutions[$part[0]])) { + throw new Exception\InvalidArgumentException(sprintf( + 'Parameter value for [%s] did not match the regex `%s`', + $part[0], + $part[1] + )); } $path .= $substitutions[$part[0]]; - $paramIdx++; } - // If number of params in route matches with number of params given, use that route. - // Otherwise try to find a route that has more params - if ($paramIdx === count($substitutions)) { - return $path; + return $path; + } + + throw new Exception\InvalidArgumentException(sprintf( + 'Expected parameter values for at least [%s], but received [%s]', + implode(',', $requiredParameters), + implode(',', array_keys($substitutions)) + )); + } + + private function hasRequiredParameters($parts, $substitutions) + { + $requiredParameters = []; + + foreach ($parts as $part) { + if (is_string($part)) { + continue; + } + + $requiredParameters[] = $part[0]; + } + + foreach ($requiredParameters as $param) { + if (! isset($substitutions[$param])) { + return $requiredParameters; } } - throw new LogicException('Too many parameters given'); + return true; } /** @@ -325,6 +347,7 @@ private function createRouter() * approach is done to allow testing against the dispatcher. * * @param array|object $data Data from RouteCollection::getData() + * * @return Dispatcher */ private function getDispatcher($data) @@ -334,6 +357,7 @@ private function getDispatcher($data) } $factory = $this->dispatcherCallback; + return $factory($data); } @@ -356,6 +380,7 @@ private function createDispatcherCallback() * methods to the factory. * * @param array $result + * * @return RouteResult */ private function marshalFailedRoute(array $result) @@ -363,14 +388,16 @@ private function marshalFailedRoute(array $result) if ($result[0] === Dispatcher::METHOD_NOT_ALLOWED) { return RouteResult::fromRouteFailure($result[1]); } + return RouteResult::fromRouteFailure(); } /** * Marshals a route result based on the results of matching and the current HTTP method. * - * @param array $result + * @param array $result * @param string $method + * * @return RouteResult */ private function marshalMatchedRoute(array $result, $method) @@ -495,7 +522,7 @@ private function loadDispatchData() )); } - $this->hasCache = true; + $this->hasCache = true; $this->dispatchData = $dispatchData; } @@ -503,6 +530,7 @@ private function loadDispatchData() * Save dispatch data to cache * * @param array $dispatchData + * * @return int|false bytes written to file or false if error * @throws Exception\InvalidCacheDirectoryException If the cache directory * does not exist. @@ -540,9 +568,10 @@ private function cacheDispatchData(array $dispatchData) * Call this method for failed HEAD or OPTIONS requests, to see if another * method matches; if so, return the match. * - * @param string $method - * @param string $path + * @param string $method + * @param string $path * @param Dispatcher $dispatcher + * * @return false|array False if no match found, array representing the match * otherwise. */ @@ -557,6 +586,7 @@ private function probeIntrospectionMethod($method, $path, Dispatcher $dispatcher return $result; } } + return false; } } diff --git a/test/FastRouteRouterTest.php b/test/FastRouteRouterTest.php index df195a2..f4f45db 100644 --- a/test/FastRouteRouterTest.php +++ b/test/FastRouteRouterTest.php @@ -448,18 +448,6 @@ public function testReturnedRouteResultShouldContainRouteName() $this->assertEquals('foo-route', $result->getMatchedRouteName()); } - public function testGenerateUriRaisesExceptionForIncompleteUriSubstitutions() - { - $router = new FastRouteRouter(); - $route = new Route('/foo[/{param}[/optional-{extra}]]', 'foo', ['GET'], 'foo'); - $router->addRoute($route); - - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('unsubstituted parameters'); - - $router->generateUri('foo', ['extra' => 'segment']); - } - public function uriGeneratorDataProvider() { return [ @@ -641,8 +629,8 @@ public function testGenerateUriRaisesExceptionForMissingMandatoryParameters() $route = new Route('/foo/{id}', 'foo', ['GET'], 'foo'); $router->addRoute($route); - $this->expectException(\LogicException::class); - $this->expectExceptionMessage('Not enough parameters given'); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Expected parameter values for at least [id], but received []'); $router->generateUri('foo'); } From 33403baba55fa3da3a57ed683c4d1f4448dd756a Mon Sep 17 00:00:00 2001 From: Geert Eltink Date: Thu, 23 Mar 2017 18:30:44 +0100 Subject: [PATCH 03/13] Add standard FastRoute tests for Uri generation --- test/UriGeneratorTest.php | 160 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 test/UriGeneratorTest.php diff --git a/test/UriGeneratorTest.php b/test/UriGeneratorTest.php new file mode 100644 index 0000000..5ab41e2 --- /dev/null +++ b/test/UriGeneratorTest.php @@ -0,0 +1,160 @@ + '/test/{param:\d+}', + 'test_param_regex_limit' => '/test/{ param : \d{1,9} }', + 'test_optional' => '/test[opt]', + 'test_optional_param' => '/test[/{param}]', + 'param_and_opt' => '/{param}[opt]', + 'test_double_opt' => '/test[/{param}[/{id:[0-9]+}]]', + 'empty' => '', + 'optional_text' => '[test]', + 'root_and_text' => '/{foo-bar}', + 'root_and_regex' => '/{_foo:.*}', + ]; + } + + /** + * @return array + */ + public function provideRouteTests() + { + return [ + // path // substitutions[] // expected // exception + ['/test', [], '/test', null], + + ['/test/{param}', ['param' => 'foo'], '/test/foo', null], + [ + '/test/{param}', + ['id' => 'foo'], + InvalidArgumentException::class, + 'Expected parameter values for at least [param], but received [id]', + ], + + ['/te{ param }st', ['param' => 'foo'], '/tefoost', null], + + ['/test/{param1}/test2/{param2}', ['param1' => 'foo', 'param2' => 'bar'], '/test/foo/test2/bar', null], + + ['/test/{param:\d+}', ['param' => 1], '/test/1', null], + //['/test/{param:\d+}', ['param' => 'foo'], 'exception', null], + + ['/test/{ param : \d{1,9} }', ['param' => 1], '/test/1', null], + ['/test/{ param : \d{1,9} }', ['param' => 123456789], '/test/123456789', null], + ['/test/{ param : \d{1,9} }', ['param' => 0], '/test/0', null], + [ + '/test/{ param : \d{1,9} }', + ['param' => 1234567890], + InvalidArgumentException::class, + 'Parameter value for [param] did not match the regex `\d{1,9}`', + ], + + ['/test[opt]', [], '/testopt', null], + + ['/test[/{param}]', [], '/test', null], + ['/test[/{param}]', ['param' => 'foo'], '/test/foo', null], + + ['/{param}[opt]', ['param' => 'foo'], '/fooopt', null], + + ['/test[/{param}[/{id:[0-9]+}]]', [], '/test', null], + ['/test[/{param}[/{id:[0-9]+}]]', ['param' => 'foo'], '/test/foo', null], + ['/test[/{param}[/{id:[0-9]+}]]', ['param' => 'foo', 'id' => 1], '/test/foo/1', null], + ['/test[/{param}[/{id:[0-9]+}]]', ['id' => 1], '/test', null], + [ + '/test[/{param}[/{id:[0-9]+}]]', + ['param' => 'foo', 'id' => 'foo'], + InvalidArgumentException::class, + 'Parameter value for [id] did not match the regex `[0-9]+`', + ], + + ['', [], '', null], + + ['[test]', [], 'test', null], + + ['/{foo-bar}', ['foo-bar' => 'bar'], '/bar', null], + + ['/{_foo:.*}', ['_foo' => 'bar'], '/bar', null], + ]; + } + + protected function setUp() + { + $this->fastRouter = $this->prophesize(RouteCollector::class); + $this->dispatcher = $this->prophesize(Dispatcher::class); + $this->dispatchCallback = function () { + return $this->dispatcher->reveal(); + }; + + $this->router = new FastRouteRouter( + $this->fastRouter->reveal(), + $this->dispatchCallback + ); + } + + /** + * @param $path + * @param $substitutions + * @param $expected + * @param $message + * + * @dataProvider provideRouteTests + */ + public function testRoutes($path, $substitutions, $expected, $message) + { + $this->router->addRoute(new Route($path, 'foo', ['GET'], 'foo')); + + if ($message !== null) { + // Test exceptions + $this->expectException($expected); + $this->expectExceptionMessage($message); + + $this->router->generateUri('foo', $substitutions); + + return; + } + + $this->assertEquals($expected, $this->router->generateUri('foo', $substitutions)); + + // Test with extra parameter + $substitutions['extra'] = 'parameter'; + $this->assertEquals($expected, $this->router->generateUri('foo', $substitutions)); + } +} From 8edc7e5cafb0866fceec25351cb6e5100213a81e Mon Sep 17 00:00:00 2001 From: Geert Eltink Date: Thu, 23 Mar 2017 18:41:25 +0100 Subject: [PATCH 04/13] Fix code standard violation --- src/FastRouteRouter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/FastRouteRouter.php b/src/FastRouteRouter.php index 8ce25de..8be2f60 100644 --- a/src/FastRouteRouter.php +++ b/src/FastRouteRouter.php @@ -287,7 +287,7 @@ public function generateUri($name, array $substitutions = [], array $options = [ // Check substitute value with regex $regex = '~^' . str_replace('/', '\/', $part[1]) . '$~'; - if (!preg_match($regex, $substitutions[$part[0]])) { + if (! preg_match($regex, $substitutions[$part[0]])) { throw new Exception\InvalidArgumentException(sprintf( 'Parameter value for [%s] did not match the regex `%s`', $part[0], From a4239339a99c38ab7206802ec1f1a1ac3b21ee5d Mon Sep 17 00:00:00 2001 From: Geert Eltink Date: Thu, 23 Mar 2017 18:46:19 +0100 Subject: [PATCH 05/13] Fix docheaders --- test/FastRouteRouterTest.php | 2 +- test/UriGeneratorTest.php | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/test/FastRouteRouterTest.php b/test/FastRouteRouterTest.php index f4f45db..14c949d 100644 --- a/test/FastRouteRouterTest.php +++ b/test/FastRouteRouterTest.php @@ -1,7 +1,7 @@ Date: Thu, 23 Mar 2017 19:00:54 +0100 Subject: [PATCH 06/13] Update dockblocks and comments --- src/FastRouteRouter.php | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/FastRouteRouter.php b/src/FastRouteRouter.php index 8be2f60..6a78aef 100644 --- a/src/FastRouteRouter.php +++ b/src/FastRouteRouter.php @@ -228,22 +228,19 @@ public function match(Request $request) * Generate a URI based on a given route. * * Replacements in FastRoute are written as `{name}` or `{name:}`; - * this method uses a regular expression to search for substitutions that - * match, and replaces them with the value provided. - * - * It does *not* use the pattern to validate that the substitution value is - * valid beforehand, however. + * this method uses `FastRoute\RouteParser\Std` to search for the best route match + * based on the available substitutions and generates a uri. * * @param string $name Route name. * @param array $substitutions Key/value pairs to substitute into the route * pattern. * @param array $options Key/value option pairs to pass to the router for - * purposes of generating a URI; takes precedence over options present - * in route used to generate URI. + * purposes of generating a URI; takes precedence over + * options present in route used to generate URI. * * @return string URI path generated. - * @throws Exception\InvalidArgumentException if the route name is not - * known. + * @throws Exception\InvalidArgumentException if the route name is not known or + * or a parameter value does not match its regex. */ public function generateUri($name, array $substitutions = [], array $options = []) { @@ -278,9 +275,11 @@ public function generateUri($name, array $substitutions = [], array $options = [ continue; } + // Generate the path $path = ''; foreach ($parts as $part) { if (is_string($part)) { + // Append the string $path .= $part; continue; } @@ -295,12 +294,15 @@ public function generateUri($name, array $substitutions = [], array $options = [ )); } + // Append the substituted value $path .= $substitutions[$part[0]]; } + // Return generated path return $path; } + // No valid route was found; explain which minimal required parameters are needed throw new Exception\InvalidArgumentException(sprintf( 'Expected parameter values for at least [%s], but received [%s]', implode(',', $requiredParameters), From 7dd04c2eb7eab012631fc36cf93b8c23754cbe6f Mon Sep 17 00:00:00 2001 From: Geert Eltink Date: Thu, 23 Mar 2017 19:12:25 +0100 Subject: [PATCH 07/13] Update dockblocks and comments for hasRequiredParameters --- src/FastRouteRouter.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/FastRouteRouter.php b/src/FastRouteRouter.php index 6a78aef..42028ad 100644 --- a/src/FastRouteRouter.php +++ b/src/FastRouteRouter.php @@ -310,10 +310,19 @@ public function generateUri($name, array $substitutions = [], array $options = [ )); } - private function hasRequiredParameters($parts, $substitutions) + /** + * Checks if all route parameters are available + * + * @param array $parts + * @param array $substitutions + * + * @return array|bool + */ + private function hasRequiredParameters(array $parts, array $substitutions) { $requiredParameters = []; + // Gather required parameters foreach ($parts as $part) { if (is_string($part)) { continue; @@ -322,12 +331,16 @@ private function hasRequiredParameters($parts, $substitutions) $requiredParameters[] = $part[0]; } + // check if all required parameters exist foreach ($requiredParameters as $param) { if (! isset($substitutions[$param])) { + // Return the required parameters so they can be used in an + // exception if needed return $requiredParameters; } } + // All required parameters are available return true; } From 849873dbf3874f72aad26150271b58c411fe0f83 Mon Sep 17 00:00:00 2001 From: Geert Eltink Date: Sat, 25 Mar 2017 16:22:20 +0100 Subject: [PATCH 08/13] Rename required parameters to missing parameters for clarification --- src/FastRouteRouter.php | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/FastRouteRouter.php b/src/FastRouteRouter.php index 42028ad..cad20a3 100644 --- a/src/FastRouteRouter.php +++ b/src/FastRouteRouter.php @@ -261,17 +261,17 @@ public function generateUri($name, array $substitutions = [], array $options = [ $substitutions = array_merge($options['defaults'], $substitutions); } - $routeParser = new RouteParser(); - $routes = array_reverse($routeParser->parse($route->getPath())); - $requiredParameters = []; + $routeParser = new RouteParser(); + $routes = array_reverse($routeParser->parse($route->getPath())); + $missingParameters = []; // One route pattern can correspond to multiple routes if it has optional parts foreach ($routes as $parts) { // Check if all parameters can be substituted - $requiredParameters = $this->hasRequiredParameters($parts, $substitutions); + $missingParameters = $this->missingParameters($parts, $substitutions); // If not all parameters can be substituted, try the next route - if ($requiredParameters !== true) { + if (! empty($missingParameters)) { continue; } @@ -302,25 +302,26 @@ public function generateUri($name, array $substitutions = [], array $options = [ return $path; } - // No valid route was found; explain which minimal required parameters are needed + // No valid route was found: list minimal required parameters throw new Exception\InvalidArgumentException(sprintf( 'Expected parameter values for at least [%s], but received [%s]', - implode(',', $requiredParameters), + implode(',', $missingParameters), implode(',', array_keys($substitutions)) )); } /** - * Checks if all route parameters are available + * Checks for any missing route parameters * * @param array $parts * @param array $substitutions * - * @return array|bool + * @return array with minimum required parameters if any are missing + * or an empty array if none are missing */ - private function hasRequiredParameters(array $parts, array $substitutions) + private function missingParameters(array $parts, array $substitutions) { - $requiredParameters = []; + $missingParameters = []; // Gather required parameters foreach ($parts as $part) { @@ -328,20 +329,20 @@ private function hasRequiredParameters(array $parts, array $substitutions) continue; } - $requiredParameters[] = $part[0]; + $missingParameters[] = $part[0]; } - // check if all required parameters exist - foreach ($requiredParameters as $param) { + // Check if all parameters exist + foreach ($missingParameters as $param) { if (! isset($substitutions[$param])) { - // Return the required parameters so they can be used in an + // Return the parameters so they can be used in an // exception if needed - return $requiredParameters; + return $missingParameters; } } // All required parameters are available - return true; + return []; } /** From 744be0a16513f50077d6414940f90b0e4fd6c0e6 Mon Sep 17 00:00:00 2001 From: Geert Eltink Date: Sat, 25 Mar 2017 16:26:18 +0100 Subject: [PATCH 09/13] Add route name to the missing parameters exception --- src/FastRouteRouter.php | 3 ++- test/FastRouteRouterTest.php | 2 +- test/UriGeneratorTest.php | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/FastRouteRouter.php b/src/FastRouteRouter.php index cad20a3..8c85b6a 100644 --- a/src/FastRouteRouter.php +++ b/src/FastRouteRouter.php @@ -304,7 +304,8 @@ public function generateUri($name, array $substitutions = [], array $options = [ // No valid route was found: list minimal required parameters throw new Exception\InvalidArgumentException(sprintf( - 'Expected parameter values for at least [%s], but received [%s]', + 'Route `%s` expects al least parameter values for [%s], but received [%s]', + $name, implode(',', $missingParameters), implode(',', array_keys($substitutions)) )); diff --git a/test/FastRouteRouterTest.php b/test/FastRouteRouterTest.php index 14c949d..ff2d84c 100644 --- a/test/FastRouteRouterTest.php +++ b/test/FastRouteRouterTest.php @@ -630,7 +630,7 @@ public function testGenerateUriRaisesExceptionForMissingMandatoryParameters() $router->addRoute($route); $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Expected parameter values for at least [id], but received []'); + $this->expectExceptionMessage('expects al least parameter values for'); $router->generateUri('foo'); } diff --git a/test/UriGeneratorTest.php b/test/UriGeneratorTest.php index 117aa9d..0458c71 100644 --- a/test/UriGeneratorTest.php +++ b/test/UriGeneratorTest.php @@ -72,7 +72,7 @@ public function provideRouteTests() '/test/{param}', ['id' => 'foo'], InvalidArgumentException::class, - 'Expected parameter values for at least [param], but received [id]', + 'expects al least parameter values for', ], ['/te{ param }st', ['param' => 'foo'], '/tefoost', null], From 2479fb07f42baf95824b7526c710ba1c7c6282a8 Mon Sep 17 00:00:00 2001 From: Geert Eltink Date: Sat, 25 Mar 2017 16:30:29 +0100 Subject: [PATCH 10/13] Fix typo --- src/FastRouteRouter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/FastRouteRouter.php b/src/FastRouteRouter.php index 8c85b6a..4e5907f 100644 --- a/src/FastRouteRouter.php +++ b/src/FastRouteRouter.php @@ -304,7 +304,7 @@ public function generateUri($name, array $substitutions = [], array $options = [ // No valid route was found: list minimal required parameters throw new Exception\InvalidArgumentException(sprintf( - 'Route `%s` expects al least parameter values for [%s], but received [%s]', + 'Route `%s` expects at least parameter values for [%s], but received [%s]', $name, implode(',', $missingParameters), implode(',', array_keys($substitutions)) From fc88301fc270f4a07201a0145c354c4281a33864 Mon Sep 17 00:00:00 2001 From: Geert Eltink Date: Sat, 25 Mar 2017 16:31:43 +0100 Subject: [PATCH 11/13] Fix all typos!!! --- test/FastRouteRouterTest.php | 2 +- test/UriGeneratorTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/FastRouteRouterTest.php b/test/FastRouteRouterTest.php index ff2d84c..bd183d6 100644 --- a/test/FastRouteRouterTest.php +++ b/test/FastRouteRouterTest.php @@ -630,7 +630,7 @@ public function testGenerateUriRaisesExceptionForMissingMandatoryParameters() $router->addRoute($route); $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('expects al least parameter values for'); + $this->expectExceptionMessage('expects at least parameter values for'); $router->generateUri('foo'); } diff --git a/test/UriGeneratorTest.php b/test/UriGeneratorTest.php index 0458c71..575c216 100644 --- a/test/UriGeneratorTest.php +++ b/test/UriGeneratorTest.php @@ -72,7 +72,7 @@ public function provideRouteTests() '/test/{param}', ['id' => 'foo'], InvalidArgumentException::class, - 'expects al least parameter values for', + 'expects at least parameter values for', ], ['/te{ param }st', ['param' => 'foo'], '/tefoost', null], From b9fed3519c8b742249e77085ced66151eb5b2a66 Mon Sep 17 00:00:00 2001 From: Geert Eltink Date: Fri, 28 Apr 2017 10:57:41 +0200 Subject: [PATCH 12/13] Fix typo and docblock formatting --- src/FastRouteRouter.php | 46 ++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/FastRouteRouter.php b/src/FastRouteRouter.php index 4e5907f..d9e6f31 100644 --- a/src/FastRouteRouter.php +++ b/src/FastRouteRouter.php @@ -132,11 +132,11 @@ class FastRouteRouter implements RouterInterface * RouteGenerator. * - A callable that returns a GroupCountBased dispatcher will be created. * - * @param null|RouteCollector $router If not provided, a default - * implementation will be used. - * @param null|callable $dispatcherFactory Callable that will return a - * FastRoute dispatcher. - * @param array $config Array of custom configuration options. + * @param null|RouteCollector $router If not provided, a default + * implementation will be used. + * @param null|callable $dispatcherFactory Callable that will return a + * FastRoute dispatcher. + * @param array $config Array of custom configuration options. */ public function __construct( RouteCollector $router = null, @@ -167,11 +167,11 @@ private function loadConfig(array $config = null) } if (isset($config[self::CONFIG_CACHE_ENABLED])) { - $this->cacheEnabled = (bool) $config[self::CONFIG_CACHE_ENABLED]; + $this->cacheEnabled = (bool)$config[self::CONFIG_CACHE_ENABLED]; } if (isset($config[self::CONFIG_CACHE_FILE])) { - $this->cacheFile = (string) $config[self::CONFIG_CACHE_FILE]; + $this->cacheFile = (string)$config[self::CONFIG_CACHE_FILE]; } if ($this->cacheEnabled) { @@ -228,19 +228,19 @@ public function match(Request $request) * Generate a URI based on a given route. * * Replacements in FastRoute are written as `{name}` or `{name:}`; - * this method uses `FastRoute\RouteParser\Std` to search for the best route match - * based on the available substitutions and generates a uri. + * this method uses `FastRoute\RouteParser\Std` to search for the best route + * match based on the available substitutions and generates a uri. * - * @param string $name Route name. - * @param array $substitutions Key/value pairs to substitute into the route - * pattern. - * @param array $options Key/value option pairs to pass to the router for - * purposes of generating a URI; takes precedence over - * options present in route used to generate URI. + * @param string $name Route name. + * @param array $substitutions Key/value pairs to substitute into the route + * pattern. + * @param array $options Key/value option pairs to pass to the router for + * purposes of generating a URI; takes precedence over options present + * in route used to generate URI. * * @return string URI path generated. - * @throws Exception\InvalidArgumentException if the route name is not known or - * or a parameter value does not match its regex. + * @throws Exception\InvalidArgumentException if the route name is not known + * or a parameter value does not match its regex. */ public function generateUri($name, array $substitutions = [], array $options = []) { @@ -317,8 +317,8 @@ public function generateUri($name, array $substitutions = [], array $options = [ * @param array $parts * @param array $substitutions * - * @return array with minimum required parameters if any are missing - * or an empty array if none are missing + * @return array with minimum required parameters if any are missing or + * an empty array if none are missing */ private function missingParameters(array $parts, array $substitutions) { @@ -363,7 +363,7 @@ private function createRouter() * (which should be derived from the router's getData() method); this * approach is done to allow testing against the dispatcher. * - * @param array|object $data Data from RouteCollection::getData() + * @param array|object $data Data from RouteCollection::getData() * * @return Dispatcher */ @@ -412,7 +412,7 @@ private function marshalFailedRoute(array $result) /** * Marshals a route result based on the results of matching and the current HTTP method. * - * @param array $result + * @param array $result * @param string $method * * @return RouteResult @@ -585,8 +585,8 @@ private function cacheDispatchData(array $dispatchData) * Call this method for failed HEAD or OPTIONS requests, to see if another * method matches; if so, return the match. * - * @param string $method - * @param string $path + * @param string $method + * @param string $path * @param Dispatcher $dispatcher * * @return false|array False if no match found, array representing the match From 1ac6fd72a10b84666b61e014b5dd67ad45370b1a Mon Sep 17 00:00:00 2001 From: Geert Eltink Date: Fri, 28 Apr 2017 11:00:21 +0200 Subject: [PATCH 13/13] Improve value substitute regex per @michaelmoussa --- src/FastRouteRouter.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/FastRouteRouter.php b/src/FastRouteRouter.php index d9e6f31..851ce14 100644 --- a/src/FastRouteRouter.php +++ b/src/FastRouteRouter.php @@ -285,8 +285,7 @@ public function generateUri($name, array $substitutions = [], array $options = [ } // Check substitute value with regex - $regex = '~^' . str_replace('/', '\/', $part[1]) . '$~'; - if (! preg_match($regex, $substitutions[$part[0]])) { + if (! preg_match('~^' . $part[1] . '$~', $substitutions[$part[0]])) { throw new Exception\InvalidArgumentException(sprintf( 'Parameter value for [%s] did not match the regex `%s`', $part[0],