Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Spin-off from #914382: Contextual links incompatible with render cache.
Currently, drupal_render_cid_create() handles #cache['keys'] and #cache['granularity']. AFAIK, 'keys' are intended to be static strings, not things that can vary by user or other $request attributes. Meanwhile, 'granularity' can be a constant like DRUPAL_CACHE_PER_ROLE, DRUPAL_CACHE_PER_USER, or DRUPAL_CACHE_PER_PAGE, which drupal_render_cid_parts() expands based on the user and request.
In the case of #914382: Contextual links incompatible with render cache, I think per role granularity is too granular. Consider a site with multiple roles without 'access contextual links' permission. There's no reason for elements that render contextual links to require duplicate cache entries for each combination of roles, all of which lack that permission. But no granularity is also not ideal, because it results in extra HTML bloat sent to everyone who lacks permission to contextual links anyway (which on many sites, can be >99% of site visitors). I think the ideal is to vary the cache just by whether that permission is true or false.
A simple solution can be to simply add a 'permissions' keys to $element['#cache']. This could be set to an array of permissions (e.g., array('access contextual links')). drupal_render_cid_parts() could then convert that to $cid_parts, just like how it already does for 'granularity'.
A more expansive solution could be to introduce a 'contexts' key that can be an array of key/values. The keys could be callbacks/DIC service ids/plugin ids/whatever for code that can be passed the corresponding value and a $request object, and would return the corresponding $cid_parts. In this case, 'permissions' could be one such service, but modules could add more without needing to hack drupal_render_cid_parts().
Comments
Comment #1
effulgentsia CreditAttribution: effulgentsia commentedtagging
Comment #2
Wim LeersI like the idea. A lot.
But I don't think this would work well due to Drupal's alterability. It's entirely possible that something is being altered somewhere deep into the render array, at a different level, that depends on other permissions. That means we'd need to add something like
drupal_render_collect_cache_permissions()(like in #1605290: Enable entity render caching with cache tag support) to deal with that.So, the problem I saw is solvable.
I'm wondering if I'm then missing yet more problematic cases.
Comment #3
moshe weitzman CreditAttribution: moshe weitzman commentedNice idea. I have no problem with dynamically generated stuff #keys. I would add your permission names there and call it a day.
Comment #4
catchI've thought about drupal_render_collect_cache_keys() before but I don't think it works.
When building a full render array, you could create a cache key collecting all the #parts from child elements, and that'll work fine for setting it.
The problem comes when you want to get the item from cache - because you'll need to have access to the child elements' #keys - but you don't because you're trying to avoid building those in the first place by using render caching.
For completely raw render caching where nothing is using a #pre_render callback I think it'd work, but once #pre_render comes into play you don't have access to the child elements to collect the keys to know what the cache key is to fetch the cache item that you're trying to fetch to avoid building the child elements ;)
Would love to be wrong on that though, or if there's an alternative approach.
Comment #5
pounardWould children traversal in order to collect keys be that time consuming? It still would be faster than rendering all children. Forms are doing that all the time, and multiple time.
Comment #6
catchIt's probably not bad when the children exist, that ought to be fine at least compared to converting to a string.
However if you use #pre_render then the children don't exist yet, and #pre_render allows render caching to skip both the building of (most of) the render array as well as the rendering of it to HTML.
Comment #7
Wim LeersLooking back at this, it feels almost cute :)
This has long since been implemented. See https://www.drupal.org/developing/api/8/cache/contexts.
A "per-permissions combination" cache context was added in #2428703: Add a 'user.permissions' cache context (was: "Should cache contexts be able to associate a cache tag?").