- 🇦🇺Australia mstrelan
Can we write a test for this? Or is this covered by existing tests?
Exposed in ✨ Abstract RenderCache into a separate service that is capable of cache redirects in a non-render array-specific way Fixed so you can read the full thought process there but here's the TLDR:
Both X-Drupal-Cache-Tags
and X-Drupal-Cache-Contexts
are set in FinishResponseSubscriber
(FRS). This is before page cache gets the response but after dynamic page cache has handled it. This is wrong for both tags and contexts.
To add insult to injury, FRS folds the cache contexts before setting them in the header, but does not similarly affect the cache tags. This is very misleading for numerous reasons:
user.permissions
and not user
, but RARS would add user
to the response, then X-Drupal-Cache-Contexts
would simply show user
, even though the DPC stored the page with user.permissions
X-Drupal-Cache-Tags
The reason this currently works with our thousands of tests checking for the 2 headers is twofold:
user
after folding user.permissions
, but not the tags set by said folding.In the long run, we should move X-Drupal-Cache-Tags
to PageCache, where it actually makes sense. We should also move X-Drupal-Cache-Contexts
to DPCS where it makes sense for contexts to be debugged.
This will, however lead to ambiguity in X-Drupal-Cache-Tags
because we might want to debug tags for DPC and PC individually. So I suggest we add a third header X-Drupal-Dynamic-Cache-Tags
and rename the context one to X-Drupal-Dynamic-Cache-Contexts
.
We can finally add X-Drupal-Cache-Contexts-Deprecated
to PageCache and allow failing tests to check that header while we clean them up on a smaller scale in follow-ups. The end goal would be to remove said header altogether.
Needs work
10.1 ✨
Enhances developer experience.
The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.
Not all content is available!
It's likely this issue predates Contrib.social: some issue and comment data are missing.
Can we write a test for this? Or is this covered by existing tests?