diff --git a/dev/tests/integration/testsuite/Magento/Framework/View/Layout/MergeTest.php b/dev/tests/integration/testsuite/Magento/Framework/View/Layout/MergeTest.php index 8404884a7cf5c..c8c80a9647020 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/View/Layout/MergeTest.php +++ b/dev/tests/integration/testsuite/Magento/Framework/View/Layout/MergeTest.php @@ -41,6 +41,11 @@ class MergeTest extends \PHPUnit\Framework\TestCase */ protected $_cache; + /** + * @var \Magento\Framework\Serialize\SerializerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + protected $_serializer; + /** * @var \PHPUnit_Framework_MockObject_MockObject */ @@ -74,7 +79,8 @@ class MergeTest extends \PHPUnit\Framework\TestCase protected function setUp() { $files = []; - foreach (glob(__DIR__ . '/_mergeFiles/layout/*.xml') as $filename) { + $fileDriver = new \Magento\Framework\Filesystem\Driver\File(); + foreach ($fileDriver->readDirectory(__DIR__ . '/_mergeFiles/layout/') as $filename) { $files[] = new \Magento\Framework\View\File($filename, 'Magento_Widget'); } $fileSource = $this->getMockForAbstractClass(\Magento\Framework\View\File\CollectorInterface::class); @@ -100,6 +106,8 @@ protected function setUp() $this->_cache = $this->getMockForAbstractClass(\Magento\Framework\Cache\FrontendInterface::class); + $this->_serializer = $this->getMockForAbstractClass(\Magento\Framework\Serialize\SerializerInterface::class); + $this->_theme = $this->createMock(\Magento\Theme\Model\Theme::class); $this->_theme->expects($this->any())->method('isPhysical')->will($this->returnValue(true)); $this->_theme->expects($this->any())->method('getArea')->will($this->returnValue('area')); @@ -140,6 +148,7 @@ function ($filename) use ($fileDriver) { 'resource' => $this->_resource, 'appState' => $this->_appState, 'cache' => $this->_cache, + 'serializer' => $this->_serializer, 'theme' => $this->_theme, 'validator' => $this->_layoutValidator, 'logger' => $this->_logger, @@ -276,9 +285,16 @@ public function testLoadFileSystemWithPageLayout() public function testLoadCache() { + $cacheValue = [ + "pageLayout" => "1column", + "layout" => self::FIXTURE_LAYOUT_XML + ]; + $this->_cache->expects($this->at(0))->method('load') - ->with('LAYOUT_area_STORE20_100c6a4ccd050e33acef0553f24ef399961') - ->will($this->returnValue(self::FIXTURE_LAYOUT_XML)); + ->with('LAYOUT_area_STORE20_100c6a4ccd050e33acef0553f24ef399961_page_layout_merged') + ->will($this->returnValue(json_encode($cacheValue))); + + $this->_serializer->expects($this->once())->method('unserialize')->willReturn($cacheValue); $this->assertEmpty($this->_model->getHandles()); $this->assertEmpty($this->_model->asString()); @@ -424,8 +440,10 @@ public function testLoadWithInvalidLayout() ->method('isValid') ->willThrowException(new \Exception('Layout is invalid.')); + // phpcs:ignore Magento2.Security.InsecureFunction $suffix = md5(implode('|', $this->_model->getHandles())); - $cacheId = "LAYOUT_{$this->_theme->getArea()}_STORE{$this->scope->getId()}_{$this->_theme->getId()}{$suffix}"; + $cacheId = "LAYOUT_{$this->_theme->getArea()}_STORE{$this->scope->getId()}" + . "_{$this->_theme->getId()}{$suffix}_page_layout_merged"; $messages = $this->_layoutValidator->getMessages(); // Testing error message is logged with logger diff --git a/lib/internal/Magento/Framework/View/Model/Layout/Merge.php b/lib/internal/Magento/Framework/View/Model/Layout/Merge.php index a0cdbfb7d8fe7..3ccc144ebecd5 100644 --- a/lib/internal/Magento/Framework/View/Model/Layout/Merge.php +++ b/lib/internal/Magento/Framework/View/Model/Layout/Merge.php @@ -5,10 +5,12 @@ */ namespace Magento\Framework\View\Model\Layout; +use Magento\Framework\App\ObjectManager; use Magento\Framework\App\State; use Magento\Framework\Config\Dom\ValidationException; use Magento\Framework\Filesystem\DriverPool; use Magento\Framework\Filesystem\File\ReadFactory; +use Magento\Framework\Serialize\SerializerInterface; use Magento\Framework\View\Layout\LayoutCacheKeyInterface; use Magento\Framework\View\Model\Layout\Update\Validator; @@ -42,7 +44,7 @@ class Merge implements \Magento\Framework\View\Layout\ProcessorInterface /** * Cache id suffix for page layout */ - const PAGE_LAYOUT_CACHE_SUFFIX = 'page_layout'; + const PAGE_LAYOUT_CACHE_SUFFIX = 'page_layout_merged'; /** * @var \Magento\Framework\View\Design\ThemeInterface @@ -54,6 +56,11 @@ class Merge implements \Magento\Framework\View\Layout\ProcessorInterface */ private $scope; + /** + * @var SerializerInterface + */ + private $serializer; + /** * In-memory cache for loaded layout updates * @@ -173,10 +180,11 @@ class Merge implements \Magento\Framework\View\Layout\ProcessorInterface * @param \Magento\Framework\Cache\FrontendInterface $cache * @param \Magento\Framework\View\Model\Layout\Update\Validator $validator * @param \Psr\Log\LoggerInterface $logger - * @param ReadFactory $readFactory , + * @param ReadFactory $readFactory * @param \Magento\Framework\View\Design\ThemeInterface $theme Non-injectable theme instance * @param string $cacheSuffix * @param LayoutCacheKeyInterface $layoutCacheKey + * @param SerializerInterface|null $serializer * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function __construct( @@ -191,7 +199,8 @@ public function __construct( ReadFactory $readFactory, \Magento\Framework\View\Design\ThemeInterface $theme = null, $cacheSuffix = '', - LayoutCacheKeyInterface $layoutCacheKey = null + LayoutCacheKeyInterface $layoutCacheKey = null, + SerializerInterface $serializer = null ) { $this->theme = $theme ?: $design->getDesignTheme(); $this->scope = $scopeResolver->getScope(); @@ -205,6 +214,7 @@ public function __construct( $this->cacheSuffix = $cacheSuffix; $this->layoutCacheKey = $layoutCacheKey ?: \Magento\Framework\App\ObjectManager::getInstance()->get(LayoutCacheKeyInterface::class); + $this->serializer = $serializer ?: ObjectManager::getInstance()->get(SerializerInterface::class); } /** @@ -283,6 +293,7 @@ public function getHandles() /** * Add the first existing (declared in layout updates) page handle along with all parents to the update. + * * Return whether any page handles have been added or not. * * @param string[] $handlesToTry @@ -315,6 +326,8 @@ public function pageHandleExists($handleName) } /** + * Page layout type + * * @return string|null */ public function getPageLayout() @@ -437,12 +450,12 @@ public function load($handles = []) $this->addHandle($handles); - $cacheId = $this->getCacheId(); - $cacheIdPageLayout = $cacheId . '_' . self::PAGE_LAYOUT_CACHE_SUFFIX; + $cacheId = $this->getCacheId() . '_' . self::PAGE_LAYOUT_CACHE_SUFFIX; $result = $this->_loadCache($cacheId); - if ($result) { - $this->addUpdate($result); - $this->pageLayout = $this->_loadCache($cacheIdPageLayout); + if ($result !== false && $result !== null) { + $data = $this->serializer->unserialize($result); + $this->pageLayout = $data["pageLayout"]; + $this->addUpdate($data["layout"]); foreach ($this->getHandles() as $handle) { $this->allHandles[$handle] = $this->handleProcessed; } @@ -455,8 +468,13 @@ public function load($handles = []) $layout = $this->asString(); $this->_validateMergedLayout($cacheId, $layout); - $this->_saveCache($layout, $cacheId, $this->getHandles()); - $this->_saveCache((string)$this->pageLayout, $cacheIdPageLayout, $this->getHandles()); + + $data = [ + "pageLayout" => (string)$this->pageLayout, + "layout" => $layout + ]; + $this->_saveCache($this->serializer->serialize($data), $cacheId, $this->getHandles()); + return $this; } @@ -602,7 +620,7 @@ protected function _fetchDbLayoutUpdates($handle) */ public function validateUpdate($handle, $updateXml) { - return; + return null; } /** @@ -930,6 +948,7 @@ public function getScope() public function getCacheId() { $layoutCacheKeys = $this->layoutCacheKey->getCacheKeys(); + // phpcs:ignore Magento2.Security.InsecureFunction return $this->generateCacheId(md5(implode('|', array_merge($this->getHandles(), $layoutCacheKeys)))); } } diff --git a/lib/internal/Magento/Framework/View/Test/Unit/Model/Layout/MergeTest.php b/lib/internal/Magento/Framework/View/Test/Unit/Model/Layout/MergeTest.php index 112d171f2574b..3060ac64d74bf 100644 --- a/lib/internal/Magento/Framework/View/Test/Unit/Model/Layout/MergeTest.php +++ b/lib/internal/Magento/Framework/View/Test/Unit/Model/Layout/MergeTest.php @@ -11,6 +11,11 @@ use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; use Magento\Framework\View\Layout\LayoutCacheKeyInterface; +/** + * Class MergeTest + * + * @package Magento\Framework\View\Test\Unit\Model\Layout + */ class MergeTest extends \PHPUnit\Framework\TestCase { /** @@ -28,11 +33,21 @@ class MergeTest extends \PHPUnit\Framework\TestCase */ private $scope; + /** + * @var \Magento\Framework\Cache\FrontendInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $cache; + /** * @var \Magento\Framework\View\Model\Layout\Update\Validator|\PHPUnit_Framework_MockObject_MockObject */ private $layoutValidator; + /** + * @var \Magento\Framework\Serialize\SerializerInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $serializer; + /** * @var \Psr\Log\LoggerInterface|\PHPUnit_Framework_MockObject_MockObject */ @@ -53,10 +68,12 @@ protected function setUp() $this->objectManagerHelper = new ObjectManager($this); $this->scope = $this->getMockForAbstractClass(\Magento\Framework\Url\ScopeInterface::class); + $this->cache = $this->getMockForAbstractClass(\Magento\Framework\Cache\FrontendInterface::class); $this->layoutValidator = $this->getMockBuilder(\Magento\Framework\View\Model\Layout\Update\Validator::class) ->disableOriginalConstructor() ->getMock(); $this->logger = $this->getMockForAbstractClass(\Psr\Log\LoggerInterface::class); + $this->serializer = $this->getMockForAbstractClass(\Magento\Framework\Serialize\SerializerInterface::class); $this->appState = $this->getMockBuilder(\Magento\Framework\App\State::class) ->disableOriginalConstructor() ->getMock(); @@ -70,10 +87,12 @@ protected function setUp() \Magento\Framework\View\Model\Layout\Merge::class, [ 'scope' => $this->scope, + 'cache' => $this->cache, 'layoutValidator' => $this->layoutValidator, 'logger' => $this->logger, 'appState' => $this->appState, 'layoutCacheKey' => $this->layoutCacheKeyMock, + 'serializer' => $this->serializer, ] ); } @@ -104,4 +123,33 @@ public function testValidateMergedLayoutThrowsException() $this->model->load(); } + + /** + * Test that merged layout is saved to cache if it wasn't cached before. + */ + public function testSaveToCache() + { + $this->scope->expects($this->once())->method('getId')->willReturn(1); + $this->cache->expects($this->once())->method('save'); + + $this->model->load(); + } + + /** + * Test that merged layout is not re-saved to cache when it was loaded from cache. + */ + public function testNoSaveToCacheWhenCachePresent() + { + $cacheValue = [ + "pageLayout" => "1column", + "layout" => "" + ]; + + $this->scope->expects($this->once())->method('getId')->willReturn(1); + $this->cache->expects($this->once())->method('load')->willReturn(json_encode($cacheValue)); + $this->serializer->expects($this->once())->method('unserialize')->willReturn($cacheValue); + $this->cache->expects($this->never())->method('save'); + + $this->model->load(); + } }