Problem/Motivation

For a long time, including in Drupal 7, it was possible to render cache something. For all that time, you had 2 ways of generating a cache ID:

  1. set #cache[cid]
  2. set #cache[keys]

The first is basically just a concatenated version of the second. So they were almost interchangeable. In any case, it was fine for both to exist. In practice, almost nobody uses #cache[cid] though — almost everybody uses #cache[keys] (because it's easier to manipulate in alter hooks, for starters).

But ever since #2158003: Remove Block Cache API in favor of blocks returning #cache with cache tags landed a year ago, we've also had #cache[contexts], basically cid == keys + contexts_to_keys(contexts). So they weren't interchangeable anymore.

And since then, about a month ago #2429257: Bubble cache contexts landed. Ever since, we have a bigger problem: cache contexts must bubble and thus must affect the cache ID generated from keys+contexts of render tree ancestors. But, we can't merge cache contexts with #cache[cid], only with #cache[keys]. Hence, using #cache[cid] effectively breaks cache bubbling.

Proposed resolution

Remove #cache[cid].

Remaining tasks

TBD

User interface changes

None.

API changes

Removed #cache[cid].

CommentFileSizeAuthor
#2 render_cache_cid-2459003-2.patch9.95 KBWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Fabianx’s picture

Fabianx CreditAttribution: Fabianx for Drupal Association commented

+1 to removing it, because as a user you can convert a normal $cid to keys, there is no real BC break.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Active » Needs review
FileSize
render_cache_cid-2459003-2.patch9.95 KB

Even more arguments in favor of removing #cache[cid]:

  1. It's documented as "use only if you have special requirements":
    -   *     - 'cid': Specify the cache ID directly. Either 'keys' or 'cid' is
-   *       required. If 'cid' is set, 'keys' is ignored. Use only if you have
-   *       special requirements.
  2. The only users in core are in tests.
Fabianx’s picture

Fabianx CreditAttribution: Fabianx for Drupal Association commented
Status: Needs review » Reviewed & tested by the community

RTBC - if tests pass.

Render Caching is very uncommon in D7 anyway and keys is always better, so no reason to keep this special case.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

Fine with the API change, very unlikely that anyone's relying on this. However it needs a change record.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Issue tags: +API change

CR created: https://www.drupal.org/node/2459367

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed 17d2e5c on 8.0.x 
    Issue #2459003 by Wim Leers: #cache[cid] breaks bubbling
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

CR published.

Status: Fixed » Closed (fixed)

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