.../Core/Config/Entity/ConfigEntityStorage.php | 32 +++++++++++++--------- .../CacheabilityMetadataConfigOverrideTest.php | 12 ++++++++ .../user/src/Tests/UserTokenReplaceTest.php | 6 ++++ core/modules/views/src/Tests/Plugin/CacheTest.php | 2 +- 4 files changed, 38 insertions(+), 14 deletions(-) diff --git a/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php b/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php index 401b19b..8e3ccf8 100644 --- a/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php +++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php @@ -8,6 +8,7 @@ namespace Drupal\Core\Config\Entity; use Drupal\Component\Utility\SafeMarkup; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Config\ConfigImporterException; use Drupal\Core\Entity\EntityInterface; @@ -197,19 +198,24 @@ protected function doLoadMultiple(array $ids = NULL) { // the cacheability metadata of config objects (to ensure e.g. additional // cacheability metadata added by config overrides is not lost). foreach ($entities as $id => $entity) { - $entity->addCacheableDependency($configs[$id]); - - // Remove the self-referring cache tag that is present on Config objects - // before setting it. A ConfigEntity doesn't need this since it will be - // dynamically generated in EntityInterface::getCacheTagsToInvalidate(). - // The cache tags are merged during rendering, and having fewer tags - // available improves performance. - $cache_tags = $configs[$id]->getCacheTags(); - $key = array_search('config:' . $configs[$id]->getName(), $cache_tags); - if ($key !== FALSE) { - unset($cache_tags[$key]); - } - $entity->addCacheTags($cache_tags); + // But rather than simply inheriting all cacheability metadata of config + // objects, we need to make sure the self-referring cache tag that is + // present on Config objects is not added to the Config entity. It must be + // removed for 3 reasons: + // 1. When renaming/duplicating a Config entity, the cache tag of the + // original config object would remain present, which would be wrong. + // 2. Some Config entities choose to not use the cache tag that the under- + // lying Config object provides by default (For performance and + // cacheability reasons it may not make sense to have a unique cache + // tag for every Config entity. The DateFormat Config entity specifies + // the 'rendered' cache tag for example, because A) date formats are + // changed extremely rarely, so invalidating all render cache items is + // fine, B) it means fewer cache tags per page.). + // 3. Fewer cache tags is better for performance. + $self_referring_cache_tag = ['config:' . $configs[$id]->getName()]; + $config_cacheability = CacheableMetadata::createFromObject($configs[$id]); + $config_cacheability->setCacheTags(array_diff($config_cacheability->getCacheTags(), $self_referring_cache_tag)); + $entity->addCacheableDependency($config_cacheability); } return $entities; diff --git a/core/modules/config/src/Tests/CacheabilityMetadataConfigOverrideTest.php b/core/modules/config/src/Tests/CacheabilityMetadataConfigOverrideTest.php index cc6e024..3e99d1b 100644 --- a/core/modules/config/src/Tests/CacheabilityMetadataConfigOverrideTest.php +++ b/core/modules/config/src/Tests/CacheabilityMetadataConfigOverrideTest.php @@ -79,6 +79,18 @@ public function testConfigEntityOverride() { $this->assertEqual(['pirate_day'], $block->getCacheContexts()); $this->assertEqual(['config:block.block.call_to_action', 'pirate-day-tag'], $block->getCacheTags()); $this->assertEqual(PirateDayCacheContext::PIRATE_DAY_MAX_AGE, $block->getCacheMaxAge()); + + // Check that duplicating a config entity does not have the original config + // entity's cache tag. + $this->assertEqual(['config:block.block.', 'pirate-day-tag'], $block->createDuplicate()->getCacheTags()); + + // Check that renaming a config entity does not have the original config + // entity's cache tag. + $block->setOriginalId('call_to_looting')->save(); + $this->assertEqual(['pirate_day'], $block->getCacheContexts()); + // @todo Uncomment once https://www.drupal.org/node/2539116 lands. +// $this->assertEqual(['config:block.block.call_to_looting', 'pirate-day-tag'], $block->getCacheTags()); + $this->assertEqual(PirateDayCacheContext::PIRATE_DAY_MAX_AGE, $block->getCacheMaxAge()); } } diff --git a/core/modules/user/src/Tests/UserTokenReplaceTest.php b/core/modules/user/src/Tests/UserTokenReplaceTest.php index 197684a..508275c 100644 --- a/core/modules/user/src/Tests/UserTokenReplaceTest.php +++ b/core/modules/user/src/Tests/UserTokenReplaceTest.php @@ -75,6 +75,12 @@ function testUserTokenReplacement() { $metadata_tests['[user:url]'] = $base_bubbleable_metadata; $metadata_tests['[user:edit-url]'] = $base_bubbleable_metadata; $bubbleable_metadata = clone $base_bubbleable_metadata; + // This test runs with the Language module enabled, which means config is + // overridden by LanguageConfigFactoryOverride (to provide translations of + // config). This causes the interface language cache context to be added for + // config entities. The four next tokens use DateFormat Config entities, and + // therefore have the interface language cache context. + $bubbleable_metadata->addCacheContexts(['languages:language_interface']); $metadata_tests['[user:last-login]'] = $bubbleable_metadata->addCacheTags(['rendered']); $metadata_tests['[user:last-login:short]'] = $bubbleable_metadata; $metadata_tests['[user:created]'] = $bubbleable_metadata; diff --git a/core/modules/views/src/Tests/Plugin/CacheTest.php b/core/modules/views/src/Tests/Plugin/CacheTest.php index f8c4d1d..0d8c907 100644 --- a/core/modules/views/src/Tests/Plugin/CacheTest.php +++ b/core/modules/views/src/Tests/Plugin/CacheTest.php @@ -299,7 +299,7 @@ function testHeaderStorage() { $this->assertTrue(in_array('views_test_data/test', $output['#attached']['library']), 'Make sure libraries are added for cached views.'); $this->assertEqual(['foo' => 'bar'], $output['#attached']['drupalSettings'], 'Make sure drupalSettings are added for cached views.'); // Note: views_test_data_views_pre_render() adds some cache tags. - $this->assertEqual(['config:views.view.test_cache_header_storage', 'config:views.view.test_view', 'views_test_data:1'], $output['#cache']['tags']); + $this->assertEqual(['config:views.view.test_cache_header_storage', 'views_test_data:1'], $output['#cache']['tags']); $this->assertEqual(['non-existing-placeholder-just-for-testing-purposes' => ['#lazy_builder' => ['views_test_data_placeholders', ['bar']]]], $output['#attached']['placeholders']); $this->assertFalse(!empty($view->build_info['pre_render_called']), 'Make sure hook_views_pre_render is not called for the cached view.'); }