ENGCOM-4223: Make the module list more deterministic #21020

magento · Feb 24, 2019 · b15af15 · b15af15
2 parents d36c704 + 02dc3ff
commit b15af15
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 5 deletions.
diff --git a/lib/internal/Magento/Framework/Module/ModuleList/Loader.php b/lib/internal/Magento/Framework/Module/ModuleList/Loader.php
@@ -126,24 +126,29 @@ private function getModuleConfigs()
      *
      * @param array $origList
      * @return array
-     * @SuppressWarnings(PHPMD.UnusedLocalVariable)
+     * @throws \Exception
      */
-    private function sortBySequence($origList)
+    private function sortBySequence(array $origList): array
     {
         ksort($origList);
+        $modules = $this->prearrangeModules($origList);
+
         $expanded = [];
-        foreach ($origList as $moduleName => $value) {
+        foreach (array_keys($modules) as $moduleName) {
+            $sequence = $this->expandSequence($origList, $moduleName);
+            asort($sequence);
+
             $expanded[] = [
                 'name' => $moduleName,
-                'sequence' => $this->expandSequence($origList, $moduleName),
+                'sequence' => $sequence,
             ];
         }
 
         // Use "bubble sorting" because usort does not check each pair of elements and in this case it is important
         $total = count($expanded);
         for ($i = 0; $i < $total - 1; $i++) {
             for ($j = $i; $j < $total; $j++) {
-                if (in_array($expanded[$j]['name'], $expanded[$i]['sequence'])) {
+                if (in_array($expanded[$j]['name'], $expanded[$i]['sequence'], true)) {
                     $temp = $expanded[$i];
                     $expanded[$i] = $expanded[$j];
                     $expanded[$j] = $temp;
@@ -159,6 +164,27 @@ private function sortBySequence($origList)
         return $result;
     }
 
+    /**
+     * Prearrange all modules by putting those from Magento before the others
+     *
+     * @param array $modules
+     * @return array
+     */
+    private function prearrangeModules(array $modules): array
+    {
+        $breakdown = ['magento' => [], 'others' => []];
+
+        foreach ($modules as $moduleName => $moduleDetails) {
+            if (strpos($moduleName, 'Magento_') !== false) {
+                $breakdown['magento'][$moduleName] = $moduleDetails;
+            } else {
+                $breakdown['others'][$moduleName] = $moduleDetails;
+            }
+        }
+
+        return array_merge($breakdown['magento'], $breakdown['others']);
+    }
+
     /**
      * Accumulate information about all transitive "sequence" references
      *

diff --git a/lib/internal/Magento/Framework/Module/Test/Unit/ModuleList/LoaderTest.php b/lib/internal/Magento/Framework/Module/Test/Unit/ModuleList/LoaderTest.php
@@ -160,4 +160,55 @@ public function testLoadCircular()
         ]));
         $this->loader->load();
     }
+
+    /**
+     * @throws \Magento\Framework\Exception\LocalizedException
+     */
+    public function testLoadPrearranged(): void
+    {
+        $fixtures = [
+            'Foo_Bar' => ['name' => 'Foo_Bar', 'sequence' => ['Magento_Store']],
+            'Magento_Directory' => ['name' => 'Magento_Directory', 'sequence' => ['Magento_Store']],
+            'Magento_Store' => ['name' => 'Magento_Store', 'sequence' => []],
+            'Magento_Theme' => ['name' => 'Magento_Theme', 'sequence' => ['Magento_Store', 'Magento_Directory']],
+            'Test_HelloWorld' => ['name' => 'Test_HelloWorld', 'sequence' => ['Magento_Theme']]
+        ];
+
+        $index = 0;
+        foreach ($fixtures as $name => $fixture) {
+            $this->converter->expects($this->at($index++))->method('convert')->willReturn([$name => $fixture]);
+        }
+
+        $this->registry->expects($this->once())
+            ->method('getPaths')
+            ->willReturn([
+                '/path/to/Foo_Bar',
+                '/path/to/Magento_Directory',
+                '/path/to/Magento_Store',
+                '/path/to/Magento_Theme',
+                '/path/to/Test_HelloWorld'
+            ]);
+
+        $this->driver->expects($this->exactly(5))
+            ->method('fileGetContents')
+            ->will($this->returnValueMap([
+                ['/path/to/Foo_Bar/etc/module.xml', null, null, self::$sampleXml],
+                ['/path/to/Magento_Directory/etc/module.xml', null, null, self::$sampleXml],
+                ['/path/to/Magento_Store/etc/module.xml', null, null, self::$sampleXml],
+                ['/path/to/Magento_Theme/etc/module.xml', null, null, self::$sampleXml],
+                ['/path/to/Test_HelloWorld/etc/module.xml', null, null, self::$sampleXml],
+            ]));
+
+        // Load the full module list information
+        $result = $this->loader->load();
+
+        $this->assertSame(
+            ['Magento_Store', 'Magento_Directory', 'Magento_Theme', 'Foo_Bar', 'Test_HelloWorld'],
+            array_keys($result)
+        );
+
+        foreach ($fixtures as $name => $fixture) {
+            $this->assertSame($fixture, $result[$name]);
+        }
+    }
 }