From f2468b5680879d3b50fed47fd766d1f074e6ffda Mon Sep 17 00:00:00 2001 From: Svyatoslav Date: Fri, 19 Jun 2020 22:21:23 +0300 Subject: [PATCH 1/2] Improvement PageLayout Config Builder. Added save in cache config files. --- .../Theme/Model/PageLayout/Config/Builder.php | 27 +++++++++++++++---- .../Model/PageLayout/Config/BuilderTest.php | 20 ++++++++++++-- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/app/code/Magento/Theme/Model/PageLayout/Config/Builder.php b/app/code/Magento/Theme/Model/PageLayout/Config/Builder.php index 13b8aa23073ce..a982d6f6fd68f 100644 --- a/app/code/Magento/Theme/Model/PageLayout/Config/Builder.php +++ b/app/code/Magento/Theme/Model/PageLayout/Config/Builder.php @@ -12,6 +12,8 @@ */ class Builder implements \Magento\Framework\View\Model\PageLayout\Config\BuilderInterface { + const CACHE_KEY_LAYOUTS = 'THEME_LAYOUTS_FILES_MERGED'; + /** * @var \Magento\Framework\View\PageLayout\ConfigFactory */ @@ -32,19 +34,27 @@ class Builder implements \Magento\Framework\View\Model\PageLayout\Config\Builder */ private $configFiles = []; + /** + * @var \Magento\Framework\App\Cache\Type\Layout + */ + protected $cacheModel; + /** * @param \Magento\Framework\View\PageLayout\ConfigFactory $configFactory * @param \Magento\Framework\View\PageLayout\File\Collector\Aggregated $fileCollector * @param \Magento\Theme\Model\ResourceModel\Theme\Collection $themeCollection + * @param \Magento\Framework\App\Cache\Type\Layout $cacheModel */ public function __construct( \Magento\Framework\View\PageLayout\ConfigFactory $configFactory, \Magento\Framework\View\PageLayout\File\Collector\Aggregated $fileCollector, - \Magento\Theme\Model\ResourceModel\Theme\Collection $themeCollection + \Magento\Theme\Model\ResourceModel\Theme\Collection $themeCollection, + \Magento\Framework\App\Cache\Type\Layout $cacheModel ) { $this->configFactory = $configFactory; $this->fileCollector = $fileCollector; $this->themeCollection = $themeCollection; + $this->cacheModel = $cacheModel; $this->themeCollection->setItemObjectClass(\Magento\Theme\Model\Theme\Data::class); } @@ -57,7 +67,7 @@ public function getPageLayoutsConfig() } /** - * Retrieve configuration files. + * Retrieve configuration files. Caches merged layouts.xml XML files. * * @return array */ @@ -65,10 +75,17 @@ protected function getConfigFiles() { if (!$this->configFiles) { $configFiles = []; - foreach ($this->themeCollection->loadRegisteredThemes() as $theme) { - $configFiles[] = $this->fileCollector->getFilesContent($theme, 'layouts.xml'); + $this->configFiles = $this->cacheModel->load(self::CACHE_KEY_LAYOUTS); + if (!empty($this->configFiles)) { + $this->configFiles = @unserialize($this->configFiles);//if value in cache is corrupted. + } + if (empty($this->configFiles)) { + foreach ($this->themeCollection->loadRegisteredThemes() as $theme) { + $configFiles[] = $this->fileCollector->getFilesContent($theme, 'layouts.xml'); + } + $this->configFiles = array_merge(...$configFiles); + $this->cacheModel->save(serialize($this->configFiles), self::CACHE_KEY_LAYOUTS); } - $this->configFiles = array_merge(...$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 index d9eccdb871222..f8975172a87b1 100644 --- a/app/code/Magento/Theme/Test/Unit/Model/PageLayout/Config/BuilderTest.php +++ b/app/code/Magento/Theme/Test/Unit/Model/PageLayout/Config/BuilderTest.php @@ -10,14 +10,19 @@ */ namespace Magento\Theme\Test\Unit\Model\PageLayout\Config; +use Magento\Framework\App\Cache\Type\FrontendPool; +use Magento\Framework\App\Cache\Type\Layout as LayoutCache; +use Magento\Framework\Cache\FrontendInterface; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; use Magento\Framework\View\PageLayout\Config; use Magento\Framework\View\PageLayout\File\Collector\Aggregated; +use Magento\TestFramework\Helper\Bootstrap; use Magento\Theme\Model\PageLayout\Config\Builder; use Magento\Theme\Model\ResourceModel\Theme\Collection; use Magento\Theme\Model\Theme\Data; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Magento\Framework\App\Cache\Type\Layout; class BuilderTest extends TestCase { @@ -41,6 +46,11 @@ class BuilderTest extends TestCase */ protected $themeCollection; + /** + * @var Layout|MockObject + */ + protected $cacheModel; + /** * SetUp method * @@ -58,21 +68,26 @@ protected function setUp(): void )->disableOriginalConstructor() ->getMock(); + $helper = new ObjectManager($this); $this->themeCollection = $this->getMockBuilder(Collection::class) ->disableOriginalConstructor() ->getMock(); + $this->cacheModel = $this->getMockBuilder(Layout::class) + ->disableOriginalConstructor() + ->getMock(); + $this->themeCollection->expects($this->once()) ->method('setItemObjectClass') ->with(Data::class) ->willReturnSelf(); - $helper = new ObjectManager($this); $this->builder = $helper->getObject( Builder::class, [ 'configFactory' => $this->configFactory, 'fileCollector' => $this->fileCollector, - 'themeCollection' => $this->themeCollection + 'themeCollection' => $this->themeCollection, + 'cacheModel' => $this->cacheModel, ] ); } @@ -84,6 +99,7 @@ protected function setUp(): void */ public function testGetPageLayoutsConfig() { + $this->cacheModel->clean(); $files1 = ['content layouts_1.xml', 'content layouts_2.xml']; $files2 = ['content layouts_3.xml', 'content layouts_4.xml']; From 276cb1c46ac46ce9cf96c615ce74da395ac96afc Mon Sep 17 00:00:00 2001 From: engcom-Echo Date: Thu, 10 Sep 2020 14:11:46 +0300 Subject: [PATCH 2/2] failing tests fixed --- .../Theme/Model/PageLayout/Config/Builder.php | 57 +++++++++++++------ .../Model/PageLayout/Config/BuilderTest.php | 37 ++++++++---- 2 files changed, 64 insertions(+), 30 deletions(-) diff --git a/app/code/Magento/Theme/Model/PageLayout/Config/Builder.php b/app/code/Magento/Theme/Model/PageLayout/Config/Builder.php index a982d6f6fd68f..e528f9e88d8a4 100644 --- a/app/code/Magento/Theme/Model/PageLayout/Config/Builder.php +++ b/app/code/Magento/Theme/Model/PageLayout/Config/Builder.php @@ -5,27 +5,38 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ +declare(strict_types=1); + namespace Magento\Theme\Model\PageLayout\Config; +use Magento\Framework\App\Cache\Type\Layout; +use Magento\Framework\App\ObjectManager; +use Magento\Framework\View\Model\PageLayout\Config\BuilderInterface; +use Magento\Framework\View\PageLayout\ConfigFactory; +use Magento\Framework\View\PageLayout\File\Collector\Aggregated; +use Magento\Theme\Model\ResourceModel\Theme\Collection; +use Magento\Theme\Model\Theme\Data; +use Magento\Framework\Serialize\SerializerInterface; + /** * Page layout config builder */ -class Builder implements \Magento\Framework\View\Model\PageLayout\Config\BuilderInterface +class Builder implements BuilderInterface { const CACHE_KEY_LAYOUTS = 'THEME_LAYOUTS_FILES_MERGED'; /** - * @var \Magento\Framework\View\PageLayout\ConfigFactory + * @var ConfigFactory */ protected $configFactory; /** - * @var \Magento\Framework\View\PageLayout\File\Collector\Aggregated + * @var Aggregated */ protected $fileCollector; /** - * @var \Magento\Theme\Model\ResourceModel\Theme\Collection + * @var Collection */ protected $themeCollection; @@ -35,27 +46,36 @@ class Builder implements \Magento\Framework\View\Model\PageLayout\Config\Builder private $configFiles = []; /** - * @var \Magento\Framework\App\Cache\Type\Layout + * @var Layout|null + */ + private $cacheModel; + /** + * @var SerializerInterface|null */ - protected $cacheModel; + private $serializer; /** - * @param \Magento\Framework\View\PageLayout\ConfigFactory $configFactory - * @param \Magento\Framework\View\PageLayout\File\Collector\Aggregated $fileCollector - * @param \Magento\Theme\Model\ResourceModel\Theme\Collection $themeCollection - * @param \Magento\Framework\App\Cache\Type\Layout $cacheModel + * @param ConfigFactory $configFactory + * @param Aggregated $fileCollector + * @param Collection $themeCollection + * @param Layout|null $cacheModel + * @param SerializerInterface|null $serializer */ public function __construct( - \Magento\Framework\View\PageLayout\ConfigFactory $configFactory, - \Magento\Framework\View\PageLayout\File\Collector\Aggregated $fileCollector, - \Magento\Theme\Model\ResourceModel\Theme\Collection $themeCollection, - \Magento\Framework\App\Cache\Type\Layout $cacheModel + ConfigFactory $configFactory, + Aggregated $fileCollector, + Collection $themeCollection, + ?Layout $cacheModel = null, + ?SerializerInterface $serializer = null ) { $this->configFactory = $configFactory; $this->fileCollector = $fileCollector; $this->themeCollection = $themeCollection; - $this->cacheModel = $cacheModel; - $this->themeCollection->setItemObjectClass(\Magento\Theme\Model\Theme\Data::class); + $this->themeCollection->setItemObjectClass(Data::class); + $this->cacheModel = $cacheModel + ?? ObjectManager::getInstance()->get(Layout::class); + $this->serializer = $serializer + ?? ObjectManager::getInstance()->get(SerializerInterface::class); } /** @@ -77,14 +97,15 @@ protected function getConfigFiles() $configFiles = []; $this->configFiles = $this->cacheModel->load(self::CACHE_KEY_LAYOUTS); if (!empty($this->configFiles)) { - $this->configFiles = @unserialize($this->configFiles);//if value in cache is corrupted. + //if value in cache is corrupted. + $this->configFiles = $this->serializer->unserialize($this->configFiles); } if (empty($this->configFiles)) { foreach ($this->themeCollection->loadRegisteredThemes() as $theme) { $configFiles[] = $this->fileCollector->getFilesContent($theme, 'layouts.xml'); } $this->configFiles = array_merge(...$configFiles); - $this->cacheModel->save(serialize($this->configFiles), self::CACHE_KEY_LAYOUTS); + $this->cacheModel->save($this->serializer->serialize($this->configFiles), self::CACHE_KEY_LAYOUTS); } } 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 index f8975172a87b1..2e2117b79e5ab 100644 --- a/app/code/Magento/Theme/Test/Unit/Model/PageLayout/Config/BuilderTest.php +++ b/app/code/Magento/Theme/Test/Unit/Model/PageLayout/Config/BuilderTest.php @@ -10,19 +10,17 @@ */ namespace Magento\Theme\Test\Unit\Model\PageLayout\Config; -use Magento\Framework\App\Cache\Type\FrontendPool; -use Magento\Framework\App\Cache\Type\Layout as LayoutCache; -use Magento\Framework\Cache\FrontendInterface; +use Magento\Framework\App\Cache\Type\Layout; +use Magento\Framework\Serialize\SerializerInterface; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; use Magento\Framework\View\PageLayout\Config; +use Magento\Framework\View\PageLayout\ConfigFactory; use Magento\Framework\View\PageLayout\File\Collector\Aggregated; -use Magento\TestFramework\Helper\Bootstrap; use Magento\Theme\Model\PageLayout\Config\Builder; use Magento\Theme\Model\ResourceModel\Theme\Collection; use Magento\Theme\Model\Theme\Data; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Magento\Framework\App\Cache\Type\Layout; class BuilderTest extends TestCase { @@ -32,7 +30,7 @@ class BuilderTest extends TestCase protected $builder; /** - * @var \Magento\Framework\View\PageLayout\ConfigFactory|MockObject + * @var ConfigFactory|MockObject */ protected $configFactory; @@ -50,6 +48,10 @@ class BuilderTest extends TestCase * @var Layout|MockObject */ protected $cacheModel; + /** + * @var SerializerInterface|MockObject + */ + protected $serializer; /** * SetUp method @@ -58,17 +60,15 @@ class BuilderTest extends TestCase */ protected function setUp(): void { - $this->configFactory = $this->getMockBuilder(\Magento\Framework\View\PageLayout\ConfigFactory::class) + $this->configFactory = $this->getMockBuilder(ConfigFactory::class) ->disableOriginalConstructor() ->setMethods(['create']) ->getMock(); - $this->fileCollector = $this->getMockBuilder( - Aggregated::class - )->disableOriginalConstructor() + $this->fileCollector = $this->getMockBuilder(Aggregated::class) + ->disableOriginalConstructor() ->getMock(); - $helper = new ObjectManager($this); $this->themeCollection = $this->getMockBuilder(Collection::class) ->disableOriginalConstructor() ->getMock(); @@ -76,10 +76,13 @@ protected function setUp(): void ->disableOriginalConstructor() ->getMock(); + $this->serializer = $this->getMockForAbstractClass(SerializerInterface::class); + $this->themeCollection->expects($this->once()) ->method('setItemObjectClass') ->with(Data::class) ->willReturnSelf(); + $helper = new ObjectManager($this); $this->builder = $helper->getObject( Builder::class, @@ -88,6 +91,7 @@ protected function setUp(): void 'fileCollector' => $this->fileCollector, 'themeCollection' => $this->themeCollection, 'cacheModel' => $this->cacheModel, + 'serializer' => $this->serializer, ] ); } @@ -102,6 +106,7 @@ public function testGetPageLayoutsConfig() $this->cacheModel->clean(); $files1 = ['content layouts_1.xml', 'content layouts_2.xml']; $files2 = ['content layouts_3.xml', 'content layouts_4.xml']; + $configFiles = array_merge($files1, $files2); $theme1 = $this->getMockBuilder(Data::class) ->disableOriginalConstructor() @@ -129,9 +134,17 @@ public function testGetPageLayoutsConfig() $this->configFactory->expects($this->once()) ->method('create') - ->with(['configFiles' => array_merge($files1, $files2)]) + ->with(['configFiles' => $configFiles]) ->willReturn($config); + $this->serializer->expects($this->once()) + ->method('serialize') + ->with($configFiles); + + $this->cacheModel->expects($this->once()) + ->method('save') + ->willReturnSelf(); + $this->assertSame($config, $this->builder->getPageLayoutsConfig()); } }