core/lib/Drupal/Core/Render/RenderCache.php | 55 +++++++++++++--------- .../Tests/Core/Render/RendererBubblingTest.php | 32 +++++++++++-- 2 files changed, 60 insertions(+), 27 deletions(-) diff --git a/core/lib/Drupal/Core/Render/RenderCache.php b/core/lib/Drupal/Core/Render/RenderCache.php index 776bfd4..e2b0276 100644 --- a/core/lib/Drupal/Core/Render/RenderCache.php +++ b/core/lib/Drupal/Core/Render/RenderCache.php @@ -8,6 +8,7 @@ namespace Drupal\Core\Render; use Drupal\Core\Cache\Cache; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Cache\Context\CacheContextsManager; use Drupal\Core\Cache\CacheFactoryInterface; use Symfony\Component\HttpFoundation\RequestStack; @@ -96,7 +97,6 @@ public function set(array &$elements, array $pre_bubbling_elements) { $data = $this->getCacheableRenderArray($elements); $bin = isset($elements['#cache']['bin']) ? $elements['#cache']['bin'] : 'render'; - $expire = ($elements['#cache']['max-age'] === Cache::PERMANENT) ? Cache::PERMANENT : (int) $this->requestStack->getMasterRequest()->server->get('REQUEST_TIME') + $elements['#cache']['max-age']; $cache = $this->cacheFactory->get($bin); // Calculate the pre-bubbling CID. @@ -207,26 +207,20 @@ public function set(array &$elements, array $pre_bubbling_elements) { // across requests. That's the strategy employed below and tested in // \Drupal\Tests\Core\Render\RendererBubblingTest::testConditionalCacheContextBubblingSelfHealing(). - // The set of cache contexts for this element, including the bubbled ones, - // for which we are handling a cache miss. - $cache_contexts = $data['#cache']['contexts']; - - // Get the contexts by which this element should be varied according to - // the current redirecting cache item, if any. - $stored_cache_contexts = []; - $stored_cache_tags = []; + // Get the cacheability of this element according to the current (stored) + // redirecting cache item, if any. + $redirect_cacheability = new CacheableMetadata(); if ($stored_cache_redirect = $cache->get($pre_bubbling_cid)) { - $stored_cache_contexts = $stored_cache_redirect->data['#cache']['contexts']; - $stored_cache_tags = $stored_cache_redirect->data['#cache']['tags']; + $redirect_cacheability = CacheableMetadata::createFromRenderArray($stored_cache_redirect->data); } - // Calculate the union of the cache contexts for this request and the - // stored cache contexts. - $merged_cache_contexts = Cache::mergeContexts($stored_cache_contexts, $cache_contexts); + // Calculate the union of the cacheability for this request and the + // current (stored) redirecting cache item. + $redirect_cacheability_updated = CacheableMetadata::createFromRenderArray($data)->merge($redirect_cacheability); // Stored cache contexts incomplete: this request causes cache contexts to // be added to the redirecting cache item. - if (array_diff($merged_cache_contexts, $stored_cache_contexts)) { + if (array_diff($redirect_cacheability_updated->getCacheContexts(), $redirect_cacheability->getCacheContexts())) { $redirect_data = [ '#cache_redirect' => TRUE, '#cache' => [ @@ -234,14 +228,16 @@ public function set(array &$elements, array $pre_bubbling_elements) { // across requests. 'keys' => $elements['#cache']['keys'], // The union of the current element's and stored cache contexts. - 'contexts' => $merged_cache_contexts, + 'contexts' => $redirect_cacheability_updated->getCacheContexts(), // The union of the current element's and stored cache tags. - 'tags' => Cache::mergeTags($stored_cache_tags, $data['#cache']['tags']), + 'tags' => $redirect_cacheability_updated->getCacheTags(), + // The union of the current element's and stored cache max-ages. + 'max-age' => $redirect_cacheability_updated->getCacheMaxAge(), // The same cache bin as the one for the actual render cache items. 'bin' => $bin, ], ]; - $cache->set($pre_bubbling_cid, $redirect_data, $expire, Cache::mergeTags($redirect_data['#cache']['tags'], ['rendered'])); + $cache->set($pre_bubbling_cid, $redirect_data, $this->maxAgeToExpire($redirect_cacheability_updated->getCacheMaxAge()), Cache::mergeTags($redirect_data['#cache']['tags'], ['rendered'])); } // Current cache contexts incomplete: this request only uses a subset of @@ -249,20 +245,35 @@ public function set(array &$elements, array $pre_bubbling_elements) { // additional (conditional) cache contexts as well, otherwise the // redirecting cache item would be pointing to a cache item that can never // exist. - if (array_diff($merged_cache_contexts, $cache_contexts)) { + if (array_diff($redirect_cacheability_updated->getCacheContexts(), $data['#cache']['contexts'])) { // Recalculate the cache ID. $recalculated_cid_pseudo_element = [ '#cache' => [ 'keys' => $elements['#cache']['keys'], - 'contexts' => $merged_cache_contexts, + 'contexts' => $redirect_cacheability_updated->getCacheContexts(), ] ]; $cid = $this->createCacheID($recalculated_cid_pseudo_element); // Ensure the about-to-be-cached data uses the merged cache contexts. - $data['#cache']['contexts'] = $merged_cache_contexts; + $data['#cache']['contexts'] = $redirect_cacheability_updated->getCacheContexts(); } } - $cache->set($cid, $data, $expire, Cache::mergeTags($data['#cache']['tags'], ['rendered'])); + $cache->set($cid, $data, $this->maxAgeToExpire($elements['#cache']['max-age']), Cache::mergeTags($data['#cache']['tags'], ['rendered'])); + } + + /** + * Maps a #cache[max-age] value to an "expire" value for the Cache API. + * + * @param int $max_age + * A #cache[max-age] value. + * + * @return int + * A corresponding "expire" value. + * + * @see \Drupal\Core\Cache\CacheBackendInterface::set() + */ + protected function maxAgeToExpire($max_age) { + return ($max_age === Cache::PERMANENT) ? Cache::PERMANENT : (int) $this->requestStack->getMasterRequest()->server->get('REQUEST_TIME') + $max_age; } /** diff --git a/core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php b/core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php index e0c7ff4..aabe892 100644 --- a/core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php +++ b/core/tests/Drupal/Tests/Core/Render/RendererBubblingTest.php @@ -125,6 +125,7 @@ public function testContextBubblingCustomCacheBin() { 'contexts' => ['bar', 'foo'], 'tags' => [], 'bin' => $bin, + 'max-age' => 3600, ], ], $bin); } @@ -279,6 +280,7 @@ public function providerTestContextBubblingEdgeCases() { 'contexts' => ['bar', 'foo'], 'tags' => ['dee', 'fiddle', 'har', 'yar'], 'bin' => 'render', + 'max-age' => Cache::PERMANENT, ], ], 'parent:bar:foo' => [ @@ -325,6 +327,8 @@ public function testConditionalCacheContextBubblingSelfHealing() { '#cache' => [ 'contexts' => ['foo'], 'tags' => ['c'], + // A lower max-age; the redirecting cache item should be updated. + 'max-age' => 1800, ], 'grandgrandchild' => [ '#access_callback' => function () { @@ -335,6 +339,8 @@ public function testConditionalCacheContextBubblingSelfHealing() { '#cache' => [ 'contexts' => ['bar'], 'tags' => ['d'], + // A lower max-age; the redirecting cache item should be updated. + 'max-age' => 300, ], ], ], @@ -353,6 +359,7 @@ public function testConditionalCacheContextBubblingSelfHealing() { 'contexts' => ['user.roles'], 'tags' => ['a', 'b'], 'bin' => 'render', + 'max-age' => Cache::PERMANENT, ], ]); $this->assertRenderCacheItem('parent:r.A', [ @@ -366,7 +373,7 @@ public function testConditionalCacheContextBubblingSelfHealing() { ]); // Request 2: role B, the grandchild is accessible => bubbled cache - // contexts: foo, user.roles. + // contexts: foo, user.roles + merged max-age: 1800. $element = $test_element; $current_user_role = 'B'; $this->renderer->render($element); @@ -377,6 +384,7 @@ public function testConditionalCacheContextBubblingSelfHealing() { 'contexts' => ['foo', 'user.roles'], 'tags' => ['a', 'b', 'c'], 'bin' => 'render', + 'max-age' => 1800, ], ]); $this->assertRenderCacheItem('parent:foo:r.B', [ @@ -384,7 +392,7 @@ public function testConditionalCacheContextBubblingSelfHealing() { '#cache' => [ 'contexts' => ['foo', 'user.roles'], 'tags' => ['a', 'b', 'c'], - 'max-age' => Cache::PERMANENT, + 'max-age' => 1800, ], '#markup' => 'parent', ]); @@ -409,6 +417,7 @@ public function testConditionalCacheContextBubblingSelfHealing() { 'contexts' => ['foo', 'user.roles'], 'tags' => ['a', 'b', 'c'], 'bin' => 'render', + 'max-age' => 1800, ], ]); $this->assertRenderCacheItem('parent:foo:r.A', [ @@ -416,13 +425,17 @@ public function testConditionalCacheContextBubblingSelfHealing() { '#cache' => [ 'contexts' => ['foo', 'user.roles'], 'tags' => ['a', 'b'], + // Note that the max-age here is unaffected. When role A, the grandchild + // is never rendered, so neither is its max-age of 1800 present here, + // despite 1800 being the max-age of the redirecting cache item. 'max-age' => Cache::PERMANENT, ], '#markup' => 'parent', ]); // Request 4: role C, both the grandchild and the grandgrandchild are - // accessible => bubbled cache contexts: foo, bar, user.roles. + // accessible => bubbled cache contexts: foo, bar, user.roles + merged + // max-age: 300. $element = $test_element; $current_user_role = 'C'; $this->renderer->render($element); @@ -433,6 +446,7 @@ public function testConditionalCacheContextBubblingSelfHealing() { 'contexts' => ['bar', 'foo', 'user.roles'], 'tags' => ['a', 'b', 'c', 'd'], 'bin' => 'render', + 'max-age' => 300, ], ]; $this->assertRenderCacheItem('parent', $final_parent_cache_item); @@ -441,7 +455,7 @@ public function testConditionalCacheContextBubblingSelfHealing() { '#cache' => [ 'contexts' => ['bar', 'foo', 'user.roles'], 'tags' => ['a', 'b', 'c', 'd'], - 'max-age' => Cache::PERMANENT, + 'max-age' => 300, ], '#markup' => 'parent', ]); @@ -456,6 +470,10 @@ public function testConditionalCacheContextBubblingSelfHealing() { '#cache' => [ 'contexts' => ['bar', 'foo', 'user.roles'], 'tags' => ['a', 'b'], + // Note that the max-age here is unaffected. When role A, the grandchild + // is never rendered, so neither is its max-age of 1800 present here, + // nor the grandgrandchild's max-age of 300, despite 300 being the + // max-age of the redirecting cache item. 'max-age' => Cache::PERMANENT, ], '#markup' => 'parent', @@ -471,7 +489,11 @@ public function testConditionalCacheContextBubblingSelfHealing() { '#cache' => [ 'contexts' => ['bar', 'foo', 'user.roles'], 'tags' => ['a', 'b', 'c'], - 'max-age' => Cache::PERMANENT, + // Note that the max-age here is unaffected. When role B, the + // grandgrandchild is never rendered, so neither is its max-age of 300 + // present here, despite 300 being the max-age of the redirecting cache + // item. + 'max-age' => 1800, ], '#markup' => 'parent', ]);