Throw error if required route params missing (#3118)

Co-authored-by: Daniël Klabbers <daniel@klabbers.email> Co-authored-by: luceos <luceos@users.noreply.github.com> Co-authored-by: David Wheatley <hi@davwheat.dev> Co-authored-by: Sami Mazouz <sychocouldy@gmail.com>
flarum · Oct 23, 2021 · 87f6774 · 87f6774
1 parent 4add23a
commit 87f6774
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 5 deletions.
diff --git a/src/Http/RouteCollection.php b/src/Http/RouteCollection.php
@@ -121,11 +121,17 @@ public function getRouteData()
         return $this->dataGenerator->getData();
     }
 
-    protected function fixPathPart(&$part, $key, array $parameters)
+    protected function fixPathPart($part, array $parameters, string $routeName)
     {
-        if (is_array($part) && array_key_exists($part[0], $parameters)) {
-            $part = $parameters[$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 = [])
@@ -151,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
@@ -44,4 +44,41 @@ public function can_add_routes_late()
 
         $this->assertEquals('/posts', $routeCollection->getPath('forum.posts.delete'));
     }
+
+    /** @test */
+    public function must_provide_required_parameters()
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        $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';
+        });
+
+        $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);
+    }
+
+    /** @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);
+    }
 }