From a9ce260cfa0c9f26e2541b781f1b5d5dd6e8e1da Mon Sep 17 00:00:00 2001
From: Sascha Grossenbacher <saschagros@gmail.com>
Date: Tue, 1 Apr 2025 17:59:47 +0200
Subject: [PATCH 1/4] applied patch
---
core/core.services.yml | 1 +
.../CacheContextOptimizableInterface.php | 39 ++++++
.../Cache/Context/CacheContextsManager.php | 18 ++-
.../Cache/Context/TimeZoneCacheContext.php | 40 ++++++-
.../Cache/Context/UserRolesCacheContext.php | 16 ++-
.../BlockContentTranslationUITest.php | 1 -
.../Functional/CommentTranslationUITest.php | 1 -
.../CommentDefaultFormatterCacheTagsTest.php | 2 +
.../tests/src/Functional/HistoryTest.php | 3 +-
.../MenuLinkContentTranslationUITest.php | 2 +-
.../tests/src/Functional/MenuUiNodeTest.php | 3 +-
.../NodeCacheTagsNoTimezoneTest.php | 45 +++++++
.../src/Functional/Routing/RouterTest.php | 6 +-
.../Functional/System/AccessDeniedTest.php | 6 +-
.../Functional/System/PageNotFoundTest.php | 6 +-
.../src/Functional/Views/AccessRoleTest.php | 19 +--
.../Context/CacheContextsManagerTest.php | 113 ++++++++++++++++++
17 files changed, 297 insertions(+), 24 deletions(-)
create mode 100644 core/lib/Drupal/Core/Cache/Context/CacheContextOptimizableInterface.php
create mode 100644 core/modules/node/tests/src/Functional/NodeCacheTagsNoTimezoneTest.php
diff --git a/core/core.services.yml b/core/core.services.yml
index 55d8d2ebc984..eb6a576d257e 100644
--- a/core/core.services.yml
+++ b/core/core.services.yml
@@ -205,6 +205,7 @@ services:
- { name: cache.context}
cache_context.timezone:
class: Drupal\Core\Cache\Context\TimeZoneCacheContext
+ arguments: ['@config.factory']
tags:
- { name: cache.context}
diff --git a/core/lib/Drupal/Core/Cache/Context/CacheContextOptimizableInterface.php b/core/lib/Drupal/Core/Cache/Context/CacheContextOptimizableInterface.php
new file mode 100644
index 000000000000..c1eeecc25db7
--- /dev/null
+++ b/core/lib/Drupal/Core/Cache/Context/CacheContextOptimizableInterface.php
@@ -0,0 +1,39 @@
+<?php
+
+namespace Drupal\Core\Cache\Context;
+
+/**
+ * Allows cache contexts to influence how cache contexts are optimized.
+ */
+interface CacheContextOptimizableInterface {
+
+ /**
+ * Returns whether a cache context value has variations.
+ *
+ * For example, the timezone cache context can define itself as global
+ * if a site does not have configurable timezones and the language contexts
+ * are global on a site that is not multilingual.
+ *
+ * @return bool
+ * TRUE if the context has more than one possible variations.
+ */
+ public function hasVariations();
+
+ /**
+ * Returns parent contexts for this context.
+ *
+ * By default, parents are derived based on the cache context name, user is
+ * a parent of user.permissions. This allows e.g. the user.roles context to
+ * expand on this and indicate that user.permissions is also a valid
+ * parent/replacement.
+ *
+ * Note that if implementing this, it is necessary to also include parents of
+ * the parent contexts, e.g. user.roles must provide both user.permissions and
+ * user.
+ *
+ * @return array
+ * A list of parents cache contexts.
+ */
+ public function getParentContexts();
+
+}
diff --git a/core/lib/Drupal/Core/Cache/Context/CacheContextsManager.php b/core/lib/Drupal/Core/Cache/Context/CacheContextsManager.php
index 91d1b3ab9e99..7a7b9979882a 100644
--- a/core/lib/Drupal/Core/Cache/Context/CacheContextsManager.php
+++ b/core/lib/Drupal/Core/Cache/Context/CacheContextsManager.php
@@ -172,6 +172,22 @@ public function optimizeTokens(array $context_tokens) {
[$context_id, $parameter] = explode(':', $context_token);
}
+ // Cache contexts can explicitly provide a list of parents or indicate
+ // that they are global.
+ // @todo Also support to optimize them away if a parent of those is
+ // provided.
+ $service = $this->getService($context_id);
+ if ($service instanceof CacheContextOptimizableInterface) {
+ if (!$service->hasVariations()) {
+ continue;
+ }
+
+ if (array_intersect($context_tokens, $service->getParentContexts())) {
+ // If there is at least one parent context available, skip this one.
+ continue;
+ }
+ }
+
// Context tokens without:
// - a period means they don't have a parent
// - a colon means they're not a specific value of a cache context
@@ -181,7 +197,7 @@ public function optimizeTokens(array $context_tokens) {
}
// Check cacheability. If the context defines a max-age of 0, then it
// can not be optimized away. Pass the parameter along if we have one.
- elseif ($this->getService($context_id)->getCacheableMetadata($parameter)->getCacheMaxAge() === 0) {
+ elseif ($service->getCacheableMetadata($parameter)->getCacheMaxAge() === 0) {
$optimized_content_tokens[] = $context_token;
}
// The context token has a period or a colon. Iterate over all ancestor
diff --git a/core/lib/Drupal/Core/Cache/Context/TimeZoneCacheContext.php b/core/lib/Drupal/Core/Cache/Context/TimeZoneCacheContext.php
index 318817264cc7..4cddaa09f330 100644
--- a/core/lib/Drupal/Core/Cache/Context/TimeZoneCacheContext.php
+++ b/core/lib/Drupal/Core/Cache/Context/TimeZoneCacheContext.php
@@ -3,6 +3,7 @@
namespace Drupal\Core\Cache\Context;
use Drupal\Core\Cache\CacheableMetadata;
+use Drupal\Core\Config\ConfigFactoryInterface;
/**
* Defines the TimeZoneCacheContext service, for "per time zone" caching.
@@ -11,7 +12,24 @@
*
* @see \Drupal\Core\Session\AccountProxy::setAccount()
*/
-class TimeZoneCacheContext implements CacheContextInterface {
+class TimeZoneCacheContext implements CacheContextInterface, CacheContextOptimizableInterface {
+
+ /**
+ * The config factory.
+ *
+ * @var \Drupal\Core\Config\ConfigFactoryInterface
+ */
+ protected $configFactory;
+
+ /**
+ * Constructor for the TimeZoneCacheContext object.
+ *
+ * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
+ * The config factory.
+ */
+ public function __construct(ConfigFactoryInterface $configFactory) {
+ $this->configFactory = $configFactory;
+ }
/**
* {@inheritdoc}
@@ -33,7 +51,25 @@ public function getContext() {
* {@inheritdoc}
*/
public function getCacheableMetadata() {
- return new CacheableMetadata();
+ $cacheability_metadata = new CacheableMetadata();
+ $cacheability_metadata->addCacheableDependency($this->configFactory->get('system.date'));
+ return $cacheability_metadata;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function hasVariations() {
+ // The timezone context can not have different values if the site does not
+ // use configurable timezones.
+ return (bool) $this->configFactory->get('system.date')->get('timezone.user.configurable');
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function getParentContexts() {
+ return [];
}
}
diff --git a/core/lib/Drupal/Core/Cache/Context/UserRolesCacheContext.php b/core/lib/Drupal/Core/Cache/Context/UserRolesCacheContext.php
index cc978750c4da..18080d06d55a 100644
--- a/core/lib/Drupal/Core/Cache/Context/UserRolesCacheContext.php
+++ b/core/lib/Drupal/Core/Cache/Context/UserRolesCacheContext.php
@@ -14,7 +14,7 @@
* Calculated cache context ID: 'user.roles:%role', e.g. 'user.roles:anonymous'
* (to vary by the presence/absence of a specific role).
*/
-class UserRolesCacheContext extends UserCacheContextBase implements CalculatedCacheContextInterface {
+class UserRolesCacheContext extends UserCacheContextBase implements CalculatedCacheContextInterface, CacheContextOptimizableInterface {
/**
* {@inheritdoc}
@@ -41,4 +41,18 @@ public function getCacheableMetadata($role = NULL) {
return (new CacheableMetadata())->setCacheTags(['user:' . $this->user->id()]);
}
+ /**
+ * {@inheritdoc}
+ */
+ public function hasVariations() {
+ return TRUE;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function getParentContexts() {
+ return ['user', 'user.permissions'];
+ }
+
}
diff --git a/core/modules/block_content/tests/src/Functional/BlockContentTranslationUITest.php b/core/modules/block_content/tests/src/Functional/BlockContentTranslationUITest.php
index 6266b71f1cb0..99b081edc801 100644
--- a/core/modules/block_content/tests/src/Functional/BlockContentTranslationUITest.php
+++ b/core/modules/block_content/tests/src/Functional/BlockContentTranslationUITest.php
@@ -40,7 +40,6 @@ class BlockContentTranslationUITest extends ContentTranslationUITestBase {
'url.path',
'url.query_args',
'user.permissions',
- 'user.roles:authenticated',
];
/**
diff --git a/core/modules/comment/tests/src/Functional/CommentTranslationUITest.php b/core/modules/comment/tests/src/Functional/CommentTranslationUITest.php
index 4d6db8c4bfd2..1cef130d13bf 100644
--- a/core/modules/comment/tests/src/Functional/CommentTranslationUITest.php
+++ b/core/modules/comment/tests/src/Functional/CommentTranslationUITest.php
@@ -49,7 +49,6 @@ class CommentTranslationUITest extends ContentTranslationUITestBase {
'url.query_args.pagers:0',
'url.site',
'user.permissions',
- 'user.roles',
];
/**
diff --git a/core/modules/comment/tests/src/Kernel/CommentDefaultFormatterCacheTagsTest.php b/core/modules/comment/tests/src/Kernel/CommentDefaultFormatterCacheTagsTest.php
index 699df300f067..ef175911e7b3 100644
--- a/core/modules/comment/tests/src/Kernel/CommentDefaultFormatterCacheTagsTest.php
+++ b/core/modules/comment/tests/src/Kernel/CommentDefaultFormatterCacheTagsTest.php
@@ -91,6 +91,7 @@ public function testCacheTags(): void {
'config:field.field.entity_test.entity_test.comment',
'config:field.storage.comment.comment_body',
'config:user.settings',
+ 'user:' . \Drupal::currentUser()->id(),
];
$this->assertEqualsCanonicalizing($expected_cache_tags, $build['#cache']['tags']);
@@ -134,6 +135,7 @@ public function testCacheTags(): void {
'user_view',
'CACHE_MISS_IF_UNCACHEABLE_HTTP_METHOD:form',
'user:' . $user->id(),
+ 'user:' . \Drupal::currentUser()->id(),
'config:core.entity_form_display.comment.comment.default',
'config:field.field.comment.comment.comment_body',
'config:field.field.entity_test.entity_test.comment',
diff --git a/core/modules/history/tests/src/Functional/HistoryTest.php b/core/modules/history/tests/src/Functional/HistoryTest.php
index 25dcfb42e31c..325d7adba9dd 100644
--- a/core/modules/history/tests/src/Functional/HistoryTest.php
+++ b/core/modules/history/tests/src/Functional/HistoryTest.php
@@ -121,7 +121,8 @@ public function testHistory(): void {
// View the node.
$this->drupalGet('node/' . $nid);
- $this->assertCacheContext('user.roles:authenticated');
+ // @todo: Does this test still make sense like this?
+ $this->assertCacheContext('user.permissions');
// JavaScript present to record the node read.
$settings = $this->getDrupalSettings();
$libraries = explode(',', $settings['ajaxPageState']['libraries']);
diff --git a/core/modules/menu_link_content/tests/src/Functional/MenuLinkContentTranslationUITest.php b/core/modules/menu_link_content/tests/src/Functional/MenuLinkContentTranslationUITest.php
index c8f12118c2d0..8e20d0329145 100644
--- a/core/modules/menu_link_content/tests/src/Functional/MenuLinkContentTranslationUITest.php
+++ b/core/modules/menu_link_content/tests/src/Functional/MenuLinkContentTranslationUITest.php
@@ -17,7 +17,7 @@ class MenuLinkContentTranslationUITest extends ContentTranslationUITestBase {
/**
* {@inheritdoc}
*/
- protected $defaultCacheContexts = ['languages:language_interface', 'session', 'theme', 'url.path', 'url.query_args', 'user.permissions', 'user.roles:authenticated'];
+ protected $defaultCacheContexts = ['languages:language_interface', 'session', 'theme', 'url.path', 'url.query_args', 'user.permissions'];
/**
* {@inheritdoc}
diff --git a/core/modules/menu_ui/tests/src/Functional/MenuUiNodeTest.php b/core/modules/menu_ui/tests/src/Functional/MenuUiNodeTest.php
index 69b11f376d03..ce1226ada23a 100644
--- a/core/modules/menu_ui/tests/src/Functional/MenuUiNodeTest.php
+++ b/core/modules/menu_ui/tests/src/Functional/MenuUiNodeTest.php
@@ -81,7 +81,8 @@ public function testMenuNodeFormWidget(): void {
// item" options in menu_ui_form_node_type_form_alter(). The "log out" link
// adds the "user.roles:authenticated" cache context.
$this->drupalGet('admin/structure/types/manage/page');
- $this->assertSession()->responseHeaderContains('X-Drupal-Cache-Contexts', 'user.roles:authenticated');
+ // @todo: Does this test still make sense like this?
+ $this->assertSession()->responseHeaderContains('X-Drupal-Cache-Contexts', 'user.permissions');
// Assert the description of "Available menus" checkboxes field.
$this->assertSession()->pageTextContains('Content of this type can be placed in the selected menus.');
diff --git a/core/modules/node/tests/src/Functional/NodeCacheTagsNoTimezoneTest.php b/core/modules/node/tests/src/Functional/NodeCacheTagsNoTimezoneTest.php
new file mode 100644
index 000000000000..786bd214fab9
--- /dev/null
+++ b/core/modules/node/tests/src/Functional/NodeCacheTagsNoTimezoneTest.php
@@ -0,0 +1,45 @@
+<?php
+
+namespace Drupal\Tests\node\Functional;
+
+use Drupal\Core\Entity\EntityInterface;
+
+/**
+ * Tests the Node entity's cache tags.
+ *
+ * @group node
+ */
+class NodeCacheTagsNoTimezoneTest extends NodeCacheTagsTest {
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp(): void {
+ parent::setUp();
+
+ // Disable configurable timezones, which should result in the timezone cache
+ // context *not* being added.
+ $this->config('system.date')
+ ->set('timezone.user.configurable', FALSE)
+ ->save();
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function getAdditionalCacheContextsForEntity(EntityInterface $entity) {
+ return [];
+ }
+
+ /**
+ * {@inheritdoc}
+ *
+ * Each node must have an author.
+ */
+ protected function getAdditionalCacheTagsForEntity(EntityInterface $node) {
+ // Because timezone is optimized away, the additional system.date cache tag
+ // is added.
+ return ['user:' . $node->getOwnerId(), 'user_view', 'config:system.date'];
+ }
+
+}
diff --git a/core/modules/system/tests/src/Functional/Routing/RouterTest.php b/core/modules/system/tests/src/Functional/Routing/RouterTest.php
index 97fa05f2f99e..ada68743a60d 100644
--- a/core/modules/system/tests/src/Functional/Routing/RouterTest.php
+++ b/core/modules/system/tests/src/Functional/Routing/RouterTest.php
@@ -75,10 +75,8 @@ public function testFinishResponseSubscriber(): void {
$this->assertSession()->responseHeaderEquals('X-Drupal-Cache-Max-Age', '60');
// 2. controller result: render array, per-role cacheable route access.
$this->drupalGet('router_test/test19');
- $expected_cache_contexts = Cache::mergeContexts($renderer_required_cache_contexts, [
- 'url',
- 'user.roles',
- ]);
+ // @todo: Does this test still make sense like this?
+ $this->assertSession()->responseHeaderEquals('X-Drupal-Cache-Contexts', implode(' ', Cache::mergeContexts($renderer_required_cache_contexts, ['url'])));
sort($expected_cache_contexts);
$this->assertSession()->responseHeaderEquals('X-Drupal-Cache-Contexts', implode(' ', $expected_cache_contexts));
$this->assertSession()->responseHeaderEquals('X-Drupal-Cache-Tags', 'config:user.role.anonymous foo http_response rendered');
diff --git a/core/modules/system/tests/src/Functional/System/AccessDeniedTest.php b/core/modules/system/tests/src/Functional/System/AccessDeniedTest.php
index 0163c594a068..efe9c9e11dff 100644
--- a/core/modules/system/tests/src/Functional/System/AccessDeniedTest.php
+++ b/core/modules/system/tests/src/Functional/System/AccessDeniedTest.php
@@ -145,14 +145,16 @@ public function testAccessDeniedCustomPageWithAccessDenied(): void {
$this->assertSession()->pageTextContains('You are not authorized to access this page.');
$this->assertSession()->statusCodeEquals(403);
// Verify the access cacheability metadata for custom 403 is bubbled.
- $this->assertCacheContext('user.roles');
+ // @todo: Does this test still make sense like this?
+ $this->assertCacheContext('user.permissions');
$this->drupalLogin($this->adminUser);
$this->drupalGet('/system-test/always-denied');
$this->assertSession()->pageTextContains('Admin-only 4xx response');
$this->assertSession()->statusCodeEquals(403);
// Verify the access cacheability metadata for custom 403 is bubbled.
- $this->assertCacheContext('user.roles');
+ // @todo: Does this test still make sense like this?
+ $this->assertCacheContext('user.permissions');
}
}
diff --git a/core/modules/system/tests/src/Functional/System/PageNotFoundTest.php b/core/modules/system/tests/src/Functional/System/PageNotFoundTest.php
index 7fe74aafcc54..0b45793f3d92 100644
--- a/core/modules/system/tests/src/Functional/System/PageNotFoundTest.php
+++ b/core/modules/system/tests/src/Functional/System/PageNotFoundTest.php
@@ -91,7 +91,8 @@ public function testPageNotFoundCustomPageWithAccessDenied(): void {
$this->assertSession()->pageTextContains('The requested page could not be found.');
$this->assertSession()->statusCodeEquals(404);
// Verify the access cacheability metadata for custom 404 is bubbled.
- $this->assertCacheContext('user.roles');
+ // @todo: Does this test still make sense like this?
+ $this->assertCacheContext('user.permissions');
$this->drupalLogin($this->adminUser);
$this->drupalGet('/this-path-does-not-exist');
@@ -99,7 +100,8 @@ public function testPageNotFoundCustomPageWithAccessDenied(): void {
$this->assertSession()->pageTextNotContains('The requested page could not be found.');
$this->assertSession()->statusCodeEquals(404);
// Verify the access cacheability metadata for custom 404 is bubbled.
- $this->assertCacheContext('user.roles');
+ // @todo: Does this test still make sense like this?
+ $this->assertCacheContext('user.permissions');
}
}
diff --git a/core/modules/user/tests/src/Functional/Views/AccessRoleTest.php b/core/modules/user/tests/src/Functional/Views/AccessRoleTest.php
index 58a5b0d346b4..dad4832186b0 100644
--- a/core/modules/user/tests/src/Functional/Views/AccessRoleTest.php
+++ b/core/modules/user/tests/src/Functional/Views/AccessRoleTest.php
@@ -70,12 +70,14 @@ public function testAccessRole(): void {
$this->drupalLogin($this->webUser);
$this->drupalGet('test-role');
$this->assertSession()->statusCodeEquals(403);
- $this->assertCacheContext('user.roles');
+ // @todo: Does this test still make sense like this?
+ $this->assertCacheContext('user.permissions');
$this->drupalLogin($this->normalUser);
$this->drupalGet('test-role');
$this->assertSession()->statusCodeEquals(200);
- $this->assertCacheContext('user.roles');
+ // @todo: Does this test still make sense like this?
+ $this->assertCacheContext('user.permissions');
// Test allowing multiple roles.
$view = Views::getView('test_access_role')->storage;
@@ -99,15 +101,18 @@ public function testAccessRole(): void {
$this->drupalLogin($this->webUser);
$this->drupalGet('test-role');
$this->assertSession()->statusCodeEquals(403);
- $this->assertCacheContext('user.roles');
+ // @todo: Does this test still make sense like this?
+ $this->assertCacheContext('user.permissions');
$this->drupalLogout();
$this->drupalGet('test-role');
$this->assertSession()->statusCodeEquals(200);
- $this->assertCacheContext('user.roles');
+ // @todo: Does this test still make sense like this?
+ $this->assertCacheContext('user.permissions');
$this->drupalLogin($this->normalUser);
$this->drupalGet('test-role');
$this->assertSession()->statusCodeEquals(200);
- $this->assertCacheContext('user.roles');
+ // @todo: Does this test still make sense like this?
+ $this->assertCacheContext('user.permissions');
}
/**
@@ -133,8 +138,8 @@ public function testRenderCaching(): void {
$build = DisplayPluginBase::buildBasicRenderable('test_access_role', 'default');
$account_switcher->switchTo($this->normalUser);
$result = $renderer->renderInIsolation($build);
- $this->assertContains('user.roles', $build['#cache']['contexts']);
- $this->assertEquals(['config:views.view.test_access_role'], $build['#cache']['tags']);
+ $this->assertContains('user.permissions', $build['#cache']['contexts']);
+ $this->assertEquals(['config:views.view.test_access_role', 'user:' . $this->normalUser->id()], $build['#cache']['tags']);
$this->assertEquals(Cache::PERMANENT, $build['#cache']['max-age']);
$this->assertNotSame('', $result);
diff --git a/core/tests/Drupal/Tests/Core/Cache/Context/CacheContextsManagerTest.php b/core/tests/Drupal/Tests/Core/Cache/Context/CacheContextsManagerTest.php
index f76e8cd64743..6c3147de0af6 100644
--- a/core/tests/Drupal/Tests/Core/Cache/Context/CacheContextsManagerTest.php
+++ b/core/tests/Drupal/Tests/Core/Cache/Context/CacheContextsManagerTest.php
@@ -5,6 +5,7 @@
namespace Drupal\Tests\Core\Cache\Context;
use Drupal\Core\Cache\CacheableMetadata;
+use Drupal\Core\Cache\Context\CacheContextOptimizableInterface;
use Drupal\Core\Cache\Context\CacheContextsManager;
use Drupal\Core\Cache\Context\CacheContextInterface;
use Drupal\Core\Cache\Context\CalculatedCacheContextInterface;
@@ -57,6 +58,16 @@ public function testOptimizeTokens(array $context_tokens, array $optimized_conte
Container::EXCEPTION_ON_INVALID_REFERENCE,
new NoOptimizeCacheContext(),
],
+ [
+ 'cache_context.x.non-standard',
+ Container::EXCEPTION_ON_INVALID_REFERENCE,
+ new NonStandardOptimizeCacheContext(),
+ ],
+ [
+ 'cache_context.x.global',
+ Container::EXCEPTION_ON_INVALID_REFERENCE,
+ new GlobalOptimizeCacheContext(),
+ ],
]);
$cache_contexts_manager = new CacheContextsManager($container, $this->getContextsFixture());
@@ -96,6 +107,24 @@ public static function providerTestOptimizeTokens() {
[['a.b.c:foo', 'a'], ['a']],
[['a.b.c:foo', 'a.b.c'], ['a.b.c']],
+ // Non-standard parents.
+ [['a.b', 'x.non-standard'], ['a.b']],
+ // Calculated cache contexts are supported as well but there is no
+ // argument granularity.
+ [['a.b', 'x.non-standard:argument'], ['a.b']],
+ [['a', 'a.b', 'x.non-standard'], ['a']],
+ // Should contexts with explicit parents still respect the default logic?
+ // Not doing that would allow to deprecate the somewhat strange max-age 0
+ // check in favor of returning an empty list?
+ [['x', 'x.non-standard'], ['x']],
+ [['a.b.c', 'x.non-standard'], ['a.b.c', 'x.non-standard']],
+ // @todo Support this as well?
+ // [['a', 'non-standard'], ['a']],
+
+ // Contexts that return TRUE in isGlobal() are always optimized away.
+ [['a', 'x.global'], ['a']],
+ [['x.global'], []],
+
// max-age 0 is treated as non-optimizable.
[['a.b.no-optimize', 'a.b', 'a'], ['a.b.no-optimize', 'a']],
];
@@ -333,3 +362,87 @@ public function getCacheableMetadata() {
}
}
+
+/**
+ * Optimizable context class with a non-default parent.
+ */
+class NonStandardOptimizeCacheContext implements CacheContextInterface, CacheContextOptimizableInterface {
+
+ /**
+ * {@inheritdoc}
+ */
+ public static function getLabel() {
+ return 'Non-Standard';
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function getContext() {
+ return 'non_standard';
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function getCacheableMetadata($parameter = NULL) {
+ return new CacheableMetadata();
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function hasVariations() {
+ return TRUE;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function getParentContexts() {
+ return ['a.b'];
+ }
+
+}
+
+/**
+ * A global cache context that can always be optimized away.
+ */
+class GlobalOptimizeCacheContext implements CacheContextInterface, CacheContextOptimizableInterface {
+
+ /**
+ * {@inheritdoc}
+ */
+ public static function getLabel() {
+ return 'Non-Standard';
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function getContext() {
+ return 'non_standard';
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function getCacheableMetadata($parameter = NULL) {
+ return new CacheableMetadata();
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function hasVariations() {
+ return FALSE;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function getParentContexts() {
+ return [];
+ }
+
+}
--
GitLab
From 2510acbfeaa845d3c521053f750acd142e64616c Mon Sep 17 00:00:00 2001
From: Sascha Grossenbacher <saschagros@gmail.com>
Date: Tue, 1 Apr 2025 18:02:26 +0200
Subject: [PATCH 2/4] remove roles/permissions changes
---
.../Cache/Context/UserRolesCacheContext.php | 16 +---------------
.../BlockContentTranslationUITest.php | 1 +
.../Functional/CommentTranslationUITest.php | 1 +
.../CommentDefaultFormatterCacheTagsTest.php | 2 --
.../tests/src/Functional/HistoryTest.php | 3 +--
.../MenuLinkContentTranslationUITest.php | 2 +-
.../tests/src/Functional/MenuUiNodeTest.php | 3 +--
.../src/Functional/Routing/RouterTest.php | 6 ++++--
.../Functional/System/AccessDeniedTest.php | 6 ++----
.../Functional/System/PageNotFoundTest.php | 6 ++----
.../src/Functional/Views/AccessRoleTest.php | 19 +++++++------------
11 files changed, 21 insertions(+), 44 deletions(-)
diff --git a/core/lib/Drupal/Core/Cache/Context/UserRolesCacheContext.php b/core/lib/Drupal/Core/Cache/Context/UserRolesCacheContext.php
index 18080d06d55a..cc978750c4da 100644
--- a/core/lib/Drupal/Core/Cache/Context/UserRolesCacheContext.php
+++ b/core/lib/Drupal/Core/Cache/Context/UserRolesCacheContext.php
@@ -14,7 +14,7 @@
* Calculated cache context ID: 'user.roles:%role', e.g. 'user.roles:anonymous'
* (to vary by the presence/absence of a specific role).
*/
-class UserRolesCacheContext extends UserCacheContextBase implements CalculatedCacheContextInterface, CacheContextOptimizableInterface {
+class UserRolesCacheContext extends UserCacheContextBase implements CalculatedCacheContextInterface {
/**
* {@inheritdoc}
@@ -41,18 +41,4 @@ public function getCacheableMetadata($role = NULL) {
return (new CacheableMetadata())->setCacheTags(['user:' . $this->user->id()]);
}
- /**
- * {@inheritdoc}
- */
- public function hasVariations() {
- return TRUE;
- }
-
- /**
- * {@inheritdoc}
- */
- public function getParentContexts() {
- return ['user', 'user.permissions'];
- }
-
}
diff --git a/core/modules/block_content/tests/src/Functional/BlockContentTranslationUITest.php b/core/modules/block_content/tests/src/Functional/BlockContentTranslationUITest.php
index 99b081edc801..6266b71f1cb0 100644
--- a/core/modules/block_content/tests/src/Functional/BlockContentTranslationUITest.php
+++ b/core/modules/block_content/tests/src/Functional/BlockContentTranslationUITest.php
@@ -40,6 +40,7 @@ class BlockContentTranslationUITest extends ContentTranslationUITestBase {
'url.path',
'url.query_args',
'user.permissions',
+ 'user.roles:authenticated',
];
/**
diff --git a/core/modules/comment/tests/src/Functional/CommentTranslationUITest.php b/core/modules/comment/tests/src/Functional/CommentTranslationUITest.php
index 1cef130d13bf..4d6db8c4bfd2 100644
--- a/core/modules/comment/tests/src/Functional/CommentTranslationUITest.php
+++ b/core/modules/comment/tests/src/Functional/CommentTranslationUITest.php
@@ -49,6 +49,7 @@ class CommentTranslationUITest extends ContentTranslationUITestBase {
'url.query_args.pagers:0',
'url.site',
'user.permissions',
+ 'user.roles',
];
/**
diff --git a/core/modules/comment/tests/src/Kernel/CommentDefaultFormatterCacheTagsTest.php b/core/modules/comment/tests/src/Kernel/CommentDefaultFormatterCacheTagsTest.php
index ef175911e7b3..699df300f067 100644
--- a/core/modules/comment/tests/src/Kernel/CommentDefaultFormatterCacheTagsTest.php
+++ b/core/modules/comment/tests/src/Kernel/CommentDefaultFormatterCacheTagsTest.php
@@ -91,7 +91,6 @@ public function testCacheTags(): void {
'config:field.field.entity_test.entity_test.comment',
'config:field.storage.comment.comment_body',
'config:user.settings',
- 'user:' . \Drupal::currentUser()->id(),
];
$this->assertEqualsCanonicalizing($expected_cache_tags, $build['#cache']['tags']);
@@ -135,7 +134,6 @@ public function testCacheTags(): void {
'user_view',
'CACHE_MISS_IF_UNCACHEABLE_HTTP_METHOD:form',
'user:' . $user->id(),
- 'user:' . \Drupal::currentUser()->id(),
'config:core.entity_form_display.comment.comment.default',
'config:field.field.comment.comment.comment_body',
'config:field.field.entity_test.entity_test.comment',
diff --git a/core/modules/history/tests/src/Functional/HistoryTest.php b/core/modules/history/tests/src/Functional/HistoryTest.php
index 325d7adba9dd..25dcfb42e31c 100644
--- a/core/modules/history/tests/src/Functional/HistoryTest.php
+++ b/core/modules/history/tests/src/Functional/HistoryTest.php
@@ -121,8 +121,7 @@ public function testHistory(): void {
// View the node.
$this->drupalGet('node/' . $nid);
- // @todo: Does this test still make sense like this?
- $this->assertCacheContext('user.permissions');
+ $this->assertCacheContext('user.roles:authenticated');
// JavaScript present to record the node read.
$settings = $this->getDrupalSettings();
$libraries = explode(',', $settings['ajaxPageState']['libraries']);
diff --git a/core/modules/menu_link_content/tests/src/Functional/MenuLinkContentTranslationUITest.php b/core/modules/menu_link_content/tests/src/Functional/MenuLinkContentTranslationUITest.php
index 8e20d0329145..c8f12118c2d0 100644
--- a/core/modules/menu_link_content/tests/src/Functional/MenuLinkContentTranslationUITest.php
+++ b/core/modules/menu_link_content/tests/src/Functional/MenuLinkContentTranslationUITest.php
@@ -17,7 +17,7 @@ class MenuLinkContentTranslationUITest extends ContentTranslationUITestBase {
/**
* {@inheritdoc}
*/
- protected $defaultCacheContexts = ['languages:language_interface', 'session', 'theme', 'url.path', 'url.query_args', 'user.permissions'];
+ protected $defaultCacheContexts = ['languages:language_interface', 'session', 'theme', 'url.path', 'url.query_args', 'user.permissions', 'user.roles:authenticated'];
/**
* {@inheritdoc}
diff --git a/core/modules/menu_ui/tests/src/Functional/MenuUiNodeTest.php b/core/modules/menu_ui/tests/src/Functional/MenuUiNodeTest.php
index ce1226ada23a..69b11f376d03 100644
--- a/core/modules/menu_ui/tests/src/Functional/MenuUiNodeTest.php
+++ b/core/modules/menu_ui/tests/src/Functional/MenuUiNodeTest.php
@@ -81,8 +81,7 @@ public function testMenuNodeFormWidget(): void {
// item" options in menu_ui_form_node_type_form_alter(). The "log out" link
// adds the "user.roles:authenticated" cache context.
$this->drupalGet('admin/structure/types/manage/page');
- // @todo: Does this test still make sense like this?
- $this->assertSession()->responseHeaderContains('X-Drupal-Cache-Contexts', 'user.permissions');
+ $this->assertSession()->responseHeaderContains('X-Drupal-Cache-Contexts', 'user.roles:authenticated');
// Assert the description of "Available menus" checkboxes field.
$this->assertSession()->pageTextContains('Content of this type can be placed in the selected menus.');
diff --git a/core/modules/system/tests/src/Functional/Routing/RouterTest.php b/core/modules/system/tests/src/Functional/Routing/RouterTest.php
index ada68743a60d..97fa05f2f99e 100644
--- a/core/modules/system/tests/src/Functional/Routing/RouterTest.php
+++ b/core/modules/system/tests/src/Functional/Routing/RouterTest.php
@@ -75,8 +75,10 @@ public function testFinishResponseSubscriber(): void {
$this->assertSession()->responseHeaderEquals('X-Drupal-Cache-Max-Age', '60');
// 2. controller result: render array, per-role cacheable route access.
$this->drupalGet('router_test/test19');
- // @todo: Does this test still make sense like this?
- $this->assertSession()->responseHeaderEquals('X-Drupal-Cache-Contexts', implode(' ', Cache::mergeContexts($renderer_required_cache_contexts, ['url'])));
+ $expected_cache_contexts = Cache::mergeContexts($renderer_required_cache_contexts, [
+ 'url',
+ 'user.roles',
+ ]);
sort($expected_cache_contexts);
$this->assertSession()->responseHeaderEquals('X-Drupal-Cache-Contexts', implode(' ', $expected_cache_contexts));
$this->assertSession()->responseHeaderEquals('X-Drupal-Cache-Tags', 'config:user.role.anonymous foo http_response rendered');
diff --git a/core/modules/system/tests/src/Functional/System/AccessDeniedTest.php b/core/modules/system/tests/src/Functional/System/AccessDeniedTest.php
index efe9c9e11dff..0163c594a068 100644
--- a/core/modules/system/tests/src/Functional/System/AccessDeniedTest.php
+++ b/core/modules/system/tests/src/Functional/System/AccessDeniedTest.php
@@ -145,16 +145,14 @@ public function testAccessDeniedCustomPageWithAccessDenied(): void {
$this->assertSession()->pageTextContains('You are not authorized to access this page.');
$this->assertSession()->statusCodeEquals(403);
// Verify the access cacheability metadata for custom 403 is bubbled.
- // @todo: Does this test still make sense like this?
- $this->assertCacheContext('user.permissions');
+ $this->assertCacheContext('user.roles');
$this->drupalLogin($this->adminUser);
$this->drupalGet('/system-test/always-denied');
$this->assertSession()->pageTextContains('Admin-only 4xx response');
$this->assertSession()->statusCodeEquals(403);
// Verify the access cacheability metadata for custom 403 is bubbled.
- // @todo: Does this test still make sense like this?
- $this->assertCacheContext('user.permissions');
+ $this->assertCacheContext('user.roles');
}
}
diff --git a/core/modules/system/tests/src/Functional/System/PageNotFoundTest.php b/core/modules/system/tests/src/Functional/System/PageNotFoundTest.php
index 0b45793f3d92..7fe74aafcc54 100644
--- a/core/modules/system/tests/src/Functional/System/PageNotFoundTest.php
+++ b/core/modules/system/tests/src/Functional/System/PageNotFoundTest.php
@@ -91,8 +91,7 @@ public function testPageNotFoundCustomPageWithAccessDenied(): void {
$this->assertSession()->pageTextContains('The requested page could not be found.');
$this->assertSession()->statusCodeEquals(404);
// Verify the access cacheability metadata for custom 404 is bubbled.
- // @todo: Does this test still make sense like this?
- $this->assertCacheContext('user.permissions');
+ $this->assertCacheContext('user.roles');
$this->drupalLogin($this->adminUser);
$this->drupalGet('/this-path-does-not-exist');
@@ -100,8 +99,7 @@ public function testPageNotFoundCustomPageWithAccessDenied(): void {
$this->assertSession()->pageTextNotContains('The requested page could not be found.');
$this->assertSession()->statusCodeEquals(404);
// Verify the access cacheability metadata for custom 404 is bubbled.
- // @todo: Does this test still make sense like this?
- $this->assertCacheContext('user.permissions');
+ $this->assertCacheContext('user.roles');
}
}
diff --git a/core/modules/user/tests/src/Functional/Views/AccessRoleTest.php b/core/modules/user/tests/src/Functional/Views/AccessRoleTest.php
index dad4832186b0..58a5b0d346b4 100644
--- a/core/modules/user/tests/src/Functional/Views/AccessRoleTest.php
+++ b/core/modules/user/tests/src/Functional/Views/AccessRoleTest.php
@@ -70,14 +70,12 @@ public function testAccessRole(): void {
$this->drupalLogin($this->webUser);
$this->drupalGet('test-role');
$this->assertSession()->statusCodeEquals(403);
- // @todo: Does this test still make sense like this?
- $this->assertCacheContext('user.permissions');
+ $this->assertCacheContext('user.roles');
$this->drupalLogin($this->normalUser);
$this->drupalGet('test-role');
$this->assertSession()->statusCodeEquals(200);
- // @todo: Does this test still make sense like this?
- $this->assertCacheContext('user.permissions');
+ $this->assertCacheContext('user.roles');
// Test allowing multiple roles.
$view = Views::getView('test_access_role')->storage;
@@ -101,18 +99,15 @@ public function testAccessRole(): void {
$this->drupalLogin($this->webUser);
$this->drupalGet('test-role');
$this->assertSession()->statusCodeEquals(403);
- // @todo: Does this test still make sense like this?
- $this->assertCacheContext('user.permissions');
+ $this->assertCacheContext('user.roles');
$this->drupalLogout();
$this->drupalGet('test-role');
$this->assertSession()->statusCodeEquals(200);
- // @todo: Does this test still make sense like this?
- $this->assertCacheContext('user.permissions');
+ $this->assertCacheContext('user.roles');
$this->drupalLogin($this->normalUser);
$this->drupalGet('test-role');
$this->assertSession()->statusCodeEquals(200);
- // @todo: Does this test still make sense like this?
- $this->assertCacheContext('user.permissions');
+ $this->assertCacheContext('user.roles');
}
/**
@@ -138,8 +133,8 @@ public function testRenderCaching(): void {
$build = DisplayPluginBase::buildBasicRenderable('test_access_role', 'default');
$account_switcher->switchTo($this->normalUser);
$result = $renderer->renderInIsolation($build);
- $this->assertContains('user.permissions', $build['#cache']['contexts']);
- $this->assertEquals(['config:views.view.test_access_role', 'user:' . $this->normalUser->id()], $build['#cache']['tags']);
+ $this->assertContains('user.roles', $build['#cache']['contexts']);
+ $this->assertEquals(['config:views.view.test_access_role'], $build['#cache']['tags']);
$this->assertEquals(Cache::PERMANENT, $build['#cache']['max-age']);
$this->assertNotSame('', $result);
--
GitLab
From a0051eff5d87eb73543674f42dc859ef0e5406f0 Mon Sep 17 00:00:00 2001
From: Sascha Grossenbacher <saschagros@gmail.com>
Date: Tue, 1 Apr 2025 21:28:01 +0200
Subject: [PATCH 3/4] phpcs
---
.../tests/src/Functional/NodeCacheTagsNoTimezoneTest.php | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/core/modules/node/tests/src/Functional/NodeCacheTagsNoTimezoneTest.php b/core/modules/node/tests/src/Functional/NodeCacheTagsNoTimezoneTest.php
index 786bd214fab9..c92a32c0fddc 100644
--- a/core/modules/node/tests/src/Functional/NodeCacheTagsNoTimezoneTest.php
+++ b/core/modules/node/tests/src/Functional/NodeCacheTagsNoTimezoneTest.php
@@ -1,5 +1,7 @@
<?php
+declare(strict_types=1);
+
namespace Drupal\Tests\node\Functional;
use Drupal\Core\Entity\EntityInterface;
@@ -27,7 +29,7 @@ protected function setUp(): void {
/**
* {@inheritdoc}
*/
- protected function getAdditionalCacheContextsForEntity(EntityInterface $entity) {
+ protected function getAdditionalCacheContextsForEntity(EntityInterface $entity): array {
return [];
}
@@ -36,7 +38,7 @@ protected function getAdditionalCacheContextsForEntity(EntityInterface $entity)
*
* Each node must have an author.
*/
- protected function getAdditionalCacheTagsForEntity(EntityInterface $node) {
+ protected function getAdditionalCacheTagsForEntity(EntityInterface $node): array {
// Because timezone is optimized away, the additional system.date cache tag
// is added.
return ['user:' . $node->getOwnerId(), 'user_view', 'config:system.date'];
--
GitLab
From ef908a365099c5335ae247cf9b9a22dfead3ef1b Mon Sep 17 00:00:00 2001
From: Sascha Grossenbacher <saschagros@gmail.com>
Date: Tue, 8 Apr 2025 21:47:49 +0200
Subject: [PATCH 4/4] return type
---
.../Cache/Context/CacheContextOptimizableInterface.php | 4 ++--
.../Drupal/Core/Cache/Context/TimeZoneCacheContext.php | 4 ++--
.../Tests/Core/Cache/Context/CacheContextsManagerTest.php | 8 ++++----
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/core/lib/Drupal/Core/Cache/Context/CacheContextOptimizableInterface.php b/core/lib/Drupal/Core/Cache/Context/CacheContextOptimizableInterface.php
index c1eeecc25db7..f7abf0944448 100644
--- a/core/lib/Drupal/Core/Cache/Context/CacheContextOptimizableInterface.php
+++ b/core/lib/Drupal/Core/Cache/Context/CacheContextOptimizableInterface.php
@@ -17,7 +17,7 @@ interface CacheContextOptimizableInterface {
* @return bool
* TRUE if the context has more than one possible variations.
*/
- public function hasVariations();
+ public function hasVariations(): bool;
/**
* Returns parent contexts for this context.
@@ -34,6 +34,6 @@ public function hasVariations();
* @return array
* A list of parents cache contexts.
*/
- public function getParentContexts();
+ public function getParentContexts(): array;
}
diff --git a/core/lib/Drupal/Core/Cache/Context/TimeZoneCacheContext.php b/core/lib/Drupal/Core/Cache/Context/TimeZoneCacheContext.php
index 4cddaa09f330..b24fc09e3260 100644
--- a/core/lib/Drupal/Core/Cache/Context/TimeZoneCacheContext.php
+++ b/core/lib/Drupal/Core/Cache/Context/TimeZoneCacheContext.php
@@ -59,7 +59,7 @@ public function getCacheableMetadata() {
/**
* {@inheritdoc}
*/
- public function hasVariations() {
+ public function hasVariations(): bool {
// The timezone context can not have different values if the site does not
// use configurable timezones.
return (bool) $this->configFactory->get('system.date')->get('timezone.user.configurable');
@@ -68,7 +68,7 @@ public function hasVariations() {
/**
* {@inheritdoc}
*/
- public function getParentContexts() {
+ public function getParentContexts(): array {
return [];
}
diff --git a/core/tests/Drupal/Tests/Core/Cache/Context/CacheContextsManagerTest.php b/core/tests/Drupal/Tests/Core/Cache/Context/CacheContextsManagerTest.php
index 6c3147de0af6..178cde88b7c5 100644
--- a/core/tests/Drupal/Tests/Core/Cache/Context/CacheContextsManagerTest.php
+++ b/core/tests/Drupal/Tests/Core/Cache/Context/CacheContextsManagerTest.php
@@ -392,14 +392,14 @@ public function getCacheableMetadata($parameter = NULL) {
/**
* {@inheritdoc}
*/
- public function hasVariations() {
+ public function hasVariations(): bool {
return TRUE;
}
/**
* {@inheritdoc}
*/
- public function getParentContexts() {
+ public function getParentContexts(): array {
return ['a.b'];
}
@@ -434,14 +434,14 @@ public function getCacheableMetadata($parameter = NULL) {
/**
* {@inheritdoc}
*/
- public function hasVariations() {
+ public function hasVariations(): bool {
return FALSE;
}
/**
* {@inheritdoc}
*/
- public function getParentContexts() {
+ public function getParentContexts(): array {
return [];
}
--
GitLab