MAGETWO-62660: [GitHub] Overly aggressive performance optimizations of the static content deployment #7862

Viktor Paladiychuk · Viktor Paladiychuk · commit 2f245a07c7c6 · 2017-02-01T15:35:12.000+02:00
diff --git a/app/code/Magento/Deploy/Model/Deploy/JsDictionaryDeploy.php b/app/code/Magento/Deploy/Model/Deploy/JsDictionaryDeploy.php
@@ -0,0 +1,86 @@
+<?php
+/**
+ * Copyright © 2013-2017 Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Deploy\Model\Deploy;
+
+use Magento\Framework\App\View\Asset\Publisher;
+use Magento\Framework\View\Asset\Repository;
+use Magento\Framework\Translate\Js\Config as TranslationJsConfig;
+use Magento\Framework\TranslateInterface;
+use Magento\Framework\Console\Cli;
+use Symfony\Component\Console\Output\OutputInterface;
+
+class JsDictionaryDeploy implements DeployInterface
+{
+    /**
+     * @var Repository
+     */
+    private $assetRepo;
+
+    /**
+     * @var Publisher
+     */
+    private $assetPublisher;
+
+    /**
+     * @var TranslationJsConfig
+     */
+    private $translationJsConfig;
+
+    /**
+     * @var TranslateInterface
+     */
+    private $translator;
+
+    /**
+     * @var OutputInterface
+     */
+    private $output;
+
+    /**
+     * @param Repository $assetRepo
+     * @param Publisher $assetPublisher
+     * @param TranslationJsConfig $translationJsConfig
+     * @param TranslateInterface $translator
+     * @param OutputInterface $output
+     */
+    public function __construct(
+        Repository $assetRepo,
+        Publisher $assetPublisher,
+        TranslationJsConfig $translationJsConfig,
+        TranslateInterface $translator,
+        OutputInterface $output
+    ) {
+        $this->assetRepo = $assetRepo;
+        $this->assetPublisher = $assetPublisher;
+        $this->translationJsConfig = $translationJsConfig;
+        $this->translator = $translator;
+        $this->output = $output;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function deploy($area, $themePath, $locale)
+    {
+        $this->translator->setLocale($locale);
+        $this->translator->loadData($area, true);
+
+        $asset = $this->assetRepo->createAsset(
+            $this->translationJsConfig->getDictionaryFileName(),
+            ['area' => $area, 'theme' => $themePath, 'locale' => $locale]
+        );
+        if ($this->output->isVeryVerbose()) {
+            $this->output->writeln("\tDeploying the file to '{$asset->getPath()}'");
+        } else {
+            $this->output->write('.');
+        }
+
+        $this->assetPublisher->publish($asset);
+
+        return Cli::RETURN_SUCCESS;
+    }
+}
diff --git a/app/code/Magento/Deploy/Model/Deploy/LocaleQuickDeploy.php b/app/code/Magento/Deploy/Model/Deploy/LocaleQuickDeploy.php
@@ -13,11 +13,9 @@
 use Magento\Framework\Console\Cli;
 use Magento\Deploy\Console\Command\DeployStaticOptionsInterface as Options;
 use Magento\Framework\RequireJs\Config as RequireJsConfig;
-use Magento\Framework\App\View\Asset\Publisher;
-use Magento\Framework\View\Asset\Repository;
+use Magento\Framework\Translate\Js\Config as TranslationJsConfig;
 use Magento\Framework\App\ObjectManager;
-use Magento\Translation\Model\Js\Config as TranslationJsConfig;
-use Magento\Framework\TranslateInterface;
+use Magento\Deploy\Model\DeployStrategyFactory;
 
 class LocaleQuickDeploy implements DeployInterface
 {
@@ -42,51 +40,41 @@ class LocaleQuickDeploy implements DeployInterface
     private $options = [];
 
     /**
-     * @var Repository
-     */
-    private $assetRepo;
-
-    /**
-     * @var Publisher
+     * @var TranslationJsConfig
      */
-    private $assetPublisher;
+    private $translationJsConfig;
 
     /**
-     * @var TranslationJsConfig
+     * @var DeployStrategyFactory
      */
-    private $translationJsConfig;
+    private $deployStrategyFactory;
 
     /**
-     * @var TranslateInterface
+     * @var DeployInterface[]
      */
-    private $translator;
+    private $deploys;
 
     /**
      * @param Filesystem $filesystem
      * @param OutputInterface $output
      * @param array $options
-     * @param Repository $assetRepo
-     * @param Publisher $assetPublisher
      * @param TranslationJsConfig $translationJsConfig
-     * @param TranslateInterface $translator
+     * @param DeployStrategyFactory $deployStrategyFactory
      */
     public function __construct(
         Filesystem $filesystem,
         OutputInterface $output,
         $options = [],
-        Repository $assetRepo = null,
-        Publisher $assetPublisher = null,
         TranslationJsConfig $translationJsConfig = null,
-        TranslateInterface $translator = null
+        DeployStrategyFactory $deployStrategyFactory = null
     ) {
         $this->filesystem = $filesystem;
         $this->output = $output;
         $this->options = $options;
-        $this->assetRepo = $assetRepo ?: ObjectManager::getInstance()->get(Repository::class);
-        $this->assetPublisher = $assetPublisher ?: ObjectManager::getInstance()->get(Publisher::class);
         $this->translationJsConfig = $translationJsConfig
             ?: ObjectManager::getInstance()->get(TranslationJsConfig::class);
-        $this->translator = $translator ?: ObjectManager::getInstance()->get(TranslateInterface::class);
+        $this->deployStrategyFactory = $deployStrategyFactory
+            ?: ObjectManager::getInstance()->get(DeployStrategyFactory::class);
     }
 
     /**
@@ -140,16 +128,23 @@ public function deploy($area, $themePath, $locale)
             $jsDictionaryEnabled = $this->translationJsConfig->dictionaryEnabled();
             foreach ($localeFiles as $path) {
                 if ($this->getStaticDirectory()->isFile($path)) {
-                    $destination = $this->replaceLocaleInPath($path, $baseLocale, $locale);
                     if (!$jsDictionaryEnabled || !$this->isJsDictionary($path)) {
+                        $destination = $this->replaceLocaleInPath($path, $baseLocale, $locale);
                         $this->getStaticDirectory()->copyFile($path, $destination);
+                        $processedFiles++;
                     }
-                    $processedFiles++;
                 }
             }
 
             if ($jsDictionaryEnabled) {
-                $this->deployJsDictionary($area, $themePath, $locale);
+                $this->getDeploy(
+                        DeployStrategyFactory::DEPLOY_STRATEGY_JS_DICTIONARY,
+                        [
+                            'output' => $this->output,
+                            'translationJsConfig' => $this->translationJsConfig
+                        ]
+                    )
+                    ->deploy($area, $themePath, $locale);
                 $processedFiles++;
             }
 
@@ -160,40 +155,29 @@ public function deploy($area, $themePath, $locale)
     }
 
     /**
-     * Define if provided path is js dictionary
+     * Get deploy strategy according to required strategy
      *
-     * @param string $path
-     * @return bool
+     * @param string $strategy
+     * @param array $params
+     * @return DeployInterface
      */
-    private function isJsDictionary($path)
+    private function getDeploy($strategy, $params)
     {
-        return strpos($path, $this->translationJsConfig->getDictionaryFileName()) !== false;
+        if (empty($this->deploys[$strategy])) {
+            $this->deploys[$strategy] = $this->deployStrategyFactory->create($strategy, $params);
+        }
+        return $this->deploys[$strategy];
     }
 
     /**
-     * Deploy js-dictionary for specific locale, theme and area
+     * Define if provided path is js dictionary
      *
-     * @param string $area
-     * @param string $themePath
-     * @param string $locale
-     * @return void
+     * @param string $path
+     * @return bool
      */
-    private function deployJsDictionary($area, $themePath, $locale)
+    private function isJsDictionary($path)
     {
-        $this->translator->setLocale($locale);
-        $this->translator->loadData($area, true);
-
-        $asset = $this->assetRepo->createAsset(
-            $this->translationJsConfig->getDictionaryFileName(),
-            ['area' => $area, 'theme' => $themePath, 'locale' => $locale]
-        );
-        if ($this->output->isVeryVerbose()) {
-            $this->output->writeln("\tDeploying the file to '{$asset->getPath()}'");
-        } else {
-            $this->output->write('.');
-        }
-
-        $this->assetPublisher->publish($asset);
+        return strpos($path, $this->translationJsConfig->getDictionaryFileName()) !== false;
     }
 
     /**
diff --git a/app/code/Magento/Deploy/Model/DeployStrategyFactory.php b/app/code/Magento/Deploy/Model/DeployStrategyFactory.php
@@ -22,6 +22,11 @@ class DeployStrategyFactory
      */
     const DEPLOY_STRATEGY_QUICK = 'quick';
 
+    /**
+     * Strategy for deploying js dictionary
+     */
+    const DEPLOY_STRATEGY_JS_DICTIONARY = 'js-dictionary';
+
     /**
      * @param ObjectManagerInterface $objectManager
      */
@@ -41,6 +46,7 @@ public function create($type, array $arguments = [])
         $strategyMap = [
             self::DEPLOY_STRATEGY_STANDARD => Deploy\LocaleDeploy::class,
             self::DEPLOY_STRATEGY_QUICK => Deploy\LocaleQuickDeploy::class,
+            self::DEPLOY_STRATEGY_JS_DICTIONARY => Deploy\JsDictionaryDeploy::class
         ];
 
         if (!isset($strategyMap[$type])) {
diff --git a/app/code/Magento/Deploy/Test/Unit/Model/Deploy/JsDictionaryDeployTest.php b/app/code/Magento/Deploy/Test/Unit/Model/Deploy/JsDictionaryDeployTest.php
@@ -0,0 +1,105 @@
+<?php
+/**
+ * Copyright © 2013-2017 Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Deploy\Test\Unit\Model\Deploy;
+
+use Symfony\Component\Console\Output\OutputInterface;
+use Magento\Framework\Translate\Js\Config as TranslationJsConfig;
+use Magento\Framework\TranslateInterface;
+use Magento\Framework\View\Asset\Repository;
+use Magento\Framework\View\Asset\LocalInterface as Asset;
+use Magento\Framework\App\View\Asset\Publisher;
+use Magento\Deploy\Model\Deploy\JsDictionaryDeploy;
+use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
+
+class JsDictionaryDeployTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @var TranslationJsConfig|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $translationJsConfig;
+
+    /**
+     * @var TranslateInterface|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $translator;
+
+    /**
+     * @var Repository|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $assetRepo;
+
+    /**
+     * @var Asset|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $asset;
+
+    /**
+     * @var Publisher|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $assetPublisher;
+
+    /**
+     * @var JsDictionaryDeploy
+     */
+    private $model;
+
+    /**
+     * @var OutputInterface|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $output;
+
+    protected function setUp()
+    {
+        $this->output = $this->getMockBuilder(OutputInterface::class)
+            ->setMethods(['writeln', 'isVeryVerbose'])
+            ->getMockForAbstractClass();
+
+        $this->translationJsConfig = $this->getMock(TranslationJsConfig::class, [], [], '', false);
+        $this->translator = $this->getMockForAbstractClass(TranslateInterface::class, [], '', false, false, true);
+        $this->assetRepo = $this->getMock(Repository::class, [], [], '', false);
+        $this->asset = $this->getMockForAbstractClass(Asset::class, [], '', false, false, true);
+        $this->assetPublisher = $this->getMock(Publisher::class, [], [], '', false);
+
+        $this->model = (new ObjectManager($this))->getObject(
+            JsDictionaryDeploy::class,
+            [
+                'translationJsConfig' => $this->translationJsConfig,
+                'translator' => $this->translator,
+                'assetRepo' => $this->assetRepo,
+                'assetPublisher' => $this->assetPublisher,
+                'output' => $this->output
+            ]
+        );
+    }
+
+    public function testDeploy()
+    {
+        $area = 'adminhtml';
+        $themePath = 'Magento/backend';
+        $locale = 'uk_UA';
+
+        $dictionary = 'js-translation.json';
+
+        $this->translationJsConfig->expects(self::exactly(1))->method('getDictionaryFileName')
+            ->willReturn($dictionary);
+
+        $this->translator->expects($this->once())->method('setLocale')->with($locale);
+        $this->translator->expects($this->once())->method('loadData')->with($area, true);
+
+        $this->assetRepo->expects($this->once())->method('createAsset')
+            ->with(
+                $dictionary,
+                ['area' => $area, 'theme' => $themePath, 'locale' => $locale]
+            )
+            ->willReturn($this->asset);
+
+        $this->assetPublisher->expects($this->once())->method('publish');
+
+        $this->model->deploy($area, $themePath, $locale);
+    }
+
+}
diff --git a/app/code/Magento/Deploy/Test/Unit/Model/Deploy/LocaleQuickDeployTest.php b/app/code/Magento/Deploy/Test/Unit/Model/Deploy/LocaleQuickDeployTest.php
