Issue #2843565 by Berdir, swentel, yched:...

Issue #2843565 by Berdir, swentel, yched: getViewBuilder('node')->viewMultiple() bypasses render cache

core/lib/Drupal/Core/Entity/EntityViewBuilder.php

@@ -108,16 +108,7 @@ public static function createInstance(ContainerInterface $container, EntityTypeI
    * {@inheritdoc}
    */
   public function view(EntityInterface $entity, $view_mode = 'full', $langcode = NULL) {
-    $build_list = $this->viewMultiple(array($entity), $view_mode, $langcode);
-
-    // The default ::buildMultiple() #pre_render callback won't run, because we
-    // extract a child element of the default renderable array. Thus we must
-    // assign an alternative #pre_render callback that applies the necessary
-    // transformations and then still calls ::buildMultiple().
-    $build = $build_list[0];
-    $build['#pre_render'][] = array($this, 'build');
-
-    return $build;
+    return $this->viewMultiple(array($entity), $view_mode, $langcode)[0];
   }
 
   /**
@@ -126,7 +117,6 @@ public function view(EntityInterface $entity, $view_mode = 'full', $langcode = N
   public function viewMultiple(array $entities = array(), $view_mode = 'full', $langcode = NULL) {
     $build_list = array(
       '#sorted' => TRUE,
-      '#pre_render' => array(array($this, 'buildMultiple')),
     );
     $weight = 0;
     foreach ($entities as $key => $entity) {
@@ -139,6 +129,7 @@ public function viewMultiple(array $entities = array(), $view_mode = 'full', $la
       $entityType = $this->entityTypeId;
       $this->moduleHandler()->alter(array($entityType . '_build_defaults', 'entity_build_defaults'), $build_list[$key], $entity, $view_mode);
 
+      $build_list[$key]['#pre_render'][] = array($this, 'build');
       $build_list[$key]['#weight'] = $weight++;
     }
 
@@ -200,11 +191,12 @@ protected function getBuildDefaults(EntityInterface $entity, $view_mode) {
   /**
    * Builds an entity's view; augments entity defaults.
    *
-   * This function is assigned as a #pre_render callback in ::view().
+   * This method is assigned as a #pre_render callback in ::viewMultiple() for
+   * each entity.
    *
-   * It transforms the renderable array for a single entity to the same
-   * structure as if we were rendering multiple entities, and then calls the
-   * default ::buildMultiple() #pre_render callback.
+   * It transforms the renderable array for a single entity to a structure
+   * understood by EntityViewBuilder::buildMultiple(), which is currently not
+   * called directly.
    *
    * @param array $build
    *   A renderable array containing build information and context for an entity
@@ -213,7 +205,8 @@ protected function getBuildDefaults(EntityInterface $entity, $view_mode) {
    * @return array
    *   The updated renderable array.
    *
-   * @see drupal_render()
+   * @see \Drupal\Core\Render\RendererInterface
+   * @see \Drupal\Core\Entity\EntityViewBuilder::buildMultiple()
    */
   public function build(array $build) {
     $build_list = [$build];
@@ -224,11 +217,17 @@ public function build(array $build) {
   /**
    * Builds multiple entities' views; augments entity defaults.
    *
-   * This function is assigned as a #pre_render callback in ::viewMultiple().
+   * This method is currently only called through EntityViewBuilder::build()
+   * as the render (cache) system does not support a #pre_render callback for
+   * multiple, separately cached render arrays.
+   *
+   * @todo Either simplify this to remove support for building multiple entities
+   *   at once or support a #pre_render_multiple or similar API in the render
+   *   system.
    *
-   * By delaying the building of an entity until the #pre_render processing in
-   * drupal_render(), the processing cost of assembling an entity's renderable
-   * array is saved on cache-hit requests.
+   * By delaying the building of an entity until the #pre_render processing, the
+   * processing cost of assembling an entity's renderable array is saved on
+   * cache-hit requests.
    *
    * @param array $build_list
    *   A renderable  array containing build information and context for an

core/modules/block_content/src/BlockContentViewBuilder.php

@@ -11,13 +11,6 @@
  */
 class BlockContentViewBuilder extends EntityViewBuilder {
 
-  /**
-   * {@inheritdoc}
-   */
-  public function view(EntityInterface $entity, $view_mode = 'full', $langcode = NULL) {
-    return $this->viewMultiple(array($entity), $view_mode, $langcode)[0];
-  }
-
   /**
    * {@inheritdoc}
    */
@@ -26,7 +19,9 @@ public function viewMultiple(array $entities = array(), $view_mode = 'full', $la
     // Apply the buildMultiple() #pre_render callback immediately, to make
     // bubbling of attributes and contextual links to the actual block work.
     // @see \Drupal\block\BlockViewBuilder::buildBlock()
-    unset($build_list['#pre_render'][0]);
+    foreach ($entities as $key => $entity) {
+      unset($build_list[$key]['#pre_render'][0]);
+    }
     return $this->buildMultiple($build_list);
   }
 

core/modules/system/src/Tests/Entity/EntityRenderCacheHookTest.php

+<?php
+
+namespace Drupal\system\Tests\Entity;
+
+use Drupal\entity_test\Entity\EntityTest;
+use Drupal\simpletest\WebTestBase;
+
+/**
+ * Tests that hooks are invoked or not depending on render cache.
+ *
+ * @group Entity
+ */
+class EntityRenderCacheHookTest extends WebTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = array('entity_test_hooks', 'entity_test');
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setUp() {
+    parent::setUp();
+    \Drupal::service('module_installer')->uninstall(['page_cache', 'dynamic_page_cache']);
+  }
+
+  /**
+   * Tests that hooks are not called when entities are render cached.
+   */
+  public function testHookInvocationAndRenderCache() {
+    $entity = EntityTest::create(['name' => 'test entity']);
+    $entity->save();
+
+    $this->drupalGet('test-view/' . $entity->id());
+    $this->assertText('test entity');
+    $this->assertText('custom hook invocation called');
+
+    $this->drupalGet('test-view/' . $entity->id());
+    $this->assertText('test entity');
+    $this->assertNoText('custom hook invocation called');
+
+    $this->drupalGet('test-view-multiple/' . $entity->id());
+    $this->assertText('test entity');
+    $this->assertNoText('custom hook invocation called');
+
+  }
+}

core/modules/system/tests/modules/entity_test_hooks/entity_test_hooks.info.yml

+type: module
+name: 'Entity test hooks module'
+core: 8.x
+package: Testing
+version: VERSION

core/modules/system/tests/modules/entity_test_hooks/entity_test_hooks.module

+<?php
+
+use Drupal\Core\Entity\Display\EntityViewDisplayInterface;
+use Drupal\entity_test\Entity\EntityTest;
+
+/**
+ * Implements hook_ENTITY_TYPE_view() for entity_test.
+ */
+function entity_test_hooks_entity_test_view(array &$build, EntityTest $entity_test, EntityViewDisplayInterface $display, $view_mode) {
+  drupal_set_message('custom hook invocation called');
+}

core/modules/system/tests/modules/entity_test_hooks/entity_test_hooks.routing.yml

+test.controller_view:
+  path: '/test-view/{entity_test}'
+  defaults:
+    _controller: 'Drupal\entity_test_hooks\Controller\ViewController::view'
+    _title: 'Test view'
+  requirements:
+    _access: 'TRUE'
+
+test.controller_view_multiple:
+  path: '/test-view-multiple/{entity_test}'
+  defaults:
+    _controller: 'Drupal\entity_test_hooks\Controller\ViewController::viewMultiple'
+    _title: 'Test multiple'
+  requirements:
+    _access: 'TRUE'

core/modules/system/tests/modules/entity_test_hooks/src/Controller/ViewController.php

+<?php
+
+namespace Drupal\entity_test_hooks\Controller;
+
+use Drupal\Core\Controller\ControllerBase;
+use Drupal\entity_test\Entity\EntityTest;
+
+/**
+ * Controller to view entities.
+ */
+class ViewController extends ControllerBase {
+
+  /**
+   * View an entity using \Drupal\Core\Entity\EntityViewBuilder::view()
+   *
+   * @param \Drupal\entity_test\Entity\EntityTest $entity_test
+   *   The entity to be viewed.
+   *
+   * @return array $build
+   *   The rendered entity
+   */
+  public function view(EntityTest $entity_test) {
+    return $this->entityTypeManager()->getViewBuilder('entity_test')->view($entity_test);
+  }
+
+  /**
+   * View an entity using \Drupal\Core\Entity\EntityViewBuilder::viewMultiple()
+   *
+   * @param \Drupal\entity_test\Entity\EntityTest $entity_test
+   *   The entity to be viewed.
+   *
+   * @return array $build
+   *   The rendered entity
+   */
+  public function viewMultiple(EntityTest $entity_test) {
+    return $this->entityTypeManager()->getViewBuilder('entity_test')->viewMultiple([$entity_test]);
+  }
+}