diff --git a/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php b/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php index 4ebb915348..4154880f84 100644 --- a/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php +++ b/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php @@ -3,6 +3,7 @@ namespace Drupal\layout_builder\Plugin\SectionStorage; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Entity\EntityFieldManagerInterface; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Entity\EntityTypeManagerInterface; @@ -345,9 +346,11 @@ public function access($operation, AccountInterface $account = NULL, $return_as_ /** * {@inheritdoc} */ - public function isApplicable() { + public function isApplicable(CacheableMetadata $cacheability) { + $default_section_storage = $this->getDefaultSectionStorage(); + $cacheability->addCacheableDependency($default_section_storage)->addCacheableDependency($this); // Check that overrides are enabled and have at least one section. - return $this->getDefaultSectionStorage()->isOverridable() && count($this); + return $default_section_storage->isOverridable() && count($this); } } diff --git a/core/modules/layout_builder/src/Plugin/SectionStorage/SectionStorageBase.php b/core/modules/layout_builder/src/Plugin/SectionStorage/SectionStorageBase.php index 364afc6067..14b6b0bbb8 100644 --- a/core/modules/layout_builder/src/Plugin/SectionStorage/SectionStorageBase.php +++ b/core/modules/layout_builder/src/Plugin/SectionStorage/SectionStorageBase.php @@ -2,6 +2,7 @@ namespace Drupal\layout_builder\Plugin\SectionStorage; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Plugin\ContextAwarePluginBase; use Drupal\layout_builder\Routing\LayoutBuilderRoutesTrait; use Drupal\layout_builder\Section; @@ -111,7 +112,7 @@ public function getContextsDuringPreview() { /** * {@inheritdoc} */ - public function isApplicable() { + public function isApplicable(CacheableMetadata $cacheability) { return TRUE; } diff --git a/core/modules/layout_builder/src/SectionStorage/SectionStorageManager.php b/core/modules/layout_builder/src/SectionStorage/SectionStorageManager.php index 50fa28f40c..e8c2e6eb84 100644 --- a/core/modules/layout_builder/src/SectionStorage/SectionStorageManager.php +++ b/core/modules/layout_builder/src/SectionStorage/SectionStorageManager.php @@ -93,16 +93,14 @@ public function load($type, array $contexts = []) { /** * {@inheritdoc} */ - public function findByContext(array $contexts, CacheableMetadata &$cacheability = NULL) { + public function findByContext(array $contexts, CacheableMetadata $cacheability) { $storage_types = array_keys($this->contextHandler->filterPluginDefinitionsByContexts($contexts, $this->getDefinitions())); foreach ($storage_types as $type) { $plugin = $this->load($type, $contexts); // Each plugin used in this loop must be added as a cacheable dependency. - if ($cacheability) { - $cacheability->addCacheableDependency($plugin); - } - if ($plugin && $plugin->isApplicable()) { + $cacheability->addCacheableDependency($plugin); + if ($plugin && $plugin->isApplicable($cacheability)) { return $plugin; } } diff --git a/core/modules/layout_builder/src/SectionStorage/SectionStorageManagerInterface.php b/core/modules/layout_builder/src/SectionStorage/SectionStorageManagerInterface.php index 60f08f3469..7403008634 100644 --- a/core/modules/layout_builder/src/SectionStorage/SectionStorageManagerInterface.php +++ b/core/modules/layout_builder/src/SectionStorage/SectionStorageManagerInterface.php @@ -33,14 +33,14 @@ public function load($type, array $contexts = []); * * @param \Drupal\Component\Plugin\Context\ContextInterface[] $contexts * The contexts which should be used to determine which storage to return. - * @param \Drupal\Core\Cache\CacheableMetadata|null $cacheability - * (optional) Cacheability metadata object, which will be populated based on - * the cacheability of each section storage candidate. + * @param \Drupal\Core\Cache\CacheableMetadata $cacheability + * Cacheability metadata object, which will be populated based on the + * cacheability of each section storage candidate. * * @return \Drupal\layout_builder\SectionStorageInterface|null * The section storage if one matched all contexts, or NULL otherwise. */ - public function findByContext(array $contexts, CacheableMetadata &$cacheability = NULL); + public function findByContext(array $contexts, CacheableMetadata $cacheability); /** * Loads a section storage with no associated section list. diff --git a/core/modules/layout_builder/src/SectionStorageInterface.php b/core/modules/layout_builder/src/SectionStorageInterface.php index 1fcdde4f1a..d044f34b3f 100644 --- a/core/modules/layout_builder/src/SectionStorageInterface.php +++ b/core/modules/layout_builder/src/SectionStorageInterface.php @@ -4,6 +4,7 @@ use Drupal\Component\Plugin\PluginInspectionInterface; use Drupal\Core\Access\AccessibleInterface; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Plugin\ContextAwarePluginInterface; use Symfony\Component\Routing\RouteCollection; @@ -167,9 +168,13 @@ public function save(); /** * Returns if the section storage should be selected during plugin mapping. * + * @param \Drupal\Core\Cache\CacheableMetadata $cacheability + * Cacheability metadata object, which must be populated with any + * information used in this method. + * * @return bool * TRUE if the section storage can be used, FALSE otherwise. */ - public function isApplicable(); + public function isApplicable(CacheableMetadata $cacheability); } diff --git a/core/modules/layout_builder/tests/modules/layout_builder_overrides_test/src/Plugin/SectionStorage/TestOverridesSectionStorage.php b/core/modules/layout_builder/tests/modules/layout_builder_overrides_test/src/Plugin/SectionStorage/TestOverridesSectionStorage.php index 7a40eb33c4..7a5e8d3f02 100644 --- a/core/modules/layout_builder/tests/modules/layout_builder_overrides_test/src/Plugin/SectionStorage/TestOverridesSectionStorage.php +++ b/core/modules/layout_builder/tests/modules/layout_builder_overrides_test/src/Plugin/SectionStorage/TestOverridesSectionStorage.php @@ -2,6 +2,7 @@ namespace Drupal\layout_builder_overrides_test\Plugin\SectionStorage; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Session\AccountInterface; use Drupal\layout_builder\Plugin\SectionStorage\SectionStorageBase; use Drupal\layout_builder\Section; @@ -35,18 +36,11 @@ public function getSections() { /** * {@inheritdoc} */ - public function isApplicable() { + public function isApplicable(CacheableMetadata $cacheability) { + $cacheability->setCacheMaxAge(0); return \Drupal::state()->get('layout_builder_overrides_test', FALSE); } - /** - * {@inheritdoc} - */ - public function getCacheMaxAge() { - // This cannot be cached as applicability is determined by state. - return 0; - } - /** * {@inheritdoc} */ diff --git a/core/modules/layout_builder/tests/modules/layout_builder_test/src/Plugin/SectionStorage/SimpleConfigSectionStorage.php b/core/modules/layout_builder/tests/modules/layout_builder_test/src/Plugin/SectionStorage/SimpleConfigSectionStorage.php index f1ad760dfa..cc18cea12f 100644 --- a/core/modules/layout_builder/tests/modules/layout_builder_test/src/Plugin/SectionStorage/SimpleConfigSectionStorage.php +++ b/core/modules/layout_builder/tests/modules/layout_builder_test/src/Plugin/SectionStorage/SimpleConfigSectionStorage.php @@ -3,6 +3,7 @@ namespace Drupal\layout_builder_test\Plugin\SectionStorage; use Drupal\Core\Access\AccessResult; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\Core\Plugin\Context\Context; @@ -216,7 +217,7 @@ public function extractIdFromRoute($value, $definition, $name, array $defaults) /** * {@inheritdoc} */ - public function isApplicable() { + public function isApplicable(CacheableMetadata $cacheability) { return TRUE; } diff --git a/core/modules/layout_builder/tests/src/Unit/SectionStorageManagerTest.php b/core/modules/layout_builder/tests/src/Unit/SectionStorageManagerTest.php index 7789399b18..31b64e04e7 100644 --- a/core/modules/layout_builder/tests/src/Unit/SectionStorageManagerTest.php +++ b/core/modules/layout_builder/tests/src/Unit/SectionStorageManagerTest.php @@ -209,6 +209,7 @@ public function testFindDefinitions() { * The result for the plugin's isApplicable() method to return. */ public function testFindByContext($plugin_is_applicable) { + $cacheability = new CacheableMetadata(); $contexts = [ 'foo' => new Context(new ContextDefinition('foo')), ]; @@ -220,10 +221,10 @@ public function testFindByContext($plugin_is_applicable) { $this->discovery->getDefinitions()->willReturn($definitions); $provider_access = $this->prophesize(SectionStorageInterface::class); - $provider_access->isApplicable()->willReturn($plugin_is_applicable); + $provider_access->isApplicable($cacheability)->willReturn($plugin_is_applicable); $no_access = $this->prophesize(SectionStorageInterface::class); - $no_access->isApplicable()->willReturn(FALSE); + $no_access->isApplicable($cacheability)->willReturn(FALSE); $missing_contexts = $this->prophesize(SectionStorageInterface::class); @@ -237,7 +238,7 @@ public function testFindByContext($plugin_is_applicable) { $this->factory->createInstance('missing_contexts', [])->willReturn($missing_contexts->reveal()); $this->factory->createInstance('provider_access', [])->willReturn($provider_access->reveal()); - $result = $this->manager->findByContext($contexts); + $result = $this->manager->findByContext($contexts, $cacheability); if ($plugin_is_applicable) { $this->assertSame($provider_access->reveal(), $result); } @@ -262,6 +263,7 @@ public function providerTestFindByContext() { * @covers ::findByContext */ public function testFindByContextCacheableSectionStorage() { + $cacheability = new CacheableMetadata(); $contexts = [ 'foo' => new Context(new ContextDefinition('foo')), ]; @@ -272,19 +274,21 @@ public function testFindByContextCacheableSectionStorage() { ]; $this->discovery->getDefinitions()->willReturn($definitions); + // Create a plugin that itself has cacheability info but is not applicable. $first_plugin = $this->prophesize(SectionStorageInterface::class); $first_plugin->willImplement(CacheableDependencyInterface::class); $first_plugin->getCacheContexts()->willReturn([]); $first_plugin->getCacheTags()->willReturn(['first_plugin']); $first_plugin->getCacheMaxAge()->willReturn(-1); - $first_plugin->isApplicable()->willReturn(FALSE); + $first_plugin->isApplicable($cacheability)->willReturn(FALSE); + // Create a plugin that adds cacheability info from within ::isApplicable() + // and is applicable. $second_plugin = $this->prophesize(SectionStorageInterface::class); - $second_plugin->willImplement(CacheableDependencyInterface::class); - $second_plugin->getCacheContexts()->willReturn([]); - $second_plugin->getCacheTags()->willReturn(['second_plugin']); - $second_plugin->getCacheMaxAge()->willReturn(-1); - $second_plugin->isApplicable()->willReturn(TRUE); + $second_plugin->isApplicable($cacheability)->will(function ($arguments) { + $arguments[0]->addCacheTags(['second_plugin']); + return TRUE; + }); $this->factory->createInstance('first', [])->willReturn($first_plugin->reveal()); $this->factory->createInstance('second', [])->willReturn($second_plugin->reveal()); @@ -294,7 +298,6 @@ public function testFindByContextCacheableSectionStorage() { $this->contextHandler->applyContextMapping($first_plugin, $contexts)->shouldBeCalled(); $this->contextHandler->applyContextMapping($second_plugin, $contexts)->shouldBeCalled(); - $cacheability = new CacheableMetadata(); $result = $this->manager->findByContext($contexts, $cacheability); $this->assertSame($second_plugin->reveal(), $result); $this->assertSame(['first_plugin', 'second_plugin'], $cacheability->getCacheTags());