From d5b52248da0b5047dd1ef19754b83da57bdd6cff Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 26 Jun 2022 21:18:53 +0900 Subject: [PATCH 1/3] fix: $routes->group('/', ...) creates the route `foo///bar` --- system/Router/RouteCollection.php | 2 +- tests/system/Router/RouteCollectionTest.php | 28 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index 4998a16c9ba6..e009b06512c7 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -625,7 +625,7 @@ public function group(string $name, ...$params) // To register a route, we'll set a flag so that our router // so it will see the group name. // If the group name is empty, we go on using the previously built group name. - $this->group = $name ? ltrim($oldGroup . '/' . $name, '/') : $oldGroup; + $this->group = $name ? trim($oldGroup . '/' . $name, '/') : $oldGroup; $callback = array_pop($params); diff --git a/tests/system/Router/RouteCollectionTest.php b/tests/system/Router/RouteCollectionTest.php index f367a203473e..06125db51fcb 100644 --- a/tests/system/Router/RouteCollectionTest.php +++ b/tests/system/Router/RouteCollectionTest.php @@ -375,6 +375,34 @@ static function ($routes) { $this->assertSame($expected, $routes->getRoutes()); } + public function testNestedGroupingWorksWithRootPrefix() + { + $routes = $this->getCollector(); + + $routes->add('verify/begin', '\VerifyController::begin'); + + $routes->group('admin', static function ($routes) { + $routes->group( + '/', + static function ($routes) { + $routes->add('users/list', '\Users::list'); + + $routes->group('delegate', static function ($routes) { + $routes->add('foo', '\Users::foo'); + }); + } + ); + }); + + $expected = [ + 'verify/begin' => '\VerifyController::begin', + 'admin/users/list' => '\Users::list', + 'admin/delegate/foo' => '\Users::foo', + ]; + + $this->assertSame($expected, $routes->getRoutes()); + } + public function testHostnameOption() { $_SERVER['HTTP_HOST'] = 'example.com'; From 106eda3fe506d1669d70842491a0f209e2aada16 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 26 Jun 2022 21:39:13 +0900 Subject: [PATCH 2/3] test: add test cases --- tests/system/Router/RouteCollectionTest.php | 44 +++++++++++++++------ 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/tests/system/Router/RouteCollectionTest.php b/tests/system/Router/RouteCollectionTest.php index 06125db51fcb..97a7d6151750 100644 --- a/tests/system/Router/RouteCollectionTest.php +++ b/tests/system/Router/RouteCollectionTest.php @@ -375,15 +375,19 @@ static function ($routes) { $this->assertSame($expected, $routes->getRoutes()); } - public function testNestedGroupingWorksWithRootPrefix() - { + /** + * @dataProvider groupProvider + */ + public function testNestedGroupingWorksWithRootPrefix( + string $group, + string $subgroup, + array $expected + ) { $routes = $this->getCollector(); - $routes->add('verify/begin', '\VerifyController::begin'); - - $routes->group('admin', static function ($routes) { + $routes->group($group, static function ($routes) use ($subgroup) { $routes->group( - '/', + $subgroup, static function ($routes) { $routes->add('users/list', '\Users::list'); @@ -394,15 +398,31 @@ static function ($routes) { ); }); - $expected = [ - 'verify/begin' => '\VerifyController::begin', - 'admin/users/list' => '\Users::list', - 'admin/delegate/foo' => '\Users::foo', - ]; - $this->assertSame($expected, $routes->getRoutes()); } + public function groupProvider() + { + yield from [ + ['admin', '/', [ + 'admin/users/list' => '\Users::list', + 'admin/delegate/foo' => '\Users::foo', + ]], + ['/', '', [ + 'users/list' => '\Users::list', + 'delegate/foo' => '\Users::foo', + ]], + ['', '', [ + 'users/list' => '\Users::list', + 'delegate/foo' => '\Users::foo', + ]], + ['', '/', [ + 'users/list' => '\Users::list', + 'delegate/foo' => '\Users::foo', + ]], + ]; + } + public function testHostnameOption() { $_SERVER['HTTP_HOST'] = 'example.com'; From 66fbd4755d2d43039b13436ff3f3fa257f851024 Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 27 Jun 2022 06:47:47 +0900 Subject: [PATCH 3/3] docs: fix comment --- system/Router/RouteCollection.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index e009b06512c7..c1e093c2380f 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -623,7 +623,7 @@ public function group(string $name, ...$params) $oldOptions = $this->currentOptions; // To register a route, we'll set a flag so that our router - // so it will see the group name. + // will see the group name. // If the group name is empty, we go on using the previously built group name. $this->group = $name ? trim($oldGroup . '/' . $name, '/') : $oldGroup;