MAGETWO-80647: Fix PageCache: async rendering of blocks can corrupt l…

…ayout cache #8554 #9050 #11174 - Merge Pull Request #11174 from adrian-martinez-interactiv4/magento2:FR#FIX-PAGECACHE-LAYOUT-CACHE-KEY - Merged commits: 1. 3bdfa1b 2. 5a6a000 3. bf7df0d 4. 7feba6c 5. 1bb25ed 6. 0e830c1 7. 8a92e46 8. 0a69b5b 9. bd6ed7c 10. c6e2b39 11. 3cc51dc 12. 2ae9c26 13. 516b529 14. efa3edb 15. 3f60168
magento · Oct 2, 2017 · f72db1b · f72db1b
2 parents 205a408 + 3f60168
commit f72db1b
Show file tree

Hide file tree

Showing 12 changed files with 197 additions and 13 deletions.
diff --git a/app/code/Magento/PageCache/Controller/Block.php b/app/code/Magento/PageCache/Controller/Block.php
@@ -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
 {
@@ -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);
     }
 
     /**
@@ -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)) {

diff --git a/app/code/Magento/PageCache/Test/Unit/Controller/Block/EsiTest.php b/app/code/Magento/PageCache/Test/Unit/Controller/Block/EsiTest.php
@@ -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
      */
@@ -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();
@@ -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
             ]
         );
     }
@@ -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))

diff --git a/app/code/Magento/PageCache/Test/Unit/Controller/Block/RenderTest.php b/app/code/Magento/PageCache/Test/Unit/Controller/Block/RenderTest.php
@@ -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();
@@ -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));
@@ -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
             ]
         );
     }
@@ -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();
     }
 
@@ -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();
     }
 
@@ -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]))

diff --git a/app/code/Magento/PageCache/etc/di.xml b/app/code/Magento/PageCache/etc/di.xml
@@ -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>
diff --git a/app/etc/di.xml b/app/etc/di.xml
@@ -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" />
@@ -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">

diff --git a/dev/tests/integration/testsuite/Magento/Framework/View/Layout/MergeTest.php b/dev/tests/integration/testsuite/Magento/Framework/View/Layout/MergeTest.php
@@ -7,6 +7,7 @@
 
 use Magento\Framework\App\State;
 use Magento\Framework\Phrase;
+use Magento\Framework\View\Layout\LayoutCacheKeyInterface;
 
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
@@ -65,6 +66,11 @@ class MergeTest extends \PHPUnit\Framework\TestCase
      */
     protected $pageConfig;
 
+    /**
+     * @var LayoutCacheKeyInterface|\PHPUnit_Framework_MockObject_MockObject
+     */
+    protected $layoutCacheKeyMock;
+
     protected function setUp()
     {
         $files = [];
@@ -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,
             [
@@ -134,6 +145,7 @@ function ($filename) use ($fileDriver) {
                 'logger' => $this->_logger,
                 'readFactory' => $readFactory,
                 'pageConfig' => $this->pageConfig,
+                'layoutCacheKey' => $this->layoutCacheKeyMock,
             ]
         );
     }

diff --git a/dev/tests/integration/testsuite/Magento/Framework/View/Model/Layout/MergeTest.php b/dev/tests/integration/testsuite/Magento/Framework/View/Model/Layout/MergeTest.php
@@ -5,6 +5,8 @@
  */
 namespace Magento\Framework\View\Model\Layout;
 
+use Magento\Framework\View\Layout\LayoutCacheKeyInterface;
+
 class MergeTest extends \PHPUnit\Framework\TestCase
 {
     /**
@@ -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();
@@ -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,
+            ]
         );
     }
 

diff --git a/lib/internal/Magento/Framework/View/Layout/LayoutCacheKeyInterface.php b/lib/internal/Magento/Framework/View/Layout/LayoutCacheKeyInterface.php
@@ -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();
+}
diff --git a/lib/internal/Magento/Framework/View/Layout/ProcessorInterface.php b/lib/internal/Magento/Framework/View/Layout/ProcessorInterface.php
@@ -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
      */

diff --git a/lib/internal/Magento/Framework/View/Model/Layout/CacheKey.php b/lib/internal/Magento/Framework/View/Model/Layout/CacheKey.php
@@ -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;
+    }
+}