From 18260ee0301baa3e2b06cd0d54377ed434d206fa Mon Sep 17 00:00:00 2001
From: Jonathan Lelievre <jo.lelievre@gmail.com>
Date: Tue, 23 Jun 2020 01:26:56 +0200
Subject: [PATCH] Auto detected controllers are valid

---
 src/Adapter/Module/Tab/ModuleTabRegister.php  | 42 +++++++++++++------
 .../Module/Tab/ModuleTabRegisterTest.php      |  9 ++--
 2 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/src/Adapter/Module/Tab/ModuleTabRegister.php b/src/Adapter/Module/Tab/ModuleTabRegister.php
index 905701b9f974f..e90b4f0618490 100644
--- a/src/Adapter/Module/Tab/ModuleTabRegister.php
+++ b/src/Adapter/Module/Tab/ModuleTabRegister.php
@@ -162,16 +162,7 @@ protected function addUndeclaredTabs($moduleName, array $tabs)
             }
         }, $tabs);
 
-        $legacyControllersFilenames = $this->getModuleAdminControllersFilename($moduleName);
-        $legacyControllers = array_map(function ($legacyControllersFilename) {
-            return str_replace('Controller.php', '', $legacyControllersFilename);
-        }, $legacyControllersFilenames);
-        $legacyControllers = $legacyControllers ?? [];
-
-        $routingControllers = $this->getModuleControllersFromRouting($moduleName);
-        $routingControllers = $routingControllers ?? [];
-
-        $detectedControllers = array_merge($legacyControllers, $routingControllers);
+        $detectedControllers = $this->getDetectedModuleControllers($moduleName);
         foreach ($detectedControllers as $adminControllerName) {
             if (in_array($adminControllerName, $tabsNames)) {
                 continue;
@@ -190,6 +181,30 @@ protected function addUndeclaredTabs($moduleName, array $tabs)
         return $tabs;
     }
 
+    /**
+     * Returns a list of all detected controllers, either from admin/controllers folder
+     * or from the routing file.
+     *
+     * @param string $moduleName
+     *
+     * @return array
+     *
+     * @throws Exception
+     */
+    protected function getDetectedModuleControllers(string $moduleName): array
+    {
+        $legacyControllersFilenames = $this->getModuleAdminControllersFilename($moduleName);
+        $legacyControllers = array_map(function ($legacyControllersFilename) {
+            return str_replace('Controller.php', '', $legacyControllersFilename);
+        }, $legacyControllersFilenames);
+        $legacyControllers = $legacyControllers ?? [];
+
+        $routingControllers = $this->getModuleControllersFromRouting($moduleName);
+        $routingControllers = $routingControllers ?? [];
+
+        return array_merge($legacyControllers, $routingControllers);
+    }
+
     /**
      * Check mandatory data for tab registration, such as class name and class exists.
      *
@@ -206,10 +221,13 @@ protected function checkIsValid($moduleName, ParameterBag $data)
         if (null === $className) {
             throw new Exception('Missing class name of tab');
         }
+
         // Check controller exists
-        if (empty($data->get('route_name')) && !in_array($className . 'Controller.php', $this->getModuleAdminControllersFilename($moduleName))) {
-            throw new Exception(sprintf('Class "%sController" not found in controllers/admin', $className));
+        $detectedControllers = $this->getDetectedModuleControllers($moduleName);
+        if (empty($data->get('route_name')) && !in_array($className, $detectedControllers)) {
+            throw new Exception(sprintf('Class "%sController" not found in controllers/admin nor routing file', $className));
         }
+
         // Deprecation check
         if ($data->has('ParentClassName') && !$data->has('parent_class_name')) {
             $this->logger->warning('Tab attribute "ParentClassName" is deprecated. You must use "parent_class_name" instead.');
diff --git a/tests-legacy/Unit/Adapter/Module/Tab/ModuleTabRegisterTest.php b/tests-legacy/Unit/Adapter/Module/Tab/ModuleTabRegisterTest.php
index 87637d744694e..f7ba60df9d7f6 100644
--- a/tests-legacy/Unit/Adapter/Module/Tab/ModuleTabRegisterTest.php
+++ b/tests-legacy/Unit/Adapter/Module/Tab/ModuleTabRegisterTest.php
@@ -28,7 +28,6 @@
 
 use LegacyTests\TestCase\UnitTestCase;
 use PrestaShop\PrestaShop\Adapter\Module\Tab\ModuleTabRegister;
-use PrestaShopBundle\Routing\YamlModuleLoader;
 use Symfony\Component\Config\Loader\LoaderInterface;
 use Symfony\Component\Filesystem\Filesystem;
 use Symfony\Component\HttpFoundation\ParameterBag;
@@ -54,7 +53,7 @@ class ModuleTabRegisterTest extends UnitTestCase
             // Non-existing class file, must throw an exception
             array(
                 'class_name' => 'AdminMissing',
-                'exception' => 'Class "AdminMissingController" not found in controllers/admin',
+                'exception' => 'Class "AdminMissingController" not found in controllers/admin nor routing file',
             ),
         ),
         'symfony' => array(
@@ -65,7 +64,11 @@ class ModuleTabRegisterTest extends UnitTestCase
             ),
         ),
         // No tabs by default, the undeclared one comes from the routing parsing
-        'undeclared_symfony' => array(),
+        'undeclared_symfony' => array(
+            array(
+                'class_name' => 'UndeclaredLegacyController',
+            ),
+        ),
     );
 
     protected $moduleAdminControllers = array(