diff -u b/core/lib/Drupal/Core/Cache/Context/CacheContextsManager.php b/core/lib/Drupal/Core/Cache/Context/CacheContextsManager.php --- b/core/lib/Drupal/Core/Cache/Context/CacheContextsManager.php +++ b/core/lib/Drupal/Core/Cache/Context/CacheContextsManager.php @@ -89,7 +89,7 @@ } /** - * Converts cache context tokens to cache keys and gets their cacheable metadata. + * Converts cache context tokens to cache keys and gets cacheable metadata. * * A cache context token is either: * - a cache context ID (if the service ID is 'cache_context.foo', then 'foo' @@ -101,26 +101,28 @@ * An array of cache context tokens. * * @return mixed[] - * An array containing two items; contexts and cacheable_metadata. + * An array containing two items: + * - contexts: string[] + * - cacheable_metadata: \Drupal\Core\Cache\CacheableMetadata. * * @throws \InvalidArgumentException */ public function convertTokensToKeys(array $context_tokens) { $cacheable_metadata = new CacheableMetadata(); - - // Iterate through cache contexts and get their cacheable metadata. - foreach (static::parseTokens($context_tokens) as $context_token) { + $optimized_tokens = $this->optimizeTokens($context_tokens); + // Iterate through cache contexts that has been optimized away and get their + // cacheable metadata. + foreach (static::parseTokens(array_diff($context_tokens, $optimized_tokens)) as $context_token) { list($context_id, $parameter) = $context_token; $context = $this->getService($context_id); if ($context instanceof CacheableCacheContextInterface || $context instanceof CacheableCalculatedCacheContextInterface) { - $cacheable_metadata = $cacheable_metadata->merge(CacheableMetadata::createFromObject($context->getCacheableMetadata())); + $cacheable_metadata = $cacheable_metadata->merge($context->getCacheableMetadata()); } } - $context_tokens = $this->optimizeTokens($context_tokens); - sort($context_tokens); + sort($optimized_tokens); $keys = []; - foreach (static::parseTokens($context_tokens) as $context) { + foreach (static::parseTokens($optimized_tokens) as $context) { list($context_id, $parameter) = $context; if (!in_array($context_id, $this->contexts)) { throw new \InvalidArgumentException(SafeMarkup::format('"@context" is not a valid cache context ID.', ['@context' => $context_id])); @@ -147,6 +149,9 @@ * possible of a set of cache context tokens, that still captures the entire * universe of variations. * + * If cache context is being optimized away, it is able to set cacheable + * metadata for itself which will be bubbled up. + * * E.g. when caching per user ('user'), also caching per role ('user.roles') * is meaningless because "per role" is implied by "per user". * diff -u b/core/lib/Drupal/Core/Cache/Context/CacheableCacheContextInterface.php b/core/lib/Drupal/Core/Cache/Context/CacheableCacheContextInterface.php --- b/core/lib/Drupal/Core/Cache/Context/CacheableCacheContextInterface.php +++ b/core/lib/Drupal/Core/Cache/Context/CacheableCacheContextInterface.php @@ -12,11 +12,17 @@ +/** + * Provides an interface for cacheable cache contexts. + */ interface CacheableCacheContextInterface extends CacheContextInterface, CacheableDependencyInterface { /** * Gets the cacheable metadata for the context based on the parameter value. * - * @return \Drupal\Core\Cache\CacheableDependencyInterface - * A cacheable dependency interface implementing object, e.g. CacheableMetadata + * If the cache context is being optimized away, this cacheable metadata will + * be bubbled up. + * + * @return \Drupal\Core\Cache\CacheableMetadata + * A cacheable metadata object. */ public function getCacheableMetadata(); -} \ No newline at end of file +} diff -u b/core/lib/Drupal/Core/Cache/Context/CacheableCalculatedCacheContextInterface.php b/core/lib/Drupal/Core/Cache/Context/CacheableCalculatedCacheContextInterface.php --- b/core/lib/Drupal/Core/Cache/Context/CacheableCalculatedCacheContextInterface.php +++ b/core/lib/Drupal/Core/Cache/Context/CacheableCalculatedCacheContextInterface.php @@ -12,14 +12,20 @@ +/** + * Provides an interface for calculated cacheable cache contexts. + */ interface CacheableCalculatedCacheContextInterface extends CalculatedCacheContextInterface { /** * Gets the cacheable metadata for the context based on the parameter value. * + * If the cache context is being optimized away, this cacheable metadata will + * be bubbled up. + * * @param mixed $parameter * The parameter to get context values for. - * - * @return \Drupal\Core\Cache\CacheableDependencyInterface - * A cacheable dependency interface implementing object, e.g. CacheableMetadata + + * @return \Drupal\Core\Cache\CacheableMetadata + * A cacheable metadata object. */ public function getCacheableMetadata($parameter = NULL); -} \ No newline at end of file +} diff -u b/core/lib/Drupal/Core/Cache/Context/UserCacheContext.php b/core/lib/Drupal/Core/Cache/Context/UserCacheContext.php --- b/core/lib/Drupal/Core/Cache/Context/UserCacheContext.php +++ b/core/lib/Drupal/Core/Cache/Context/UserCacheContext.php @@ -8,6 +8,7 @@ namespace Drupal\Core\Cache\Context; use Drupal\Core\Cache\Cache; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Session\AccountInterface; /** @@ -66,7 +67,7 @@ * {@inheritdoc} */ public function getCacheableMetadata() { - return $this; + return CacheableMetadata::createFromObject($this); } } reverted: --- b/core/modules/comment/src/Tests/CommentCacheTagsTest.php +++ a/core/modules/comment/src/Tests/CommentCacheTagsTest.php @@ -102,7 +102,6 @@ /** @var \Drupal\comment\CommentInterface $entity */ return array( 'config:filter.format.plain_text', - 'config:user.role.anonymous', 'user:' . $entity->getOwnerId(), 'user_view', ); reverted: --- b/core/modules/field/src/Tests/EntityReference/EntityReferenceFormatterTest.php +++ a/core/modules/field/src/Tests/EntityReference/EntityReferenceFormatterTest.php @@ -203,8 +203,7 @@ $expected_cache_tags = Cache::mergeTags( \Drupal::entityManager()->getViewBuilder($this->entityType)->getCacheTags(), $this->referencedEntity->getCacheTags(), + FilterFormat::load('full_html')->getCacheTags() - FilterFormat::load('full_html')->getCacheTags(), - ['config:user.role.anonymous'] ); $this->assertEqual($build[0]['#cache']['tags'], $expected_cache_tags, format_string('The @formatter formatter has the expected cache tags.', array('@formatter' => $formatter))); reverted: --- b/core/modules/system/src/Tests/Render/PermissionsCacheContextTest.php +++ /dev/null @@ -1,78 +0,0 @@ -installEntitySchema('user'); - $this->installConfig(['user']); - $this->installSchema('system', ['sequences']); - } - - /** - * Ensures that permission change works with permission and user contexts. - */ - public function testPermissionChange() { - $user1 = $this->createUser(); - $this->assertEqual($user1->id(), 1); - - $authenticated_user = $this->createUser(['administer permissions']); - $role = $authenticated_user->getRoles()[1]; - - $test_element = [ - '#cache' => [ - 'keys' => ['test'], - 'contexts' => ['user', 'user.permissions'], - ], - ]; - \Drupal::service('account_switcher')->switchTo($authenticated_user); - $element = $test_element; - $element['#markup'] = 'content for authenticated users'; - $output = \Drupal::service('renderer')->renderRoot($element); - $this->assertEqual($output, 'content for authenticated users'); - - // Verify that the render caching is working. - $element = $test_element; - $element['#markup'] = 'this should not be visible'; - $output = \Drupal::service('renderer')->renderRoot($element); - $this->assertEqual($output, 'content for authenticated users'); - - // Verify that the element is being rendered after permissions have changed. - Role::load($role) - ->revokePermission('administer permissions') - ->save(); - $element = $test_element; - $element['#markup'] = 'this should be visible'; - $output = \Drupal::service('renderer')->renderRoot($element); - $this->assertEqual($output, 'this should be visible'); - } - -} only in patch2: unchanged: --- /dev/null +++ b/core/modules/system/src/Tests/Cache/CacheContextOptimizationTest.php @@ -0,0 +1,83 @@ +installEntitySchema('user'); + $this->installConfig(['user']); + $this->installSchema('system', ['sequences']); + } + + /** + * Ensures that 'user.permissions' cache context is able to define cache tags. + */ + public function testUserPermissionCacheContextOptimization() { + $user1 = $this->createUser(); + $this->assertEqual($user1->id(), 1); + + $authenticated_user = $this->createUser(['administer permissions']); + $role = $authenticated_user->getRoles()[1]; + + $test_element = [ + '#cache' => [ + 'keys' => ['test'], + 'contexts' => ['user', 'user.permissions'], + ], + ]; + \Drupal::service('account_switcher')->switchTo($authenticated_user); + $element = $test_element; + $element['#markup'] = 'content for authenticated users'; + $output = \Drupal::service('renderer')->renderRoot($element); + $this->assertEqual($output, 'content for authenticated users'); + + // Verify that the render caching is working so that other tests can be + // trusted. + $element = $test_element; + $element['#markup'] = 'this should not be visible'; + $output = \Drupal::service('renderer')->renderRoot($element); + $this->assertEqual($output, 'content for authenticated users'); + + // Even though the cache contexts have been optimized to only include 'user' + // cache context, the element should have been changed because + // 'user.permissions' cache context defined a cache tags for permission + // change, which should have bubbled up for the element when it was optimized + // away. + Role::load($role) + ->revokePermission('administer permissions') + ->save(); + $element = $test_element; + $element['#markup'] = 'this should be visible'; + $output = \Drupal::service('renderer')->renderRoot($element); + $this->assertEqual($output, 'this should be visible'); + } + +}