ENGCOM-3634: Avoid duplicate loading of configuration files #19585

magento · Jan 25, 2019 · 0b0e952 · 0b0e952
2 parents a556e3c + 3356534
commit 0b0e952
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 26 deletions.
diff --git a/app/code/Magento/Catalog/Model/Category/Attribute/Source/Layout.php b/app/code/Magento/Catalog/Model/Category/Attribute/Source/Layout.php
@@ -17,6 +17,12 @@ class Layout extends \Magento\Eav\Model\Entity\Attribute\Source\AbstractSource
      */
     protected $pageLayoutBuilder;
 
+    /**
+     * @inheritdoc
+     * @deprecated since the cache is now handled by \Magento\Theme\Model\PageLayout\Config\Builder::$configFiles
+     */
+    protected $_options = null;
+
     /**
      * @param \Magento\Framework\View\Model\PageLayout\Config\BuilderInterface $pageLayoutBuilder
      */
@@ -26,14 +32,14 @@ public function __construct(\Magento\Framework\View\Model\PageLayout\Config\Buil
     }
 
     /**
-     * {@inheritdoc}
+     * @inheritdoc
      */
     public function getAllOptions()
     {
-        if (!$this->_options) {
-            $this->_options = $this->pageLayoutBuilder->getPageLayoutsConfig()->toOptionArray();
-            array_unshift($this->_options, ['value' => '', 'label' => __('No layout updates')]);
-        }
-        return $this->_options;
+        $options = $this->pageLayoutBuilder->getPageLayoutsConfig()->toOptionArray();
+        array_unshift($options, ['value' => '', 'label' => __('No layout updates')]);
+        $this->_options = $options;
+
+        return $options;
     }
 }
diff --git a/app/code/Magento/Catalog/Model/Product/Attribute/Source/Layout.php b/app/code/Magento/Catalog/Model/Product/Attribute/Source/Layout.php
@@ -17,6 +17,12 @@ class Layout extends \Magento\Eav\Model\Entity\Attribute\Source\AbstractSource
      */
     protected $pageLayoutBuilder;
 
+    /**
+     * @inheritdoc
+     * @deprecated since the cache is now handled by \Magento\Theme\Model\PageLayout\Config\Builder::$configFiles
+     */
+    protected $_options = null;
+
     /**
      * @param \Magento\Framework\View\Model\PageLayout\Config\BuilderInterface $pageLayoutBuilder
      */
@@ -26,14 +32,14 @@ public function __construct(\Magento\Framework\View\Model\PageLayout\Config\Buil
     }
 
     /**
-     * @return array
+     * @inheritdoc
      */
     public function getAllOptions()
     {
-        if (!$this->_options) {
-            $this->_options = $this->pageLayoutBuilder->getPageLayoutsConfig()->toOptionArray();
-            array_unshift($this->_options, ['value' => '', 'label' => __('No layout updates')]);
-        }
-        return $this->_options;
+        $options = $this->pageLayoutBuilder->getPageLayoutsConfig()->toOptionArray();
+        array_unshift($options, ['value' => '', 'label' => __('No layout updates')]);
+        $this->_options = $options;
+
+        return $options;
     }
 }
diff --git a/app/code/Magento/Cms/Model/Page/Source/PageLayout.php b/app/code/Magento/Cms/Model/Page/Source/PageLayout.php
@@ -20,6 +20,7 @@ class PageLayout implements OptionSourceInterface
 
     /**
      * @var array
+     * @deprecated since the cache is now handled by \Magento\Theme\Model\PageLayout\Config\Builder::$configFiles
      */
     protected $options;
 
@@ -34,16 +35,10 @@ public function __construct(BuilderInterface $pageLayoutBuilder)
     }
 
     /**
-     * Get options
-     *
-     * @return array
+     * @inheritdoc
      */
     public function toOptionArray()
     {
-        if ($this->options !== null) {
-            return $this->options;
-        }
-
         $configOptions = $this->pageLayoutBuilder->getPageLayoutsConfig()->getOptions();
         $options = [];
         foreach ($configOptions as $key => $value) {
@@ -54,6 +49,6 @@ public function toOptionArray()
         }
         $this->options = $options;
 
-        return $this->options;
+        return $options;
     }
 }
diff --git a/app/code/Magento/Theme/Model/PageLayout/Config/Builder.php b/app/code/Magento/Theme/Model/PageLayout/Config/Builder.php
@@ -27,6 +27,11 @@ class Builder implements \Magento\Framework\View\Model\PageLayout\Config\Builder
      */
     protected $themeCollection;
 
+    /**
+     * @var array
+     */
+    private $configFiles = [];
+
     /**
      * @param \Magento\Framework\View\PageLayout\ConfigFactory $configFactory
      * @param \Magento\Framework\View\PageLayout\File\Collector\Aggregated $fileCollector
@@ -44,23 +49,28 @@ public function __construct(
     }
 
     /**
-     * @return \Magento\Framework\View\PageLayout\Config
+     * @inheritdoc
      */
     public function getPageLayoutsConfig()
     {
         return $this->configFactory->create(['configFiles' => $this->getConfigFiles()]);
     }
 
     /**
+     * Retrieve configuration files.
+     *
      * @return array
      */
     protected function getConfigFiles()
     {
-        $configFiles = [];
-        foreach ($this->themeCollection->loadRegisteredThemes() as $theme) {
-            $configFiles = array_merge($configFiles, $this->fileCollector->getFilesContent($theme, 'layouts.xml'));
+        if (!$this->configFiles) {
+            $configFiles = [];
+            foreach ($this->themeCollection->loadRegisteredThemes() as $theme) {
+                $configFiles[] = $this->fileCollector->getFilesContent($theme, 'layouts.xml');
+            }
+            $this->configFiles = array_merge(...$configFiles);
         }
 
-        return $configFiles;
+        return $this->configFiles;
     }
 }
diff --git a/app/code/Magento/Theme/Test/Unit/Model/PageLayout/Config/BuilderTest.php b/app/code/Magento/Theme/Test/Unit/Model/PageLayout/Config/BuilderTest.php
@@ -83,7 +83,7 @@ public function testGetPageLayoutsConfig()
             ->disableOriginalConstructor()
             ->getMock();
 
-        $this->themeCollection->expects($this->any())
+        $this->themeCollection->expects($this->once())
             ->method('loadRegisteredThemes')
             ->willReturn([$theme1, $theme2]);