Issue #2241229 by Wim Leers: Aggregator Feed & Item content entity types, and...

Issue #2241229 by Wim Leers: Aggregator Feed & Item content entity types, and AggregatorFeedBlock should use cache tags.

core/modules/aggregator/src/Controller/AggregatorController.php

@@ -206,6 +206,9 @@ public function sources() {
         '#theme' => 'aggregator_summary_items',
         '#summary_items' => $summary_items,
         '#source' => $feed,
+        '#cache' => array(
+          'tags' => $feed->getCacheTag(),
+        ),
       );
     }
     $build['feed_icon'] = array(

core/modules/aggregator/src/Entity/Feed.php

@@ -23,6 +23,7 @@
  *   controllers = {
  *     "storage" = "Drupal\aggregator\FeedStorage",
  *     "view_builder" = "Drupal\aggregator\FeedViewBuilder",
+ *     "access" = "Drupal\aggregator\FeedAccessController",
  *     "form" = {
  *       "default" = "Drupal\aggregator\FeedForm",
  *       "delete" = "Drupal\aggregator\Form\FeedDeleteForm",
@@ -36,6 +37,7 @@
  *   },
  *   base_table = "aggregator_feed",
  *   fieldable = TRUE,
+ *   render_cache = FALSE,
  *   entity_keys = {
  *     "id" = "fid",
  *     "label" = "title",
@@ -107,6 +109,7 @@ public static function preDelete(EntityStorageInterface $storage, array $entitie
    * {@inheritdoc}
    */
   public static function postDelete(EntityStorageInterface $storage, array $entities) {
+    parent::postDelete($storage, $entities);
     if (\Drupal::moduleHandler()->moduleExists('block')) {
       // Make sure there are no active blocks for these feeds.
       $ids = \Drupal::entityQuery('block')

core/modules/aggregator/src/Entity/Item.php

@@ -7,11 +7,13 @@
 
 namespace Drupal\aggregator\Entity;
 
+use Drupal\Core\Cache\Cache;
 use Drupal\Core\Entity\ContentEntityBase;
 use Drupal\Core\Entity\EntityStorageInterface;
 use Drupal\aggregator\ItemInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
 use Drupal\Core\Field\FieldDefinition;
+use Drupal\Core\Url;
 
 /**
  * Defines the aggregator item entity class.
@@ -21,10 +23,13 @@
  *   label = @Translation("Aggregator feed item"),
  *   controllers = {
  *     "storage" = "Drupal\aggregator\ItemStorage",
- *     "view_builder" = "Drupal\aggregator\ItemViewBuilder"
+ *     "view_builder" = "Drupal\aggregator\ItemViewBuilder",
+ *     "access" = "Drupal\aggregator\FeedAccessController",
  *   },
+ *   uri_callback = "Drupal\aggregator\Entity\Item::buildUri",
  *   base_table = "aggregator_item",
  *   fieldable = TRUE,
+ *   render_cache = FALSE,
  *   entity_keys = {
  *     "id" = "iid",
  *     "label" = "title",
@@ -184,4 +189,40 @@ public function getGuid() {
   public function setGuid($guid) {
     return $this->set('guid', $guid);
   }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function postSave(EntityStorageInterface $storage, $update = TRUE) {
+    parent::postSave($storage, $update);
+
+    // Entity::postSave() calls Entity::invalidateTagsOnSave(), which only
+    // handles the regular cases. The Item entity has one special case: a newly
+    // created Item is *also* associated with a Feed, so we must invalidate the
+    // associated Feed's cache tag.
+    Cache::invalidateTags($this->getCacheTag());
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getCacheTag() {
+    return Feed::load($this->getFeedId())->getCacheTag();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getListCacheTags() {
+    return Feed::load($this->getFeedId())->getListCacheTags();
+  }
+
+
+  /**
+   * Entity URI callback.
+   */
+  public static function buildUri(ItemInterface $item) {
+    return Url::createFromPath($item->getLink());
+  }
+
 }

core/modules/aggregator/src/Entity/ItemAccessController.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\aggregator\ItemAccessController.
+ */
+
+namespace Drupal\aggregator;
+
+use Drupal\Core\Entity\EntityAccessController;
+use Drupal\Core\Entity\EntityInterface;
+use Drupal\Core\Session\AccountInterface;
+
+/**
+ * Defines an access controller for the item entity.
+ *
+ * @see \Drupal\aggregator\Entity\Item
+ */
+class ItemAccessController extends EntityAccessController {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) {
+    switch ($operation) {
+      case 'view':
+        return $account->hasPermission('access news feeds');
+        break;
+
+      default:
+        return $account->hasPermission('administer news feeds');
+        break;
+    }
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) {
+    return $account->hasPermission('administer news feeds');
+  }
+
+}

core/modules/aggregator/src/FeedAccessController.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\aggregator\FeedAccessController.
+ */
+
+namespace Drupal\aggregator;
+
+use Drupal\Core\Entity\EntityAccessController;
+use Drupal\Core\Entity\EntityInterface;
+use Drupal\Core\Session\AccountInterface;
+
+/**
+ * Defines an access controller for the feed entity.
+ *
+ * @see \Drupal\aggregator\Entity\Feed
+ */
+class FeedAccessController extends EntityAccessController {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) {
+    switch ($operation) {
+      case 'view':
+        return $account->hasPermission('access news feeds');
+        break;
+
+      default:
+        return $account->hasPermission('administer news feeds');
+        break;
+    }
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function checkCreateAccess(AccountInterface $account, array $context, $entity_bundle = NULL) {
+    return $account->hasPermission('administer news feeds');
+  }
+
+}

core/modules/aggregator/src/Plugin/Block/AggregatorFeedBlock.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\aggregator\Plugin\Block;
 
+use Drupal\Component\Utility\NestedArray;
 use Drupal\aggregator\FeedStorageInterface;
 use Drupal\aggregator\ItemStorageInterface;
 use Drupal\block\BlockBase;
@@ -94,6 +95,13 @@ public function defaultConfiguration() {
     return array(
       'block_count' => 10,
       'feed' => NULL,
+      // Modify the default max age for the 'Aggregator Feed' blocks:
+      // modifications made to feeds or feed items will automatically invalidate
+      // corresponding cache tags, therefore allowing us to cache these blocks
+      // forever.
+      'cache' => array(
+        'max_age' => \Drupal\Core\Cache\Cache::PERMANENT,
+      ),
     );
   }
 
@@ -180,4 +188,14 @@ public function build() {
     }
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function getCacheTags() {
+    $cache_tags = parent::getCacheTags();
+    $feed = $this->feedStorage->load($this->configuration['feed']);
+    $cache_tags = NestedArray::mergeDeep($cache_tags, $feed->getCacheTag());
+    return $cache_tags;
+  }
+
 }

core/modules/aggregator/src/Tests/AggregatorRenderingTest.php

@@ -56,11 +56,19 @@ public function testBlockLinks() {
     $href = 'aggregator/sources/' . $feed->id();
     $links = $this->xpath('//a[@href = :href]', array(':href' => url($href)));
     $this->assert(isset($links[0]), format_string('Link to href %href found.', array('%href' => $href)));
+    $cache_tags_header = $this->drupalGetHeader('X-Drupal-Cache-Tags');
+    $cache_tags = explode(' ', $cache_tags_header);
+    $this->assertTrue(in_array('block_plugin:aggregator_feed_block', $cache_tags));
+    $this->assertTrue(in_array('aggregator_feed:' . $feed->id(), $cache_tags));
 
     // Visit that page.
     $this->drupalGet($href);
     $correct_titles = $this->xpath('//h1[normalize-space(text())=:title]', array(':title' => $feed->label()));
     $this->assertFalse(empty($correct_titles), 'Aggregator feed page is available and has the correct title.');
+    $cache_tags = explode(' ', $this->drupalGetHeader('X-Drupal-Cache-Tags'));
+    $this->assertTrue(in_array('aggregator_feed:' . $feed->id(), $cache_tags));
+    $this->assertTrue(in_array('aggregator_feed_view:1', $cache_tags));
+    $this->assertTrue(in_array('aggregator_item_view:1', $cache_tags));
 
     // Set the number of news items to 0 to test that the block does not show
     // up.
@@ -101,10 +109,18 @@ public function testFeedPage() {
     $href = 'aggregator/sources/' . $feed->id();
     $links = $this->xpath('//a[@href = :href]', array(':href' => url($href)));
     $this->assertTrue(isset($links[0]), String::format('Link to href %href found.', array('%href' => $href)));
+    $cache_tags_header = $this->drupalGetHeader('X-Drupal-Cache-Tags');
+    $cache_tags = explode(' ', $cache_tags_header);
+    $this->assertTrue(in_array('aggregator_feed:' . $feed->id(), $cache_tags));
 
     // Check for the presence of a pager.
     $this->drupalGet('aggregator/sources/' . $feed->id());
     $elements = $this->xpath("//ul[@class=:class]", array(':class' => 'pager'));
     $this->assertTrue(!empty($elements), 'Individual source page contains a pager.');
+    $cache_tags = explode(' ', $this->drupalGetHeader('X-Drupal-Cache-Tags'));
+    $this->assertTrue(in_array('aggregator_feed:' . $feed->id(), $cache_tags));
+    $this->assertTrue(in_array('aggregator_feed_view:1', $cache_tags));
+    $this->assertTrue(in_array('aggregator_item_view:1', $cache_tags));
   }
+
 }

core/modules/aggregator/src/Tests/FeedCacheTagsTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\aggregator\Tests\FeedCacheTagsTest.
+ */
+
+namespace Drupal\aggregator\Tests;
+
+use Drupal\aggregator\Entity\Feed;
+use Drupal\system\Tests\Entity\EntityWithUriCacheTagsTestBase;
+
+/**
+ * Tests the Feed entity's cache tags.
+ */
+class FeedCacheTagsTest extends EntityWithUriCacheTagsTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = array('aggregator');
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function getInfo() {
+    return parent::generateStandardizedInfo('Aggregator feed', 'Aggregator');
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setUp() {
+    parent::setUp();
+
+    // Give anonymous users permission to access feeds, so that we can verify
+    // the cache tags of cached versions of feeds.
+    $user_role = entity_load('user_role', DRUPAL_ANONYMOUS_RID);
+    $user_role->grantPermission('access news feeds');
+    $user_role->save();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function createEntity() {
+    // Create a "Llama" feed.
+    $feed = Feed::create(array(
+      'title' => 'Llama',
+      'url' => 'https://www.drupal.org/',
+      'refresh' => 900,
+      'checked' => 1389919932,
+      'description' => 'Drupal.org',
+    ));
+    $feed->save();
+
+    return $feed;
+  }
+
+}

core/modules/aggregator/src/Tests/ItemCacheTagsTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\aggregator\Tests\ItemCacheTagsTest.
+ */
+
+namespace Drupal\aggregator\Tests;
+
+use Drupal\aggregator\Entity\Feed;
+use Drupal\aggregator\Entity\Item;
+use Drupal\system\Tests\Entity\EntityCacheTagsTestBase;
+
+/**
+ * Tests the Item entity's cache tags.
+ */
+class ItemCacheTagsTest extends EntityCacheTagsTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public static $modules = array('aggregator');
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function getInfo() {
+    return parent::generateStandardizedInfo('Aggregator feed item', 'Aggregator');
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function setUp() {
+    parent::setUp();
+
+    // Give anonymous users permission to access feeds, so that we can verify
+    // the cache tags of cached versions of feed items.
+    $user_role = entity_load('user_role', DRUPAL_ANONYMOUS_RID);
+    $user_role->grantPermission('access news feeds');
+    $user_role->save();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function createEntity() {
+    // Create a "Camelids" feed.
+    $feed = Feed::create(array(
+      'title' => 'Camelids',
+      'url' => 'https://groups.drupal.org/not_used/167169',
+      'refresh' => 900,
+      'checked' => 1389919932,
+      'description' => 'Drupal Core Group feed',
+    ));
+    $feed->save();
+
+    // Create a "Llama" aggregator feed item.
+    $item = Item::create(array(
+      'fid' => $feed->id(),
+      'title' => t('Llama'),
+      'path' => 'https://www.drupal.org/',
+    ));
+    $item->save();
+
+    return $item;
+  }
+
+  /**
+   * Tests that when creating a feed item, the feed tag is invalidated.
+   */
+  public function testEntityCreation() {
+    // Create a cache entry that is tagged with a feed cache tag.
+    \Drupal::cache('render')->set('foo', 'bar', \Drupal\Core\Cache\CacheBackendInterface::CACHE_PERMANENT, $this->entity->getCacheTag());
+
+    // Verify a cache hit.
+    $this->verifyRenderCache('foo', array('aggregator_feed:1'));
+
+    // Now create a feed item in that feed.
+    Item::create(array(
+      'fid' => $this->entity->getFeedId(),
+      'title' => t('Llama 2'),
+      'path' => 'https://groups.drupal.org/',
+    ))->save();
+
+    // Verify a cache miss.
+    $this->assertFalse(\Drupal::cache('render')->get('foo'), 'Creating a new feed item invalidates the cache tag of the feed.');
+  }
+
+}

core/modules/aggregator/src/Tests/Views/IntegrationTest.php

@@ -62,10 +62,20 @@ protected function setUp() {
    * Tests basic aggregator_item view.
    */
   public function testAggregatorItemView() {
+    $feed = $this->feedStorage->create(array(
+      'title' => $this->randomName(),
+      'url' => 'http://drupal.org/',
+      'refresh' => 900,
+      'checked' => 123543535,
+      'description' => $this->randomName(),
+    ));
+    $feed->save();
+
     $items = array();
     $expected = array();
     for ($i = 0; $i < 10; $i++) {
       $values = array();
+      $values['fid'] = $feed->id();
       $values['timestamp'] = mt_rand(REQUEST_TIME - 10, REQUEST_TIME + 10);
       $values['title'] = $this->randomName();
       $values['description'] = $this->randomName();

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

@@ -139,6 +139,32 @@ protected function getAdditionalCacheTagsForEntity(EntityInterface $entity) {
     return array();
   }
 
+  /**
+   * Selects the preferred view mode for the given entity type.
+   *
+   * Prefers 'full', picks the first one otherwise, and if none are available,
+   * chooses 'default'.
+   */
+  protected function selectViewMode($entity_type) {
+    $view_modes = \Drupal::entityManager()
+      ->getStorage('view_mode')
+      ->loadByProperties(array('targetEntityType' => $entity_type));
+
+    if (empty($view_modes)) {
+      return 'default';
+    }
+    else {
+      // Prefer the "full" display mode.
+      if (isset($view_modes[$entity_type . '.full'])) {
+        return 'full';
+      }
+      else {
+        $view_modes = array_keys($view_modes);
+        return substr($view_modes[0], strlen($entity_type) + 1);
+      }
+    }
+  }
+
   /**
    * Creates a referencing and a non-referencing entity for testing purposes.
    *
@@ -182,13 +208,24 @@ protected function createReferenceTestEntities($referenced_entity) {
         ),
       ),
     ))->save();
-    $formatter = 'entity_reference_entity_view';
     if (!$this->entity->getEntityType()->hasControllerClass('view_builder')) {
-      $formatter = 'entity_reference_label';
+      entity_get_display($entity_type, $bundle, 'full')
+        ->setComponent($field_name, array(
+          'type' => 'entity_reference_label',
+        ))
+        ->save();
+    }
+    else {
+      $referenced_entity_view_mode = $this->selectViewMode($this->entity->getEntityTypeId());
+      entity_get_display($entity_type, $bundle, 'full')
+        ->setComponent($field_name, array(
+          'type' => 'entity_reference_entity_view',
+          'settings' => array(
+            'view_mode' => $referenced_entity_view_mode,
+          ),
+        ))
+        ->save();
     }
-    entity_get_display($entity_type, $bundle, 'full')
-      ->setComponent($field_name, array('type' => $formatter))
-      ->save();
 
     // Create an entity that does reference the entity being tested.
     $label_key = \Drupal::entityManager()->getDefinition($entity_type)->getKey('label');
@@ -326,11 +363,12 @@ public function testReferencedEntity() {
 
 
     if ($this->entity->getEntityType()->hasControllerClass('view_builder')) {
-      // Verify that after modifying the entity's "full" display, there is a cache
-      // miss for both the referencing entity, and the listing of referencing
+      // Verify that after modifying the entity's display, there is a cache miss
+      // for both the referencing entity, and the listing of referencing
       // entities, but not for the non-referencing entity.
-      $this->pass("Test modification of referenced entity's 'full' display.", 'Debug');
-      $entity_display = entity_get_display($entity_type, $this->entity->bundle(), 'full');
+      $referenced_entity_view_mode = $this->selectViewMode($this->entity->getEntityTypeId());
+      $this->pass("Test modification of referenced entity's '$referenced_entity_view_mode' display.", 'Debug');
+      $entity_display = entity_get_display($entity_type, $this->entity->bundle(), $referenced_entity_view_mode);
       $entity_display->save();
       $this->verifyPageCache($referencing_entity_path, 'MISS');
       $this->verifyPageCache($listing_path, 'MISS');

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

@@ -8,7 +8,6 @@
 namespace Drupal\system\Tests\Entity;
 
 use Drupal\Core\Cache\Cache;
-use Drupal\system\Tests\Entity\EntityCacheTagsTestBase;
 
 /**
  * Provides helper methods for Entity cache tags tests; for entities with URIs.
@@ -26,6 +25,9 @@ public function testEntityUri() {
     $entity_path = $this->entity->getSystemPath();
     $entity_type = $this->entity->getEntityTypeId();
 
+    // Selects the view mode that will be used.
+    $view_mode = $this->selectViewMode($entity_type);
+
     // Generate the standardized entity cache tags.
     $cache_tag = $entity_type . ':' . $this->entity->id();
     $view_cache_tag = $entity_type . '_view:1';
@@ -41,7 +43,7 @@ public function testEntityUri() {
     // Also verify the existence of an entity render cache entry, if this entity
     // type supports render caching.
     if (\Drupal::entityManager()->getDefinition($entity_type)->isRenderCacheable()) {
-      $cid = 'entity_view:' . $entity_type . ':' . $this->entity->id() . ':full:stark:r.anonymous:' . date_default_timezone_get();
+      $cid = 'entity_view:' . $entity_type . ':' . $this->entity->id() . ':' . $view_mode . ':stark:r.anonymous:' . date_default_timezone_get();
       $cache_entry = \Drupal::cache('render')->get($cid);
       $expected_cache_tags = array_merge(array($view_cache_tag, $cache_tag), $this->getAdditionalCacheTagsForEntity($this->entity), array($render_cache_tag));
       $this->assertIdentical($cache_entry->tags, $expected_cache_tags);
@@ -56,10 +58,9 @@ public function testEntityUri() {
     $this->verifyPageCache($entity_path, 'HIT');
 
 
-    // Verify that after modifying the entity's "full" display, there is a cache
-    // miss.
-    $this->pass("Test modification of entity's 'full' display.", 'Debug');
-    $entity_display = entity_get_display($entity_type, $this->entity->bundle(), 'full');
+    // Verify that after modifying the entity's display, there is a cache miss.
+    $this->pass("Test modification of entity's '$view_mode' display.", 'Debug');
+    $entity_display = entity_get_display($entity_type, $this->entity->bundle(), $view_mode);
     $entity_display->save();
     $this->verifyPageCache($entity_path, 'MISS');