core/modules/shortcut/shortcut.module | 7 ++ core/modules/shortcut/src/Entity/Shortcut.php | 45 ++++++-- core/modules/shortcut/src/Entity/ShortcutSet.php | 6 +- core/modules/shortcut/src/ShortcutForm.php | 8 +- .../shortcut/src/Tests/ShortcutCacheTagsTest.php | 76 +++++++++++++ .../Tests/Entity/EntityCacheTagsTestBase.php | 122 +++++++++++++-------- 6 files changed, 202 insertions(+), 62 deletions(-) diff --git a/core/modules/shortcut/shortcut.module b/core/modules/shortcut/shortcut.module index 9196fd2..81b6d19 100644 --- a/core/modules/shortcut/shortcut.module +++ b/core/modules/shortcut/shortcut.module @@ -5,6 +5,7 @@ * Allows users to manage customizable lists of shortcut links. */ +use Drupal\Component\Utility\NestedArray; use Drupal\Component\Utility\UrlHelper; use Drupal\Core\Routing\UrlMatcher; use Drupal\Core\Url; @@ -299,12 +300,15 @@ function shortcut_renderable_links($shortcut_set = NULL) { $shortcut_set = shortcut_current_displayed_set(); } + /** @var \Drupal\shortcut\ShortcutInterface[] $shortcuts */ $shortcuts = \Drupal::entityManager()->getStorage('shortcut')->loadByProperties(array('shortcut_set' => $shortcut_set->id())); + $all_cache_tags = array(); foreach ($shortcuts as $shortcut) { $links[] = array( 'title' => $shortcut->label(), 'href' => $shortcut->path->value, ); + $all_cache_tags[] = $shortcut->getCacheTag(); } if (!empty($links)) { @@ -314,6 +318,9 @@ function shortcut_renderable_links($shortcut_set = NULL) { '#attributes' => array( 'class' => array('menu'), ), + '#cache' => array( + 'tags' => NestedArray::mergeDeepArray($all_cache_tags), + ), ); } diff --git a/core/modules/shortcut/src/Entity/Shortcut.php b/core/modules/shortcut/src/Entity/Shortcut.php index 71a33e4..200ecf8 100644 --- a/core/modules/shortcut/src/Entity/Shortcut.php +++ b/core/modules/shortcut/src/Entity/Shortcut.php @@ -7,11 +7,11 @@ namespace Drupal\shortcut\Entity; +use Drupal\Core\Cache\Cache; use Drupal\Core\Entity\ContentEntityBase; use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Field\FieldDefinition; -use Drupal\Core\Url; use Drupal\shortcut\ShortcutInterface; /** @@ -25,6 +25,7 @@ * "form" = { * "default" = "Drupal\shortcut\ShortcutForm", * "add" = "Drupal\shortcut\ShortcutForm", + * "edit" = "Drupal\shortcut\ShortcutForm", * "delete" = "Drupal\shortcut\Form\ShortcutDeleteForm" * }, * "translation" = "Drupal\content_translation\ContentTranslationHandler" @@ -39,9 +40,12 @@ * "label" = "title" * }, * links = { + * "canonical" = "shortcut.link_edit", * "delete-form" = "shortcut.link_delete", - * "edit-form" = "shortcut.link_edit" - * } + * "edit-form" = "shortcut.link_edit", + * "admin-form" = "shortcut.link_edit" + * }, + * bundle_entity_type = "shortcut_set" * ) */ class Shortcut extends ContentEntityBase implements ShortcutInterface { @@ -121,12 +125,16 @@ public static function preCreate(EntityStorageInterface $storage, array &$values /** * {@inheritdoc} */ - public function preSave(EntityStorageInterface $storage) { - parent::preSave($storage); - - $url = Url::createFromPath($this->path->value); - $this->setRouteName($url->getRouteName()); - $this->setRouteParams($url->getRouteParameters()); + public function postSave(EntityStorageInterface $storage, $update = TRUE) { + parent::postSave($storage, $update); + + // Entity::postSave() calls Entity::invalidateTagsOnSave(), which only + // handles the regular cases. The Shortcut entity has one special case: a + // newly created shortcut is *also* added to a shortcut set, so we must + // invalidate the associated shortcut set's cache tag. + if (!$update) { + Cache::invalidateTags($this->getCacheTag()); + } } /** @@ -199,4 +207,23 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) { return $fields; } + /** + * {@inheritdoc} + */ + public function getCacheTag() { + $field_value = $this->get('shortcut_set') + ->getValue(TRUE); + $shortcut_set = $field_value[0]['entity']; + return $shortcut_set->getCacheTag(); + } + + /** + * {@inheritdoc} + */ + public function getListCacheTags() { + $field_value = $this->get('shortcut_set')->getValue(TRUE); + $shortcut_set = $field_value[0]['entity']; + return $shortcut_set->getListCacheTags(); + } + } diff --git a/core/modules/shortcut/src/Entity/ShortcutSet.php b/core/modules/shortcut/src/Entity/ShortcutSet.php index f9338d9..32c1bb6 100644 --- a/core/modules/shortcut/src/Entity/ShortcutSet.php +++ b/core/modules/shortcut/src/Entity/ShortcutSet.php @@ -60,11 +60,11 @@ class ShortcutSet extends ConfigEntityBase implements ShortcutSetInterface { /** * {@inheritdoc} */ - public function postCreate(EntityStorageInterface $storage) { - parent::postCreate($storage); + public function postSave(EntityStorageInterface $storage, $update = TRUE) { + parent::postSave($storage, $update); // Generate menu-compatible set name. - if (!$this->getOriginalId()) { + if (!$update && !$this->getOriginalId()) { // Save a new shortcut set with links copied from the user's default set. $default_set = shortcut_default_set(); foreach ($default_set->getShortcuts() as $shortcut) { diff --git a/core/modules/shortcut/src/ShortcutForm.php b/core/modules/shortcut/src/ShortcutForm.php index 0e101ff..4764ddc 100644 --- a/core/modules/shortcut/src/ShortcutForm.php +++ b/core/modules/shortcut/src/ShortcutForm.php @@ -9,6 +9,7 @@ use Drupal\Core\Entity\ContentEntityForm; use Drupal\Core\Language\Language; +use Drupal\Core\Url; /** * Form controller for the shortcut entity forms. @@ -53,9 +54,10 @@ public function form(array $form, array &$form_state) { public function buildEntity(array $form, array &$form_state) { $entity = parent::buildEntity($form, $form_state); - // Set the computed 'path' value so it can used in the preSave() method to - // derive the route name and parameters. - $entity->path->value = $form_state['values']['path']; + // Transform the (computed) 'path' value to a route name and parameters. + $url = Url::createFromPath($form_state['values']['path']); + $entity->setRouteName($url->getRouteName()); + $entity->setRouteParams($url->getRouteParameters()); return $entity; } diff --git a/core/modules/shortcut/src/Tests/ShortcutCacheTagsTest.php b/core/modules/shortcut/src/Tests/ShortcutCacheTagsTest.php new file mode 100644 index 0000000..6178538 --- /dev/null +++ b/core/modules/shortcut/src/Tests/ShortcutCacheTagsTest.php @@ -0,0 +1,76 @@ +grantPermission('customize shortcut links'); + $user_role->save(); + } + + /** + * {@inheritdoc} + */ + protected function createEntity() { + // Create a "Llama" shortcut. + $shortcut = entity_create('shortcut', array( + 'set' => 'default', + 'title' => t('Llama'), + 'weight' => 0, + 'path' => '', + )); + $shortcut->save(); + + return $shortcut; + } + + /** + * Tests that when creating a shortcut, the shortcut set tag is invalidated. + */ + public function testEntityCreation() { + // Create a cache entry that is tagged with a shortcut set cache tag. + $cache_tags = array('shortcut_set' => 'default'); + \Drupal::cache('render')->set('foo', 'bar', \Drupal\Core\Cache\CacheBackendInterface::CACHE_PERMANENT, $cache_tags); + + // Verify a cache hit. + $this->verifyRenderCache('foo', array('shortcut_set:default')); + + // Now create a shortcut entity in that shortcut set. + $this->createEntity(); + + // Verify a cache miss. + $this->assertFalse(\Drupal::cache('render')->get('foo'), 'Creating a new shortcut invalidates the cache tag of the shortcut set.'); + } + +} diff --git a/core/modules/system/lib/Drupal/system/Tests/Entity/EntityCacheTagsTestBase.php b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityCacheTagsTestBase.php index 7e6506b..2326794 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Entity/EntityCacheTagsTestBase.php +++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityCacheTagsTestBase.php @@ -7,9 +7,10 @@ namespace Drupal\system\Tests\Entity; -use Drupal\Component\Utility\String; +use Drupal\Component\Utility\NestedArray; use Drupal\Core\Cache\Cache; use Drupal\Core\Entity\EntityInterface; +use Drupal\Core\EventSubscriber\HtmlViewSubscriber; use Drupal\Core\Field\FieldDefinitionInterface; use Drupal\system\Tests\Cache\PageCacheTagsTestBase; @@ -182,8 +183,12 @@ 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_entity_view')) + ->setComponent($field_name, array('type' => $formatter)) ->save(); // Create an entity that does reference the entity being tested. @@ -227,23 +232,27 @@ public function testReferencedEntity() { $theme_cache_tags = array('content:1', 'theme:stark', 'theme_global_settings:1'); - // Generate the standardized entity cache tags. - $cache_tag = $entity_type . ':' . $this->entity->id(); - $view_cache_tag = $entity_type . '_view:1'; + $view_cache_tag = array(); + if ($this->entity->getEntityType()->hasControllerClass('view_builder')) { + $view_cache_tag = \Drupal::entityManager()->getViewBuilder($entity_type) + ->getCacheTag(); + } // Generate the cache tags for the (non) referencing entities. - $referencing_entity_cache_tags = array( - 'entity_test_view:1', - 'entity_test:' . $this->referencing_entity->id(), + $referencing_entity_cache_tags = NestedArray::mergeDeep( + $this->referencing_entity->getCacheTag(), + \Drupal::entityManager()->getViewBuilder('entity_test')->getCacheTag(), // Includes the main entity's cache tags, since this entity references it. - $view_cache_tag, - $cache_tag, + $this->entity->getCacheTag(), + $view_cache_tag ); + $referencing_entity_cache_tags = explode(' ', HtmlViewSubscriber::convertCacheTagsToHeader($referencing_entity_cache_tags)); $referencing_entity_cache_tags = array_merge($referencing_entity_cache_tags, $this->getAdditionalCacheTagsForEntity($this->entity)); - $non_referencing_entity_cache_tags = array( - 'entity_test_view:1', - 'entity_test:' . $this->non_referencing_entity->id(), + $non_referencing_entity_cache_tags = NestedArray::mergeDeep( + $this->non_referencing_entity->getCacheTag(), + \Drupal::entityManager()->getViewBuilder('entity_test')->getCacheTag() ); + $non_referencing_entity_cache_tags = explode(' ', HtmlViewSubscriber::convertCacheTagsToHeader($non_referencing_entity_cache_tags)); // Prime the page cache for the referencing entity. @@ -255,9 +264,7 @@ public function testReferencedEntity() { // Also verify the existence of an entity render cache entry. $cid = 'entity_view:entity_test:' . $this->referencing_entity->id() . ':full:stark:r.anonymous'; - $cache_entry = \Drupal::cache('render')->get($cid); - $this->assertIdentical($cache_entry->tags, $referencing_entity_cache_tags); - + $this->verifyRenderCache($cid, $referencing_entity_cache_tags); // Prime the page cache for the non-referencing entity. $this->verifyPageCache($non_referencing_entity_path, 'MISS'); @@ -268,8 +275,7 @@ public function testReferencedEntity() { // Also verify the existence of an entity render cache entry. $cid = 'entity_view:entity_test:' . $this->non_referencing_entity->id() . ':full:stark:r.anonymous'; - $cache_entry = \Drupal::cache('render')->get($cid); - $this->assertIdentical($cache_entry->tags, $non_referencing_entity_cache_tags); + $this->verifyRenderCache($cid, $non_referencing_entity_cache_tags); @@ -322,19 +328,21 @@ public function testReferencedEntity() { $this->verifyPageCache($non_referencing_entity_path, 'HIT'); - // Verify that after modifying the entity's "full" 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'); - $entity_display->save(); - $this->verifyPageCache($referencing_entity_path, 'MISS'); - $this->verifyPageCache($listing_path, 'MISS'); - $this->verifyPageCache($non_referencing_entity_path, 'HIT'); + 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 + // 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'); + $entity_display->save(); + $this->verifyPageCache($referencing_entity_path, 'MISS'); + $this->verifyPageCache($listing_path, 'MISS'); + $this->verifyPageCache($non_referencing_entity_path, 'HIT'); - // Verify cache hits. - $this->verifyPageCache($referencing_entity_path, 'HIT'); - $this->verifyPageCache($listing_path, 'HIT'); + // Verify cache hits. + $this->verifyPageCache($referencing_entity_path, 'HIT'); + $this->verifyPageCache($listing_path, 'HIT'); + } $bundle_entity_type = $this->entity->getEntityType()->getBundleEntityType(); @@ -391,7 +399,7 @@ public function testReferencedEntity() { // a cache miss for both the referencing entity, and the listing of // referencing entities, but not for the non-referencing entity. $this->pass("Test invalidation of referenced entity's cache tag.", 'Debug'); - Cache::invalidateTags(array($entity_type => array($this->entity->id()))); + Cache::invalidateTags($this->entity->getCacheTag()); $this->verifyPageCache($referencing_entity_path, 'MISS'); $this->verifyPageCache($listing_path, 'MISS'); $this->verifyPageCache($non_referencing_entity_path, 'HIT'); @@ -401,19 +409,20 @@ public function testReferencedEntity() { $this->verifyPageCache($listing_path, 'HIT'); - // Verify that after invalidating the generic entity type's view cache tag - // directly, 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 invalidation of referenced entity's 'view' cache tag.", 'Debug'); - Cache::invalidateTags(array($entity_type . '_view' => TRUE)); - $this->verifyPageCache($referencing_entity_path, 'MISS'); - $this->verifyPageCache($listing_path, 'MISS'); - $this->verifyPageCache($non_referencing_entity_path, 'HIT'); - - // Verify cache hits. - $this->verifyPageCache($referencing_entity_path, 'HIT'); - $this->verifyPageCache($listing_path, 'HIT'); + if (!empty($view_cache_tag)) { + // Verify that after invalidating the generic entity type's view cache tag + // directly, 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 invalidation of referenced entity's 'view' cache tag.", 'Debug'); + Cache::invalidateTags($view_cache_tag); + $this->verifyPageCache($referencing_entity_path, 'MISS'); + $this->verifyPageCache($listing_path, 'MISS'); + $this->verifyPageCache($non_referencing_entity_path, 'HIT'); + // Verify cache hits. + $this->verifyPageCache($referencing_entity_path, 'HIT'); + $this->verifyPageCache($listing_path, 'HIT'); + } // Verify that after deleting the entity, there is a cache miss for both the // referencing entity, and the listing of referencing entities, but not for @@ -425,12 +434,31 @@ public function testReferencedEntity() { $this->verifyPageCache($non_referencing_entity_path, 'HIT'); // Verify cache hits. - $tags = array_merge($theme_cache_tags, array( - 'entity_test_view:1', - 'entity_test:' . $this->referencing_entity->id(), - )); + $referencing_entity_cache_tags = NestedArray::mergeDeep( + $this->referencing_entity->getCacheTag(), + \Drupal::entityManager()->getViewBuilder('entity_test')->getCacheTag() + ); + $referencing_entity_cache_tags = explode(' ', HtmlViewSubscriber::convertCacheTagsToHeader($referencing_entity_cache_tags)); + $tags = array_merge($theme_cache_tags, $referencing_entity_cache_tags); $this->verifyPageCache($referencing_entity_path, 'HIT', $tags); $this->verifyPageCache($listing_path, 'HIT', $theme_cache_tags); } + /** + * Verify that a given render cache entry exists, with the correct cache tags. + * + * @param string $cid + * The render cache item ID. + * @param array $tags + * An array of expected cache tags. + */ + protected function verifyRenderCache($cid, array $tags) { + // Also verify the existence of an entity render cache entry. + $cache_entry = \Drupal::cache('render')->get($cid); + $this->assertTrue($cache_entry, 'A render cache entry exists.'); + sort($cache_entry->tags); + sort($tags); + $this->assertIdentical($cache_entry->tags, $tags); + } + }