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

#9050 #9560 Create cache key object to be injected separately with its own interface
magento · Sep 21, 2017 · bd6ed7c · bd6ed7c
1 parent 0a69b5b
commit bd6ed7c
Show file tree

Hide file tree

Showing 6 changed files with 92 additions and 36 deletions.
diff --git a/app/code/Magento/PageCache/Controller/Block.php b/app/code/Magento/PageCache/Controller/Block.php
@@ -27,6 +27,13 @@ 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 \Magento\Framework\View\Layout\CacheKeyInterface
+     */
+    private $cacheKey;
+
     /**
      * @var string
      */
@@ -37,19 +44,22 @@ abstract class Block extends \Magento\Framework\App\Action\Action
      * @param \Magento\Framework\Translate\InlineInterface $translateInline
      * @param Json $jsonSerializer
      * @param Base64Json $base64jsonSerializer
+     * @param \Magento\Framework\View\Layout\CacheKeyInterface $cacheKey
      */
     public function __construct(
         \Magento\Framework\App\Action\Context $context,
         \Magento\Framework\Translate\InlineInterface $translateInline,
         Json $jsonSerializer = null,
-        Base64Json $base64jsonSerializer = null
+        Base64Json $base64jsonSerializer = null,
+        \Magento\Framework\View\Layout\CacheKeyInterface $cacheKey
     ) {
         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->cacheKey = $cacheKey;
     }
 
     /**
@@ -69,7 +79,7 @@ protected function _getBlocks()
         $handles = $this->base64jsonSerializer->unserialize($handles);
 
         $layout = $this->_view->getLayout();
-        $layout->getUpdate()->addCacheKey($this->layoutCacheKey);
+        $this->cacheKey->addCacheKey($this->layoutCacheKey);
 
         $this->_view->loadLayout($handles, true, true, false);
         $data = [];

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\CacheKeyInterface" 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" />

diff --git a/lib/internal/Magento/Framework/View/Layout/CacheKeyInterface.php b/lib/internal/Magento/Framework/View/Layout/CacheKeyInterface.php
@@ -0,0 +1,26 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+namespace Magento\Framework\View\Layout;
+
+/**
+ * Interface CacheKeyInterface
+ */
+interface CacheKeyInterface
+{
+    /**
+     * Add cache key for generating different cache id for same handles
+     *
+     * @param array|string $cacheKey
+     */
+    public function addCacheKey($cacheKey);
+
+    /**
+     * Return cache keys array stored
+     *
+     * @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
@@ -125,14 +125,6 @@ public function getFileLayoutUpdatesXml();
      */
     public function getContainers();
 
-    /**
-     * Add cache key for generating different cache id for same handles
-     *
-     * @param array|string $cacheKey
-     * @return ProcessorInterface
-     */
-    public function addCacheKey($cacheKey);
-
     /**
      * Return cache ID based current area/package/theme/store, handles and cache key(s)
      *

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,41 @@
+<?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\CacheKeyInterface
+{
+    /**
+     * 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
+     */
+    public function addCacheKey($cacheKeys)
+    {
+        if (!is_array($cacheKeys)) {
+            $cacheKeys = [$cacheKeys];
+        }
+        $this->cacheKeys = array_merge($this->cacheKeys, $cacheKeys);
+    }
+
+    /**
+     * Return cache keys array stored
+     *
+     * @return array
+     */
+    public function getCacheKeys() {
+        return $this->cacheKeys;
+    }
+}
diff --git a/lib/internal/Magento/Framework/View/Model/Layout/Merge.php b/lib/internal/Magento/Framework/View/Model/Layout/Merge.php
@@ -9,7 +9,6 @@
 use Magento\Framework\Config\Dom\ValidationException;
 use Magento\Framework\Filesystem\DriverPool;
 use Magento\Framework\Filesystem\File\ReadFactory;
-use Magento\Framework\View\Layout\ProcessorInterface;
 use Magento\Framework\View\Model\Layout\Update\Validator;
 
 /**
@@ -104,6 +103,13 @@ class Merge implements \Magento\Framework\View\Layout\ProcessorInterface
      */
     private $appState;
 
+    /**
+     * Cache keys to be able to generate different cache id for same handles
+     *
+     * @var \Magento\Framework\View\Layout\CacheKeyInterface
+     */
+    private $cacheKey;
+
     /**
      * @var \Magento\Framework\Cache\FrontendInterface
      */
@@ -129,13 +135,6 @@ class Merge implements \Magento\Framework\View\Layout\ProcessorInterface
      */
     protected $cacheSuffix;
 
-    /**
-     * Cache keys to be able to generate different cache id for same handles
-     *
-     * @var array
-     */
-    protected $cacheKeys = [];
-
     /**
      * All processed handles used in this update
      *
@@ -173,8 +172,9 @@ 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 \Magento\Framework\View\Layout\CacheKeyInterface $cacheKey
      * @param string $cacheSuffix
      * @SuppressWarnings(PHPMD.ExcessiveParameterList)
      */
@@ -189,6 +189,7 @@ public function __construct(
         \Psr\Log\LoggerInterface $logger,
         ReadFactory $readFactory,
         \Magento\Framework\View\Design\ThemeInterface $theme = null,
+        \Magento\Framework\View\Layout\CacheKeyInterface $cacheKey,
         $cacheSuffix = ''
     ) {
         $this->theme = $theme ?: $design->getDesignTheme();
@@ -200,6 +201,7 @@ public function __construct(
         $this->layoutValidator = $validator;
         $this->logger = $logger;
         $this->readFactory = $readFactory;
+        $this->cacheKey = $cacheKey;
         $this->cacheSuffix = $cacheSuffix;
     }
 
@@ -915,29 +917,13 @@ public function getScope()
         return $this->scope;
     }
 
-    /**
-     * Add cache key(s) for generating different cache id for same handles
-     *
-     * @param array|string $cacheKeys
-     * @return $this
-     */
-    public function addCacheKey($cacheKeys)
-    {
-        if (!is_array($cacheKeys)) {
-            $cacheKeys = [$cacheKeys];
-        }
-        $this->cacheKeys = array_merge($this->cacheKeys, $cacheKeys);
-
-        return $this;
-    }
-
     /**
      * Return cache ID based current area/package/theme/store and handles
      *
      * @return string
      */
     public function getCacheId()
     {
-        return $this->generateCacheId(md5(implode('|', array_merge($this->getHandles(), $this->cacheKeys))));
+        return $this->generateCacheId(md5(implode('|', array_merge($this->getHandles(), $this->cacheKey->getCacheKeys()))));
     }
 }