diff --git a/system/Router/RouteCollection.php b/system/Router/RouteCollection.php index ca080152ff28..66d6f276c334 100644 --- a/system/Router/RouteCollection.php +++ b/system/Router/RouteCollection.php @@ -771,7 +771,19 @@ public function group(string $name, ...$params) $callback = array_pop($params); if ($params && is_array($params[0])) { - $this->currentOptions = array_shift($params); + $options = array_shift($params); + + if (isset($options['filter'])) { + // Merge filters. + $currentFilter = (array) ($this->currentOptions['filter'] ?? []); + $options['filter'] = array_merge($currentFilter, (array) $options['filter']); + } + + // Merge options other than filters. + $this->currentOptions = array_merge( + $this->currentOptions ?? [], + $options + ); } if (is_callable($callback)) { diff --git a/tests/system/Router/RouteCollectionTest.php b/tests/system/Router/RouteCollectionTest.php index 37844fed6cdb..d6b7e0dfe6ac 100644 --- a/tests/system/Router/RouteCollectionTest.php +++ b/tests/system/Router/RouteCollectionTest.php @@ -337,6 +337,104 @@ static function ($routes): void { $this->assertSame($expected, $routes->getRoutes()); } + public function testGroupNestedWithOuterOptionsWithoutInnerOptions(): void + { + $routes = $this->getCollector(); + + $routes->group( + 'admin', + ['namespace' => 'Admin', 'filter' => ['csrf']], + static function ($routes) { + $routes->get('dashboard', static function () { + }); + + $routes->group('profile', static function ($routes) { + $routes->get('/', static function () { + }); + }); + } + ); + + $expected = [ + 'admin/dashboard' => [ + 'namespace' => 'Admin', + 'filter' => ['csrf'], + ], + 'admin/profile' => [ + 'namespace' => 'Admin', + 'filter' => ['csrf'], + ], + ]; + $this->assertSame($expected, $routes->getRoutesOptions()); + } + + public function testGroupNestedWithOuterAndInnerOption(): void + { + $routes = $this->getCollector(); + + $routes->group( + 'admin', + ['filter' => ['csrf']], + static function ($routes) { + $routes->get('dashboard', static function () { + }); + + $routes->group( + 'profile', + ['filter' => ['honeypot']], + static function ($routes) { + $routes->get('/', static function () { + }); + } + ); + } + ); + + $expected = [ + 'admin/dashboard' => [ + 'filter' => ['csrf'], + ], + 'admin/profile' => [ + 'filter' => ['csrf', 'honeypot'], + ], + ]; + $this->assertSame($expected, $routes->getRoutesOptions()); + } + + public function testGroupNestedWithoutOuterOptionWithInnerOption(): void + { + $routes = $this->getCollector(); + + $routes->group( + 'admin', + ['filter' => 'csrf'], + static function ($routes) { + $routes->get('dashboard', static function () { + }); + + $routes->group( + 'profile', + ['namespace' => 'Admin'], + static function ($routes) { + $routes->get('/', static function () { + }); + } + ); + } + ); + + $expected = [ + 'admin/dashboard' => [ + 'filter' => ['csrf'], + ], + 'admin/profile' => [ + 'filter' => ['csrf'], + 'namespace' => 'Admin', + ], + ]; + $this->assertSame($expected, $routes->getRoutesOptions()); + } + public function testGroupingWorksWithEmptyStringPrefix(): void { $routes = $this->getCollector(); diff --git a/user_guide_src/source/changelogs/v4.5.0.rst b/user_guide_src/source/changelogs/v4.5.0.rst index ad96b7574b92..7f03e498b152 100644 --- a/user_guide_src/source/changelogs/v4.5.0.rst +++ b/user_guide_src/source/changelogs/v4.5.0.rst @@ -27,6 +27,13 @@ Filter Execution Order The order in which Controller Filters are executed has changed. See :ref:`Upgrading Guide ` for details. +Nested Route Groups and Options +------------------------------- + +Due to a bug fix, the behavior has changed so that options passed to the outer +``group()`` are merged with the options of the inner ``group()``. +See :ref:`Upgrading Guide ` for details. + Others ------ diff --git a/user_guide_src/source/incoming/routing.rst b/user_guide_src/source/incoming/routing.rst index f956c68fb4df..39b5d3db8cdd 100644 --- a/user_guide_src/source/incoming/routing.rst +++ b/user_guide_src/source/incoming/routing.rst @@ -535,6 +535,8 @@ given route config options: .. literalinclude:: routing/027.php +.. _routing-nesting-groups: + Nesting Groups ============== @@ -544,7 +546,15 @@ It is possible to nest groups within groups for finer organization if you need i This would handle the URL at **admin/users/list**. -.. note:: Options passed to the outer ``group()`` (for example ``namespace`` and ``filter``) are not merged with the inner ``group()`` options. +**Filter** option passed to the outer ``group()`` are merged with the inner +``group()`` filter option. +The above code runs ``myfilter:config`` for the route ``admin``, and ``myfilter:config`` +and ``myfilter:region`` for the route ``admin/users/list``. + +Any other overlapping options passed to the inner `group()` will overwrite their values. + +.. note:: Prior to v4.5.0, due to a bug, options passed to the outer ``group()`` + are not merged with the inner ``group()`` options. .. _routing-priority: diff --git a/user_guide_src/source/incoming/routing/026.php b/user_guide_src/source/incoming/routing/026.php index 1ed2a8a96ee6..b83e33fce0cb 100644 --- a/user_guide_src/source/incoming/routing/026.php +++ b/user_guide_src/source/incoming/routing/026.php @@ -1,7 +1,9 @@ group('admin', static function ($routes) { - $routes->group('users', static function ($routes) { +$routes->group('admin', ['filter' => 'myfilter:config'], static function ($routes) { + $routes->get('/', 'Admin\Admin::index'); + + $routes->group('users', ['filter' => 'myfilter:region'], static function ($routes) { $routes->get('list', 'Admin\Users::list'); }); }); diff --git a/user_guide_src/source/installation/upgrade_450.rst b/user_guide_src/source/installation/upgrade_450.rst index 878ecbd1ed21..db022059d585 100644 --- a/user_guide_src/source/installation/upgrade_450.rst +++ b/user_guide_src/source/installation/upgrade_450.rst @@ -18,6 +18,37 @@ Mandatory File Changes Breaking Changes **************** +.. _upgrade-450-nested-route-groups-and-options: + +Nested Route Groups and Options +=============================== + +A bug that prevented options passed to outer ``group()`` from being merged with +options in inner ``group()`` has been fixed. + +Check and correct your route configuration as it could change the values of the +options applied. + +For example, + +.. code-block:: php + + $routes->group('admin', ['filter' => 'csrf'], static function ($routes) { + $routes->get('/', static function () { + // ... + }); + + $routes->group('users', ['namespace' => 'Users'], static function ($routes) { + $routes->get('/', static function () { + // ... + }); + }); + }); + +Now the ``csrf`` filter is executed for both the route ``admin`` and ``admin/users``. +In previous versions, it is executed only for the route ``admin``. +See also :ref:`routing-nesting-groups`. + Method Signature Changes ========================