diff --git a/core/modules/layout_builder/src/Access/LayoutBuilderAccessCheck.php b/core/modules/layout_builder/src/Access/LayoutBuilderAccessCheck.php index 0a70db0633..033e153011 100644 --- a/core/modules/layout_builder/src/Access/LayoutBuilderAccessCheck.php +++ b/core/modules/layout_builder/src/Access/LayoutBuilderAccessCheck.php @@ -20,17 +20,12 @@ class LayoutBuilderAccessCheck implements AccessInterface { * * @param \Drupal\layout_builder\SectionStorageInterface $section_storage * The section storage. - * @param \Drupal\Core\Session\AccountInterface $account - * The current user. - * @param \Symfony\Component\Routing\Route $route - * The route to check against. * * @return \Drupal\Core\Access\AccessResultInterface * The access result. */ - public function access(SectionStorageInterface $section_storage, AccountInterface $account, Route $route) { - $operation = $route->getRequirement('_layout_builder_access'); - $access = $section_storage->access($operation, $account, TRUE); + public function access(SectionStorageInterface $section_storage) { + $access = $section_storage->routingAccess(); if ($access instanceof RefinableCacheableDependencyInterface) { $access->addCacheableDependency($section_storage); } diff --git a/core/modules/layout_builder/src/Plugin/SectionStorage/DefaultsSectionStorage.php b/core/modules/layout_builder/src/Plugin/SectionStorage/DefaultsSectionStorage.php index 32ea0edb8a..d490bb1be4 100644 --- a/core/modules/layout_builder/src/Plugin/SectionStorage/DefaultsSectionStorage.php +++ b/core/modules/layout_builder/src/Plugin/SectionStorage/DefaultsSectionStorage.php @@ -446,9 +446,8 @@ public function getThirdPartyProviders() { /** * {@inheritdoc} */ - public function access($operation, AccountInterface $account = NULL, $return_as_object = FALSE) { - $result = AccessResult::allowedIf($this->isLayoutBuilderEnabled()); - return $return_as_object ? $result : $result->isAllowed(); + public function routingAccess() { + return AccessResult::allowedIf($this->isLayoutBuilderEnabled()); } } diff --git a/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php b/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php index 37a89ba1f7..88d15d4bb9 100644 --- a/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php +++ b/core/modules/layout_builder/src/Plugin/SectionStorage/OverridesSectionStorage.php @@ -329,18 +329,20 @@ public function save() { /** * {@inheritdoc} */ - public function access($operation, AccountInterface $account = NULL, $return_as_object = FALSE) { + public function routingAccess() { $default_section_storage = $this->getDefaultSectionStorage(); - $result = AccessResult::allowedIf($default_section_storage->isLayoutBuilderEnabled())->addCacheableDependency($default_section_storage); - return $return_as_object ? $result : $result->isAllowed(); + return AccessResult::allowedIf($default_section_storage->isLayoutBuilderEnabled())->addCacheableDependency($default_section_storage); } /** * {@inheritdoc} */ - public function isApplicable() { + public function renderAccess() { // Check that overrides are enabled and have at least one section. - return $this->getDefaultSectionStorage()->isOverridable() && count($this); + $default_section_storage = $this->getDefaultSectionStorage(); + return AccessResult::allowedIf($default_section_storage->isOverridable() && count($this)) + ->addCacheableDependency($default_section_storage) + ->addCacheableDependency($this->getEntity()); } } diff --git a/core/modules/layout_builder/src/Plugin/SectionStorage/SectionStorageBase.php b/core/modules/layout_builder/src/Plugin/SectionStorage/SectionStorageBase.php index 364afc6067..7c1b3425eb 100644 --- a/core/modules/layout_builder/src/Plugin/SectionStorage/SectionStorageBase.php +++ b/core/modules/layout_builder/src/Plugin/SectionStorage/SectionStorageBase.php @@ -2,7 +2,9 @@ namespace Drupal\layout_builder\Plugin\SectionStorage; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Plugin\ContextAwarePluginBase; +use Drupal\Core\Session\AccountInterface; use Drupal\layout_builder\Routing\LayoutBuilderRoutesTrait; use Drupal\layout_builder\Section; use Drupal\layout_builder\SectionListInterface; @@ -111,8 +113,17 @@ public function getContextsDuringPreview() { /** * {@inheritdoc} */ - public function isApplicable() { - return TRUE; + public function access($operation, AccountInterface $account = NULL, $return_as_object = FALSE) { + @trigger_error('@todo', E_USER_DEPRECATED); + $result = $this->routingAccess(); + return $return_as_object ? $result : $result->isAllowed(); + } + + /** + * {@inheritdoc} + */ + public function renderAccess() { + return AccessResult::allowed(); } } diff --git a/core/modules/layout_builder/src/SectionStorage/SectionStorageManager.php b/core/modules/layout_builder/src/SectionStorage/SectionStorageManager.php index 50fa28f40c..47f117ed53 100644 --- a/core/modules/layout_builder/src/SectionStorage/SectionStorageManager.php +++ b/core/modules/layout_builder/src/SectionStorage/SectionStorageManager.php @@ -97,13 +97,14 @@ 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()) { - return $plugin; + if ($plugin = $this->load($type, $contexts)) { + $access = $plugin->renderAccess(); + if ($cacheability) { + $cacheability->addCacheableDependency($access); + } + if ($access->isAllowed()) { + return $plugin; + } } } return NULL; diff --git a/core/modules/layout_builder/src/SectionStorageInterface.php b/core/modules/layout_builder/src/SectionStorageInterface.php index 1fcdde4f1a..41a1055c72 100644 --- a/core/modules/layout_builder/src/SectionStorageInterface.php +++ b/core/modules/layout_builder/src/SectionStorageInterface.php @@ -14,6 +14,8 @@ * Layout Builder is currently experimental and should only be leveraged by * experimental modules and development releases of contributed modules. * See https://www.drupal.org/core/experimental for more information. + * + * @todo Remove AccessibleInterface */ interface SectionStorageInterface extends SectionListInterface, PluginInspectionInterface, ContextAwarePluginInterface, AccessibleInterface { @@ -165,11 +167,13 @@ public function label(); public function save(); /** - * Returns if the section storage should be selected during plugin mapping. - * - * @return bool - * TRUE if the section storage can be used, FALSE otherwise. + * @return \Drupal\Core\Access\AccessResultInterface */ - public function isApplicable(); + public function routingAccess(); + + /** + * @return \Drupal\Core\Access\AccessResultInterface + */ + public function renderAccess(); } 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..6dca1ee900 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\Access\AccessResult; use Drupal\Core\Session\AccountInterface; use Drupal\layout_builder\Plugin\SectionStorage\SectionStorageBase; use Drupal\layout_builder\Section; @@ -35,23 +36,10 @@ public function getSections() { /** * {@inheritdoc} */ - public function isApplicable() { - return \Drupal::state()->get('layout_builder_overrides_test', FALSE); + public function routingAccess() { + return AccessResult::allowedIf(\Drupal::state()->get('layout_builder_overrides_test', FALSE))->setCacheMaxAge(0); } - /** - * {@inheritdoc} - */ - public function getCacheMaxAge() { - // This cannot be cached as applicability is determined by state. - return 0; - } - - /** - * {@inheritdoc} - */ - public function access($operation, AccountInterface $account = NULL, $return_as_object = FALSE) {} - /** * {@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..a280b7f028 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 @@ -186,7 +186,8 @@ public function getRedirectUrl() { * {@inheritdoc} */ public function access($operation, AccountInterface $account = NULL, $return_as_object = FALSE) { - $result = AccessResult::allowed(); + @trigger_error('@todo', E_USER_DEPRECATED); + $result = $this->routingAccess(); return $return_as_object ? $result : $result->isAllowed(); } @@ -213,11 +214,4 @@ public function extractIdFromRoute($value, $definition, $name, array $defaults) return $value ?: $defaults['id']; } - /** - * {@inheritdoc} - */ - public function isApplicable() { - return TRUE; - } - } diff --git a/core/modules/layout_builder/tests/src/Kernel/DefaultsSectionStorageTest.php b/core/modules/layout_builder/tests/src/Kernel/DefaultsSectionStorageTest.php index 5e3e1a2cb5..ddb76cd23a 100644 --- a/core/modules/layout_builder/tests/src/Kernel/DefaultsSectionStorageTest.php +++ b/core/modules/layout_builder/tests/src/Kernel/DefaultsSectionStorageTest.php @@ -2,6 +2,8 @@ namespace Drupal\Tests\layout_builder\Kernel; +use Drupal\Core\Access\AccessResult; +use Drupal\Core\Access\AccessResultInterface; use Drupal\Core\Plugin\Context\EntityContext; use Drupal\entity_test\Entity\EntityTest; use Drupal\KernelTests\KernelTestBase; @@ -55,6 +57,9 @@ protected function setUp() { * @covers ::access * @dataProvider providerTestAccess * + * @group legacy + * @expectedDeprecation @todo + * * @param bool $expected * The expected outcome of ::access(). * @param string $operation @@ -106,6 +111,74 @@ public function providerTestAccess() { return $data; } + /** + * @covers ::routingAccess + * @dataProvider providerTestRoutingAccess + * + * @param \Drupal\Core\Access\AccessResultInterface $expected + * The expected outcome of ::routingAccess(). + * @param bool $is_enabled + * Whether Layout Builder is enabled for this display. + * @param array $section_data + * Data to store as the sections value for Layout Builder. + */ + public function testRoutingAccess(AccessResultInterface $expected, $is_enabled, array $section_data) { + $display = LayoutBuilderEntityViewDisplay::create([ + 'targetEntityType' => 'entity_test', + 'bundle' => 'entity_test', + 'mode' => 'default', + 'status' => TRUE, + ]); + if ($is_enabled) { + $display->enableLayoutBuilder(); + } + $display + ->setThirdPartySetting('layout_builder', 'sections', $section_data) + ->save(); + + $this->plugin->setContext('display', EntityContext::fromEntity($display)); + $result = $this->plugin->routingAccess(); + $this->assertEquals($expected, $result); + } + + /** + * Provides test data for ::testRoutingAccess(). + */ + public function providerTestRoutingAccess() { + $section_data = [ + new Section('layout_onecol', [], [ + 'first-uuid' => new SectionComponent('first-uuid', 'content', ['id' => 'foo']), + ]), + ]; + + // Data provider values are: + // - the expected outcome of the call to ::routingAccess() + // - whether Layout Builder has been enabled for this display + // - whether this display has any section data. + $data = []; + $data['disabled, no data'] = [ + AccessResult::neutral()->addCacheTags(['config:core.entity_view_display.entity_test.entity_test.default']), + FALSE, + [], + ]; + $data['enabled, no data'] = [ + AccessResult::allowed()->addCacheTags(['config:core.entity_view_display.entity_test.entity_test.default']), + TRUE, + [], + ]; + $data['disabled, data'] = [ + AccessResult::neutral()->addCacheTags(['config:core.entity_view_display.entity_test.entity_test.default']), + FALSE, + $section_data, + ]; + $data['enabled, data'] = [ + AccessResult::allowed()->addCacheTags(['config:core.entity_view_display.entity_test.entity_test.default']), + TRUE, + $section_data, + ]; + return $data; + } + /** * @covers ::getContexts */ diff --git a/core/modules/layout_builder/tests/src/Kernel/OverridesSectionStorageTest.php b/core/modules/layout_builder/tests/src/Kernel/OverridesSectionStorageTest.php index cb8a0d35d7..d29687588a 100644 --- a/core/modules/layout_builder/tests/src/Kernel/OverridesSectionStorageTest.php +++ b/core/modules/layout_builder/tests/src/Kernel/OverridesSectionStorageTest.php @@ -2,6 +2,8 @@ namespace Drupal\Tests\layout_builder\Kernel; +use Drupal\Core\Access\AccessResult; +use Drupal\Core\Access\AccessResultInterface; use Drupal\Core\Plugin\Context\EntityContext; use Drupal\entity_test\Entity\EntityTest; use Drupal\KernelTests\KernelTestBase; @@ -54,6 +56,9 @@ protected function setUp() { * @covers ::access * @dataProvider providerTestAccess * + * @group legacy + * @expectedDeprecation @todo + * * @param bool $expected * The expected outcome of ::access(). * @param string $operation @@ -108,6 +113,77 @@ public function providerTestAccess() { return $data; } + /** + * @covers ::routingAccess + * @dataProvider providerTestRoutingAccess + * + * @param \Drupal\Core\Access\AccessResultInterface $expected + * The expected outcome of ::routingAccess(). + * @param bool $is_enabled + * Whether Layout Builder is enabled for this display. + * @param array $section_data + * Data to store as the sections value for Layout Builder. + */ + public function testRoutingAccess(AccessResultInterface $expected, $is_enabled, array $section_data) { + $display = LayoutBuilderEntityViewDisplay::create([ + 'targetEntityType' => 'entity_test', + 'bundle' => 'entity_test', + 'mode' => 'default', + 'status' => TRUE, + ]); + if ($is_enabled) { + $display->enableLayoutBuilder(); + } + $display + ->setOverridable() + ->save(); + + $entity = EntityTest::create([OverridesSectionStorage::FIELD_NAME => $section_data]); + $entity->save(); + + $this->plugin->setContext('entity', EntityContext::fromEntity($entity)); + $result = $this->plugin->routingAccess(); + $this->assertEquals($expected, $result); + } + + /** + * Provides test data for ::testRoutingAccess(). + */ + public function providerTestRoutingAccess() { + $section_data = [ + new Section('layout_default', [], [ + 'first-uuid' => new SectionComponent('first-uuid', 'content', ['id' => 'foo']), + ]), + ]; + + // Data provider values are: + // - the expected outcome of the call to ::routingAccess() + // - whether Layout Builder has been enabled for this display + // - whether this display has any section data. + $data = []; + $data['disabled, no data'] = [ + AccessResult::neutral()->addCacheTags(['config:core.entity_view_display.entity_test.entity_test.default']), + FALSE, + [], + ]; + $data['enabled, no data'] = [ + AccessResult::allowed()->addCacheTags(['config:core.entity_view_display.entity_test.entity_test.default']), + TRUE, + [], + ]; + $data['disabled, data'] = [ + AccessResult::neutral()->addCacheTags(['config:core.entity_view_display.entity_test.entity_test.default']), + FALSE, + $section_data, + ]; + $data['enabled, data'] = [ + AccessResult::allowed()->addCacheTags(['config:core.entity_view_display.entity_test.entity_test.default']), + TRUE, + $section_data, + ]; + return $data; + } + /** * @covers ::getContexts */ diff --git a/core/modules/layout_builder/tests/src/Unit/SectionStorageManagerTest.php b/core/modules/layout_builder/tests/src/Unit/SectionStorageManagerTest.php index 7789399b18..93749a49c5 100644 --- a/core/modules/layout_builder/tests/src/Unit/SectionStorageManagerTest.php +++ b/core/modules/layout_builder/tests/src/Unit/SectionStorageManagerTest.php @@ -6,7 +6,8 @@ use Drupal\Component\Plugin\Discovery\DiscoveryInterface; use Drupal\Component\Plugin\Exception\ContextException; use Drupal\Component\Plugin\Factory\FactoryInterface; -use Drupal\Core\Cache\CacheableDependencyInterface; +use Drupal\Core\Access\AccessResult; +use Drupal\Core\Access\AccessResultInterface; use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\Extension\ModuleHandlerInterface; @@ -205,10 +206,12 @@ public function testFindDefinitions() { * * @dataProvider providerTestFindByContext * - * @param bool $plugin_is_applicable - * The result for the plugin's isApplicable() method to return. + * @param \Drupal\Core\Access\AccessResultInterface $plugin_access + * The result for the plugin's access method to return. + * @param bool $boolean_access + * The Boolean equivalent of $plugin_access. */ - public function testFindByContext($plugin_is_applicable) { + public function testFindByContext(AccessResultInterface $plugin_access, $boolean_access) { $contexts = [ 'foo' => new Context(new ContextDefinition('foo')), ]; @@ -220,10 +223,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->renderAccess()->willReturn($plugin_access); $no_access = $this->prophesize(SectionStorageInterface::class); - $no_access->isApplicable()->willReturn(FALSE); + $no_access->renderAccess()->willReturn(AccessResult::neutral()); $missing_contexts = $this->prophesize(SectionStorageInterface::class); @@ -238,7 +241,7 @@ public function testFindByContext($plugin_is_applicable) { $this->factory->createInstance('provider_access', [])->willReturn($provider_access->reveal()); $result = $this->manager->findByContext($contexts); - if ($plugin_is_applicable) { + if ($boolean_access) { $this->assertSame($provider_access->reveal(), $result); } else { @@ -251,10 +254,11 @@ public function testFindByContext($plugin_is_applicable) { */ public function providerTestFindByContext() { // Data provider values are: - // - the result for the plugin's isApplicable() method to return. + // - the result for the plugin's access method to return. + // - the Boolean equivalent of that result. $data = []; - $data['plugin access: true'] = [TRUE]; - $data['plugin access: false'] = [FALSE]; + $data['plugin access: true'] = [AccessResult::allowed(), TRUE]; + $data['plugin access: false'] = [AccessResult::neutral(), FALSE]; return $data; } @@ -273,18 +277,10 @@ public function testFindByContextCacheableSectionStorage() { $this->discovery->getDefinitions()->willReturn($definitions); $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->renderAccess()->willReturn(AccessResult::neutral()->addCacheTags(['first_plugin'])); $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->renderAccess()->willReturn(AccessResult::allowed()->addCacheTags(['second_plugin'])); $this->factory->createInstance('first', [])->willReturn($first_plugin->reveal()); $this->factory->createInstance('second', [])->willReturn($second_plugin->reveal());