Commit 938e2f64 authored by catch's avatar catch
Browse files

Issue #2428805 by Wim Leers: Remove the ability to configure a block's cache contexts

parent 3fbe92b0
......@@ -295,12 +295,6 @@ block_settings:
max_age:
type: integer
label: 'Maximum age'
contexts:
type: sequence
label: 'Vary by context'
sequence:
type: string
label: 'Context'
status:
type: boolean
label: 'Status'
......
......@@ -10,7 +10,6 @@
use Drupal\block\BlockInterface;
use Drupal\Component\Utility\String;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Cache\CacheContexts;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Plugin\ContextAwarePluginBase;
use Drupal\Component\Utility\Unicode;
......@@ -91,8 +90,8 @@ protected function baseConfigurationDefaults() {
'provider' => $this->pluginDefinition['provider'],
'label_display' => BlockInterface::BLOCK_LABEL_VISIBLE,
'cache' => array(
'max_age' => 0,
'contexts' => array(),
// Blocks are cacheable by default.
'max_age' => Cache::PERMANENT,
),
);
}
......@@ -204,36 +203,6 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
'#default_value' => $this->configuration['cache']['max_age'],
'#options' => $period,
);
$contexts = \Drupal::service("cache_contexts")->getLabels();
// Blocks are always rendered in the "per language" and "per theme" cache
// contexts. No need to show those options to the end user.
unset($contexts['languages']);
unset($contexts['theme']);
$form['cache']['contexts'] = array(
'#type' => 'checkboxes',
'#title' => $this->t('Vary by context'),
'#description' => $this->t('The contexts this cached block must be varied by. <em>All</em> blocks are varied by language and theme.'),
'#default_value' => $this->configuration['cache']['contexts'],
'#options' => $contexts,
'#states' => array(
'disabled' => array(
':input[name="settings[cache][max_age]"]' => array('value' => (string) 0),
),
),
);
if (count($this->getRequiredCacheContexts()) > 0) {
// Remove the required cache contexts from the list of contexts a user can
// choose to modify by: they must always be applied.
$context_labels = array();
$all_contexts = \Drupal::service("cache_contexts")->getLabels(TRUE);
foreach (CacheContexts::parseTokens($this->getRequiredCacheContexts()) as $context) {
$context_id = $context[0];
$context_labels[] = $all_contexts[$context_id];
unset($form['cache']['contexts']['#options'][$context_id]);
}
$required_context_list = implode(', ', $context_labels);
$form['cache']['contexts']['#description'] .= ' ' . $this->t('This block is <em>always</em> varied by the following contexts: %required-context-list.', array('%required-context-list' => $required_context_list));
}
// Add plugin-specific settings for this block type.
$form += $this->blockForm($form, $form_state);
......@@ -259,10 +228,6 @@ public function validateConfigurationForm(array &$form, FormStateInterface $form
// Remove the admin_label form item element value so it will not persist.
$form_state->unsetValue('admin_label');
// Transform the #type = checkboxes value to a numerically indexed array.
$contexts = $form_state->getValue(array('cache', 'contexts'));
$form_state->setValue(array('cache', 'contexts'), array_values(array_filter($contexts)));
$this->blockValidate($form, $form_state);
}
......@@ -340,16 +305,6 @@ public function setTransliteration(TransliterationInterface $transliteration) {
$this->transliteration = $transliteration;
}
/**
* Returns the cache contexts required for this block.
*
* @return array
* The required cache contexts IDs.
*/
protected function getRequiredCacheContexts() {
return array();
}
/**
* {@inheritdoc}
*/
......@@ -361,9 +316,7 @@ public function getCacheKeys() {
* {@inheritdoc}
*/
public function getCacheContexts() {
// Return the required cache contexts, merged with the user-configured cache
// contexts, if any.
return array_merge($this->getRequiredCacheContexts(), $this->configuration['cache']['contexts']);
return [];
}
/**
......
......@@ -96,13 +96,6 @@ public function defaultConfiguration() {
return array(
'block_count' => 10,
'feed' => NULL,
// Modify the default max age for the 'Aggregator Feed' blocks:
// modifications made to feeds or feed items will automatically invalidate
// corresponding cache tags, therefore allowing us to cache these blocks
// forever.
'cache' => array(
'max_age' => \Drupal\Core\Cache\Cache::PERMANENT,
),
);
}
......
......@@ -75,10 +75,7 @@ protected function setUp() {
* Test "user.roles" cache context.
*/
function testCachePerRole() {
$this->setBlockCacheConfig(array(
'max_age' => 600,
'contexts' => array('user.roles'),
));
\Drupal::state()->set('block_test.cache_contexts', ['user.roles']);
// Enable our test block. Set some content for it to display.
$current_content = $this->randomMachineName();
......@@ -125,9 +122,7 @@ function testCachePerRole() {
* Test a cacheable block without any cache context.
*/
function testCacheGlobal() {
$this->setBlockCacheConfig(array(
'max_age' => 600,
));
\Drupal::state()->set('block_test.cache_contexts', []);
$current_content = $this->randomMachineName();
\Drupal::state()->set('block_test.content', $current_content);
......@@ -170,10 +165,7 @@ function testNoCache() {
* Test "user" cache context.
*/
function testCachePerUser() {
$this->setBlockCacheConfig(array(
'max_age' => 600,
'contexts' => array('user'),
));
\Drupal::state()->set('block_test.cache_contexts', ['user']);
$current_content = $this->randomMachineName();
\Drupal::state()->set('block_test.content', $current_content);
......@@ -202,10 +194,7 @@ function testCachePerUser() {
* Test "url" cache context.
*/
function testCachePerPage() {
$this->setBlockCacheConfig(array(
'max_age' => 600,
'contexts' => array('url'),
));
\Drupal::state()->set('block_test.cache_contexts', ['url']);
$current_content = $this->randomMachineName();
\Drupal::state()->set('block_test.content', $current_content);
......
......@@ -8,6 +8,7 @@
namespace Drupal\block\Tests;
use Drupal\Component\Utility\String;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Form\FormState;
use Drupal\simpletest\KernelTestBase;
use Drupal\block\BlockInterface;
......@@ -45,8 +46,7 @@ public function testBlockInterface() {
'provider' => 'block_test',
'label_display' => BlockInterface::BLOCK_LABEL_VISIBLE,
'cache' => array(
'max_age' => 0,
'contexts' => array(),
'max_age' => Cache::PERMANENT,
),
'display_message' => 'no message set',
);
......@@ -65,9 +65,6 @@ public function testBlockInterface() {
$period = array_map(array(\Drupal::service('date.formatter'), 'formatInterval'), array_combine($period, $period));
$period[0] = '<' . t('no caching') . '>';
$period[\Drupal\Core\Cache\Cache::PERMANENT] = t('Forever');
$contexts = \Drupal::service("cache_contexts")->getLabels();
unset($contexts['theme']);
unset($contexts['languages']);
$expected_form = array(
'provider' => array(
'#type' => 'value',
......@@ -98,21 +95,9 @@ public function testBlockInterface() {
'#type' => 'select',
'#title' => t('Maximum age'),
'#description' => t('The maximum time this block may be cached.'),
'#default_value' => 0,
'#default_value' => Cache::PERMANENT,
'#options' => $period,
),
'contexts' => array(
'#type' => 'checkboxes',
'#title' => t('Vary by context'),
'#description' => t('The contexts this cached block must be varied by. <em>All</em> blocks are varied by language and theme.'),
'#default_value' => array(),
'#options' => $contexts,
'#states' => array(
'disabled' => array(
':input[name="settings[cache][max_age]"]' => array('value' => (string) 0),
),
),
),
),
'display_message' => array(
'#type' => 'textfield',
......
......@@ -7,6 +7,7 @@
namespace Drupal\block\Tests;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Config\Entity\ConfigEntityStorage;
use Drupal\simpletest\KernelTestBase;
use Drupal\block_test\Plugin\Block\TestHtmlBlock;
......@@ -99,8 +100,7 @@ protected function createTests() {
'provider' => 'block_test',
'label_display' => BlockInterface::BLOCK_LABEL_VISIBLE,
'cache' => array(
'max_age' => 0,
'contexts' => array(),
'max_age' => Cache::PERMANENT,
),
),
'visibility' => array(),
......
......@@ -8,7 +8,6 @@
namespace Drupal\block\Tests;
use Drupal\Component\Utility\Html;
use Drupal\Core\Cache\Cache;
use Drupal\simpletest\WebTestBase;
use Drupal\Component\Utility\String;
use Drupal\block\Entity\Block;
......@@ -320,7 +319,7 @@ public function testBlockCacheTags() {
$config->save();
// Place the "Powered by Drupal" block.
$block = $this->drupalPlaceBlock('system_powered_by_block', array('id' => 'powered', 'cache' => array('max_age' => 315360000)));
$block = $this->drupalPlaceBlock('system_powered_by_block', array('id' => 'powered'));
// Prime the page cache.
$this->drupalGet('<front>');
......@@ -361,7 +360,7 @@ public function testBlockCacheTags() {
$this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT');
// Place the "Powered by Drupal" block another time; verify a cache miss.
$block_2 = $this->drupalPlaceBlock('system_powered_by_block', array('id' => 'powered-2', 'cache' => array('max_age' => 315360000)));
$block_2 = $this->drupalPlaceBlock('system_powered_by_block', array('id' => 'powered-2'));
$this->drupalGet('<front>');
$this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS');
......
......@@ -149,15 +149,6 @@ protected function verifyRenderCacheHandling() {
$request_method = $request->server->get('REQUEST_METHOD');
$request->setMethod('GET');
// Test that entities with caching disabled do not generate a cache entry.
$build = $this->getBlockRenderArray();
$this->assertTrue(isset($build['#cache']) && array_keys($build['#cache']) == array('contexts', 'tags', 'max-age'), 'The render array element of uncacheable blocks is not cached, but does have cache contexts, tags & max-age set.');
// Enable block caching.
$this->setBlockCacheConfig(array(
'max_age' => 600,
));
// Test that a cache entry is created.
$build = $this->getBlockRenderArray();
$cid = 'entity_view:block:test_block:en:core';
......@@ -194,19 +185,10 @@ public function testBlockViewBuilderAlter() {
// Enable the block view alter hook that adds a suffix, for basic testing.
\Drupal::state()->set('block_test_view_alter_suffix', TRUE);
// Basic: non-empty block.
Cache::invalidateTags($this->block->getCacheTags());
$build = $this->getBlockRenderArray();
$this->assertTrue(isset($build['#suffix']) && $build['#suffix'] === '<br>Goodbye!', 'A block with content is altered.');
$this->assertIdentical(drupal_render($build), 'Llamas &gt; unicorns!<br>Goodbye!');
// Basic: empty block.
\Drupal::state()->set('block_test.content', NULL);
$build = $this->getBlockRenderArray();
$this->assertTrue(isset($build['#suffix']) && $build['#suffix'] === '<br>Goodbye!', 'A block without content is altered.');
$this->assertIdentical(drupal_render($build), '<br>Goodbye!');
// Disable the block view alter hook that adds a suffix, for basic testing.
\Drupal::state()->set('block_test_view_alter_suffix', FALSE);
// Force a request via GET so we can get drupal_render() cache working.
......@@ -214,13 +196,13 @@ public function testBlockViewBuilderAlter() {
$request_method = $request->server->get('REQUEST_METHOD');
$request->setMethod('GET');
\Drupal::state()->set('block_test.content', NULL);
Cache::invalidateTags($this->block->getCacheTags());
$default_keys = array('entity_view', 'block', 'test_block');
$default_tags = array('block_view', 'config:block.block.test_block');
// Advanced: cached block, but an alter hook adds an additional cache key.
$this->setBlockCacheConfig(array(
'max_age' => 600,
));
$alter_add_key = $this->randomMachineName();
\Drupal::state()->set('block_test_view_alter_cache_key', $alter_add_key);
$cid = 'entity_view:block:test_block:' . $alter_add_key . ':en:core';
......@@ -262,75 +244,6 @@ public function testBlockViewBuilderAlter() {
$request->setMethod($request_method);
}
/**
* Tests block render cache handling with configurable cache contexts.
*
* This is only intended to test that an existing block can be configured with
* additional contexts, not to test that each context works correctly.
*
* @see \Drupal\block\Tests\BlockCacheTest
*/
public function testBlockViewBuilderCacheContexts() {
$cache_contexts = \Drupal::service("cache_contexts");
// Force a request via GET so we can get drupal_render() cache working.
$request = \Drupal::request();
$request_method = $request->server->get('REQUEST_METHOD');
$request->setMethod('GET');
// First: no cache context.
$this->setBlockCacheConfig(array(
'max_age' => 600,
));
$build = $this->getBlockRenderArray();
$cid = implode(':', $build['#cache']['keys']);
drupal_render($build);
$this->assertTrue($this->container->get('cache.render', $cid), 'The block render element has been cached.');
// Second: the "per URL" cache context.
$this->setBlockCacheConfig(array(
'max_age' => 600,
'contexts' => array('url'),
));
$old_cid = $cid;
$build = $this->getBlockRenderArray();
$cid_parts = array_merge($build['#cache']['keys'], $cache_contexts->convertTokensToKeys($build['#cache']['contexts']));
$cid = implode(':', $cid_parts);
drupal_render($build);
$this->assertTrue($this->container->get('cache.render', $cid), 'The block render element has been cached.');
$this->assertNotEqual($cid, $old_cid, 'The cache ID has changed.');
// Third: the same block configuration, but a different URL.
$original_url_cache_context = $this->container->get('cache_context.url');
$request_stack = new RequestStack();
$request_stack->push(Request::create('/foo'));
$temp_context = new UrlCacheContext($request_stack);
$this->container->set('cache_context.url', $temp_context);
$old_cid = $cid;
$build = $this->getBlockRenderArray();
$cid_parts = array_merge($build['#cache']['keys'], $cache_contexts->convertTokensToKeys($build['#cache']['contexts']));
$cid = implode(':', $cid_parts);
drupal_render($build);
$this->assertTrue($this->container->get('cache.render', $cid), 'The block render element has been cached.');
$this->assertNotEqual($cid, $old_cid, 'The cache ID has changed.');
$this->container->set('cache_context.url', $original_url_cache_context);
// Restore the previous request method.
$request->setMethod($request_method);
}
/**
* Sets the test block's cache configuration.
*
* @param array $cache_config
* The desired cache configuration.
*/
protected function setBlockCacheConfig(array $cache_config) {
$block = $this->block->getPlugin();
$block->setConfigurationValue('cache', $cache_config);
$this->block->save();
}
/**
* Get a fully built render array for a block.
*
......
......@@ -273,7 +273,7 @@ public function testBlockRendering() {
public function testBlockContextualLinks() {
$this->drupalLogin($this->drupalCreateUser(array('administer views', 'access contextual links', 'administer blocks')));
$block = $this->drupalPlaceBlock('views_block:test_view_block-block_1');
$cached_block = $this->drupalPlaceBlock('views_block:test_view_block-block_1', array('cache' => array('max_age' => 3600)));
$cached_block = $this->drupalPlaceBlock('views_block:test_view_block-block_1');
$this->drupalGet('test-page');
$id = 'block:block=' . $block->id() . ':|entity.view.edit_form:view=test_view_block:location=block&name=test_view_block&display_id=block_1';
......
......@@ -32,4 +32,11 @@ public function build() {
return $build;
}
/**
* {@inheritdoc}
*/
public function getCacheContexts() {
return \Drupal::state()->get('block_test.cache_contexts', []);
}
}
......@@ -98,12 +98,6 @@ public function defaultConfiguration() {
'status' => TRUE,
'info' => '',
'view_mode' => 'full',
// Modify the default max age for custom block blocks: modifications made
// to them will automatically invalidate corresponding cache tags, thus
// allowing us to cache custom block blocks forever.
'cache' => array(
'max_age' => \Drupal\Core\Cache\Cache::PERMANENT,
),
);
}
......
......@@ -64,7 +64,7 @@ protected function getAdditionalCacheTagsForEntity(EntityInterface $entity) {
* Tests that the block is cached with the correct contexts and tags.
*/
public function testBlock() {
$block = $this->drupalPlaceBlock('block_content:' . $this->entity->uuid(), ['cache' => []]);
$block = $this->drupalPlaceBlock('block_content:' . $this->entity->uuid());
$build = $this->container->get('entity.manager')->getViewBuilder('block')->view($block, 'block');
// Render the block.
......
......@@ -16,9 +16,6 @@ settings:
label: 'Foobar Gorilla'
provider: block_content
label_display: visible
cache:
max_age: -1
contexts: { }
status: true
info: ''
view_mode: default
......
......@@ -181,7 +181,7 @@ public function build() {
/**
* {@inheritdoc}
*/
protected function getRequiredCacheContexts() {
public function getCacheContexts() {
// The "Book navigation" block must be cached per role and book navigation
// context.
return [
......@@ -190,4 +190,13 @@ protected function getRequiredCacheContexts() {
];
}
/**
* {@inheritdoc}
*
* @todo Make cacheable as part of https://drupal.org/node/1805054
*/
public function getCacheMaxAge() {
return 0;
}
}
......@@ -87,7 +87,16 @@ public function blockSubmit($form, FormStateInterface $form_state) {
* {@inheritdoc}
*/
public function getCacheKeys() {
return array_merge(parent::getCacheKeys(), Cache::keyFromQuery($this->buildForumQuery()));
$keys = parent::getCacheKeys();
$keys[] = Cache::keyFromQuery($this->buildForumQuery());
return $keys;
}
/**
* {@inheritdoc}
*/
public function getCacheTags() {
return ['node_list'];
}
}
......@@ -127,18 +127,7 @@ public function build() {
/**
* {@inheritdoc}
*/
public function defaultConfiguration() {
// Modify the default max age for the Help block: help text is
// static for a given URL, except when a module is updated, in which case
// update.php must be run, which clears all caches. Thus it's safe to cache
// the output for this block forever on a per-URL basis.
return array('cache' => array('max_age' => \Drupal\Core\Cache\Cache::PERMANENT));
}
/**
* {@inheritdoc}
*/
protected function getRequiredCacheContexts() {
public function getCacheContexts() {
// The "Help" block must be cached per URL: help is defined for a
// given path, and does not come with any access restrictions.
return array('url');
......
......@@ -107,4 +107,13 @@ public function build() {
return $build;
}
/**
* {@inheritdoc}
*
* @todo Make cacheable in https://drupal.org/node/2232375.
*/
public function getCacheMaxAge() {
return 0;
}
}
......@@ -578,12 +578,6 @@ public function testBlockContextualLinks() {
$this->assertResponse(200);
$json = Json::decode($response);
$this->assertIdentical($json[$id], '<ul class="contextual-links"><li class="block-configure"><a href="' . base_path() . 'admin/structure/block/manage/' . $block->id() . '">Configure block</a></li><li class="entitymenuedit-form"><a href="' . base_path() . 'admin/structure/menu/manage/' . $custom_menu->id() . '">Edit menu</a></li></ul>');
// Test the contextual links are available when block caching is enabled.
$this->drupalPostForm('admin/structure/block/manage/' . $block->id(), ['settings[cache][max_age]' => Cache::PERMANENT], t('Save block'));
$this->drupalGet('test-page');
$id = 'block:block=' . $block->id() . ':|menu:menu=' . $custom_menu->id() . ':';
$this->assertRaw('<div data-contextual-id="'. $id . '"></div>', format_string('Contextual link placeholder with id @id exists.', array('@id' => $id)));
}
/**
......
......@@ -11,6 +11,7 @@
use Drupal\Component\Utility\Html;
use Drupal\Component\Utility\Xss;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Database\Query\AlterableInterface;
use Drupal\Core\Database\Query\SelectInterface;
use Drupal\Core\Database\StatementInterface;
......@@ -188,15 +189,17 @@ function node_entity_view_display_alter(EntityViewDisplayInterface $display, $co
function node_title_list(StatementInterface $result, $title = NULL) {
$items = array();
$num_rows = FALSE;
$nids = [];
foreach ($result as $row) {
// Do not use $node->label() or $node->urlInfo() here, because we only have
// database rows, not actual nodes.
$nids[] = $row->nid;
$options = !empty($row->comment_count) ? array('attributes' => array('title' => \Drupal::translation()->formatPlural($row->comment_count, '1 comment', '@count comments'))) : array();
$items[] = \Drupal::l($row->title, new Url('entity.node.canonical', ['node' => $row->nid], $options));
$num_rows = TRUE;
}
return $num_rows ? array('#theme' => 'item_list__node', '#items' => $items, '#title' => $title) : FALSE;
return $num_rows ? array('#theme' => 'item_list__node', '#items' => $items, '#title' => $title, '#cache' => ['tags' => Cache::mergeTags(['node_list'], Cache::buildTags('node', $nids))]) : FALSE;
}
/**
......
......@@ -37,4 +37,13 @@ public function build() {
return \Drupal::formBuilder()->getForm('Drupal\search\Form\SearchBlockForm');
}
/**
* {@inheritdoc}
*
* @todo Make cacheable once https://www.drupal.org/node/2351015 lands.
*/
public function getCacheMaxAge() {
return 0;
}
}
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