diff --git a/core/lib/Drupal/Component/Plugin/ContextAwarePluginBase.php b/core/lib/Drupal/Component/Plugin/ContextAwarePluginBase.php index e925407..d9c9c8a 100644 --- a/core/lib/Drupal/Component/Plugin/ContextAwarePluginBase.php +++ b/core/lib/Drupal/Component/Plugin/ContextAwarePluginBase.php @@ -22,7 +22,7 @@ * * @var \Drupal\Component\Plugin\Context\ContextInterface[] */ - protected $context; + protected $context = []; /** * Overrides \Drupal\Component\Plugin\PluginBase::__construct(). diff --git a/core/lib/Drupal/Core/Block/BlockBase.php b/core/lib/Drupal/Core/Block/BlockBase.php index fe31b48..ef127e9 100644 --- a/core/lib/Drupal/Core/Block/BlockBase.php +++ b/core/lib/Drupal/Core/Block/BlockBase.php @@ -10,6 +10,7 @@ use Drupal\block\BlockInterface; use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Plugin\ContextAwarePluginBase; use Drupal\Component\Utility\Unicode; @@ -273,6 +274,16 @@ public function getMachineNameSuggestion() { } /** + * {@inheritdoc} + */ + public function getCacheMaxAge() { + $max_age = parent::getCacheMaxAge(); + // @todo Configurability of this will be removed in + // https://www.drupal.org/node/2458763. + return Cache::mergeMaxAges($max_age, (int) $this->configuration['cache']['max_age']); + } + + /** * Wraps the transliteration service. * * @return \Drupal\Component\Transliteration\TransliterationInterface @@ -294,25 +305,4 @@ public function setTransliteration(TransliterationInterface $transliteration) { $this->transliteration = $transliteration; } - /** - * {@inheritdoc} - */ - public function getCacheContexts() { - return []; - } - - /** - * {@inheritdoc} - */ - public function getCacheTags() { - return []; - } - - /** - * {@inheritdoc} - */ - public function getCacheMaxAge() { - return (int)$this->configuration['cache']['max_age']; - } - } diff --git a/core/lib/Drupal/Core/Condition/ConditionInterface.php b/core/lib/Drupal/Core/Condition/ConditionInterface.php index e0b6115..c53059d 100644 --- a/core/lib/Drupal/Core/Condition/ConditionInterface.php +++ b/core/lib/Drupal/Core/Condition/ConditionInterface.php @@ -9,6 +9,7 @@ use Drupal\Component\Plugin\ConfigurablePluginInterface; use Drupal\Component\Plugin\PluginInspectionInterface; +use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\Executable\ExecutableInterface; use Drupal\Core\Executable\ExecutableManagerInterface; use Drupal\Core\Plugin\PluginFormInterface; @@ -46,7 +47,7 @@ * * @ingroup plugin_api */ -interface ConditionInterface extends ExecutableInterface, PluginFormInterface, ConfigurablePluginInterface, PluginInspectionInterface { +interface ConditionInterface extends ExecutableInterface, PluginFormInterface, ConfigurablePluginInterface, PluginInspectionInterface, CacheableDependencyInterface { /** * Determines whether condition result will be negated. diff --git a/core/lib/Drupal/Core/Condition/ConditionPluginBase.php b/core/lib/Drupal/Core/Condition/ConditionPluginBase.php index 5237d0d..6f6ffc3 100644 --- a/core/lib/Drupal/Core/Condition/ConditionPluginBase.php +++ b/core/lib/Drupal/Core/Condition/ConditionPluginBase.php @@ -7,6 +7,8 @@ namespace Drupal\Core\Condition; +use Drupal\Core\Cache\Cache; +use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\Executable\ExecutableManagerInterface; use Drupal\Core\Executable\ExecutablePluginBase; use Drupal\Core\Form\FormStateInterface; diff --git a/core/lib/Drupal/Core/Plugin/Context/Context.php b/core/lib/Drupal/Core/Plugin/Context/Context.php index 6178398..5a5ab6c 100644 --- a/core/lib/Drupal/Core/Plugin/Context/Context.php +++ b/core/lib/Drupal/Core/Plugin/Context/Context.php @@ -10,6 +10,8 @@ use Drupal\Component\Plugin\Context\Context as ComponentContext; use Drupal\Component\Plugin\Exception\ContextException; use Drupal\Component\Utility\SafeMarkup; +use Drupal\Core\Cache\CacheableDependencyInterface; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\TypedData\TypedDataInterface; use Drupal\Core\TypedData\TypedDataTrait; @@ -35,6 +37,21 @@ class Context extends ComponentContext implements ContextInterface { protected $contextDefinition; /** + * The cacheability metadata. + * + * @var \Drupal\Core\Cache\CacheableMetadata + */ + protected $cacheabilityMetadata; + + /** + * {@inheritdoc} + */ + public function __construct(ContextDefinitionInterface $context_definition) { + parent::__construct($context_definition); + $this->cacheabilityMetadata = new CacheableMetadata(); + } + + /** * {@inheritdoc} */ public function getContextValue() { @@ -67,6 +84,11 @@ public function hasContextValue() { * {@inheritdoc} */ public function setContextValue($value) { + // Add the value as a cacheable dependency only if implements the interface + // to prevent it from disabling caching with a max-age 0. + if ($value instanceof CacheableDependencyInterface) { + $this->addCacheableDependency($value); + } if ($value instanceof TypedDataInterface) { return $this->setContextData($value); } @@ -120,4 +142,33 @@ public function validate() { return $this->getContextData()->validate(); } + /** + * {@inheritdoc} + */ + public function addCacheableDependency($dependency) { + $this->cacheabilityMetadata = $this->cacheabilityMetadata->merge(CacheableMetadata::createFromObject($dependency)); + return $this; + } + + /** + * {@inheritdoc} + */ + public function getCacheContexts() { + return $this->cacheabilityMetadata->getCacheContexts(); + } + + /** + * {@inheritdoc} + */ + public function getCacheTags() { + return $this->cacheabilityMetadata->getCacheTags(); + } + + /** + * {@inheritdoc} + */ + public function getCacheMaxAge() { + return $this->cacheabilityMetadata->getCacheMaxAge(); + } + } diff --git a/core/lib/Drupal/Core/Plugin/Context/ContextHandler.php b/core/lib/Drupal/Core/Plugin/Context/ContextHandler.php index 06930e1..39c2460 100644 --- a/core/lib/Drupal/Core/Plugin/Context/ContextHandler.php +++ b/core/lib/Drupal/Core/Plugin/Context/ContextHandler.php @@ -9,6 +9,7 @@ use Drupal\Component\Plugin\Exception\ContextException; use Drupal\Component\Utility\SafeMarkup; +use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\Plugin\ContextAwarePluginInterface; /** @@ -84,6 +85,14 @@ public function applyContextMapping(ContextAwarePluginInterface $plugin, $contex // This assignment has been used, remove it. unset($mappings[$plugin_context_id]); + // Plugins have their on context objects, only the value is applied. + // They also need to know about the cacheable metadata of where that + // value is coming from, so pass them through to those objects. + $plugin_context = $plugin->getContext($plugin_context_id); + if ($plugin_context instanceof ContextInterface && $contexts[$context_id] instanceof CacheableDependencyInterface) { + $plugin_context->addCacheableDependency($contexts[$context_id]); + } + // Pass the value to the plugin if there is one. if ($contexts[$context_id]->hasContextValue()) { $plugin->setContextValue($plugin_context_id, $contexts[$context_id]->getContextValue()); diff --git a/core/lib/Drupal/Core/Plugin/Context/ContextInterface.php b/core/lib/Drupal/Core/Plugin/Context/ContextInterface.php index bb38617..5b1e6bd 100644 --- a/core/lib/Drupal/Core/Plugin/Context/ContextInterface.php +++ b/core/lib/Drupal/Core/Plugin/Context/ContextInterface.php @@ -8,12 +8,13 @@ namespace Drupal\Core\Plugin\Context; use Drupal\Component\Plugin\Context\ContextInterface as ComponentContextInterface; +use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\TypedData\TypedDataInterface; /** * Interface for context. */ -interface ContextInterface extends ComponentContextInterface { +interface ContextInterface extends ComponentContextInterface, CacheableDependencyInterface { /** * Gets the context value as typed data object. @@ -32,4 +33,22 @@ public function getContextData(); */ public function setContextData(TypedDataInterface $data); + /** + * Adds a dependency on an object: merges its cacheability metadata. + * + * E.g. when a context depends on some configuration, an entity, or an access + * result, we must make sure their cacheability metadata is present on the + * response. This method makes doing that simple. + * + * @param \Drupal\Core\Cache\CacheableDependencyInterface|mixed $dependency + * The dependency. If the object implements CacheableDependencyInterface, + * then its cacheability metadata will be used. Otherwise, the passed in + * object must be assumed to be uncacheable, so max-age 0 is set. + * + * @return $this + * + * @see \Drupal\Core\Cache\CacheableMetadata::createFromObject() + */ + public function addCacheableDependency($dependency); + } diff --git a/core/lib/Drupal/Core/Plugin/ContextAwarePluginBase.php b/core/lib/Drupal/Core/Plugin/ContextAwarePluginBase.php index e58e39e..51a39d7 100644 --- a/core/lib/Drupal/Core/Plugin/ContextAwarePluginBase.php +++ b/core/lib/Drupal/Core/Plugin/ContextAwarePluginBase.php @@ -10,6 +10,8 @@ use Drupal\Component\Plugin\ConfigurablePluginInterface; use Drupal\Component\Plugin\ContextAwarePluginBase as ComponentContextAwarePluginBase; use Drupal\Component\Plugin\Exception\ContextException; +use Drupal\Core\Cache\Cache; +use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\DependencyInjection\DependencySerializationTrait; use Drupal\Core\Plugin\Context\Context; use Drupal\Core\StringTranslation\StringTranslationTrait; @@ -91,4 +93,53 @@ protected function contextHandler() { return \Drupal::service('context.handler'); } + /** + * {@inheritdoc} + */ + public function getCacheContexts() { + $cache_contexts = []; + // Applied contexts can affect the cache contexts when this plugin is + // involved in caching, collect and return them. + foreach ($this->getContexts() as $context) { + /** @var $context \Drupal\Core\Cache\CacheableDependencyInterface */ + if ($context instanceof CacheableDependencyInterface) { + $cache_contexts = Cache::mergeContexts($cache_contexts, $context->getCacheContexts()); + } + } + return $cache_contexts; + } + + /** + * {@inheritdoc} + */ + public function getCacheTags() { + $tags = []; + // Applied contexts can affect the cache tags when this plugin is + // involved in caching, collect and return them. + foreach ($this->getContexts() as $context) { + /** @var $context \Drupal\Core\Cache\CacheableDependencyInterface */ + if ($context instanceof CacheableDependencyInterface) { + $tags = Cache::mergeTags($tags, $context->getCacheTags()); + } + } + return $tags; + } + + /** + * {@inheritdoc} + */ + public function getCacheMaxAge() { + $max_age = Cache::PERMANENT; + + // Applied contexts can affect the cache max age when this plugin is + // involved in caching, collect and return them. + foreach ($this->getContexts() as $context) { + /** @var $context \Drupal\Core\Cache\CacheableDependencyInterface */ + if ($context instanceof CacheableDependencyInterface) { + $max_age = Cache::mergeMaxAges($max_age, $context->getCacheMaxAge()); + } + } + return $max_age; + } + } diff --git a/core/modules/block/src/BlockAccessControlHandler.php b/core/modules/block/src/BlockAccessControlHandler.php index 148e3e3..473e58e 100644 --- a/core/modules/block/src/BlockAccessControlHandler.php +++ b/core/modules/block/src/BlockAccessControlHandler.php @@ -9,6 +9,8 @@ use Drupal\Component\Plugin\Exception\ContextException; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\Cache; +use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\Condition\ConditionAccessResolverTrait; use Drupal\Core\Entity\EntityAccessControlHandler; use Drupal\Core\Entity\EntityHandlerInterface; @@ -87,31 +89,60 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A else { $contexts = $entity->getContexts(); $conditions = []; + $missing_context = FALSE; foreach ($entity->getVisibilityConditions() as $condition_id => $condition) { if ($condition instanceof ContextAwarePluginInterface) { try { $this->contextHandler->applyContextMapping($condition, $contexts); } catch (ContextException $e) { - return AccessResult::forbidden()->setCacheMaxAge(0); + $missing_context = TRUE; } } $conditions[$condition_id] = $condition; } - if ($this->resolveConditions($conditions, 'and') !== FALSE) { + + if ($missing_context) { + // If any context is missing then we might be missing cacheable + // metadata, and don't know based on what conditions the block is + // accessible or not. For example, blocks that have a node type + // condition will have a missing context on any non-node route like the + // frontpage. + // @todo Avoid setting max-age 0 for some or all cases, for example by + // treating available contexts without value differently in + // https://www.drupal.org/node/2521956. + $access = AccessResult::forbidden()->setCacheMaxAge(0); + } + elseif ($this->resolveConditions($conditions, 'and') !== FALSE) { // Delegate to the plugin. $access = $entity->getPlugin()->access($account, TRUE); } else { $access = AccessResult::forbidden(); } - // This should not be hardcoded to an uncacheable access check result, but - // in order to fix that, we need condition plugins to return cache contexts, - // otherwise it will be impossible to determine by which cache contexts the - // result should be varied. - // @todo Change this to use $access->cacheUntilEntityChanges($entity) once - // https://www.drupal.org/node/2375695 is resolved. - return $access->setCacheMaxAge(0); + + $this->mergeCacheabilityFromConditions($access, $conditions); + + // Ensure that access is evaluated again when the block changes. + return $access->cacheUntilEntityChanges($entity); + } + } + + /** + * Merges cacheable metadata from conditions onto the access result object. + * + * @param \Drupal\Core\Access\AccessResult $access + * The access result object. + * @param \Drupal\Core\Condition\ConditionInterface[] $conditions + * List of visibility conditions. + */ + protected function mergeCacheabilityFromConditions(AccessResult $access, array $conditions) { + foreach ($conditions as $condition) { + if ($condition instanceof CacheableDependencyInterface) { + $access->addCacheTags($condition->getCacheTags()); + $access->addCacheContexts($condition->getCacheContexts()); + $access->setCacheMaxAge(Cache::mergeMaxAges($access->getCacheMaxAge(), $condition->getCacheMaxAge())); + } } } diff --git a/core/modules/block/src/BlockRepository.php b/core/modules/block/src/BlockRepository.php index 980e0ad..bc5eb8c 100644 --- a/core/modules/block/src/BlockRepository.php +++ b/core/modules/block/src/BlockRepository.php @@ -7,6 +7,7 @@ namespace Drupal\block; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Plugin\Context\ContextHandlerInterface; use Drupal\Core\Theme\ThemeManagerInterface; @@ -49,7 +50,7 @@ public function __construct(EntityManagerInterface $entity_manager, ThemeManager /** * {@inheritdoc} */ - public function getVisibleBlocksPerRegion(array $contexts) { + public function getVisibleBlocksPerRegion(array $contexts, array &$cacheable_metadata = []) { $active_theme = $this->themeManager->getActiveTheme(); // Build an array of the region names in the right order. $empty = array_fill_keys($active_theme->getRegions(), array()); @@ -57,9 +58,19 @@ public function getVisibleBlocksPerRegion(array $contexts) { $full = array(); foreach ($this->blockStorage->loadByProperties(array('theme' => $active_theme->getName())) as $block_id => $block) { /** @var \Drupal\block\BlockInterface $block */ + $block->setContexts($contexts); + $access = $block->access('view', NULL, TRUE); + $region = $block->getRegion(); + if (!isset($cacheable_metadata[$region])) { + $cacheable_metadata[$region] = CacheableMetadata::createFromObject($access); + } + else { + $cacheable_metadata[$region] = $cacheable_metadata[$region]->merge(CacheableMetadata::createFromObject($access)); + } + // Set the contexts on the block before checking access. - if ($block->setContexts($contexts)->access('view')) { - $full[$block->getRegion()][$block_id] = $block; + if ($access->isAllowed()) { + $full[$region][$block_id] = $block; } } diff --git a/core/modules/block/src/BlockRepositoryInterface.php b/core/modules/block/src/BlockRepositoryInterface.php index 082456f..00eb5e4 100644 --- a/core/modules/block/src/BlockRepositoryInterface.php +++ b/core/modules/block/src/BlockRepositoryInterface.php @@ -14,11 +14,14 @@ * * @param \Drupal\Component\Plugin\Context\ContextInterface[] $contexts * An array of contexts to set on the blocks. + * @param \Drupal\Core\Cache\CacheableMetadata[] $cacheable_metadata + * (optional) List of CacheableMetadata objects, keyed by region. This is + * by reference and is used to pass this information back to the caller. * * @return array * The array is first keyed by region machine name, with the values * containing an array keyed by block ID, with block entities as the values. */ - public function getVisibleBlocksPerRegion(array $contexts); + public function getVisibleBlocksPerRegion(array $contexts, array &$cacheable_metadata = []); } diff --git a/core/modules/block/src/EventSubscriber/CurrentLanguageContext.php b/core/modules/block/src/EventSubscriber/CurrentLanguageContext.php index 023f7cc..7b1a206 100644 --- a/core/modules/block/src/EventSubscriber/CurrentLanguageContext.php +++ b/core/modules/block/src/EventSubscriber/CurrentLanguageContext.php @@ -8,6 +8,7 @@ namespace Drupal\block\EventSubscriber; use Drupal\block\Event\BlockContextEvent; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Language\LanguageManagerInterface; use Drupal\Core\Plugin\Context\Context; use Drupal\Core\Plugin\Context\ContextDefinition; @@ -48,6 +49,11 @@ public function onBlockActiveContext(BlockContextEvent $event) { if (isset($info[$type_key]['name'])) { $context = new Context(new ContextDefinition('language', $info[$type_key]['name'])); $context->setContextValue($this->languageManager->getCurrentLanguage($type_key)); + + $cacheability = new CacheableMetadata(); + $cacheability->setCacheContexts(['languages:' . $type_key]); + $context->addCacheableDependency($cacheability); + $event->setContext('language.' . $type_key, $context); } } diff --git a/core/modules/block/src/EventSubscriber/CurrentUserContext.php b/core/modules/block/src/EventSubscriber/CurrentUserContext.php index dcf95c9..194a252 100644 --- a/core/modules/block/src/EventSubscriber/CurrentUserContext.php +++ b/core/modules/block/src/EventSubscriber/CurrentUserContext.php @@ -8,6 +8,7 @@ namespace Drupal\block\EventSubscriber; use Drupal\block\Event\BlockContextEvent; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Plugin\Context\Context; use Drupal\Core\Plugin\Context\ContextDefinition; @@ -56,6 +57,9 @@ public function onBlockActiveContext(BlockContextEvent $event) { $context = new Context(new ContextDefinition('entity:user', $this->t('Current user'))); $context->setContextValue($current_user); + $cacheability = new CacheableMetadata(); + $cacheability->setCacheContexts(['user']); + $context->addCacheableDependency($cacheability); $event->setContext('user.current_user', $context); } diff --git a/core/modules/block/src/EventSubscriber/NodeRouteContext.php b/core/modules/block/src/EventSubscriber/NodeRouteContext.php index 66458c0..89d24f5 100644 --- a/core/modules/block/src/EventSubscriber/NodeRouteContext.php +++ b/core/modules/block/src/EventSubscriber/NodeRouteContext.php @@ -8,6 +8,7 @@ namespace Drupal\block\EventSubscriber; use Drupal\block\Event\BlockContextEvent; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Plugin\Context\Context; use Drupal\Core\Plugin\Context\ContextDefinition; use Drupal\Core\Routing\RouteMatchInterface; @@ -39,8 +40,8 @@ public function __construct(RouteMatchInterface $route_match) { * {@inheritdoc} */ public function onBlockActiveContext(BlockContextEvent $event) { + $context = new Context(new ContextDefinition('entity:node', NULL, FALSE)); if (($route_object = $this->routeMatch->getRouteObject()) && ($route_contexts = $route_object->getOption('parameters')) && isset($route_contexts['node'])) { - $context = new Context(new ContextDefinition($route_contexts['node']['type'])); if ($node = $this->routeMatch->getParameter('node')) { $context->setContextValue($node); } @@ -48,10 +49,12 @@ public function onBlockActiveContext(BlockContextEvent $event) { } elseif ($this->routeMatch->getRouteName() == 'node.add') { $node_type = $this->routeMatch->getParameter('node_type'); - $context = new Context(new ContextDefinition('entity:node')); $context->setContextValue(Node::create(array('type' => $node_type->id()))); - $event->setContext('node.node', $context); } + $cacheability = new CacheableMetadata(); + $cacheability->setCacheContexts(['route']); + $context->addCacheableDependency($cacheability); + $event->setContext('node.node', $context); } /** diff --git a/core/modules/block/src/Plugin/DisplayVariant/BlockPageVariant.php b/core/modules/block/src/Plugin/DisplayVariant/BlockPageVariant.php index 7b67093..f5caee4 100644 --- a/core/modules/block/src/Plugin/DisplayVariant/BlockPageVariant.php +++ b/core/modules/block/src/Plugin/DisplayVariant/BlockPageVariant.php @@ -12,6 +12,7 @@ use Drupal\block\Event\BlockEvents; use Drupal\Core\Block\MainContentBlockPluginInterface; use Drupal\Core\Block\MessagesBlockPluginInterface; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Display\PageVariantInterface; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Entity\EntityViewBuilderInterface; @@ -130,7 +131,8 @@ public function build() { ]; $contexts = $this->getActiveBlockContexts(); // Load all region content assigned via blocks. - foreach ($this->blockRepository->getVisibleBlocksPerRegion($contexts) as $region => $blocks) { + $cacheable_metadata_list = []; + foreach ($this->blockRepository->getVisibleBlocksPerRegion($contexts, $cacheable_metadata_list) as $region => $blocks) { /** @var $blocks \Drupal\block\BlockInterface[] */ foreach ($blocks as $key => $block) { $block_plugin = $block->getPlugin(); @@ -172,6 +174,17 @@ public function build() { ]; } + // The access results' cacheability is currently added to the top level of the + // render array. This is done to prevent issues with empty regions being + // displayed. + // This would need to be changed to allow caching of block regions, as each + // region must then have the relevant cacheable metadata. + $merged_cacheable_metadata = CacheableMetadata::createFromRenderArray($build); + foreach ($cacheable_metadata_list as $cacheable_metadata) { + $merged_cacheable_metadata = $merged_cacheable_metadata->merge($cacheable_metadata); + } + $merged_cacheable_metadata->applyTo($build); + return $build; } diff --git a/core/modules/block/src/Tests/BlockLanguageTest.php b/core/modules/block/src/Tests/BlockLanguageTest.php index db908b0..56ee191 100644 --- a/core/modules/block/src/Tests/BlockLanguageTest.php +++ b/core/modules/block/src/Tests/BlockLanguageTest.php @@ -90,6 +90,7 @@ public function testLanguageBlockVisibilityLanguageDelete() { 'langcodes' => array( 'fr' => 'fr', ), + 'context_mapping' => ['language' => 'language.language_interface'], ), ), ); diff --git a/core/modules/block/tests/src/Unit/BlockRepositoryTest.php b/core/modules/block/tests/src/Unit/BlockRepositoryTest.php index 56b90ca..d77d64e 100644 --- a/core/modules/block/tests/src/Unit/BlockRepositoryTest.php +++ b/core/modules/block/tests/src/Unit/BlockRepositoryTest.php @@ -8,6 +8,7 @@ namespace Drupal\Tests\block\Unit; use Drupal\block\BlockRepository; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Block\BlockPluginInterface; use Drupal\Core\Plugin\ContextAwarePluginInterface; use Drupal\Tests\UnitTestCase; @@ -113,18 +114,18 @@ public function testGetVisibleBlocksPerRegion(array $blocks_config, array $expec public function providerBlocksConfig() { $blocks_config = array( 'block1' => array( - TRUE, 'top', 0 + AccessResult::allowed(), 'top', 0 ), // Test a block without access. 'block2' => array( - FALSE, 'bottom', 0 + AccessResult::forbidden(), 'bottom', 0 ), // Test two blocks in the same region with specific weight. 'block3' => array( - TRUE, 'bottom', 5 + AccessResult::allowed(), 'bottom', 5 ), 'block4' => array( - TRUE, 'bottom', -5 + AccessResult::allowed(), 'bottom', -5 ), ); @@ -151,7 +152,7 @@ public function testGetVisibleBlocksPerRegionWithContext() { ->willReturnSelf(); $block->expects($this->once()) ->method('access') - ->willReturn(TRUE); + ->willReturn(AccessResult::allowed()->addCacheTags(['config:block.block.block_id'])); $block->expects($this->once()) ->method('getRegion') ->willReturn('top'); @@ -163,7 +164,8 @@ public function testGetVisibleBlocksPerRegionWithContext() { ->with(['theme' => $this->theme]) ->willReturn($blocks); $result = []; - foreach ($this->blockRepository->getVisibleBlocksPerRegion($contexts) as $region => $resulting_blocks) { + $cacheable_metadata = []; + foreach ($this->blockRepository->getVisibleBlocksPerRegion($contexts, $cacheable_metadata) as $region => $resulting_blocks) { $result[$region] = []; foreach ($resulting_blocks as $plugin_id => $block) { $result[$region][] = $plugin_id; @@ -177,6 +179,10 @@ public function testGetVisibleBlocksPerRegionWithContext() { 'bottom' => [], ]; $this->assertSame($expected, $result); + + // Assert that the cacheable metadata from the block access results was + // collected. + $this->assertEquals(['config:block.block.block_id'], $cacheable_metadata['top']->getCacheTags()); } } diff --git a/core/modules/block/tests/src/Unit/Plugin/DisplayVariant/BlockPageVariantTest.php b/core/modules/block/tests/src/Unit/Plugin/DisplayVariant/BlockPageVariantTest.php index bb5390c..5438a99 100644 --- a/core/modules/block/tests/src/Unit/Plugin/DisplayVariant/BlockPageVariantTest.php +++ b/core/modules/block/tests/src/Unit/Plugin/DisplayVariant/BlockPageVariantTest.php @@ -7,6 +7,8 @@ namespace Drupal\Tests\block\Unit\Plugin\DisplayVariant; +use Drupal\Core\Cache\CacheableMetadata; +use Drupal\Core\DependencyInjection\Container; use Drupal\Tests\UnitTestCase; /** @@ -55,6 +57,18 @@ class BlockPageVariantTest extends UnitTestCase { * A mocked display variant plugin. */ public function setUpDisplayVariant($configuration = array(), $definition = array()) { + + $container = new Container(); + $cache_context_manager = $this->getMockBuilder('Drupal\Core\Cache\CacheContextsManager') + ->disableOriginalConstructor() + ->getMock(); + $container->set('cache_contexts_manager', $cache_context_manager); + $cache_context_manager->expects($this->any()) + ->method('validateTokens') + ->with([]) + ->willReturn([]); + \Drupal::setContainer($container); + $this->blockRepository = $this->getMock('Drupal\block\BlockRepositoryInterface'); $this->blockViewBuilder = $this->getMock('Drupal\Core\Entity\EntityViewBuilderInterface'); $this->dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); @@ -96,7 +110,10 @@ public function providerBuild() { '#cache' => [ 'tags' => [ 'config:block_list', + 'route', ], + 'contexts' => [], + 'max-age' => -1, ], 'top' => [ 'block1' => [], @@ -121,7 +138,10 @@ public function providerBuild() { '#cache' => [ 'tags' => [ 'config:block_list', + 'route', ], + 'contexts' => [], + 'max-age' => -1, ], 'top' => [ 'block1' => [], @@ -152,7 +172,10 @@ public function providerBuild() { '#cache' => [ 'tags' => [ 'config:block_list', + 'route', ], + 'contexts' => [], + 'max-age' => -1, ], 'top' => [ 'block1' => [], @@ -205,7 +228,10 @@ public function testBuild(array $blocks_config, $visible_block_count, array $exp ->will($this->returnValue(array())); $this->blockRepository->expects($this->once()) ->method('getVisibleBlocksPerRegion') - ->will($this->returnValue($blocks)); + ->willReturnCallback(function ($contexts, &$cacheable_metadata) use ($blocks) { + $cacheable_metadata['top'] = (new CacheableMetadata())->addCacheTags(['route']); + return $blocks; + }); $this->assertSame($expected_render_array, $display_variant->build()); } @@ -226,6 +252,8 @@ public function testBuildWithoutMainContent() { 'tags' => [ 'config:block_list', ], + 'contexts' => [], + 'max-age' => -1, ], 'content' => [ 'system_main' => [], diff --git a/core/modules/node/src/Tests/NodeBlockFunctionalTest.php b/core/modules/node/src/Tests/NodeBlockFunctionalTest.php index 32bb692..13a835a 100644 --- a/core/modules/node/src/Tests/NodeBlockFunctionalTest.php +++ b/core/modules/node/src/Tests/NodeBlockFunctionalTest.php @@ -8,6 +8,7 @@ namespace Drupal\node\Tests; use Drupal\block\Entity\Block; +use Drupal\system\Tests\Cache\AssertPageCacheContextsAndTagsTrait; use Drupal\user\RoleInterface; /** @@ -17,6 +18,8 @@ */ class NodeBlockFunctionalTest extends NodeTestBase { + use AssertPageCacheContextsAndTagsTrait; + /** * An administrative user for testing. * @@ -122,6 +125,8 @@ public function testRecentNodeBlock() { $this->assertText($node3->label(), 'Node found in block.'); $this->assertText($node4->label(), 'Node found in block.'); + $this->assertCacheContexts(['languages:language_content', 'languages:language_interface', 'theme', 'user']); + // Enable the "Powered by Drupal" block only on article nodes. $edit = [ 'id' => strtolower($this->randomMachineName()), @@ -145,12 +150,16 @@ public function testRecentNodeBlock() { $this->drupalGet(''); $label = $block->label(); $this->assertNoText($label, 'Block was not displayed on the front page.'); + $this->assertCacheContexts(['languages:language_content', 'languages:language_interface', 'theme', 'user', 'route']); $this->drupalGet('node/add/article'); $this->assertText($label, 'Block was displayed on the node/add/article page.'); + $this->assertCacheContexts(['languages:language_content', 'languages:language_interface', 'theme', 'user', 'route']); $this->drupalGet('node/' . $node1->id()); $this->assertText($label, 'Block was displayed on the node/N when node is of type article.'); + $this->assertCacheContexts(['languages:language_content', 'languages:language_interface', 'theme', 'user', 'route', 'timezone']); $this->drupalGet('node/' . $node5->id()); $this->assertNoText($label, 'Block was not displayed on nodes of type page.'); + $this->assertCacheContexts(['languages:language_content', 'languages:language_interface', 'theme', 'user', 'route', 'timezone']); $this->drupalLogin($this->adminUser); $this->drupalGet('admin/structure/block'); diff --git a/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php b/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php index d8dec6a..ebf0867 100644 --- a/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php +++ b/core/modules/page_cache/src/Tests/PageCacheTagsIntegrationTest.php @@ -75,12 +75,20 @@ function testPageCacheTags() { 'route.menu_active_trails:footer', 'route.menu_active_trails:main', 'route.menu_active_trails:tools', + // The user login block access is not visible on certain routes. + 'route.name', 'theme', 'timezone', 'user.permissions', + // The user login block access depends on whether the current user is + // logged in or not. + 'user.roles:anonymous', // The cache contexts associated with the (in)accessible menu links are // bubbled. 'user.roles:authenticated', + // The placed block is only visible on certain URLs through a visibility + // condition. + 'url', ]; // Full node page 1. @@ -93,6 +101,9 @@ function testPageCacheTags() { 'config:block.block.bartik_tools', 'config:block.block.bartik_login', 'config:block.block.bartik_footer', + 'config:block.block.bartik_help', + 'config:block.block.bartik_search', + 'config:block.block.' . $block->id(), 'config:block.block.bartik_powered', 'config:block.block.bartik_main_menu', 'config:block.block.bartik_account_menu', @@ -123,6 +134,8 @@ function testPageCacheTags() { 'config:block.block.bartik_content', 'config:block.block.bartik_tools', 'config:block.block.bartik_login', + 'config:block.block.bartik_help', + 'config:block.block.bartik_search', 'config:block.block.' . $block->id(), 'config:block.block.bartik_footer', 'config:block.block.bartik_powered', diff --git a/core/modules/system/src/Plugin/Condition/CurrentThemeCondition.php b/core/modules/system/src/Plugin/Condition/CurrentThemeCondition.php index 50eb373..3b82d48 100644 --- a/core/modules/system/src/Plugin/Condition/CurrentThemeCondition.php +++ b/core/modules/system/src/Plugin/Condition/CurrentThemeCondition.php @@ -123,4 +123,13 @@ public function summary() { return $this->t('The current theme is @theme', array('@theme' => $this->configuration['theme'])); } + /** + * {@inheritdoc} + */ + public function getCacheContexts() { + $contexts = parent::getCacheContexts(); + $contexts[] = 'theme'; + return $contexts; + } + } diff --git a/core/modules/system/src/Plugin/Condition/RequestPath.php b/core/modules/system/src/Plugin/Condition/RequestPath.php index 876656c..135e05b 100644 --- a/core/modules/system/src/Plugin/Condition/RequestPath.php +++ b/core/modules/system/src/Plugin/Condition/RequestPath.php @@ -159,4 +159,15 @@ public function evaluate() { return $this->pathMatcher->matchPath($path_alias, $pages) || (($path != $path_alias) && $this->pathMatcher->matchPath($path, $pages)); } + /** + * {@inheritdoc} + */ + public function getCacheContexts() { + $contexts = parent::getCacheContexts(); + // @todo Add a url.path cache context in + // https://www.drupal.org/node/2521978. + $contexts[] = 'url'; + return $contexts; + } + } diff --git a/core/modules/system/src/Tests/Cache/AssertPageCacheContextsAndTagsTrait.php b/core/modules/system/src/Tests/Cache/AssertPageCacheContextsAndTagsTrait.php index 44aeda6..f8ce454 100644 --- a/core/modules/system/src/Tests/Cache/AssertPageCacheContextsAndTagsTrait.php +++ b/core/modules/system/src/Tests/Cache/AssertPageCacheContextsAndTagsTrait.php @@ -89,8 +89,8 @@ protected function assertPageCacheContextsAndTags(Url $url, array $expected_cont */ protected function debugCacheTags(array $actual_tags, array $expected_tags) { if ($actual_tags !== $expected_tags) { - debug('Missing cache tags: ' . implode(',', array_diff($expected_tags, $actual_tags))); - debug('Unwanted cache tags: ' . implode(',', array_diff($actual_tags, $expected_tags))); + debug('Unwanted cache tags in response: ' . implode(',', array_diff($actual_tags, $expected_tags))); + debug('Missing cache tags in response: ' . implode(',', array_diff($expected_tags, $actual_tags))); } } @@ -125,8 +125,8 @@ protected function assertCacheContexts(array $expected_contexts, $message = NULL sort($actual_contexts); $return = $this->assertIdentical($actual_contexts, $expected_contexts, $message); if (!$return) { - debug('Missing cache contexts: ' . implode(',', array_diff($actual_contexts, $expected_contexts))); - debug('Unwanted cache contexts: ' . implode(',', array_diff($expected_contexts, $actual_contexts))); + debug('Unwanted cache contexts in response: ' . implode(',', array_diff($actual_contexts, $expected_contexts))); + debug('Missing cache contexts in response: ' . implode(',', array_diff($expected_contexts, $actual_contexts))); } return $return; } diff --git a/core/modules/user/src/Plugin/Block/UserLoginBlock.php b/core/modules/user/src/Plugin/Block/UserLoginBlock.php index 564870e..e694577 100644 --- a/core/modules/user/src/Plugin/Block/UserLoginBlock.php +++ b/core/modules/user/src/Plugin/Block/UserLoginBlock.php @@ -79,7 +79,7 @@ protected function blockAccess(AccountInterface $account) { $route_name = $this->routeMatch->getRouteName(); if ($account->isAnonymous() && !in_array($route_name, array('user.register', 'user.login', 'user.logout'))) { return AccessResult::allowed() - ->addCacheContexts(['route', 'user.roles:anonymous']); + ->addCacheContexts(['route.name', 'user.roles:anonymous']); } return AccessResult::forbidden(); } diff --git a/core/modules/user/src/Plugin/Condition/UserRole.php b/core/modules/user/src/Plugin/Condition/UserRole.php index b64a28f..3316cbc 100644 --- a/core/modules/user/src/Plugin/Condition/UserRole.php +++ b/core/modules/user/src/Plugin/Condition/UserRole.php @@ -87,4 +87,17 @@ public function evaluate() { return (bool) array_intersect($this->configuration['roles'], $user->getRoles()); } + /** + * {@inheritdoc} + */ + public function getCacheContexts() { + // Optimize cache context, if a user cache context is provided, only use + // user.roles, since that's the only part this condition cares about. + $contexts = []; + foreach (parent::getCacheContexts() as $context) { + $contexts[] = $context == 'user' ? 'user.roles' : $context; + } + return $contexts; + } + } diff --git a/core/tests/Drupal/Tests/Core/Plugin/Context/ContextTest.php b/core/tests/Drupal/Tests/Core/Plugin/Context/ContextTest.php index 97702c7..b59ed8a 100644 --- a/core/tests/Drupal/Tests/Core/Plugin/Context/ContextTest.php +++ b/core/tests/Drupal/Tests/Core/Plugin/Context/ContextTest.php @@ -7,8 +7,11 @@ namespace Drupal\Tests\Core\Plugin\Context; +use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\Plugin\Context\Context; +use Drupal\Core\TypedData\TypedDataInterface; use Drupal\Tests\UnitTestCase; +use Symfony\Component\DependencyInjection\Container; /** * @coversDefaultClass \Drupal\Core\Plugin\Context\Context @@ -43,6 +46,91 @@ class ContextTest extends UnitTestCase { public function setUp() { parent::setUp(); + $this->typedDataManager = $this->getMockBuilder('Drupal\Core\TypedData\TypedDataManager') + ->disableOriginalConstructor() + ->setMethods(array('create')) + ->getMock(); + } + + /** + * @covers ::getContextValue + */ + public function testDefaultValue() { + $this->setUpDefaultValue(); + + $context = new Context($this->contextDefinition); + $context->setTypedDataManager($this->typedDataManager); + $this->assertEquals('test', $context->getContextValue()); + } + + /** + * @covers ::getContextData + */ + public function testDefaultDataValue() { + $this->setUpDefaultValue(); + + $context = new Context($this->contextDefinition); + $context->setTypedDataManager($this->typedDataManager); + $this->assertEquals($this->typedData, $context->getContextData()); + } + + /** + * @covers ::setContextValue + */ + public function testSetContextValueTypedData() { + + $this->contextDefinition = $this->getMockBuilder('Drupal\Core\Plugin\Context\ContextDefinitionInterface') + ->setMethods(array('getDefaultValue', 'getDataDefinition')) + ->getMockForAbstractClass(); + + $context = new Context($this->contextDefinition); + $context->setTypedDataManager($this->typedDataManager); + $typed_data = $this->getMock('Drupal\Core\TypedData\TypedDataInterface'); + $context->setContextValue($typed_data); + $this->assertSame($typed_data, $context->getContextData()); + } + + /** + * @covers ::setContextValue + */ + public function testSetContextValueCacheableDependency() { + $container = new Container(); + $cache_context_manager = $this->getMockBuilder('Drupal\Core\Cache\CacheContextsManager') + ->disableOriginalConstructor() + ->getMock(); + $container->set('cache_contexts_manager', $cache_context_manager); + $cache_context_manager->expects($this->any()) + ->method('validateTokens') + ->with(['route']) + ->willReturn(['route']); + \Drupal::setContainer($container); + + $this->contextDefinition = $this->getMock('Drupal\Core\Plugin\Context\ContextDefinitionInterface'); + + $context = new Context($this->contextDefinition); + $context->setTypedDataManager($this->typedDataManager); + $cacheable_dependency = $this->getMock('Drupal\Tests\Core\Plugin\Context\TypedDataCacheableDependencyInterface'); + $cacheable_dependency->expects($this->once()) + ->method('getCacheTags') + ->willReturn(['node:1']); + $cacheable_dependency->expects($this->once()) + ->method('getCacheContexts') + ->willReturn(['route']); + $cacheable_dependency->expects($this->once()) + ->method('getCacheMaxAge') + ->willReturn(60); + + $context->setContextValue($cacheable_dependency); + $this->assertSame($cacheable_dependency, $context->getContextData()); + $this->assertEquals(['node:1'], $context->getCacheTags()); + $this->assertEquals(['route'], $context->getCacheContexts()); + $this->assertEquals(60, $context->getCacheMaxAge()); + } + + /** + * Set up mocks for the getDefaultValue() method call. + */ + protected function setUpDefaultValue() { $mock_data_definition = $this->getMock('Drupal\Core\TypedData\DataDefinitionInterface'); $this->contextDefinition = $this->getMockBuilder('Drupal\Core\Plugin\Context\ContextDefinitionInterface') @@ -59,33 +147,14 @@ public function setUp() { $this->typedData = $this->getMock('Drupal\Core\TypedData\TypedDataInterface'); - $this->typedDataManager = $this->getMockBuilder('Drupal\Core\TypedData\TypedDataManager') - ->disableOriginalConstructor() - ->setMethods(array('create')) - ->getMock(); - $this->typedDataManager->expects($this->once()) ->method('create') ->with($mock_data_definition, 'test') ->willReturn($this->typedData); } - - /** - * @covers ::getContextValue - */ - public function testDefaultValue() { - $context = new Context($this->contextDefinition); - $context->setTypedDataManager($this->typedDataManager); - $this->assertEquals('test', $context->getContextValue()); - } - - /** - * @covers ::getContextData - */ - public function testDefaultDataValue() { - $context = new Context($this->contextDefinition); - $context->setTypedDataManager($this->typedDataManager); - $this->assertEquals($this->typedData, $context->getContextData()); - } - } + +/** + * Test interface used for mocking. + */ +interface TypedDataCacheableDependencyInterface extends CacheableDependencyInterface, TypedDataInterface { } diff --git a/core/tests/Drupal/Tests/Core/Plugin/ContextHandlerTest.php b/core/tests/Drupal/Tests/Core/Plugin/ContextHandlerTest.php index 3dcf628..8900fdd 100644 --- a/core/tests/Drupal/Tests/Core/Plugin/ContextHandlerTest.php +++ b/core/tests/Drupal/Tests/Core/Plugin/ContextHandlerTest.php @@ -258,6 +258,16 @@ public function testApplyContextMapping() { ->method('setContextValue') ->with('hit', array('foo')); + // Make sure that the cacheability metadata is passed to the plugin context. + $plugin_context = $this->getMock('Drupal\Core\Plugin\Context\ContextInterface'); + $plugin_context->expects($this->once()) + ->method('addCacheableDependency') + ->with($context_hit); + $plugin->expects($this->once()) + ->method('getContext') + ->with('hit') + ->willReturn($plugin_context); + $this->contextHandler->applyContextMapping($plugin, $contexts); } @@ -291,6 +301,10 @@ public function testApplyContextMappingMissingRequired() { $plugin->expects($this->never()) ->method('setContextValue'); + // No context, so no cacheability metadata can be passed along. + $plugin->expects($this->never()) + ->method('getContext'); + $this->contextHandler->applyContextMapping($plugin, $contexts); } @@ -321,6 +335,10 @@ public function testApplyContextMappingMissingNotRequired() { $plugin->expects($this->never()) ->method('setContextValue'); + // No context, so no cacheability metadata can be passed along. + $plugin->expects($this->never()) + ->method('getContext'); + $this->contextHandler->applyContextMapping($plugin, $contexts); } @@ -423,6 +441,16 @@ public function testApplyContextMappingConfigurableAssigned() { ->method('setContextValue') ->with('hit', array('foo')); + // Make sure that the cacheability metadata is passed to the plugin context. + $plugin_context = $this->getMock('Drupal\Core\Plugin\Context\ContextInterface'); + $plugin_context->expects($this->once()) + ->method('addCacheableDependency') + ->with($context); + $plugin->expects($this->once()) + ->method('getContext') + ->with('hit') + ->willReturn($plugin_context); + $this->contextHandler->applyContextMapping($plugin, $contexts, ['hit' => 'name']); }