Problem/Motivation
Example use case:
Override node/{node} with a page and then display the node through the entity view block. If you enable caching, then every node page will render the same node, because the block has the same cache key and contexts.
Proposed resolution
#2375695: Condition plugins should provide cache contexts AND cacheability metadata needs to be exposed almost solves this for us. We just need to make sure that the custom context providers provide the cacheability metadata for this, specifically the current user (user) and route-based contexts (route).
Extend the existing tests for overriding the node page for this scenario by enabling caching for the placed blocks.
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#7 | interdiff-4-7.txt | 722 bytes | giancarlosotelo |
#7 | add_cacheability_metadata-2540772-7.patch | 3.1 KB | giancarlosotelo |
#4 | add_cacheability_metadata-2540772-3.patch | 3.1 KB | giancarlosotelo |
#4 | add_cacheability_metadata-2540772-TEST_ONLY.patch | 933 bytes | giancarlosotelo |
Comments
Comment #1
BerdirComment #2
Wim LeersI just opened #2541344: BlockBase subclasses should merge their cache tags/contexts with the parent's (BlockBase's), which is a related issue.
It's still not 100% clear to me what the problem here is, other than the problem that #2541344 already points out. Is it context providers that aren't returning the proper cacheability metadata?
Comment #3
Wim LeersComment #4
giancarlosotelo CreditAttribution: giancarlosotelo at MD Systems GmbH commentedFixed and a test only patch provided.
Comment #7
giancarlosotelo CreditAttribution: giancarlosotelo at MD Systems GmbH commentedFixed that test.
Comment #8
dsnopekI just wanted to add a note that there's plans in the works to switch from Page Manager's current context providers to a "context stack" from CTools:
#2511568: Create "context stack" service where available contexts can be registered
However, if this issue gets done first, we'll just port the approach here into CTools. :-)
Comment #9
BerdirLooks good, test makes the problem obvious enough :)
Comment could be a bit clearer, but not exactly sure what.
@dsnopet: Sure, but that issue hasn't moved since it was created, this is a serious bug.
And core now actually already provides the a global context provider system, but that doesn't completely fulfill the needs of page manager (context specific contexts, aka based on the page you're looking at.. you can't just globally register that).
Comment #10
dsnopekYes, I agree! I wasn't trying to delay work on this, just post the note so that all the things are linked. I also didn't know how close this was to being finished, and based on your RTBC, it sounds like it's very close!
I'm hoping to at least have a proof-of-concept started on the CTools "context stack" next week at MWDS. :-)
Comment #12
tim.plunkettThanks!