ENGCOM-6176: Fixed improper ObjectManager usage and respective unit tests #25263

Author: VladimirZaets

app/code/Magento/Email/Model/Template/Filter.php

@@ -165,15 +165,20 @@ class Filter extends \Magento\Framework\Filter\Template
     protected $configVariables;
 
     /**
-     * @var \Magento\Email\Model\Template\Css\Processor
+     * @var Css\Processor
      */
     private $cssProcessor;
 
     /**
-     * @var ReadInterface
+     * @var Filesystem
      */
     private $pubDirectory;
 
+    /**
+     * @var \Magento\Framework\Filesystem\Directory\Read
+     */
+    private $pubDirectoryRead;
+
     /**
      * @param \Magento\Framework\Stdlib\StringUtils $string
      * @param \Psr\Log\LoggerInterface $logger
@@ -190,7 +195,8 @@ class Filter extends \Magento\Framework\Filter\Template
      * @param \Magento\Variable\Model\Source\Variables $configVariables
      * @param array $variables
      * @param \Magento\Framework\Css\PreProcessor\Adapter\CssInliner|null $cssInliner
-     *
+     * @param Css\Processor|null $cssProcessor
+     * @param Filesystem|null $pubDirectory
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
     public function __construct(
@@ -208,7 +214,9 @@ public function __construct(
         \Pelago\Emogrifier $emogrifier,
         \Magento\Variable\Model\Source\Variables $configVariables,
         $variables = [],
-        \Magento\Framework\Css\PreProcessor\Adapter\CssInliner $cssInliner = null
+        \Magento\Framework\Css\PreProcessor\Adapter\CssInliner $cssInliner = null,
+        Css\Processor $cssProcessor = null,
+        Filesystem $pubDirectory = null
     ) {
         $this->_escaper = $escaper;
         $this->_assetRepo = $assetRepo;
@@ -224,6 +232,10 @@ public function __construct(
         $this->emogrifier = $emogrifier;
         $this->cssInliner = $cssInliner ?: \Magento\Framework\App\ObjectManager::getInstance()
             ->get(\Magento\Framework\Css\PreProcessor\Adapter\CssInliner::class);
+        $this->cssProcessor = $cssProcessor ?: ObjectManager::getInstance()
+            ->get(Css\Processor::class);
+        $this->pubDirectory = $pubDirectory ?: ObjectManager::getInstance()
+            ->get(Filesystem::class);
         $this->configVariables = $configVariables;
         parent::__construct($string, $variables);
     }
@@ -321,32 +333,14 @@ public function setDesignParams(array $designParams)
     }
 
     /**
-     * Get CSS processor
-     *
-     * @deprecated 100.1.2
-     * @return Css\Processor
-     */
-    private function getCssProcessor()
-    {
-        if (!$this->cssProcessor) {
-            $this->cssProcessor = ObjectManager::getInstance()->get(Css\Processor::class);
-        }
-        return $this->cssProcessor;
-    }
-
-    /**
-     * Get pub directory
+     * Sets pub directory
      *
-     * @deprecated 100.1.2
      * @param string $dirType
-     * @return ReadInterface
+     * @return void
      */
-    private function getPubDirectory($dirType)
+    private function setPubDirectory($dirType)
     {
-        if (!$this->pubDirectory) {
-            $this->pubDirectory = ObjectManager::getInstance()->get(Filesystem::class)->getDirectoryRead($dirType);
-        }
-        return $this->pubDirectory;
+        $this->pubDirectoryRead = $this->pubDirectory->getDirectoryRead($dirType);
     }
 
     /**
@@ -844,7 +838,7 @@ public function cssDirective($construction)
             return '/* ' . __('"file" parameter must be specified') . ' */';
         }
 
-        $css = $this->getCssProcessor()->process(
+        $css = $this->cssProcessor->process(
             $this->getCssFilesContent([$params['file']])
         );
 
@@ -947,9 +941,9 @@ public function getCssFilesContent(array $files)
         try {
             foreach ($files as $file) {
                 $asset = $this->_assetRepo->createAsset($file, $designParams);
-                $pubDirectory = $this->getPubDirectory($asset->getContext()->getBaseDirType());
-                if ($pubDirectory->isExist($asset->getPath())) {
-                    $css .= $pubDirectory->readFile($asset->getPath());
+                $this->setPubDirectory($asset->getContext()->getBaseDirType());
+                if ($this->pubDirectoryRead->isExist($asset->getPath())) {
+                    $css .= $this->pubDirectoryRead->readFile($asset->getPath());
                 } else {
                     $css .= $asset->getContent();
                 }
@@ -979,7 +973,7 @@ public function applyInlineCss($html)
         $cssToInline = $this->getCssFilesContent(
             $this->getInlineCssFiles()
         );
-        $cssToInline = $this->getCssProcessor()->process($cssToInline);
+        $cssToInline = $this->cssProcessor->process($cssToInline);
 
         // Only run Emogrify if HTML and CSS contain content
         if ($html && $cssToInline) {

app/code/Magento/Email/Test/Unit/Model/Template/FilterTest.php

@@ -3,6 +3,9 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
+declare(strict_types=1);
+
 namespace Magento\Email\Test\Unit\Model\Template;
 
 use Magento\Email\Model\Template\Css\Processor;
@@ -14,6 +17,7 @@
 
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ * @SuppressWarnings(PHPMD.TooManyFields)
  */
 class FilterTest extends \PHPUnit\Framework\TestCase
 {
@@ -92,6 +96,21 @@ class FilterTest extends \PHPUnit\Framework\TestCase
      */
     private $cssInliner;
 
+    /**
+     * @var \PHPUnit\Framework\MockObject\MockObject|\Magento\Email\Model\Template\Css\Processor
+     */
+    private $cssProcessor;
+
+    /**
+     * @var \PHPUnit\Framework\MockObject\MockObject|\Magento\Framework\Filesystem
+     */
+    private $pubDirectory;
+
+    /**
+     * @var \PHPUnit\Framework\MockObject\MockObject|\Magento\Framework\Filesystem\Directory\Read
+     */
+    private $pubDirectoryRead;
+
     protected function setUp()
     {
         $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
@@ -147,6 +166,18 @@ protected function setUp()
         $this->cssInliner = $this->objectManager->getObject(
             \Magento\Framework\Css\PreProcessor\Adapter\CssInliner::class
         );
+
+        $this->cssProcessor = $this->getMockBuilder(\Magento\Email\Model\Template\Css\Processor::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $this->pubDirectory = $this->getMockBuilder(\Magento\Framework\Filesystem::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $this->pubDirectoryRead = $this->getMockBuilder(\Magento\Framework\Filesystem\Directory\Read::class)
+            ->disableOriginalConstructor()
+            ->getMock();
     }
 
     /**
@@ -173,6 +204,8 @@ protected function getModel($mockedMethods = null)
                     $this->configVariables,
                     [],
                     $this->cssInliner,
+                    $this->cssProcessor,
+                    $this->pubDirectory
                 ]
             )
             ->setMethods($mockedMethods)
@@ -329,17 +362,16 @@ public function testGetCssFilesContent()
             ->with($file, $designParams)
             ->willReturn($asset);
 
-        $pubDirectory = $this->getMockBuilder(ReadInterface::class)
-            ->getMockForAbstractClass();
-        $reflectionClass = new \ReflectionClass(Filter::class);
-        $reflectionProperty = $reflectionClass->getProperty('pubDirectory');
-        $reflectionProperty->setAccessible(true);
-        $reflectionProperty->setValue($filter, $pubDirectory);
-        $pubDirectory->expects($this->once())
+        $this->pubDirectory
+            ->expects($this->once())
+            ->method('getDirectoryRead')
+            ->willReturn($this->pubDirectoryRead);
+
+        $this->pubDirectoryRead->expects($this->once())
             ->method('isExist')
             ->with($path . DIRECTORY_SEPARATOR . $file)
             ->willReturn(true);
-        $pubDirectory->expects($this->once())
+        $this->pubDirectoryRead->expects($this->once())
             ->method('readFile')
             ->with($path . DIRECTORY_SEPARATOR . $file)
             ->willReturn($css);