diff --git a/core/lib/Drupal/Component/Plugin/ContextAwarePluginInterface.php b/core/lib/Drupal/Component/Plugin/ContextAwarePluginInterface.php index 751b50e..b58d098 100644 --- a/core/lib/Drupal/Component/Plugin/ContextAwarePluginInterface.php +++ b/core/lib/Drupal/Component/Plugin/ContextAwarePluginInterface.php @@ -130,9 +130,9 @@ public function validateContexts(); * * @return array * A mapping of the expected assignment names to their context names. For - * example, if one of the $contexts is named 'entity', but the plugin - * expects a context named 'node', then this map would contain - * 'entity' => 'node'. + * example, if one of the $contexts is named 'current_user', but the plugin + * expects a context named 'user', then this map would contain + * 'current_user' => 'user'. */ public function getContextMapping(); @@ -141,9 +141,9 @@ public function getContextMapping(); * * @param array $context_mapping * A mapping of the expected assignment names to their context names. For - * example, if one of the $contexts is named 'entity', but the plugin - * expects a context named 'node', then this map would contain - * 'entity' => 'node'. + * example, if one of the $contexts is named 'current_user', but the plugin + * expects a context named 'user', then this map would contain + * 'current_user' => 'user'. * * @return $this */ diff --git a/core/lib/Drupal/Core/Annotation/ContextDefinition.php b/core/lib/Drupal/Core/Annotation/ContextDefinition.php index a532cae..987bab95 100644 --- a/core/lib/Drupal/Core/Annotation/ContextDefinition.php +++ b/core/lib/Drupal/Core/Annotation/ContextDefinition.php @@ -99,6 +99,7 @@ public function __construct(array $values) { // Annotation classes extract data from passed annotation classes directly // used in the classes they pass to. foreach (['label', 'description'] as $key) { + // @todo Remove this workaround in https://www.drupal.org/node/2362727. if (isset($values[$key]) && $values[$key] instanceof TranslationWrapper) { $values[$key] = (string) $values[$key]->get(); } diff --git a/core/lib/Drupal/Core/Block/BlockBase.php b/core/lib/Drupal/Core/Block/BlockBase.php index dfd4573..ba1026b 100644 --- a/core/lib/Drupal/Core/Block/BlockBase.php +++ b/core/lib/Drupal/Core/Block/BlockBase.php @@ -120,7 +120,9 @@ public function calculateDependencies() { * {@inheritdoc} */ public function access(AccountInterface $account) { - // result should be varied. + // @todo Remove self::blockAccess() and force individual plugins to return + // their own AccessResult logic. Until that is done in + // https://www.drupal.org/node/2375689 the access will be set uncacheable. if ($this->blockAccess($account)) { $access = AccessResult::allowed(); } diff --git a/core/lib/Drupal/Core/Condition/ConditionPluginCollection.php b/core/lib/Drupal/Core/Condition/ConditionPluginCollection.php index 6da4f03..02b3e11 100644 --- a/core/lib/Drupal/Core/Condition/ConditionPluginCollection.php +++ b/core/lib/Drupal/Core/Condition/ConditionPluginCollection.php @@ -41,9 +41,11 @@ public function getConfiguration() { $default_config = array(); $default_config['id'] = $instance_id; $default_config += $this->get($instance_id)->defaultConfiguration(); - // For the purposes of comparison, remove the context mapping. If the - // current configuration matches the default configuration, the context - // mapping was not used anyway. + // In order to determine if a plugin is configured, we must compare it to + // its default. The default configuration of a plugin does not contain + // context_mapping and it is not used when the plugin is not configured, + // so remove the context_mapping from the instance config to compare the + // remaining values. unset($instance_config['context_mapping']); if ($default_config === $instance_config) { unset($configuration[$instance_id]); diff --git a/core/lib/Drupal/Core/Entity/EntityForm.php b/core/lib/Drupal/Core/Entity/EntityForm.php index b82d283..a078aee 100644 --- a/core/lib/Drupal/Core/Entity/EntityForm.php +++ b/core/lib/Drupal/Core/Entity/EntityForm.php @@ -291,18 +291,18 @@ public function buildEntity(array $form, FormStateInterface $form_state) { * The current state of the form. */ protected function copyFormValuesToEntity(EntityInterface $entity, array $form, FormStateInterface $form_state) { - $keys_to_skip = []; + $values = $form_state->getValues(); + if ($this->entity instanceof EntityWithPluginCollectionInterface) { // Do not manually update values represented by plugin collections. - $keys_to_skip += array_keys($this->entity->getPluginCollections()); + $values = array_diff_key($values, $this->entity->getPluginCollections()); } + // @todo: This relies on a method that only exists for config and content // entities, in a different way. Consider moving this logic to a config // entity specific implementation. - foreach ($form_state->getValues() as $key => $value) { - if (!in_array($key, $keys_to_skip)) { - $entity->set($key, $value); - } + foreach ($values as $key => $value) { + $entity->set($key, $value); } } diff --git a/core/modules/block/src/BlockAccessControlHandler.php b/core/modules/block/src/BlockAccessControlHandler.php index b815c41..d460521 100644 --- a/core/modules/block/src/BlockAccessControlHandler.php +++ b/core/modules/block/src/BlockAccessControlHandler.php @@ -99,10 +99,6 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A } $conditions[$condition_id] = $condition; } - // 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. if ($this->resolveConditions($conditions, 'and') !== FALSE) { // Delegate to the plugin. $access = $entity->getPlugin()->access($account); @@ -110,7 +106,13 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A else { $access = AccessResult::forbidden(); } - return $access->setCacheable(FALSE)->cacheUntilEntityChanges($entity); + // 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->setCacheable(FALSE); } } diff --git a/core/modules/block/src/BlockRepository.php b/core/modules/block/src/BlockRepository.php index e3279b8..4687d37 100644 --- a/core/modules/block/src/BlockRepository.php +++ b/core/modules/block/src/BlockRepository.php @@ -78,13 +78,7 @@ public function getVisibleBlocksPerRegion(array $contexts) { foreach ($this->blockStorage->loadByProperties(array('theme' => $this->getTheme())) as $block_id => $block) { /** @var \Drupal\block\BlockInterface $block */ // Set the contexts on the block before checking access. - $block->setAvailableContexts($contexts); - $block_plugin = $block->getPlugin(); - if ($block_plugin instanceof ContextAwarePluginInterface) { - $this->contextHandler->applyContextMapping($block_plugin, $contexts); - } - - if ($block->access('view')) { + if ($block->setAvailableContexts($contexts)->access('view')) { $full[$block->get('region')][$block_id] = $block; } } diff --git a/core/modules/block/src/Event/BlockContextEvent.php b/core/modules/block/src/Event/BlockContextEvent.php index 9feac5e..99b0bbd 100644 --- a/core/modules/block/src/Event/BlockContextEvent.php +++ b/core/modules/block/src/Event/BlockContextEvent.php @@ -11,7 +11,7 @@ use Symfony\Component\EventDispatcher\Event; /** - * Wraps block contexts in order for event subscribers to add context. + * Event subscribers can add context to be used by the block and its conditions. * * @see \Drupal\block\Event\BlockEvents::ACTIVE_CONTEXT * @see \Drupal\block\Event\BlockEvents::ADMINISTRATIVE_CONTEXT diff --git a/core/modules/block/src/EventSubscriber/CurrentUserContext.php b/core/modules/block/src/EventSubscriber/CurrentUserContext.php index 659395b..b16f2c1 100644 --- a/core/modules/block/src/EventSubscriber/CurrentUserContext.php +++ b/core/modules/block/src/EventSubscriber/CurrentUserContext.php @@ -61,10 +61,6 @@ public function onBlockActiveContext(BlockContextEvent $event) { /** * {@inheritdoc} - * - * Since administrative contexts are not executed against and any user object - * will work, and we always have a current user, we can simply execute the - * active context method. */ public function onBlockAdministrativeContext(BlockContextEvent $event) { $this->onBlockActiveContext($event); diff --git a/core/modules/block/tests/src/Unit/BlockRepositoryTest.php b/core/modules/block/tests/src/Unit/BlockRepositoryTest.php index ef46dc9..c22f099 100644 --- a/core/modules/block/tests/src/Unit/BlockRepositoryTest.php +++ b/core/modules/block/tests/src/Unit/BlockRepositoryTest.php @@ -87,6 +87,9 @@ public function testGetVisibleBlocksPerRegion(array $blocks_config, array $expec foreach ($blocks_config as $block_id => $block_config) { $block = $this->getMock('Drupal\block\BlockInterface'); $block->expects($this->once()) + ->method('setAvailableContexts') + ->willReturnSelf(); + $block->expects($this->once()) ->method('access') ->will($this->returnValue($block_config[0])); $block->expects($block_config[0] ? $this->atLeastOnce() : $this->never()) @@ -150,23 +153,18 @@ public function providerBlocksConfig() { public function testGetVisibleBlocksPerRegionWithContext() { $block = $this->getMock('Drupal\block\BlockInterface'); $block->expects($this->once()) + ->method('setAvailableContexts') + ->willReturnSelf(); + $block->expects($this->once()) ->method('access') ->willReturn(TRUE); $block->expects($this->once()) ->method('get') ->with('region') ->willReturn('top'); - $block_plugin = $this->getMock('Drupal\Tests\block\Unit\TestContextAwareBlockInterface'); - $block->expects($this->once()) - ->method('getPlugin') - ->willReturn($block_plugin); $blocks['block_id'] = $block; $contexts = []; - $this->contextHandler->expects($this->once()) - ->method('applyContextMapping') - ->with($block_plugin, $contexts); - $this->blockStorage->expects($this->once()) ->method('loadByProperties') ->with(['theme' => $this->theme]) diff --git a/core/tests/Drupal/Tests/Core/Entity/EntityFormTest.php b/core/tests/Drupal/Tests/Core/Entity/EntityFormTest.php index 35f8913..3586df9 100644 --- a/core/tests/Drupal/Tests/Core/Entity/EntityFormTest.php +++ b/core/tests/Drupal/Tests/Core/Entity/EntityFormTest.php @@ -8,6 +8,7 @@ namespace Drupal\Tests\Core\Entity; use Drupal\Core\Entity\EntityForm; +use Drupal\Core\Form\FormState; use Drupal\Tests\UnitTestCase; /** @@ -94,4 +95,32 @@ public function providerTestFormIds() { ); } + /** + * @covers ::copyFormValuesToEntity + */ + public function testCopyFormValuesToEntity() { + $entity_id = 'test_config_entity_id'; + $values = ['id' => $entity_id]; + $entity = $this->getMockBuilder('\Drupal\Tests\Core\Config\Entity\Fixtures\ConfigEntityBaseWithPluginCollections') + ->setConstructorArgs([$values, 'test_config_entity']) + ->setMethods(['getPluginCollections']) + ->getMock(); + $entity->expects($this->atLeastOnce()) + ->method('getPluginCollections') + ->willReturn(['key_controlled_by_plugin_collection' => NULL]); + $this->entityForm->setEntity($entity); + + $form_state = (new FormState())->setValues([ + 'regular_key' => 'foo', + 'key_controlled_by_plugin_collection' => 'bar', + ]); + $result = $this->entityForm->buildEntity([], $form_state); + + $this->assertSame($entity_id, $result->id()); + // The regular key should have a value, but the one controlled by a plugin + // collection should not have been set. + $this->assertSame('foo', $result->get('regular_key')); + $this->assertNull($result->get('key_controlled_by_plugin_collection')); + } + }