Merge pull request #4988 from magento-engcom/2.3-develop-expedited-prs

[Magento Community Engineering] Community Contributions - 2.3-develop expedited
magento · Nov 12, 2019 · 29cbd2f · 29cbd2f
2 parents a74a8a8 + c8bc476
commit 29cbd2f
Show file tree

Hide file tree

Showing 6 changed files with 268 additions and 79 deletions.
diff --git a/README.md b/README.md
@@ -1,7 +1,8 @@
 [![Open Source Helpers](https://www.codetriage.com/magento/magento2/badges/users.svg)](https://www.codetriage.com/magento/magento2)
 [![Gitter](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/magento/magento2?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge)
 [![Crowdin](https://d322cqt584bo4o.cloudfront.net/magento-2/localized.svg)](https://crowdin.com/project/magento-2)
-<h2>Welcome</h2>
+
+## Welcome
 Welcome to Magento 2 installation! We're glad you chose to install Magento 2, a cutting-edge, feature-rich eCommerce solution that gets results.
 
 ## Magento System Requirements
@@ -30,7 +31,7 @@ To suggest documentation improvements, click [here][4].
 [4]: https://devdocs.magento.com
 
 <h3>Community Maintainers</h3>
-The members of this team have been recognized for their outstanding commitment to maintaining and improving Magento. Magento has granted them permission to accept, merge, and reject pull requests, as well as review issues, and thanks these Community Maintainers for their valuable contributions.
+The members of this team have been recognized for their outstanding commitment to maintaining and improving Magento. Magento has granted them permission to accept, merge, and reject pull requests, as well as review issues, and thanks to these Community Maintainers for their valuable contributions.
 
 <a href="https://magento.com/magento-contributors#maintainers">
     <img src="https://raw.githubusercontent.com/wiki/magento/magento2/images/maintainers.png"/>

diff --git a/app/code/Magento/Email/Model/Template/Filter.php b/app/code/Magento/Email/Model/Template/Filter.php
@@ -167,15 +167,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
@@ -192,9 +197,10 @@ 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 array $directiveProcessors
      * @param VariableResolverInterface|null $variableResolver
+     * @param Css\Processor|null $cssProcessor
+     * @param Filesystem|null $pubDirectory
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
     public function __construct(
@@ -214,7 +220,9 @@ public function __construct(
         $variables = [],
         \Magento\Framework\Css\PreProcessor\Adapter\CssInliner $cssInliner = null,
         array $directiveProcessors = [],
-        VariableResolverInterface $variableResolver = null
+        VariableResolverInterface $variableResolver = null,
+        Css\Processor $cssProcessor = null,
+        Filesystem $pubDirectory = null
     ) {
         $this->_escaper = $escaper;
         $this->_assetRepo = $assetRepo;
@@ -230,6 +238,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, $directiveProcessors, $variableResolver);
     }
@@ -327,32 +339,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);
     }
 
     /**
@@ -856,7 +850,7 @@ public function cssDirective($construction)
             return '/* ' . __('"file" parameter must be specified') . ' */';
         }
 
-        $css = $this->getCssProcessor()->process(
+        $css = $this->cssProcessor->process(
             $this->getCssFilesContent([$params['file']])
         );
 
@@ -959,9 +953,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();
                 }
@@ -991,7 +985,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) {

diff --git a/app/code/Magento/Email/Test/Unit/Model/Template/FilterTest.php b/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,31 @@ 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;
+
+    /**
+     * @var \PHPUnit\Framework\MockObject\MockObject|\Magento\Framework\Filter\VariableResolver\StrategyResolver
+     */
+    private $variableResolver;
+
+    /**
+     * @var array
+     */
+    private $directiveProcessors;
+
     protected function setUp()
     {
         $this->objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
@@ -147,6 +176,41 @@ 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();
+        $this->variableResolver =
+            $this->getMockBuilder(\Magento\Framework\Filter\VariableResolver\StrategyResolver::class)
+                ->disableOriginalConstructor()
+                ->getMock();
+
+        $this->directiveProcessors = [
+            'depend' =>
+                $this->getMockBuilder(\Magento\Framework\Filter\DirectiveProcessor\DependDirective::class)
+                    ->disableOriginalConstructor()
+                    ->getMock(),
+            'if' =>
+                $this->getMockBuilder(\Magento\Framework\Filter\DirectiveProcessor\IfDirective::class)
+                    ->disableOriginalConstructor()
+                    ->getMock(),
+            'template' =>
+                $this->getMockBuilder(\Magento\Framework\Filter\DirectiveProcessor\TemplateDirective::class)
+                    ->disableOriginalConstructor()
+                    ->getMock(),
+            'legacy' =>
+                $this->getMockBuilder(\Magento\Framework\Filter\DirectiveProcessor\LegacyDirective::class)
+                    ->disableOriginalConstructor()
+                    ->getMock(),
+        ];
     }
 
     /**
@@ -173,6 +237,10 @@ protected function getModel($mockedMethods = null)
                     $this->configVariables,
                     [],
                     $this->cssInliner,
+                    $this->directiveProcessors,
+                    $this->variableResolver,
+                    $this->cssProcessor,
+                    $this->pubDirectory
                 ]
             )
             ->setMethods($mockedMethods)
@@ -252,17 +320,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);
@@ -319,43 +386,6 @@ public function testApplyInlineCssThrowsExceptionWhenDesignParamsNotSet()
         $filter->applyInlineCss('test');
     }
 
-    /**
-     * Ensure that after filter callbacks are reset after exception is thrown during filtering
-     */
-    public function testAfterFilterCallbackGetsResetWhenExceptionTriggered()
-    {
-        $value = '{{var random_var}}';
-        $exception = new \Exception('Test exception');
-        $exceptionResult = sprintf(__('Error filtering template: %s'), $exception->getMessage());
-
-        $this->appState->expects($this->once())
-            ->method('getMode')
-            ->will($this->returnValue(\Magento\Framework\App\State::MODE_DEVELOPER));
-        $this->logger->expects($this->once())
-            ->method('critical')
-            ->with($exception);
-
-        $filter = $this->getModel(['varDirective', 'resetAfterFilterCallbacks']);
-        $filter->expects($this->once())
-            ->method('varDirective')
-            ->will($this->throwException($exception));
-
-        // Callbacks must be reset after exception is thrown
-        $filter->expects($this->once())
-            ->method('resetAfterFilterCallbacks');
-
-        // Build arbitrary object to pass into the addAfterFilterCallback method
-        $callbackObject = $this->getMockBuilder('stdObject')
-            ->setMethods(['afterFilterCallbackMethod'])
-            ->getMock();
-        // Callback should never run due to exception happening during filtering
-        $callbackObject->expects($this->never())
-            ->method('afterFilterCallbackMethod');
-        $filter->addAfterFilterCallback([$callbackObject, 'afterFilterCallbackMethod']);
-
-        $this->assertEquals($exceptionResult, $filter->filter($value));
-    }
-
     public function testConfigDirectiveAvailable()
     {
         $path = "web/unsecure/base_url";