Merge pull request #8033 from kenjis/fix-route-option-merge-2

fix: route options are not merged (outer filter is merged with inner filter)

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)) {

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();

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 <upgrade-450-filter-execution-order>` 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 <upgrade-450-nested-route-groups-and-options>` for details.
+
 Others
 ------
 

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:
 

user_guide_src/source/incoming/routing/026.php

@@ -1,7 +1,9 @@
 <?php
 
-$routes->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');
     });
 });

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
 ========================