Merge pull request #12256 from cakephp/issue-12130

Fix duplicate route errors when using loadPlugin()

src/Routing/RouteBuilder.php

@@ -602,28 +602,45 @@ class RouteBuilder
      * the current RouteBuilder instance.
      *
      * @param string $name The plugin name
-     * @param string $file The routes file to load. Defaults to `routes.php`
+     * @param string $file The routes file to load. Defaults to `routes.php`. This parameter
+     *   is deprecated and will be removed in 4.0
      * @return void
      * @throws \Cake\Core\Exception\MissingPluginException When the plugin has not been loaded.
      * @throws \InvalidArgumentException When the plugin does not have a routes file.
      */
     public function loadPlugin($name, $file = 'routes.php')
     {
-        if (!Plugin::loaded($name)) {
+        $plugins = Plugin::getCollection();
+        if (!$plugins->has($name)) {
             throw new MissingPluginException(['plugin' => $name]);
         }
+        $plugin = $plugins->get($name);
 
-        $path = Plugin::configPath($name) . DIRECTORY_SEPARATOR . $file;
-        if (!file_exists($path)) {
-            throw new InvalidArgumentException(sprintf(
-                'Cannot load routes for the plugin named %s. The %s file does not exist.',
-                $name,
-                $path
-            ));
+        // @deprecated This block should be removed in 4.0
+        if ($file !== 'routes.php') {
+            deprecationWarning(
+                'Loading plugin routes now uses the routes() hook method on the plugin class. ' .
+                'Loading non-standard files will be removed in 4.0'
+            );
+
+            $path = $plugin->getConfigPath() . DIRECTORY_SEPARATOR . $file;
+            if (!file_exists($path)) {
+                throw new InvalidArgumentException(sprintf(
+                    'Cannot load routes for the plugin named %s. The %s file does not exist.',
+                    $name,
+                    $path
+                ));
+            }
+
+            $routes = $this;
+            include $path;
+
+            return;
         }
+        $plugin->routes($this);
 
-        $routes = $this;
-        include $path;
+        // Disable the routes hook to prevent duplicate route issues.
+        $plugin->disable('routes');
     }
 
     /**

tests/TestCase/Routing/RouteBuilderTest.php

@@ -1206,11 +1206,13 @@ class RouteBuilderTest extends TestCase
      */
     public function testLoadPluginBadFile()
     {
-        $this->expectException(\InvalidArgumentException::class);
-        $this->expectExceptionMessage('Cannot load routes for the plugin named TestPlugin.');
-        Plugin::load('TestPlugin');
-        $routes = new RouteBuilder($this->collection, '/');
-        $routes->loadPlugin('TestPlugin', 'nope.php');
+        $this->deprecated(function () {
+            $this->expectException(\InvalidArgumentException::class);
+            $this->expectExceptionMessage('Cannot load routes for the plugin named TestPlugin.');
+            Plugin::load('TestPlugin');
+            $routes = new RouteBuilder($this->collection, '/');
+            $routes->loadPlugin('TestPlugin', 'nope.php');
+        });
     }
 
     /**
@@ -1225,6 +1227,9 @@ class RouteBuilderTest extends TestCase
         $routes->loadPlugin('TestPlugin');
         $this->assertCount(1, $this->collection->routes());
         $this->assertNotEmpty($this->collection->parse('/test_plugin', 'GET'));
+
+        $plugin = Plugin::getCollection()->get('TestPlugin');
+        $this->assertFalse($plugin->isEnabled('routes'), 'Hook should be disabled preventing duplicate routes');
     }
 
     /**

tests/TestCase/TestSuite/IntegrationTestCaseTest.php

@@ -15,6 +15,7 @@
 namespace Cake\Test\TestCase\TestSuite;
 
 use Cake\Core\Configure;
+use Cake\Core\Plugin;
 use Cake\Event\EventManager;
 use Cake\Http\Response;
 use Cake\Routing\DispatcherFactory;
@@ -52,6 +53,7 @@ class IntegrationTestCaseTest extends IntegrationTestCase
         Router::$initialized = true;
 
         $this->useHttpServer(true);
+        $this->configApplication(Configure::read('App.namespace') . '\Application', null);
         DispatcherFactory::clear();
     }
 
@@ -281,6 +283,23 @@ class IntegrationTestCaseTest extends IntegrationTestCase
      *
      * @return void
      */
+    public function testGetUsingApplicationWithPluginRoutes()
+    {
+        // first clean routes to have Router::$initailized === false
+        Router::reload();
+        Plugin::unload();
+
+        $this->configApplication(Configure::read('App.namespace') . '\ApplicationWithPluginRoutes', null);
+
+        $this->get('/test_plugin');
+        $this->assertResponseOk();
+    }
+
+    /**
+     * Test sending get request and using default `test_app/config/routes.php`.
+     *
+     * @return void
+     */
     public function testGetUsingApplicationWithDefaultRoutes()
     {
         // first clean routes to have Router::$initailized === false

tests/test_app/Plugin/TestPlugin/config/routes.php

@@ -4,5 +4,9 @@ use Cake\Core\Configure;
 Configure::write('PluginTest.test_plugin.routes', 'loaded plugin routes');
 
 if (isset($routes)) {
-    $routes->get('/test_plugin', ['controller' => 'TestPlugin', 'plugin' => 'TestPlugin', 'action' => 'index']);
+    $routes->get(
+        '/test_plugin',
+        ['controller' => 'TestPlugin', 'plugin' => 'TestPlugin', 'action' => 'index'],
+        'test_plugin:index'
+    );
 }

tests/test_app/TestApp/ApplicationWithPluginRoutes.php

@@ -0,0 +1,48 @@
+<?php
+/**
+ * CakePHP(tm) : Rapid Development Framework (https://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ * @link          https://cakephp.org CakePHP(tm) Project
+ * @since         3.6.6
+ * @license       https://opensource.org/licenses/mit-license.php MIT License
+ */
+namespace TestApp;
+
+use Cake\Http\BaseApplication;
+use Cake\Routing\Middleware\RoutingMiddleware;
+
+class ApplicationWithPluginRoutes extends BaseApplication
+{
+    public function bootstrap()
+    {
+        parent::bootstrap();
+        $this->addPlugin('TestPlugin');
+    }
+
+    public function middleware($middleware)
+    {
+        $middleware->add(new RoutingMiddleware($this));
+
+        return $middleware;
+    }
+
+    /**
+     * Routes hook, used for testing with RoutingMiddleware.
+     *
+     * @param \Cake\Routing\RouteBuilder $routes
+     * @return void
+     */
+    public function routes($routes)
+    {
+        $routes->scope('/app', function ($routes) {
+            $routes->connect('/articles', ['controller' => 'Articles']);
+        });
+        $routes->loadPlugin('TestPlugin');
+    }
+}