Merge pull request #6644 from kenjis/fix-routes-processing

fix: routes registration bug

Author: kenjis

system/Router/RouteCollection.php

@@ -110,7 +110,7 @@ class RouteCollection implements RouteCollectionInterface
      *     verb => [
      *         routeName => [
      *             'route' => [
-     *                 routeKey => handler,
+     *                 routeKey(or from) => handler,
      *             ]
      *         ]
      *     ],
@@ -133,6 +133,14 @@ class RouteCollection implements RouteCollectionInterface
      * Array of routes options
      *
      * @var array
+     *
+     * [
+     *     verb => [
+     *         routeKey(or from) => [
+     *             key => value,
+     *         ]
+     *     ],
+     * ]
      */
     protected $routesOptions = [];
 
@@ -1239,12 +1247,15 @@ protected function create(string $verb, string $from, $to, ?array $options = nul
 
         $name = $options['as'] ?? $from;
 
+        helper('array');
+
         // Don't overwrite any existing 'froms' so that auto-discovered routes
         // do not overwrite any app/Config/Routes settings. The app
         // routes should always be the "source of truth".
         // this works only because discovered routes are added just prior
         // to attempting to route the request.
-        if (isset($this->routes[$verb][$name]) && ! $overwrite) {
+        $fromExists = dot_array_search('*.route.' . $from, $this->routes[$verb] ?? []) !== null;
+        if ((isset($this->routes[$verb][$name]) || $fromExists) && ! $overwrite) {
             return;
         }
 

tests/system/HTTP/RedirectResponseTest.php

@@ -73,12 +73,12 @@ public function testRedirectRoute()
         $this->assertTrue($response->hasHeader('Location'));
         $this->assertSame('http://example.com/index.php/exampleRoute', $response->getHeaderLine('Location'));
 
-        $this->routes->add('exampleRoute', 'Home::index', ['as' => 'home']);
+        $this->routes->add('exampleRoute2', 'Home::index', ['as' => 'home']);
 
         $response->route('home');
 
         $this->assertTrue($response->hasHeader('Location'));
-        $this->assertSame('http://example.com/index.php/exampleRoute', $response->getHeaderLine('Location'));
+        $this->assertSame('http://example.com/index.php/exampleRoute2', $response->getHeaderLine('Location'));
     }
 
     public function testRedirectRouteBadNamedRoute()

tests/system/Router/RouteCollectionTest.php

@@ -11,7 +11,6 @@
 
 namespace CodeIgniter\Router;
 
-use App\Controllers\Home;
 use CodeIgniter\Config\Services;
 use CodeIgniter\Test\CIUnitTestCase;
 use Config\Modules;
@@ -1189,6 +1188,73 @@ static function () {},
         $this->assertSame($options, ['as' => 'admin', 'foo' => 'baz']);
     }
 
+    /**
+     * @dataProvider optionsProvider
+     */
+    public function testRoutesOptionsWithSameFromTwoRoutes(array $options1, array $options2)
+    {
+        $routes = $this->getCollector();
+
+        // This is the first route for `administrator`.
+        $routes->get(
+            'administrator',
+            static function () {},
+            $options1
+        );
+        // The second route for `administrator` should be ignored.
+        $routes->get(
+            'administrator',
+            static function () {},
+            $options2
+        );
+
+        $options = $routes->getRoutesOptions('administrator');
+
+        $this->assertSame($options, $options1);
+    }
+
+    public function optionsProvider()
+    {
+        yield from [
+            [
+                [
+                    'foo' => 'options1',
+                ],
+                [
+                    'foo' => 'options2',
+                ],
+            ],
+            [
+                [
+                    'as'  => 'admin',
+                    'foo' => 'options1',
+                ],
+                [
+                    'foo' => 'options2',
+                ],
+            ],
+            [
+                [
+                    'foo' => 'options1',
+                ],
+                [
+                    'as'  => 'admin',
+                    'foo' => 'options2',
+                ],
+            ],
+            [
+                [
+                    'as'  => 'admin',
+                    'foo' => 'options1',
+                ],
+                [
+                    'as'  => 'admin',
+                    'foo' => 'options2',
+                ],
+            ],
+        ];
+    }
+
     public function testRoutesOptionsForDifferentVerbs()
     {
         $routes = $this->getCollector();
@@ -1460,12 +1526,12 @@ public function testRouteOverwritingTwoRules()
         $routes->setDefaultController('Home');
         $routes->setDefaultMethod('index');
 
+        // The subdomain of the current URL is `doc`, so this route is registered.
         $routes->get('/', '\App\Controllers\Site\CDoc::index', ['subdomain' => 'doc', 'as' => 'doc_index']);
+        // The subdomain route is already registered, so this route is not registered.
         $routes->get('/', 'Home::index');
 
-        // the second rule applies, so overwrites the first
-        $expects = '\\' . Home::class;
-
+        $expects = '\App\Controllers\Site\CDoc';
         $this->assertSame($expects, $router->handle('/'));
     }