src/EntityEmbedDisplay/EntityEmbedDisplayBase.php | 28 +++----- .../EntityEmbedDisplayInterface.php | 4 +- .../FieldFormatterEntityEmbedDisplayBase.php | 9 +-- src/EntityHelperTrait.php | 79 ++++++++-------------- src/Plugin/Filter/EntityEmbedFilter.php | 21 ++++-- .../EntityEmbedDisplay/ImageFieldFormatter.php | 17 +++-- src/Tests/EntityEmbedFilterTest.php | 18 +++-- src/Twig/EntityEmbedTwigExtension.php | 2 +- 8 files changed, 88 insertions(+), 90 deletions(-) diff --git a/src/EntityEmbedDisplay/EntityEmbedDisplayBase.php b/src/EntityEmbedDisplay/EntityEmbedDisplayBase.php index 2e6ef49..9f1b644 100644 --- a/src/EntityEmbedDisplay/EntityEmbedDisplayBase.php +++ b/src/EntityEmbedDisplay/EntityEmbedDisplayBase.php @@ -1,13 +1,9 @@ isValidEntityType()) { - return FALSE; - } - - // Check that the entity itself can be viewed by the user. - if ($entity = $this->getEntityFromContext()) { - return $entity->access('view', $account); - } - - return TRUE; + return AccessResult::allowedIf($this->isValidEntityType()) + // @see \Drupal\Core\Entity\EntityTypeManager + ->addCacheTags(['entity_types']); } /** @@ -261,6 +248,13 @@ abstract class EntityEmbedDisplayBase extends PluginBase implements ContainerFac /** * Gets the entity from the current context. * + * @todo Where doe sthis come from? The value must come from somewhere, yet + * this does not implement any context-related interfaces. This is an *input*, + * so we need cache contexts and possibly cache tags to reflect where this + * came from. We need that for *everything* that this class does that relies + * on this, plus any of its subclasses. Right now, this is effectively a + * global that breaks cacheability metadata. + * * @return \Drupal\Core\Entity\EntityInterface */ public function getEntityFromContext() { diff --git a/src/EntityEmbedDisplay/EntityEmbedDisplayInterface.php b/src/EntityEmbedDisplay/EntityEmbedDisplayInterface.php index 2284444..6e9e6af 100644 --- a/src/EntityEmbedDisplay/EntityEmbedDisplayInterface.php +++ b/src/EntityEmbedDisplay/EntityEmbedDisplayInterface.php @@ -58,8 +58,8 @@ interface EntityEmbedDisplayInterface extends ConfigurablePluginInterface, Plugi * (optional) The user for which to check access, or NULL to check access * for the current user. Defaults to NULL. * - * @return bool - * TRUE if this Entity Embed Display plugin can be used, or FALSE otherwise. + * @return \Drupal\Core\Access\AccessResultInterface + * The access result. */ public function access(AccountInterface $account = NULL); diff --git a/src/EntityEmbedDisplay/FieldFormatterEntityEmbedDisplayBase.php b/src/EntityEmbedDisplay/FieldFormatterEntityEmbedDisplayBase.php index 79456c2..f681c56 100644 --- a/src/EntityEmbedDisplay/FieldFormatterEntityEmbedDisplayBase.php +++ b/src/EntityEmbedDisplay/FieldFormatterEntityEmbedDisplayBase.php @@ -7,6 +7,7 @@ namespace Drupal\entity_embed\EntityEmbedDisplay; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Field\BaseFieldDefinition; use Drupal\Core\Field\FormatterPluginManager; @@ -112,12 +113,12 @@ abstract class FieldFormatterEntityEmbedDisplayBase extends EntityEmbedDisplayBa * {@inheritdoc} */ public function access(AccountInterface $account = NULL) { - if (!parent::access($account)) { - return FALSE; - } + return parent::access($account)->andIf($this->isApplicableFieldFormatter()); + } + protected function isApplicableFieldFormatter() { $definition = $this->formatterPluginManager->getDefinition($this->getDerivativeId()); - return $definition['class']::isApplicable($this->getFieldDefinition()); + return AccessResult::allowedIf($definition['class']::isApplicable($this->getFieldDefinition())); } /** diff --git a/src/EntityHelperTrait.php b/src/EntityHelperTrait.php index 9736fe4..1516507 100644 --- a/src/EntityHelperTrait.php +++ b/src/EntityHelperTrait.php @@ -1,10 +1,5 @@ entityManager()->getViewBuilder($entity->getEntityTypeId()); - return $render_controller->view($entity, $view_mode, $langcode); + protected function buildEntity(EntityInterface $entity, $view_mode, $langcode = NULL) { + return $this->entityManager() + ->getViewBuilder($entity->getEntityTypeId()) + ->view($entity, $view_mode, $langcode); } /** - * Renders an embedded entity. + * Builds the render array for an embedded entity. * * @param \Drupal\Core\Entity\EntityInterface $entity * The entity to be rendered. @@ -159,10 +156,12 @@ trait EntityHelperTrait { * (optional) Array of context values, corresponding to the attributes on * the embed HTML tag. * - * @return string - * The HTML of the entity rendered with the Entity Embed Display plugin. + * @return array + * A render array. + * + * @todo improve documentation */ - protected function renderEntityEmbed(EntityInterface $entity, array $context = array()) { + protected function buildEntityEmbed(EntityInterface $entity, array $context = array()) { // Support the deprecated view-mode data attribute. if (isset($context['data-view-mode']) && !isset($context['data-entity-embed-display']) && !isset($context['data-entity-embed-settings'])) { $context['data-entity-embed-display'] = 'entity_reference:entity_reference_entity_view'; @@ -199,7 +198,7 @@ trait EntityHelperTrait { '#entity' => $entity, '#context' => $context, ); - $build['entity'] = $this->renderEntityEmbedDisplayPlugin( + $build['entity'] = $this->buildEntityEmbedDisplayPlugin( $entity, $context['data-entity-embed-display'], $context['data-entity-embed-settings'], @@ -219,15 +218,16 @@ trait EntityHelperTrait { $build['#attributes']['data-caption'] = $context['data-caption']; } + // Make sure that access to the entity is respected. + $build['#access'] = $entity->access('view', NULL, TRUE); + // @todo Should this hook get invoked if $build is an empty array? $this->moduleHandler()->alter(array("{$context['data-entity-type']}_embed", 'entity_embed'), $build, $entity, $context); - $entity_output = $this->renderer()->render($build); - - return $entity_output; + return $build; } /** - * Renders an entity using an Entity Embed Display plugin. + * Builds the render array for an entity using an Entity Embed Display plugin. * * @param \Drupal\Core\Entity\EntityInterface $entity * The entity to be rendered. @@ -242,7 +242,7 @@ trait EntityHelperTrait { * @return array * A render array for the Entity Embed Display plugin. */ - protected function renderEntityEmbedDisplayPlugin(EntityInterface $entity, $plugin_id, array $plugin_configuration = array(), array $context = array()) { + protected function buildEntityEmbedDisplayPlugin(EntityInterface $entity, $plugin_id, array $plugin_configuration = array(), array $context = array()) { // Build the Entity Embed Display plugin. /** @var \Drupal\entity_embed\EntityEmbedDisplay\EntityEmbedDisplayBase $display */ $display = $this->displayPluginManager()->createInstance($plugin_id, $plugin_configuration); @@ -336,29 +336,4 @@ trait EntityHelperTrait { return $this; } - /** - * Returns the renderer. - * - * @return \Drupal\Core\Render\RendererInterface - * The renderer. - */ - protected function renderer() { - if (!isset($this->renderer)) { - $this->renderer = \Drupal::service('renderer'); - } - return $this->renderer; - } - - /** - * Sets the renderer. - * - * @param \Drupal\Core\Render\RendererInterface $renderer - * The renderer. - * - * @return self - */ - public function setRenderer(RendererInterface $renderer) { - $this->renderer = $renderer; - return $this; - } } diff --git a/src/Plugin/Filter/EntityEmbedFilter.php b/src/Plugin/Filter/EntityEmbedFilter.php index c8b2614..e8059d5 100644 --- a/src/Plugin/Filter/EntityEmbedFilter.php +++ b/src/Plugin/Filter/EntityEmbedFilter.php @@ -12,6 +12,8 @@ use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\Core\Cache\CacheableMetadata; +use Drupal\Core\Render\BubbleableMetadata; +use Drupal\Core\Render\RenderContext; use Drupal\entity_embed\EntityHelperTrait; use Drupal\entity_embed\Exception\EntityNotFoundException; use Drupal\entity_embed\Exception\RecursiveRenderingException; @@ -102,13 +104,24 @@ class EntityEmbedFilter extends FilterBase implements ContainerFactoryPluginInte } $access = $entity->access('view', NULL, TRUE); - $access_metadata = CacheableMetadata::createFromObject($access); - $entity_metadata = CacheableMetadata::createFromObject($entity); - $result = $result->merge($entity_metadata)->merge($access_metadata); + $result->addCacheableDependency($access); $context = $this->getNodeAttributesAsArray($node); $context += array('data-langcode' => $langcode); - $entity_output = $this->renderEntityEmbed($entity, $context); + $build = $this->buildEntityEmbed($entity, $context); + // We need to render the embedded entity: + // - without replacing placeholders, so that the placeholders are + // only replaced at the last possible moment. Hence we cannot use + // either renderPlain() or renderRoot(), so we must use render(). + // - without bubbling beyond this filter, because filters must + // ensure that the bubbleable metadata for the changes they make + // when filtering text makes it onto the FilterProcessResult + // object that they return ($result). To prevent that bubbling, we + // must wrap the call to render() in a render context. + $entity_output = $this->renderer()->executeInRenderContext(new RenderContext(), function () use (&$build) { + return $this->renderer()->render($build); + }); + $result = $result->merge(BubbleableMetadata::createFromRenderArray($build)); $depth--; } diff --git a/src/Plugin/entity_embed/EntityEmbedDisplay/ImageFieldFormatter.php b/src/Plugin/entity_embed/EntityEmbedDisplay/ImageFieldFormatter.php index b75fd14..e66c5fc 100644 --- a/src/Plugin/entity_embed/EntityEmbedDisplay/ImageFieldFormatter.php +++ b/src/Plugin/entity_embed/EntityEmbedDisplay/ImageFieldFormatter.php @@ -7,6 +7,7 @@ namespace Drupal\entity_embed\Plugin\entity_embed\EntityEmbedDisplay; +use Drupal\Core\Access\AccessResult; use Drupal\Core\Entity\EntityManagerInterface; use Drupal\Core\Field\FormatterPluginManager; use Drupal\Core\Form\FormStateInterface; @@ -85,15 +86,21 @@ class ImageFieldFormatter extends FileFieldFormatter { * {@inheritdoc} */ public function access(AccountInterface $account = NULL) { - if (!parent::access($account)) { - return FALSE; - } + return parent::access($account)->andIf($this->isValidImage()); + } + + protected function isValidImage() { + $access = AccessResult::allowed(); + // @todo needs cacheability metadata for getEntityFromContext. + // @see \Drupal\entity_embed\EntityEmbedDisplay\EntityEmbedDisplayBase::getEntityFromContext() if ($entity = $this->getEntityFromContext()) { - return $this->imageFactory->get($entity->getFileUri())->isValid(); + $access = AccessResult::allowedIf($this->imageFactory->get($entity->getFileUri())->isValid()) + // See the above @todo, this is the best we can do for now. + ->addCacheableDependency($entity); } - return TRUE; + return $access; } /** diff --git a/src/Tests/EntityEmbedFilterTest.php b/src/Tests/EntityEmbedFilterTest.php index ce77ea8..cafcbdc 100644 --- a/src/Tests/EntityEmbedFilterTest.php +++ b/src/Tests/EntityEmbedFilterTest.php @@ -1,10 +1,5 @@ assertNoText(strip_tags($content), 'Placeholder does not appear in the output when embed is successful.'); $this->assertRaw('
', 'Embed container found.'); + // Tests that embedded entity is not rendered if not accessible. + $this->node->setPublished(FALSE)->save(); + $settings = []; + $settings['type'] = 'page'; + $settings['title'] = 'Test un-accessible entity embed with entity-id and view-mode'; + $settings['body'] = [['value' => $content, 'format' => 'custom_format']]; + $node = $this->drupalCreateNode($settings); + $this->drupalGet('node/' . $node->id()); + $this->assertNoRaw('assertNoText($this->node->body->value, 'Embedded node does not exist in the page.'); + $this->assertNoText(strip_tags($content), 'Placeholder does not appear in the output when embed is successful.'); + $this->node->setPublished(TRUE)->save(); + // Tests entity embed using entity UUID and view mode. $content = 'This placeholder should not be rendered.'; $settings = array(); diff --git a/src/Twig/EntityEmbedTwigExtension.php b/src/Twig/EntityEmbedTwigExtension.php index 9352e60..518088c 100644 --- a/src/Twig/EntityEmbedTwigExtension.php +++ b/src/Twig/EntityEmbedTwigExtension.php @@ -74,7 +74,7 @@ class EntityEmbedTwigExtension extends \Twig_Extension { 'data-entity-embed-display' => $display_plugin, 'data-entity-embed-settings' => $display_settings, ); - return $this->renderEntityEmbed($entity, $context); + return $this->buildEntityEmbed($entity, $context); } }