Commit 6b371cc5 authored by catch's avatar catch

Issue #2463581 by Wim Leers, swentel: #cache_redirect cache items should have...

Issue #2463581 by Wim Leers, swentel: #cache_redirect cache items should have an 'expire' timestamp that matches the merged max-age
parent 7f420bd9
......@@ -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,28 @@ 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. We need:
// - the union of cache contexts, because that is how we know which cache
// item to redirect to;
// - the union of cache tags, because that is how we know when the cache
// redirect cache item itself is invalidated;
// - the union of max ages, because that is how we know when the cache
// redirect cache item itself becomes stale. (Without this, we might end
// up toggling between a permanently and a briefly cacheable cache
// redirect, because the last update's max-age would always "win".)
$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 +236,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 +253,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;
}
/**
......
......@@ -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->renderRoot($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->renderRoot($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',
]);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment