#D8 cacheability

#260 applied for me on Drupal 10.3.7 and
#262 did not
thanks @socialnicheguru

Same for me on 10.3.8
Thank you

Attached an attempt at creating a patch from MR 2417 that is compatible with Drupal 10.3.x

Attached an attempt at creating a patch from MR 2417 that is compatible with Drupal 10.3.x

I recently discussed thie with catch.

Using the header and vary on that is very dangerous for cache purposes. It will make very very many variations, which results in bad caching and performance. Not to mention possible memroy issues in caches or high cache trashing.

We did think about how we could possible work around that.

What if;

We normalize the prermutation of the language into the resulting language, and cache that normalization and the resulting language.

So perhaps a site with en_EN and fr_FR. A browser comes along and tells the site "Hi i'm en-GB,en-US;q=0.9,en;q=0.8". The site would find out what this will end up as so

en-GB,en-US;q=0.9,en;q=0.8 magic => en_EN, we cache this and threat serve cache asif en_EN. This would mean (at least on the drupal side) only the records for inLang => outLang exist, which are pretty small and we dont end up with loads of difference permutations.

Not entirely sure how we would implement that, but feels like a way to have drupal cache be happy.

The little devil on my shoulder also has a voice though. Which does tell me this is kinda a. can of worms as this could also easily break caching on the caches in front of Drupal, be it varnisch or maybe smoething like cloudflare. That is kinda the scary part. Perhaps contrib is better to have this live, instead of trying to get this into core.

> That route has the _csrf_token requirement. That gets removed by the array_diff in AccessManager::check because there is no request, which the csrf_token requirement needs.

We tried the patch and did just run into this as well.

On one hand, I think that can be improved in masquerade and is arguably even a bug. The controller still checks access, so it's not a security issue.. masquerade_target_user_access() or masquerade_switch_user_validate() really should be exposed as an access check and the route should not _only_ have the csrf access check.

But I'm also not sure that the approach here is correct. It's kind a neat to fall back to the url access, but probably also has some performance overhead, because it involves creating route matches and lookups and directly doing access checks on the entity really should be faster. And it's an implicit API change anyway because all implementations need to now always return their URL.

An alternative, more "direct" approach would be to pass around a cacheability object to getDefaultOperations (tricky to add with BC) and the hook (easy to add an extra argument) and require that implementations add their access cacheability to do that and let calls deal with that. Similar to how hook_tokens() works for example.