Commit b2303acf authored by catch's avatar catch

Issue #2543332 by Wim Leers, effulgentsia, Fabianx, Crell, dawehner,...

Issue #2543332 by Wim Leers, effulgentsia, Fabianx, Crell, dawehner, borisson_: Auto-placeholdering for #lazy_builder without bubbling
parent c3e2000b
......@@ -3,6 +3,10 @@ parameters:
twig.config: {}
renderer.config:
required_cache_contexts: ['languages:language_interface', 'theme', 'user.permissions']
auto_placeholder_conditions:
max-age: 0
contexts: ['session', 'user']
tags: []
factory.keyvalue:
default: keyvalue.database
factory.keyvalue.expirable:
......
......@@ -170,6 +170,9 @@ protected function renderPlaceholder($placeholder, array $elements) {
// Get the render array for the given placeholder
$placeholder_elements = $elements['#attached']['placeholders'][$placeholder];
// Prevent the render array from being auto-placeholdered again.
$placeholder_elements['#create_placeholder'] = FALSE;
// Render the placeholder into markup.
$markup = $this->renderPlain($placeholder_elements);
......@@ -337,6 +340,10 @@ protected function doRender(&$elements, $is_root_call = FALSE) {
throw new \DomainException(sprintf('When a #lazy_builder callback is specified, no properties can exist; all properties must be generated by the #lazy_builder callback. You specified the following properties: %s.', implode(', ', $unsupported_keys)));
}
}
// Determine whether to do auto-placeholdering.
if (isset($elements['#lazy_builder']) && (!isset($elements['#create_placeholder']) || $elements['#create_placeholder'] !== FALSE) && $this->shouldAutomaticallyPlaceholder($elements)) {
$elements['#create_placeholder'] = TRUE;
}
// If instructed to create a placeholder, and a #lazy_builder callback is
// present (without such a callback, it would be impossible to replace the
// placeholder), replace the current element with a placeholder.
......@@ -635,6 +642,37 @@ protected function replacePlaceholders(array &$elements) {
return TRUE;
}
/**
* Whether the given render array should be automatically placeholdered.
*
* @param array $element
* The render array whose cacheability to analyze.
*
* @return bool
* Whether the given render array's cacheability meets the placeholdering
* conditions.
*/
protected function shouldAutomaticallyPlaceholder(array $element) {
$conditions = $this->rendererConfig['auto_placeholder_conditions'];
// Auto-placeholder if max-age is at or below the configured threshold.
if (isset($element['#cache']['max-age']) && $element['#cache']['max-age'] !== Cache::PERMANENT && $element['#cache']['max-age'] <= $conditions['max-age']) {
return TRUE;
}
// Auto-placeholder if a high-cardinality cache context is set.
if (isset($element['#cache']['contexts']) && array_intersect($element['#cache']['contexts'], $conditions['contexts'])) {
return TRUE;
}
// Auto-placeholder if a high-invalidation frequency cache tag is set.
if (isset($element['#cache']['tags']) && array_intersect($element['#cache']['tags'], $conditions['tags'])) {
return TRUE;
}
return FALSE;
}
/**
* Turns this element into a placeholder.
*
......
......@@ -78,6 +78,16 @@ public function form(array $form, FormStateInterface $form_state) {
$field_definition = $this->entityManager->getFieldDefinitions($entity->getEntityTypeId(), $entity->bundle())[$comment->getFieldName()];
$config = $this->config('user.settings');
// In several places within this function, we vary $form on:
// - The current user's permissions.
// - Whether the current user is authenticated or anonymous.
// - The 'user.settings' configuration.
// - The comment field's definition.
$form['#cache']['contexts'][] = 'user.permissions';
$form['#cache']['contexts'][] = 'user.roles:authenticated';
$this->renderer->addCacheableDependency($form, $config);
$this->renderer->addCacheableDependency($form, $field_definition->getConfig($entity->bundle()));
// Use #comment-form as unique jump target, regardless of entity type.
$form['#id'] = Html::getUniqueId('comment_form');
$form['#theme'] = array('comment_form__' . $entity->getEntityTypeId() . '__' . $entity->bundle() . '__' . $field_name, 'comment_form');
......@@ -90,10 +100,6 @@ public function form(array $form, FormStateInterface $form_state) {
$form['#attributes']['data-user-info-from-browser'] = TRUE;
}
// Vary per role, because we check a permission above and attach an asset
// library only for authenticated users.
$form['#cache']['contexts'][] = 'user.roles';
// If not replying to a comment, use our dedicated page callback for new
// Comments on entities.
if (!$comment->id() && !$comment->hasParentComment()) {
......@@ -164,6 +170,7 @@ public function form(array $form, FormStateInterface $form_state) {
$form['author']['name']['#value'] = $form['author']['name']['#default_value'];
$form['author']['name']['#theme'] = 'username';
$form['author']['name']['#account'] = $this->currentUser;
$form['author']['name']['#cache']['contexts'][] = 'user';
}
elseif($this->currentUser->isAnonymous()) {
$form['author']['name']['#attributes']['data-drupal-default-value'] = $config->get('anonymous');
......@@ -210,10 +217,6 @@ public function form(array $form, FormStateInterface $form_state) {
'#access' => $is_admin,
);
$this->renderer->addCacheableDependency($form, $config);
// The form depends on the field definition.
$this->renderer->addCacheableDependency($form, $field_definition->getConfig($entity->bundle()));
return parent::form($form, $form_state, $comment);
}
......
......@@ -184,30 +184,23 @@ public function viewElements(FieldItemListInterface $items) {
// Only show the add comment form if the user has permission.
$elements['#cache']['contexts'][] = 'user.roles';
if ($this->currentUser->hasPermission('post comments')) {
// All users in the "anonymous" role can use the same form: it is fine
// for this form to be stored in the render cache.
if ($this->currentUser->isAnonymous()) {
$comment = $this->storage->create(array(
'entity_type' => $entity->getEntityTypeId(),
'entity_id' => $entity->id(),
'field_name' => $field_name,
'comment_type' => $this->getFieldSetting('comment_type'),
'pid' => NULL,
));
$output['comment_form'] = $this->entityFormBuilder->getForm($comment);
}
// All other users need a user-specific form, which would break the
// render cache: hence use a #lazy_builder callback.
else {
$output['comment_form'] = [
'#lazy_builder' => ['comment.lazy_builders:renderForm', [
$entity->getEntityTypeId(),
$entity->id(),
$field_name,
$this->getFieldSetting('comment_type'),
]],
'#create_placeholder' => TRUE,
];
$output['comment_form'] = [
'#lazy_builder' => ['comment.lazy_builders:renderForm', [
$entity->getEntityTypeId(),
$entity->id(),
$field_name,
$this->getFieldSetting('comment_type'),
]],
];
// @todo Remove this in https://www.drupal.org/node/2543334. Until
// then, \Drupal\Core\Render\Renderer::hasPoorCacheability() isn't
// integrated with cache context bubbling, so this duplicates the
// contexts added by \Drupal\comment\CommentForm::form().
$output['comment_form']['#cache']['contexts'][] = 'user.permissions';
$output['comment_form']['#cache']['contexts'][] = 'user.roles:authenticated';
if ($this->currentUser->isAuthenticated()) {
$output['comment_form']['#cache']['contexts'][] = 'user';
}
}
}
......
......@@ -70,8 +70,7 @@ public function testCacheTags() {
->getViewBuilder('entity_test')
->view($commented_entity);
$renderer->renderRoot($build);
$cache_context_tags = \Drupal::service('cache_contexts_manager')->convertTokensToKeys($build['#cache']['contexts'])->getCacheTags();
$expected_cache_tags = Cache::mergeTags($cache_context_tags, [
$expected_cache_tags = [
'entity_test_view',
'entity_test:' . $commented_entity->id(),
'comment_list',
......@@ -80,7 +79,7 @@ public function testCacheTags() {
'config:field.field.entity_test.entity_test.comment',
'config:field.storage.comment.comment_body',
'config:user.settings',
]);
];
sort($expected_cache_tags);
$this->assertEqual($build['#cache']['tags'], $expected_cache_tags);
......@@ -113,8 +112,7 @@ public function testCacheTags() {
->getViewBuilder('entity_test')
->view($commented_entity);
$renderer->renderRoot($build);
$cache_context_tags = \Drupal::service('cache_contexts_manager')->convertTokensToKeys($build['#cache']['contexts'])->getCacheTags();
$expected_cache_tags = Cache::mergeTags($cache_context_tags, [
$expected_cache_tags = [
'entity_test_view',
'entity_test:' . $commented_entity->id(),
'comment_list',
......@@ -128,7 +126,7 @@ public function testCacheTags() {
'config:field.field.entity_test.entity_test.comment',
'config:field.storage.comment.comment_body',
'config:user.settings',
]);
];
sort($expected_cache_tags);
$this->assertEqual($build['#cache']['tags'], $expected_cache_tags);
}
......
......@@ -38,10 +38,9 @@ class CommentTranslationUITest extends ContentTranslationUITestBase {
protected $defaultCacheContexts = [
'languages:language_interface',
'theme',
'user.permissions',
'timezone',
'url.query_args.pagers:0',
'user.roles'
'user'
];
/**
......
......@@ -27,13 +27,12 @@ class NodeTranslationUITest extends ContentTranslationUITestBase {
protected $defaultCacheContexts = [
'languages:language_interface',
'theme',
'user.permissions',
'route.menu_active_trails:account',
'route.menu_active_trails:footer',
'route.menu_active_trails:main',
'route.menu_active_trails:tools',
'timezone',
'user.roles'
'user'
];
/**
......
......@@ -89,6 +89,11 @@ class RendererTestBase extends UnitTestCase {
'languages:language_interface',
'theme',
],
'auto_placeholder_conditions' => [
'max-age' => 0,
'contexts' => ['session', 'user'],
'tags' => ['current-temperature'],
],
];
/**
......
......@@ -84,6 +84,37 @@ parameters:
#
# @default ['languages:language_interface', 'theme', 'user.permissions']
required_cache_contexts: ['languages:language_interface', 'theme', 'user.permissions']
# Renderer automatic placeholdering conditions:
#
# Drupal allows portions of the page to be automatically deferred when
# rendering to improve cache performance. That is especially helpful for
# cache contexts that vary widely, such as the active user. On some sites
# those may be different, however, such as sites with only a handful of
# users. If you know what the high-cardinality cache contexts are for your
# site, specify those here. If you're not sure, the defaults are fairly safe
# in general.
#
# For more information about rendering optimizations see
# https://www.drupal.org/developing/api/8/render/arrays/cacheability#optimizing
auto_placeholder_conditions:
# Max-age at or below which caching is not considered worthwhile.
#
# Disable by setting to -1.
#
# @default 0
max-age: 0
# Cache contexts with a high cardinality.
#
# Disable by setting to [].
#
# @default ['session', 'user']
contexts: ['session', 'user']
# Tags with a high invalidation frequency.
#
# Disable by setting to [].
#
# @default []
tags: []
factory.keyvalue:
{}
# Default key/value storage service to use.
......
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