Fix issue 24666 (Static content is deploying for disabled modules)

lib/internal/Magento/Framework/Css/PreProcessor/File/Collector/Aggregated.php

@@ -6,6 +6,7 @@
 
 namespace Magento\Framework\Css\PreProcessor\File\Collector;
 
+use Magento\Framework\Module\ModuleManagerInterface;
 use Magento\Framework\View\Design\ThemeInterface;
 use Magento\Framework\View\File\CollectorInterface;
 use Magento\Framework\View\File\FileList\Factory;
@@ -36,6 +37,11 @@ class Aggregated implements CollectorInterface
      */
     protected $overriddenBaseFiles;
 
+    /**
+     * @var \Magento\Framework\Module\ModuleManagerInterface
+     */
+    protected $moduleManager;
+
     /**
      * @var LoggerInterface
      */
@@ -46,19 +52,22 @@ class Aggregated implements CollectorInterface
      * @param CollectorInterface $libraryFiles
      * @param CollectorInterface $baseFiles
      * @param CollectorInterface $overriddenBaseFiles
+     * @param ModuleManagerInterface $moduleManager
      * @param LoggerInterface $logger
      */
     public function __construct(
         Factory $fileListFactory,
         CollectorInterface $libraryFiles,
         CollectorInterface $baseFiles,
         CollectorInterface $overriddenBaseFiles,
+        ModuleManagerInterface $moduleManager,
         LoggerInterface $logger
     ) {
         $this->fileListFactory = $fileListFactory;
         $this->libraryFiles = $libraryFiles;
         $this->baseFiles = $baseFiles;
         $this->overriddenBaseFiles = $overriddenBaseFiles;
+        $this->moduleManager = $moduleManager;
         $this->logger = $logger;
     }
 
@@ -83,11 +92,32 @@ public function getFiles(ThemeInterface $theme, $filePath)
             $list->replace($files);
         }
         $result = $list->getAll();
-        if (empty($result)) {
+
+        $finalResult = [];
+
+        if(!empty($result))
+        {
+            foreach ($result as $file)
+            {
+                if ($this->hasEnabledModule($file)) {
+                    $finalResult[] = $file;
+                }
+            }
+        }
+
+        if (empty($finalResult)) {
             $this->logger->notice(
                 'magento_import returns empty result by path ' . $filePath . ' for theme ' . $theme->getCode()
             );
         }
-        return $result;
+
+
+        return $finalResult;
+    }
+
+    private function hasEnabledModule(\Magento\Framework\View\File $file)
+    {
+        return $file instanceof \Magento\Framework\View\File && (!$file->getModule() ||
+                $this->moduleManager->isEnabled($file->getModule()));
     }
 }

lib/internal/Magento/Framework/Css/PreProcessor/Instruction/MagentoImport.php

@@ -13,8 +13,11 @@
 use Magento\Framework\View\Design\Theme\ThemeProviderInterface;
 use Magento\Framework\View\DesignInterface;
 use Magento\Framework\View\File\CollectorInterface;
+use Magento\Framework\Module\ModuleManagerInterface;
 
 /**
+ * Processes import statements for css files
+ *
  * @magento_import instruction preprocessor
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects) Must be deleted after moving themeProvider to construct
  */
@@ -57,29 +60,37 @@ class MagentoImport implements PreProcessorInterface
      */
     private $themeProvider;
 
+    /**
+     * @var ModuleManagerInterface
+     */
+    private $moduleManager;
+
     /**
      * @param DesignInterface $design
      * @param CollectorInterface $fileSource
      * @param ErrorHandlerInterface $errorHandler
      * @param \Magento\Framework\View\Asset\Repository $assetRepo
      * @param \Magento\Framework\View\Design\Theme\ListInterface $themeList
+     * @param ModuleManagerInterface $moduleManager
      */
     public function __construct(
         DesignInterface $design,
         CollectorInterface $fileSource,
         ErrorHandlerInterface $errorHandler,
         \Magento\Framework\View\Asset\Repository $assetRepo,
-        \Magento\Framework\View\Design\Theme\ListInterface $themeList
+        \Magento\Framework\View\Design\Theme\ListInterface $themeList,
+        ModuleManagerInterface $moduleManager = null
     ) {
         $this->design = $design;
         $this->fileSource = $fileSource;
         $this->errorHandler = $errorHandler;
         $this->assetRepo = $assetRepo;
         $this->themeList = $themeList;
+        $this->moduleManager = $moduleManager ?: ObjectManager::getInstance()->get(ModuleManagerInterface::class);
     }
 
     /**
-     * {@inheritdoc}
+     * @inheritdoc
      */
     public function process(\Magento\Framework\View\Asset\PreProcessor\Chain $chain)
     {
@@ -105,9 +116,13 @@ protected function replace(array $matchedContent, LocalInterface $asset)
             $isReference = !empty($matchedContent['reference']);
             $relatedAsset = $this->assetRepo->createRelated($matchedFileId, $asset);
             $resolvedPath = $relatedAsset->getFilePath();
+
             $importFiles = $this->fileSource->getFiles($this->getTheme($relatedAsset), $resolvedPath);
             /** @var $importFile \Magento\Framework\View\File */
             foreach ($importFiles as $importFile) {
+//                if ($importFile->getModule() && !$this->moduleManager->isEnabled($importFile->getModule())) {
+//                    continue;
+//                }
                 $referenceString = $isReference ? '(reference) ' : '';
                 $importsContent .= $importFile->getModule()
                     ? "@import $referenceString'{$importFile->getModule()}::{$resolvedPath}';\n"
@@ -137,6 +152,8 @@ protected function getTheme(LocalInterface $asset)
     }
 
     /**
+     * Get theme provider
+     *
      * @return ThemeProviderInterface
      * @deprecated 100.1.1
      */

lib/internal/Magento/Framework/Css/Test/Unit/PreProcessor/File/Collector/AggregatedTest.php

@@ -42,6 +42,11 @@ class AggregatedTest extends \PHPUnit\Framework\TestCase
      */
     protected $themeMock;
 
+    /**
+     * @var \Magento\Framework\Module\ModuleManagerInterface
+     */
+    protected $moduleManagerMock;
+
     /**
      * @var \Psr\Log\LoggerInterface|\PHPUnit_Framework_MockObject_MockObject
      */
@@ -61,6 +66,8 @@ public function setup()
             ->will($this->returnValue($this->fileListMock));
         $this->libraryFilesMock = $this->getMockBuilder(\Magento\Framework\View\File\CollectorInterface::class)
             ->getMock();
+        $this->moduleManagerMock = $this->getMockBuilder(\Magento\Framework\Module\ModuleManagerInterface::class)
+                                 ->getMock();
         $this->loggerMock = $this->getMockBuilder(\Psr\Log\LoggerInterface::class)
             ->getMock();
 
@@ -81,6 +88,7 @@ public function testGetFilesEmpty()
             $this->libraryFilesMock,
             $this->baseFilesMock,
             $this->overriddenBaseFilesMock,
+            $this->moduleManagerMock,
             $this->loggerMock
         );
 
@@ -106,9 +114,12 @@ public function testGetFilesEmpty()
      */
     public function testGetFiles($libraryFiles, $baseFiles, $themeFiles)
     {
+
+        $returnFile = new \Magento\Framework\View\File('returnFile','myModule');
+
         $this->fileListMock->expects($this->at(0))->method('add')->with($this->equalTo($libraryFiles));
         $this->fileListMock->expects($this->at(1))->method('add')->with($this->equalTo($baseFiles));
-        $this->fileListMock->expects($this->any())->method('getAll')->will($this->returnValue(['returnedFile']));
+        $this->fileListMock->expects($this->any())->method('getAll')->will($this->returnValue([$returnFile]));
 
         $subPath = '*';
         $this->libraryFilesMock->expects($this->atLeastOnce())
@@ -125,19 +136,22 @@ public function testGetFiles($libraryFiles, $baseFiles, $themeFiles)
             ->method('getFiles')
             ->will($this->returnValue($themeFiles));
 
+        $this->moduleManagerMock->expects($this->any())->method('isEnabled')->will($this->returnValue(true));
+
         $aggregated = new Aggregated(
             $this->fileListFactoryMock,
             $this->libraryFilesMock,
             $this->baseFilesMock,
             $this->overriddenBaseFilesMock,
+            $this->moduleManagerMock,
             $this->loggerMock
         );
 
         $inheritedThemeMock = $this->getMockBuilder(\Magento\Framework\View\Design\ThemeInterface::class)->getMock();
         $this->themeMock->expects($this->any())->method('getInheritedThemes')
             ->will($this->returnValue([$inheritedThemeMock]));
 
-        $this->assertEquals(['returnedFile'], $aggregated->getFiles($this->themeMock, $subPath));
+        $this->assertEquals([$returnFile], $aggregated->getFiles($this->themeMock, $subPath));
     }
 
     /**

lib/internal/Magento/Framework/Css/Test/Unit/PreProcessor/Instruction/MagentoImportTest.php

@@ -7,6 +7,7 @@
 namespace Magento\Framework\Css\Test\Unit\PreProcessor\Instruction;
 
 use Magento\Framework\Css\PreProcessor\Instruction\MagentoImport;
+use Magento\Framework\Module\ModuleManagerInterface;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
 use Magento\Framework\View\Design\Theme\ThemeProviderInterface;
 
@@ -45,6 +46,10 @@ class MagentoImportTest extends \PHPUnit\Framework\TestCase
      */
     private $themeProvider;
 
+    /**
+     * @var ModuleManagerInterface||\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $moduleManager;
     /**
      * @var \Magento\Framework\Css\PreProcessor\Instruction\Import
      */
@@ -61,12 +66,15 @@ protected function setUp()
         $this->asset->expects($this->any())->method('getContentType')->will($this->returnValue('css'));
         $this->assetRepo = $this->createMock(\Magento\Framework\View\Asset\Repository::class);
         $this->themeProvider = $this->createMock(ThemeProviderInterface::class);
+        $this->moduleManager = $this->createMock(ModuleManagerInterface::class);
         $this->object = (new ObjectManager($this))->getObject(MagentoImport::class, [
             'design' => $this->design,
             'fileSource' => $this->fileSource,
             'errorHandler' => $this->errorHandler,
             'assetRepo' => $this->assetRepo,
-            'themeProvider' => $this->themeProvider
+            'themeProvider' => $this->themeProvider,
+            'moduleManager' => $this->moduleManager
+
         ]);
     }
 
@@ -94,16 +102,22 @@ public function testProcess($originalContent, $foundPath, $resolvedPath, $foundF
         $relatedAsset->expects($this->once())->method('getContext')->will($this->returnValue($context));
         $theme = $this->getMockForAbstractClass(\Magento\Framework\View\Design\ThemeInterface::class);
         $this->themeProvider->expects($this->once())->method('getThemeByFullPath')->will($this->returnValue($theme));
+
+        $this->moduleManager->expects($this->any())->method('isEnabled')->will($this->returnValue(true));
+
         $files = [];
         foreach ($foundFiles as $file) {
             $fileObject = $this->createMock(\Magento\Framework\View\File::class);
             $fileObject->expects($this->any())
-                ->method('getModule')
-                ->will($this->returnValue($file['module']));
+                       ->method('getModule')
+                       ->will($this->returnValue($file['module']));
             $fileObject->expects($this->any())
-                ->method('getFilename')
-                ->will($this->returnValue($file['filename']));
-            $files[] = $fileObject;
+                       ->method('getFilename')
+                       ->will($this->returnValue($file['filename']));
+
+            if (\is_null($file['module']) || (isset($file['isEnabled']) && $file['isEnabled'] === true)) {
+                $files[] = $fileObject;
+            }
         }
         $this->fileSource->expects($this->once())
             ->method('getFiles')
@@ -135,11 +149,33 @@ public function processDataProvider()
                 'Magento_Module::some/file.css',
                 'some/file.css',
                 [
-                    ['module' => 'Magento_Module', 'filename' => 'some/file.css'],
-                    ['module' => 'Magento_Two', 'filename' => 'some/file.css'],
+                    ['module' => 'Magento_Module','isEnabled' => true, 'filename' => 'some/file.css'],
+                    ['module' => 'Magento_Two','isEnabled' => true, 'filename' => 'some/file.css'],
                 ],
                 "@import 'Magento_Module::some/file.css';\n@import 'Magento_Two::some/file.css';\n",
             ],
+            'modular one module disabled' => [
+                '//@magento_import "Magento_Module::some/file.css";',
+                'Magento_Module::some/file.css',
+                'some/file.css',
+                [
+                    ['module' => 'Magento_Module','isEnabled' => true, 'filename' => 'some/file.css'],
+                    ['module' => 'Magento_Two','isEnabled' => true, 'filename' => 'some/file.css'],
+                    ['module' => 'Magento_Disabled','isEnabled' => false, 'filename' => 'some/file.css'],
+                ],
+                "@import 'Magento_Module::some/file.css';\n@import 'Magento_Two::some/file.css';\n",
+            ],
+            'modular all modules disabled' => [
+                '//@magento_import "Magento_Module::some/file.css";',
+                'Magento_Module::some/file.css',
+                'some/file.css',
+                [
+                    ['module' => 'Magento_Module','isEnabled' => false, 'filename' => 'some/file.css'],
+                    ['module' => 'Magento_Two','isEnabled' => false, 'filename' => 'some/file.css'],
+                    ['module' => 'Magento_Disabled','isEnabled' => false, 'filename' => 'some/file.css'],
+                ],
+                "",
+            ],
             'non-modular reference notation' => [
                 '//@magento_import (reference) "some/file.css";',
                 'some/file.css',
@@ -155,12 +191,35 @@ public function processDataProvider()
                 'Magento_Module::some/file.css',
                 'some/file.css',
                 [
-                    ['module' => 'Magento_Module', 'filename' => 'some/file.css'],
-                    ['module' => 'Magento_Two', 'filename' => 'some/file.css'],
+                    ['module' => 'Magento_Module','isEnabled' => true, 'filename' => 'some/file.css'],
+                    ['module' => 'Magento_Two','isEnabled' => true, 'filename' => 'some/file.css'],
+                ],
+                "@import (reference) 'Magento_Module::some/file.css';\n" .
+                "@import (reference) 'Magento_Two::some/file.css';\n",
+            ],
+            'modular reference one disabled module' => [
+                '//@magento_import (reference) "Magento_Module::some/file.css";',
+                'Magento_Module::some/file.css',
+                'some/file.css',
+                [
+                    ['module' => 'Magento_Module','isEnabled' => true, 'filename' => 'some/file.css'],
+                    ['module' => 'Magento_Two','isEnabled' => true, 'filename' => 'some/file.css'],
+                    ['module' => 'Magento_Disabled','isEnabled' => false, 'filename' => 'some/file.css'],
                 ],
                 "@import (reference) 'Magento_Module::some/file.css';\n" .
                 "@import (reference) 'Magento_Two::some/file.css';\n",
             ],
+            'modular reference all modules disabled' => [
+                '//@magento_import (reference) "Magento_Module::some/file.css";',
+                'Magento_Module::some/file.css',
+                'some/file.css',
+                [
+                    ['module' => 'Magento_Module','isEnabled' => false, 'filename' => 'some/file.css'],
+                    ['module' => 'Magento_Two','isEnabled' => false, 'filename' => 'some/file.css'],
+                    ['module' => 'Magento_Disabled','isEnabled' => false, 'filename' => 'some/file.css'],
+                ],
+                "",
+            ],
         ];
     }