From 725e8138a547b628c8f72c4b65011b5e278799af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Klabbers?= Date: Tue, 8 Jun 2021 01:43:56 +0200 Subject: [PATCH 1/6] prevents missing route params fixes #2513 --- src/Http/RouteCollection.php | 4 ++++ tests/unit/Http/RouteCollectionTest.php | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/src/Http/RouteCollection.php b/src/Http/RouteCollection.php index 78c6998e24..c0f721440e 100644 --- a/src/Http/RouteCollection.php +++ b/src/Http/RouteCollection.php @@ -123,8 +123,12 @@ public function getRouteData() protected function fixPathPart(&$part, $key, array $parameters) { + // This assumes that $parameters always has the required param + // the route needs. This isn't always the case. if (is_array($part) && array_key_exists($part[0], $parameters)) { $part = $parameters[$part[0]]; + } elseif(is_array($part) && ! array_key_exists($part[0], $parameters)) { + throw new \InvalidArgumentException("Route is missing argument for part $part[0]."); } } diff --git a/tests/unit/Http/RouteCollectionTest.php b/tests/unit/Http/RouteCollectionTest.php index 968ed93402..ad29403904 100644 --- a/tests/unit/Http/RouteCollectionTest.php +++ b/tests/unit/Http/RouteCollectionTest.php @@ -44,4 +44,17 @@ public function can_add_routes_late() $this->assertEquals('/posts', $routeCollection->getPath('forum.posts.delete')); } + + /** @test */ + public function can_add_empty_parameters() + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage("Route is missing argument for part user."); + + $routeCollection = (new RouteCollection)->addRoute('GET', '/user/{user}', 'user', function () { + echo 'user'; + }); + + $routeCollection->getPath('user', []); + } } From 009779e2b8b107652e376b4358c0e1896ac1fa02 Mon Sep 17 00:00:00 2001 From: luceos Date: Mon, 7 Jun 2021 23:44:56 +0000 Subject: [PATCH 2/6] Apply fixes from StyleCI [ci skip] [skip ci] --- src/Http/RouteCollection.php | 2 +- tests/unit/Http/RouteCollectionTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/RouteCollection.php b/src/Http/RouteCollection.php index c0f721440e..b2e3d3d6dc 100644 --- a/src/Http/RouteCollection.php +++ b/src/Http/RouteCollection.php @@ -127,7 +127,7 @@ protected function fixPathPart(&$part, $key, array $parameters) // the route needs. This isn't always the case. if (is_array($part) && array_key_exists($part[0], $parameters)) { $part = $parameters[$part[0]]; - } elseif(is_array($part) && ! array_key_exists($part[0], $parameters)) { + } elseif (is_array($part) && ! array_key_exists($part[0], $parameters)) { throw new \InvalidArgumentException("Route is missing argument for part $part[0]."); } } diff --git a/tests/unit/Http/RouteCollectionTest.php b/tests/unit/Http/RouteCollectionTest.php index ad29403904..f6afda47a9 100644 --- a/tests/unit/Http/RouteCollectionTest.php +++ b/tests/unit/Http/RouteCollectionTest.php @@ -49,7 +49,7 @@ public function can_add_routes_late() public function can_add_empty_parameters() { $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage("Route is missing argument for part user."); + $this->expectExceptionMessage('Route is missing argument for part user.'); $routeCollection = (new RouteCollection)->addRoute('GET', '/user/{user}', 'user', function () { echo 'user'; From 0cd86b069aa6476fad01536a76f4d67cd127e306 Mon Sep 17 00:00:00 2001 From: David Wheatley Date: Mon, 27 Sep 2021 14:47:21 +0100 Subject: [PATCH 3/6] Update unit test name Co-authored-by: Sami Mazouz --- tests/unit/Http/RouteCollectionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/Http/RouteCollectionTest.php b/tests/unit/Http/RouteCollectionTest.php index f6afda47a9..68191712f5 100644 --- a/tests/unit/Http/RouteCollectionTest.php +++ b/tests/unit/Http/RouteCollectionTest.php @@ -46,7 +46,7 @@ public function can_add_routes_late() } /** @test */ - public function can_add_empty_parameters() + public function must_provide_required_parameters() { $this->expectException(\InvalidArgumentException::class); $this->expectExceptionMessage('Route is missing argument for part user.'); From 853479e030dec28fdcaf9d46067f04352371899c Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 22 Oct 2021 11:22:07 -0400 Subject: [PATCH 4/6] Cleanup --- src/Http/RouteCollection.php | 22 +++++++++++++--------- tests/unit/Http/RouteCollectionTest.php | 16 ++++++++++++++-- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/Http/RouteCollection.php b/src/Http/RouteCollection.php index b2e3d3d6dc..e28a3eee3c 100644 --- a/src/Http/RouteCollection.php +++ b/src/Http/RouteCollection.php @@ -121,15 +121,17 @@ public function getRouteData() return $this->dataGenerator->getData(); } - protected function fixPathPart(&$part, $key, array $parameters) + protected function fixPathPart($part, array $parameters, string $routeName) { - // This assumes that $parameters always has the required param - // the route needs. This isn't always the case. - if (is_array($part) && array_key_exists($part[0], $parameters)) { - $part = $parameters[$part[0]]; - } elseif (is_array($part) && ! array_key_exists($part[0], $parameters)) { - throw new \InvalidArgumentException("Route is missing argument for part $part[0]."); + if (!is_array($part)) { + return $part; } + + if (!array_key_exists($part[0], $parameters)) { + throw new \InvalidArgumentException("Could not generate URL for route '$routeName': no value provided for required part '$part[0]'."); + } + + return $parameters[$part[0]]; } public function getPath($name, array $parameters = []) @@ -155,9 +157,11 @@ public function getPath($name, array $parameters = []) } } - array_walk($matchingParts, [$this, 'fixPathPart'], $parameters); + $fixedParts = array_map(function($part) use ($parameters, $name) { + return $this->fixPathPart($part, $parameters, $name); + }, $matchingParts); - return '/'.ltrim(implode('', $matchingParts), '/'); + return '/'.ltrim(implode('', $fixedParts), '/'); } throw new \RuntimeException("Route $name not found"); diff --git a/tests/unit/Http/RouteCollectionTest.php b/tests/unit/Http/RouteCollectionTest.php index 68191712f5..06f88124de 100644 --- a/tests/unit/Http/RouteCollectionTest.php +++ b/tests/unit/Http/RouteCollectionTest.php @@ -49,7 +49,7 @@ public function can_add_routes_late() public function must_provide_required_parameters() { $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage('Route is missing argument for part user.'); + $this->expectExceptionMessage("Could not generate URL for route 'user': no value provided for required part 'user'."); $routeCollection = (new RouteCollection)->addRoute('GET', '/user/{user}', 'user', function () { echo 'user'; @@ -57,4 +57,16 @@ public function must_provide_required_parameters() $routeCollection->getPath('user', []); } -} + + /** @test */ + public function dont_need_to_provide_optional_parameters() + { + $routeCollection = (new RouteCollection)->addRoute('GET', '/user/{user}[/{test}]', 'user', function () { + echo 'user'; + }); + + $path = $routeCollection->getPath('user', ['user' => 'SomeUser']); + + $this->assertEquals('/user/SomeUser', $path); + } +} \ No newline at end of file From 047dd327f4c51cc1b99b8abab0782948ac684ac8 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 22 Oct 2021 11:26:45 -0400 Subject: [PATCH 5/6] Add unit test for optional parameters --- tests/unit/Http/RouteCollectionTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/unit/Http/RouteCollectionTest.php b/tests/unit/Http/RouteCollectionTest.php index 06f88124de..5d51d09c52 100644 --- a/tests/unit/Http/RouteCollectionTest.php +++ b/tests/unit/Http/RouteCollectionTest.php @@ -69,4 +69,16 @@ public function dont_need_to_provide_optional_parameters() $this->assertEquals('/user/SomeUser', $path); } + + /** @test */ + public function can_provide_optional_parameters() + { + $routeCollection = (new RouteCollection)->addRoute('GET', '/user/{user}[/{test}]', 'user', function () { + echo 'user'; + }); + + $path = $routeCollection->getPath('user', ['user' => 'SomeUser', 'test' => 'Flarum']); + + $this->assertEquals('/user/SomeUser/Flarum', $path); + } } \ No newline at end of file From 9e1247d117db4e79d9ced3e840aa65b3360dc026 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 22 Oct 2021 15:26:58 +0000 Subject: [PATCH 6/6] Apply fixes from StyleCI [ci skip] [skip ci] --- src/Http/RouteCollection.php | 6 +++--- tests/unit/Http/RouteCollectionTest.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Http/RouteCollection.php b/src/Http/RouteCollection.php index e28a3eee3c..6b19b9df9d 100644 --- a/src/Http/RouteCollection.php +++ b/src/Http/RouteCollection.php @@ -123,11 +123,11 @@ public function getRouteData() protected function fixPathPart($part, array $parameters, string $routeName) { - if (!is_array($part)) { + if (! is_array($part)) { return $part; } - if (!array_key_exists($part[0], $parameters)) { + if (! array_key_exists($part[0], $parameters)) { throw new \InvalidArgumentException("Could not generate URL for route '$routeName': no value provided for required part '$part[0]'."); } @@ -157,7 +157,7 @@ public function getPath($name, array $parameters = []) } } - $fixedParts = array_map(function($part) use ($parameters, $name) { + $fixedParts = array_map(function ($part) use ($parameters, $name) { return $this->fixPathPart($part, $parameters, $name); }, $matchingParts); diff --git a/tests/unit/Http/RouteCollectionTest.php b/tests/unit/Http/RouteCollectionTest.php index 5d51d09c52..00db58d5fe 100644 --- a/tests/unit/Http/RouteCollectionTest.php +++ b/tests/unit/Http/RouteCollectionTest.php @@ -81,4 +81,4 @@ public function can_provide_optional_parameters() $this->assertEquals('/user/SomeUser/Flarum', $path); } -} \ No newline at end of file +}