Merge pull request #5930 from magento-performance/MC-35885-2

vzabaznov · web-flow · commit 48d786827644 · 2020-07-27T21:58:32.000-05:00
MC-35885
diff --git a/dev/tests/integration/testsuite/Magento/Setup/Model/ConfigOptionsListCollectorTest.php b/dev/tests/integration/testsuite/Magento/Setup/Model/ConfigOptionsListCollectorTest.php
@@ -5,6 +5,10 @@
  */
 namespace Magento\Setup\Model;
 
+use Magento\Framework\Component\ComponentRegistrarInterface;
+use Magento\Setup\Validator\DbValidator;
+use Laminas\ServiceManager\ServiceLocatorInterface;
+
 class ConfigOptionsListCollectorTest extends \PHPUnit\Framework\TestCase
 {
     /**
@@ -14,7 +18,7 @@ class ConfigOptionsListCollectorTest extends \PHPUnit\Framework\TestCase
 
     protected function setUp(): void
     {
-        $this->objectManagerProvider = $this->createMock(\Magento\Setup\Model\ObjectManagerProvider::class);
+        $this->objectManagerProvider = $this->createMock(ObjectManagerProvider::class);
         $this->objectManagerProvider
             ->expects($this->any())
             ->method('get')
@@ -24,11 +28,13 @@ protected function setUp(): void
     public function testCollectOptionsLists()
     {
         $objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
-        $fullModuleListMock = $this->createMock(\Magento\Framework\Module\FullModuleList::class);
-        $fullModuleListMock->expects($this->once())->method('getNames')->willReturn(['Magento_Backend']);
+        $componentRegistrar = $this->createMock(ComponentRegistrarInterface::class);
+        $componentRegistrar->expects($this->once())
+            ->method('getPaths')
+            ->willReturn(['Magento_Backend'=>'app/code/Magento/Backend']);
 
-        $dbValidator = $this->createMock(\Magento\Setup\Validator\DbValidator::class);
-        $configGenerator = $this->createMock(\Magento\Setup\Model\ConfigGenerator::class);
+        $dbValidator = $this->createMock(DbValidator::class);
+        $configGenerator = $this->createMock(ConfigGenerator::class);
 
         $setupOptions = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()
             ->create(
@@ -39,7 +45,7 @@ public function testCollectOptionsLists()
                 ]
             );
 
-        $serviceLocator = $this->getMockForAbstractClass(\Laminas\ServiceManager\ServiceLocatorInterface::class);
+        $serviceLocator = $this->getMockForAbstractClass(ServiceLocatorInterface::class);
 
         $serviceLocator->expects($this->once())
             ->method('get')
@@ -51,7 +57,7 @@ public function testCollectOptionsLists()
             \Magento\Setup\Model\ConfigOptionsListCollector::class,
             [
                 'objectManagerProvider' => $this->objectManagerProvider,
-                'fullModuleList' => $fullModuleListMock,
+                'componentRegistrar' => $componentRegistrar,
                 'serviceLocator' => $serviceLocator
             ]
         );
diff --git a/lib/internal/Magento/Framework/Module/DependencyChecker.php b/lib/internal/Magento/Framework/Module/DependencyChecker.php
@@ -13,43 +13,32 @@
 class DependencyChecker
 {
     /**
-     * Enabled module list from configuration
-     *
-     * @var array
-     */
-    private $enabledModuleList;
-
-    /**
-     * The full module list information from filesystem
-     *
-     * @var array
+     * @var PackageInfo
      */
-    private $fullModuleList;
+    private $packageInfo;
 
     /**
-     * Graph
-     *
-     * @var Graph
+     * @var ModuleList
      */
-    private $graph;
+    private $list;
 
     /**
-     * @var PackageInfo
+     * @var ModuleList\Loader
      */
-    protected $packageInfo;
+    private $loader;
 
     /**
      * Constructor
      *
      * @param ModuleList $list
      * @param ModuleList\Loader $loader
-     * @param PackageInfoFactory $packageInfoFactory
+     * @param PackageInfo $packageInfo
      */
-    public function __construct(ModuleList $list, ModuleList\Loader $loader, PackageInfoFactory $packageInfoFactory)
+    public function __construct(ModuleList $list, ModuleList\Loader $loader, PackageInfo $packageInfo)
     {
-        $this->enabledModuleList = $list->getNames();
-        $this->fullModuleList = $loader->load();
-        $this->packageInfo = $packageInfoFactory->create();
+        $this->list = $list;
+        $this->loader = $loader;
+        $this->packageInfo = $packageInfo;
     }
 
     /**
@@ -58,10 +47,11 @@ public function __construct(ModuleList $list, ModuleList\Loader $loader, Package
      * @param string[] $toBeDisabledModules
      * @param string[] $currentlyEnabledModules
      * @return array
+     * @throws \Magento\Framework\Exception\LocalizedException
      */
     public function checkDependenciesWhenDisableModules($toBeDisabledModules, $currentlyEnabledModules = null)
     {
-        $masterList = isset($currentlyEnabledModules) ? $currentlyEnabledModules : $this->enabledModuleList;
+        $masterList = $currentlyEnabledModules ?? $this->list->getNames();
         // assume disable succeeds: currently enabled modules - to-be-disabled modules
         $enabledModules = array_diff($masterList, $toBeDisabledModules);
         return $this->checkDependencyGraph(false, $toBeDisabledModules, $enabledModules);
@@ -73,10 +63,11 @@ public function checkDependenciesWhenDisableModules($toBeDisabledModules, $curre
      * @param string[] $toBeEnabledModules
      * @param string[] $currentlyEnabledModules
      * @return array
+     * @throws \Magento\Framework\Exception\LocalizedException
      */
-    public function checkDependenciesWhenEnableModules($toBeEnabledModules, $currentlyEnabledModules = null)
+    public function checkDependenciesWhenEnableModules(array $toBeEnabledModules, array $currentlyEnabledModules = null)
     {
-        $masterList = isset($currentlyEnabledModules) ? $currentlyEnabledModules : $this->enabledModuleList;
+        $masterList = $currentlyEnabledModules ?? $this->list->getNames();
         // assume enable succeeds: union of currently enabled modules and to-be-enabled modules
         $enabledModules = array_unique(array_merge($masterList, $toBeEnabledModules));
         return $this->checkDependencyGraph(true, $toBeEnabledModules, $enabledModules);
@@ -89,19 +80,21 @@ public function checkDependenciesWhenEnableModules($toBeEnabledModules, $current
      * @param string[] $moduleNames list of modules to be enabled/disabled
      * @param string[] $enabledModules list of enabled modules assuming enable/disable succeeds
      * @return array
+     * @throws \Magento\Framework\Exception\LocalizedException
      */
-    private function checkDependencyGraph($isEnable, $moduleNames, $enabledModules)
+    private function checkDependencyGraph(bool $isEnable, array $moduleNames, array $enabledModules)
     {
-        $this->graph = $this->createGraph();
+        $fullModuleList = $this->loader->load();
+        $graph = $this->createGraph($fullModuleList);
         $dependenciesMissingAll = [];
         $graphMode = $isEnable ? Graph::DIRECTIONAL : Graph::INVERSE;
+        $modules = array_merge(
+            array_keys($fullModuleList),
+            $this->packageInfo->getNonExistingDependencies()
+        );
         foreach ($moduleNames as $moduleName) {
             $dependenciesMissing = [];
-            $paths = $this->graph->findPathsToReachableNodes($moduleName, $graphMode);
-            $modules = array_merge(
-                array_keys($this->fullModuleList),
-                $this->packageInfo->getNonExistingDependencies()
-            );
+            $paths = $graph->findPathsToReachableNodes($moduleName, $graphMode);
             foreach ($modules as $module) {
                 if (isset($paths[$module])) {
                     if ($isEnable && !in_array($module, $enabledModules)) {
@@ -119,15 +112,16 @@ private function checkDependencyGraph($isEnable, $moduleNames, $enabledModules)
     /**
      * Create the dependency graph
      *
+     * @param array $fullModuleList
      * @return Graph
      */
-    private function createGraph()
+    private function createGraph(array $fullModuleList): Graph
     {
         $nodes = [];
         $dependencies = [];
 
         // build the graph data
-        foreach (array_keys($this->fullModuleList) as $moduleName) {
+        foreach (array_keys($fullModuleList) as $moduleName) {
             $nodes[] = $moduleName;
             foreach ($this->packageInfo->getRequire($moduleName) as $dependModuleName) {
                 if ($dependModuleName) {
diff --git a/lib/internal/Magento/Framework/Module/Dir/Reader.php b/lib/internal/Magento/Framework/Module/Dir/Reader.php
@@ -125,7 +125,11 @@ private function getFiles($filename, $subDir = '')
     {
         $result = [];
         foreach ($this->modulesList->getNames() as $moduleName) {
-            $moduleSubDir = $this->getModuleDir($subDir, $moduleName);
+            try {
+                $moduleSubDir = $this->getModuleDir($subDir, $moduleName);
+            } catch (\InvalidArgumentException $e) {
+                continue;
+            }
             $file = $moduleSubDir . '/' . $filename;
             $directoryRead = $this->readFactory->create($moduleSubDir);
             $path = $directoryRead->getRelativePath($file);
diff --git a/lib/internal/Magento/Framework/Module/Test/Unit/DependencyCheckerTest.php b/lib/internal/Magento/Framework/Module/Test/Unit/DependencyCheckerTest.php
@@ -27,11 +27,6 @@ class DependencyCheckerTest extends TestCase
      */
     private $packageInfoMock;
 
-    /**
-     * @var PackageInfoFactory|MockObject
-     */
-    private $packageInfoFactoryMock;
-
     /**
      * @var ModuleList|MockObject
      */
@@ -57,13 +52,8 @@ protected function setUp(): void
             ->method('getRequire')
             ->willReturnMap($requireMap);
 
-        $this->packageInfoFactoryMock = $this->createMock(PackageInfoFactory::class);
-        $this->packageInfoFactoryMock->expects($this->once())
-            ->method('create')
-            ->willReturn($this->packageInfoMock);
-
-        $this->listMock = $this->createMock(ModuleList::class);
-        $this->loaderMock = $this->createMock(Loader::class);
+        $this->listMock = $this->createMock(\Magento\Framework\Module\ModuleList::class);
+        $this->loaderMock = $this->createMock(\Magento\Framework\Module\ModuleList\Loader::class);
         $this->loaderMock
             ->expects($this->any())
             ->method('load')
@@ -78,7 +68,7 @@ public function testCheckDependenciesWhenDisableModules()
         $this->packageInfoMock->expects($this->atLeastOnce())
             ->method('getNonExistingDependencies')
             ->willReturn([]);
-        $this->checker = new DependencyChecker($this->listMock, $this->loaderMock, $this->packageInfoFactoryMock);
+        $this->checker = new DependencyChecker($this->listMock, $this->loaderMock, $this->packageInfoMock);
 
         $actual = $this->checker->checkDependenciesWhenDisableModules(['B', 'D']);
         $expected = ['B' => ['A' => ['A', 'B']], 'D' => ['A' => ['A', 'B', 'D']]];
@@ -90,7 +80,7 @@ public function testCheckDependenciesWhenDisableModulesWithCurEnabledModules()
         $this->packageInfoMock->expects($this->atLeastOnce())
             ->method('getNonExistingDependencies')
             ->willReturn([]);
-        $this->checker = new DependencyChecker($this->listMock, $this->loaderMock, $this->packageInfoFactoryMock);
+        $this->checker = new DependencyChecker($this->listMock, $this->loaderMock, $this->packageInfoMock);
 
         $actual = $this->checker->checkDependenciesWhenDisableModules(['B', 'D'], ['C', 'D', 'E']);
         $expected = ['B' => [], 'D' => []];
@@ -105,7 +95,7 @@ public function testCheckDependenciesWhenEnableModules()
         $this->packageInfoMock->expects($this->atLeastOnce())
             ->method('getNonExistingDependencies')
             ->willReturn([]);
-        $this->checker = new DependencyChecker($this->listMock, $this->loaderMock, $this->packageInfoFactoryMock);
+        $this->checker = new DependencyChecker($this->listMock, $this->loaderMock, $this->packageInfoMock);
         $actual = $this->checker->checkDependenciesWhenEnableModules(['B', 'D']);
         $expected = [
             'B' => ['A' => ['B', 'D', 'A'], 'E' => ['B', 'E']],
@@ -119,7 +109,7 @@ public function testCheckDependenciesWhenEnableModulesWithCurEnabledModules()
         $this->packageInfoMock->expects($this->atLeastOnce())
             ->method('getNonExistingDependencies')
             ->willReturn([]);
-        $this->checker = new DependencyChecker($this->listMock, $this->loaderMock, $this->packageInfoFactoryMock);
+        $this->checker = new DependencyChecker($this->listMock, $this->loaderMock, $this->packageInfoMock);
         $actual = $this->checker->checkDependenciesWhenEnableModules(['B', 'D'], ['C']);
         $expected = [
             'B' => ['A' => ['B', 'D', 'A'], 'E' => ['B', 'E']],
diff --git a/setup/src/Magento/Setup/Model/ConfigOptionsListCollector.php b/setup/src/Magento/Setup/Model/ConfigOptionsListCollector.php
@@ -6,8 +6,9 @@
 namespace Magento\Setup\Model;
 
 use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\Component\ComponentRegistrar;
+use Magento\Framework\Component\ComponentRegistrarInterface;
 use Magento\Framework\Filesystem;
-use Magento\Framework\Module\FullModuleList;
 use Magento\Framework\Setup\ConfigOptionsListInterface;
 use Laminas\ServiceManager\ServiceLocatorInterface;
 
@@ -30,13 +31,6 @@ class ConfigOptionsListCollector
      */
     private $filesystem;
 
-    /**
-     * Module list including enabled and disabled modules
-     *
-     * @var FullModuleList
-     */
-    private $fullModuleList;
-
     /**
      * Object manager provider
      *
@@ -51,27 +45,34 @@ class ConfigOptionsListCollector
      */
     private $serviceLocator;
 
+    /**
+     * Component list
+     *
+     * @var ComponentRegistrarInterface
+     */
+    private $componentRegistrar;
+
     /**
      * Constructor
      *
      * @param DirectoryList $directoryList
      * @param Filesystem $filesystem
-     * @param FullModuleList $fullModuleList
+     * @param ComponentRegistrarInterface $componentRegistrar
      * @param ObjectManagerProvider $objectManagerProvider
      * @param ServiceLocatorInterface $serviceLocator
      */
     public function __construct(
         DirectoryList $directoryList,
         Filesystem $filesystem,
-        FullModuleList $fullModuleList,
+        ComponentRegistrarInterface $componentRegistrar,
         ObjectManagerProvider $objectManagerProvider,
         ServiceLocatorInterface $serviceLocator
     ) {
         $this->directoryList = $directoryList;
         $this->filesystem = $filesystem;
-        $this->fullModuleList = $fullModuleList;
         $this->objectManagerProvider = $objectManagerProvider;
         $this->serviceLocator = $serviceLocator;
+        $this->componentRegistrar = $componentRegistrar;
     }
 
     /**
@@ -86,8 +87,8 @@ public function collectOptionsLists()
     {
         $optionsList = [];
 
-        // go through modules
-        foreach ($this->fullModuleList->getNames() as $moduleName) {
+        $modulePaths = $this->componentRegistrar->getPaths(ComponentRegistrar::MODULE);
+        foreach (array_keys($modulePaths) as $moduleName) {
             $optionsClassName = str_replace('_', '\\', $moduleName) . '\Setup\ConfigOptionsList';
             if (class_exists($optionsClassName)) {
                 $optionsClass = $this->objectManagerProvider->get()->create($optionsClassName);
