Skip to content

Fix PageCache: async rendering of blocks can corrupt layout cache #8554 #9050 #11174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
3bdfa1b
Fix PageCache: async rendering of blocks can corrupt layout cache #8554
adrian-martinez-interactiv4 May 9, 2017
5a6a000
Fix PageCache: async rendering of blocks can corrupt layout cache #8554
adrian-martinez-interactiv4 May 9, 2017
bf7df0d
Fix PageCache: async rendering of blocks can corrupt layout cache #85…
adrian-martinez-interactiv4 May 9, 2017
7feba6c
Fix PageCache: async rendering of blocks can corrupt layout cache #85…
adrian-martinez-interactiv4 May 9, 2017
1bb25ed
Fix PageCache: async rendering of blocks can corrupt layout cache #85…
adrian-martinez-interactiv4 May 10, 2017
0e830c1
Fix PageCache: async rendering of blocks can corrupt layout cache
adrian-martinez-interactiv4 May 17, 2017
8a92e46
Fix PageCache: async rendering of blocks can corrupt layout cache
adrian-martinez-interactiv4 May 17, 2017
0a69b5b
Merge remote-tracking branch 'upstream/develop' into develop
adrian-martinez-interactiv4 Sep 21, 2017
bd6ed7c
Fix PageCache: async rendering of blocks can corrupt layout cache #85…
adrian-martinez-interactiv4 Sep 21, 2017
c6e2b39
Fix PageCache: async rendering of blocks can corrupt layout cache #85…
adrian-martinez-interactiv4 Sep 22, 2017
3cc51dc
Fix PageCache: async rendering of blocks can corrupt layout cache #85…
adrian-martinez-interactiv4 Sep 22, 2017
2ae9c26
Fix PageCache: async rendering of blocks can corrupt layout cache #85…
adrian-martinez-interactiv4 Sep 22, 2017
516b529
Fix PageCache: async rendering of blocks can corrupt layout cache #85…
adrian-martinez-interactiv4 Sep 22, 2017
efa3edb
Fix PageCache: async rendering of blocks can corrupt layout cache #85…
adrian-martinez-interactiv4 Sep 22, 2017
3f60168
Fix PageCache: async rendering of blocks can corrupt layout cache #85…
adrian-martinez-interactiv4 Sep 22, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions app/code/Magento/PageCache/Controller/Block.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

use Magento\Framework\Serialize\Serializer\Base64Json;
use Magento\Framework\Serialize\Serializer\Json;
use Magento\Framework\View\Layout\LayoutCacheKeyInterface;

abstract class Block extends \Magento\Framework\App\Action\Action
{
Expand All @@ -27,24 +28,40 @@ abstract class Block extends \Magento\Framework\App\Action\Action
*/
private $base64jsonSerializer;

/**
* Layout cache keys to be able to generate different cache id for same handles
*
* @var LayoutCacheKeyInterface
*/
private $layoutCacheKey;

/**
* @var string
*/
private $layoutCacheKeyName = 'mage_pagecache';

/**
* @param \Magento\Framework\App\Action\Context $context
* @param \Magento\Framework\Translate\InlineInterface $translateInline
* @param Json $jsonSerializer
* @param Base64Json $base64jsonSerializer
* @param LayoutCacheKeyInterface $layoutCacheKey
*/
public function __construct(
\Magento\Framework\App\Action\Context $context,
\Magento\Framework\Translate\InlineInterface $translateInline,
Json $jsonSerializer = null,
Base64Json $base64jsonSerializer = null
Base64Json $base64jsonSerializer = null,
LayoutCacheKeyInterface $layoutCacheKey = null
) {
parent::__construct($context);
$this->translateInline = $translateInline;
$this->jsonSerializer = $jsonSerializer
?: \Magento\Framework\App\ObjectManager::getInstance()->get(Json::class);
$this->base64jsonSerializer = $base64jsonSerializer
?: \Magento\Framework\App\ObjectManager::getInstance()->get(Base64Json::class);
$this->layoutCacheKey = $layoutCacheKey
?: \Magento\Framework\App\ObjectManager::getInstance()->get(LayoutCacheKeyInterface::class);
}

/**
Expand All @@ -63,10 +80,12 @@ protected function _getBlocks()
$blocks = $this->jsonSerializer->unserialize($blocks);
$handles = $this->base64jsonSerializer->unserialize($handles);

$layout = $this->_view->getLayout();
$this->layoutCacheKey->addCacheKeys($this->layoutCacheKeyName);

$this->_view->loadLayout($handles, true, true, false);
$data = [];

$layout = $this->_view->getLayout();
foreach ($blocks as $blockName) {
$blockInstance = $layout->getBlock($blockName);
if (is_object($blockInstance)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ class EsiTest extends \PHPUnit\Framework\TestCase
*/
protected $layoutMock;

/**
* @var \Magento\Framework\View\Layout\LayoutCacheKeyInterface|\PHPUnit_Framework_MockObject_MockObject
*/
protected $layoutCacheKeyMock;

/**
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Translate\InlineInterface
*/
Expand All @@ -52,6 +57,8 @@ protected function setUp()
$this->layoutMock = $this->getMockBuilder(\Magento\Framework\View\Layout::class)
->disableOriginalConstructor()->getMock();

$this->layoutCacheKeyMock = $this->getMockForAbstractClass(\Magento\Framework\View\Layout\LayoutCacheKeyInterface::class);

$contextMock =
$this->getMockBuilder(\Magento\Framework\App\Action\Context::class)
->disableOriginalConstructor()->getMock();
Expand All @@ -76,7 +83,8 @@ protected function setUp()
'context' => $contextMock,
'translateInline' => $this->translateInline,
'jsonSerializer' => new \Magento\Framework\Serialize\Serializer\Json(),
'base64jsonSerializer' => new \Magento\Framework\Serialize\Serializer\Base64Json()
'base64jsonSerializer' => new \Magento\Framework\Serialize\Serializer\Base64Json(),
'layoutCacheKey' => $this->layoutCacheKeyMock
]
);
}
Expand Down Expand Up @@ -104,6 +112,11 @@ public function testExecute($blockClass, $shouldSetHeaders)

$this->viewMock->expects($this->once())->method('getLayout')->will($this->returnValue($this->layoutMock));

$this->layoutMock->expects($this->never())
->method('getUpdate');
$this->layoutCacheKeyMock->expects($this->atLeastOnce())
->method('addCacheKeys');

$this->layoutMock->expects($this->once())
->method('getBlock')
->with($this->equalTo($block))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,26 @@ class RenderTest extends \PHPUnit\Framework\TestCase
*/
protected $layoutMock;

/**
* @var \Magento\Framework\View\Layout\ProcessorInterface|\PHPUnit_Framework_MockObject_MockObject
*/
protected $layoutProcessorMock;

/**
* @var \Magento\Framework\View\Layout\LayoutCacheKeyInterface|\PHPUnit_Framework_MockObject_MockObject
*/
protected $layoutCacheKeyMock;

/**
* Set up before test
*/
protected function setUp()
{
$this->layoutMock = $this->getMockBuilder(
\Magento\Framework\View\Layout::class
)->disableOriginalConstructor()->getMock();
$this->layoutMock = $this->getMockBuilder(\Magento\Framework\View\Layout::class)
->disableOriginalConstructor()->getMock();

$this->layoutProcessorMock = $this->getMockForAbstractClass(\Magento\Framework\View\Layout\ProcessorInterface::class);
$this->layoutCacheKeyMock = $this->getMockForAbstractClass(\Magento\Framework\View\Layout\LayoutCacheKeyInterface::class);

$contextMock = $this->getMockBuilder(\Magento\Framework\App\Action\Context::class)
->disableOriginalConstructor()->getMock();
Expand All @@ -65,6 +77,8 @@ protected function setUp()
$this->viewMock = $this->getMockBuilder(\Magento\Framework\App\View::class)
->disableOriginalConstructor()->getMock();

$this->layoutMock->expects($this->any())->method('getUpdate')->will($this->returnValue($this->layoutProcessorMock));

$contextMock->expects($this->any())->method('getRequest')->will($this->returnValue($this->requestMock));
$contextMock->expects($this->any())->method('getResponse')->will($this->returnValue($this->responseMock));
$contextMock->expects($this->any())->method('getView')->will($this->returnValue($this->viewMock));
Expand All @@ -78,7 +92,8 @@ protected function setUp()
'context' => $contextMock,
'translateInline' => $this->translateInline,
'jsonSerializer' => new \Magento\Framework\Serialize\Serializer\Json(),
'base64jsonSerializer' => new \Magento\Framework\Serialize\Serializer\Base64Json()
'base64jsonSerializer' => new \Magento\Framework\Serialize\Serializer\Base64Json(),
'layoutCacheKey' => $this->layoutCacheKeyMock
]
);
}
Expand All @@ -88,6 +103,8 @@ public function testExecuteNotAjax()
$this->requestMock->expects($this->once())->method('isAjax')->will($this->returnValue(false));
$this->requestMock->expects($this->once())->method('setActionName')->will($this->returnValue('noroute'));
$this->requestMock->expects($this->once())->method('setDispatched')->will($this->returnValue(false));
$this->layoutCacheKeyMock->expects($this->never())
->method('addCacheKeys');
$this->action->execute();
}

Expand All @@ -105,6 +122,8 @@ public function testExecuteNoParams()
->method('getParam')
->with($this->equalTo('handles'), $this->equalTo(''))
->will($this->returnValue(''));
$this->layoutCacheKeyMock->expects($this->never())
->method('addCacheKeys');
$this->action->execute();
}

Expand Down Expand Up @@ -150,6 +169,10 @@ public function testExecute()
->will($this->returnValue(base64_encode(json_encode($handles))));
$this->viewMock->expects($this->once())->method('loadLayout')->with($this->equalTo($handles));
$this->viewMock->expects($this->any())->method('getLayout')->will($this->returnValue($this->layoutMock));
$this->layoutMock->expects($this->never())
->method('getUpdate');
$this->layoutCacheKeyMock->expects($this->atLeastOnce())
->method('addCacheKeys');
$this->layoutMock->expects($this->at(0))
->method('getBlock')
->with($this->equalTo($blocks[0]))
Expand Down
5 changes: 5 additions & 0 deletions app/code/Magento/PageCache/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@
</argument>
</arguments>
</type>
<type name="Magento\PageCache\Controller\Block">
<arguments>
<argument name="layoutCacheKey" xsi:type="object">Magento\Framework\View\Layout\LayoutCacheKeyInterface</argument>
</arguments>
</type>
<preference for="Magento\PageCache\Model\VclGeneratorInterface" type="Magento\PageCache\Model\Varnish\VclGenerator"/>
<preference for="Magento\PageCache\Model\VclTemplateLocatorInterface" type="Magento\PageCache\Model\Varnish\VclTemplateLocator"/>
</config>
2 changes: 2 additions & 0 deletions app/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
<preference for="Magento\Framework\Event\ManagerInterface" type="Magento\Framework\Event\Manager\Proxy" />
<preference for="Magento\Framework\View\LayoutInterface" type="Magento\Framework\View\Layout" />
<preference for="Magento\Framework\View\Layout\ProcessorInterface" type="Magento\Framework\View\Model\Layout\Merge" />
<preference for="Magento\Framework\View\Layout\LayoutCacheKeyInterface" type="Magento\Framework\View\Model\Layout\CacheKey" />
<preference for="Magento\Framework\View\Url\ConfigInterface" type="Magento\Framework\View\Url\Config" />
<preference for="Magento\Framework\App\Route\ConfigInterface" type="Magento\Framework\App\Route\Config" />
<preference for="Magento\Framework\App\ResourceConnection\ConfigInterface" type="Magento\Framework\App\ResourceConnection\Config\Proxy" />
Expand Down Expand Up @@ -740,6 +741,7 @@
<argument name="fileSource" xsi:type="object">Magento\Framework\View\Layout\File\Collector\Aggregated\Proxy</argument>
<argument name="pageLayoutFileSource" xsi:type="object">pageLayoutFileCollectorAggregated</argument>
<argument name="cache" xsi:type="object">Magento\Framework\App\Cache\Type\Layout</argument>
<argument name="layoutCacheKey" xsi:type="object">Magento\Framework\View\Layout\LayoutCacheKeyInterface</argument>
</arguments>
</type>
<type name="CSSmin">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

use Magento\Framework\App\State;
use Magento\Framework\Phrase;
use Magento\Framework\View\Layout\LayoutCacheKeyInterface;

/**
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
Expand Down Expand Up @@ -65,6 +66,11 @@ class MergeTest extends \PHPUnit\Framework\TestCase
*/
protected $pageConfig;

/**
* @var LayoutCacheKeyInterface|\PHPUnit_Framework_MockObject_MockObject
*/
protected $layoutCacheKeyMock;

protected function setUp()
{
$files = [];
Expand Down Expand Up @@ -119,6 +125,11 @@ function ($filename) use ($fileDriver) {
)
);

$this->layoutCacheKeyMock = $this->getMockForAbstractClass(LayoutCacheKeyInterface::class);
$this->layoutCacheKeyMock->expects($this->any())
->method('getCacheKeys')
->willReturn([]);

$this->_model = $objectHelper->getObject(
\Magento\Framework\View\Model\Layout\Merge::class,
[
Expand All @@ -134,6 +145,7 @@ function ($filename) use ($fileDriver) {
'logger' => $this->_logger,
'readFactory' => $readFactory,
'pageConfig' => $this->pageConfig,
'layoutCacheKey' => $this->layoutCacheKeyMock,
]
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
*/
namespace Magento\Framework\View\Model\Layout;

use Magento\Framework\View\Layout\LayoutCacheKeyInterface;

class MergeTest extends \PHPUnit\Framework\TestCase
{
/**
Expand All @@ -18,6 +20,11 @@ class MergeTest extends \PHPUnit\Framework\TestCase
*/
protected $model;

/**
* @var LayoutCacheKeyInterface|\PHPUnit_Framework_MockObject_MockObject
*/
protected $layoutCacheKeyMock;

protected function setUp()
{
$objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
Expand Down Expand Up @@ -62,9 +69,17 @@ protected function setUp()
$link2->setLayoutUpdateId($layoutUpdate2->getId());
$link2->save();

$this->layoutCacheKeyMock = $this->getMockForAbstractClass(LayoutCacheKeyInterface::class);
$this->layoutCacheKeyMock->expects($this->any())
->method('getCacheKeys')
->willReturn([]);

$this->model = $objectManager->create(
\Magento\Framework\View\Model\Layout\Merge::class,
['theme' => $theme]
[
'theme' => $theme,
'layoutCacheKey' => $this->layoutCacheKeyMock,
]
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Framework\View\Layout;

/**
* Interface LayoutCacheKeyInterface
*/
interface LayoutCacheKeyInterface
{
/**
* Add cache key(s) for generating different cache id for same handles
*
* @param array|string $cacheKeys
* @return void
*/
public function addCacheKeys($cacheKeys);

/**
* Return cache keys array
*
* @return array
*/
public function getCacheKeys();
}
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public function getFileLayoutUpdatesXml();
public function getContainers();

/**
* Return cache ID based current area/package/theme/store and handles
* Return cache ID based current area/package/theme/store, handles and cache key(s)
*
* @return string
*/
Expand Down
43 changes: 43 additions & 0 deletions lib/internal/Magento/Framework/View/Model/Layout/CacheKey.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Framework\View\Model\Layout;

/**
* Layout cache key model
*/
class CacheKey implements \Magento\Framework\View\Layout\LayoutCacheKeyInterface
{
/**
* Cache keys to be able to generate different cache id for same handles
*
* @var array
*/
private $cacheKeys = [];

/**
* Add cache key(s) for generating different cache id for same handles
*
* @param array|string $cacheKeys
* @return void
*/
public function addCacheKeys($cacheKeys)
{
if (!is_array($cacheKeys)) {
$cacheKeys = [$cacheKeys];
}
$this->cacheKeys = array_merge($this->cacheKeys, $cacheKeys);
}

/**
* Return cache keys array
*
* @return array
*/
public function getCacheKeys()
{
return $this->cacheKeys;
}
}
Loading