Problem/Motivation
One of the worst things that could happen in regards to #2429287: [meta] Finalize the cache contexts API & DX/usage, enable a leap forward in performance is that we ship 8.0.x and then find something that should have had cacheable metadata, but does not and either introduces a security issue (compare tokens) or that it should at least mark itself uncacheable, but lets one opt-in to it being cacheable.
Proposed resolution
Audit core for things missing cacheable metadata, e.g.:
- Does it have tags (pretty complete), but is missing contexts? (e.g. is it varying on something external, but not specifying that)
- Does it have contexts, but is missing tags? (e.g. Is invalidation a problem once this becomes cacheable)
Remaining tasks
- Audit
- Discuss
Comments
Comment #1
Fabianx CreditAttribution: Fabianx as a volunteer commentedComment #2
Wim LeersI don't think we'll find more skeletons in the closet.
Thanks to #1805054: Cache localized, access filtered, URL resolved, and rendered menu trees, #606840: Enable internal page cache by default and #2429617: Make D8 2x as fast: Dynamic Page Cache: context-dependent page caching (for *all* users!), we've uncovered piles of things missing cacheability metadata. At this point, the only things we're discovering are APIs that are pretty much unused in Drupal 8 core (i.e. #2525910: Ensure token replacements have cacheability + attachments metadata and that it is bubbled in any case), of which we only have very few.
I think that if anything remains, we'll only actually discover it after D8 ships. But, sure, we should do another sweep.
Comment #3
cosmicdreams CreditAttribution: cosmicdreams as a volunteer commentedHow would one even audit core to see if it needs better support for cacheability. Are we talking about reading through all the code?
Comment #4
Fabianx CreditAttribution: Fabianx as a volunteer commented#3 Yes! Of course.
I already have ordered a horde of monkeys to do so ...
Uhm, no, not really. The important thing is to look at subsystems we do support in Core and see if there is any hidden cacheability problems.
But then for each subsystem we do need to look at the code, yes.
Comment #5
BerdirNot sure how this issue is different from e.g. #2467071: [meta] Find, fix & finish cache tag support: try to find broken scenarios as the anonymous user
The title is only about cache tags and anonymous users atm, but there's no reason to not expand it to auth users and cache contexts as well?
Comment #6
Wim Leers#5: That other issue was specifically about Page Cache. So indeed only for anon user.
Comment #7
Fabianx CreditAttribution: Fabianx as a volunteer commentedI think it makes sense to split things up; clarified title.
Comment #8
moshe weitzman CreditAttribution: moshe weitzman at Acquia commentedThis issue is not actionable IMO. Its sort of like 'Find any bugs in my subsystem'. I think it should be closed and bugs can be reported separately as needed.
Comment #12
BerdirAgreed. This was open for two years, there is no specific goal. I'd say we close this.