Problem/Motivation

Some use cases would benefit from limited cache context, e.g. the RequestPath condition as added in #2375695: Condition plugins should provide cache contexts AND cacheability metadata needs to be exposed.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#7 2375695-request-path-cache-7.patch606 byteskim.pepper

Comments

fabianx’s picture

fabianx commented
Parent issue: » #2429287: [meta] Finalize the cache contexts API & DX/usage, enable a leap forward in performance
fabianx’s picture

fabianx commented
Related issues: +#2483183: Make breadcrumb block cacheable

#2483183: Make breadcrumb block cacheable introduces such a cache context.

dawehner’s picture

dawehner
Primary language German
commented

Well, should we mark this issue now as duplicated?

fabianx’s picture

fabianx commented

Maybe after that other issue is in as its currently at needs work?

fabianx’s picture

fabianx commented
Title: Add a url.path cache context » Add a url.path cache context and make core/modules/system/src/Plugin/Condition/RequestPath use it
Related issues: +#2375695: Condition plugins should provide cache contexts AND cacheability metadata needs to be exposed

Nope, as part of this core/modules/system/src/Plugin/Condition/RequestPath.php should be updated to use it.

@berdir:

Did you intend for url.path to include basePath() or not?

The breadcrumb issue uses getBasePath() . getPathInfo().

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Add a url.path cache context and make core/modules/system/src/Plugin/Condition/RequestPath use it » core/modules/system/src/Plugin/Condition/RequestPath should use the 'url.path' cache context
Issue tags: +D8 cacheability

#2483183: Make breadcrumb block cacheable was RTBC, got kicked back, and is now RTBC again. Which means that in all likelihood, that issue will land before this one, which means we can use the 'url.path' cache context being introduced there.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Active » Needs review
StatusFileSize
new 2375695-request-path-cache-7.patch606 bytes

Looks pretty simple.

dawehner’s picture

dawehner
Primary language German
commented

Ideally it would allow us to remove some of the longer lists of cache contexts.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

Ideally it would allow us to remove some of the longer lists of cache contexts.

You mean e.g. PageCacheTagsIntegrationTest?

We don't have any integration tests using the RequestPath condition, clearly. So… not sure what else to do here. This is straightforward enough.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 8b4ceac and pushed to 8.0.x. Thanks!

  • alexpott committed 8b4ceac on 8.0.x 
    Issue #2521978 by kim.pepper: core/modules/system/src/Plugin/Condition/...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.