Problem/Motivation

Some things are not ever cacheable (e.g. if they depend on data that is stored externally and that can be changed any second). In that case, they should return max-age = 0. Consequently, cache tags and cache contexts do not matter anymore: the thing is not cacheable at all anyway.

Implementing all three methods of CacheableDependencyInterface in these cases is a chore.

Proposed resolution

Provide an UncacheableDependencyTrait, just like we provide its opposite already: UnchangingCacheableDependencyTrait.

Remaining tasks

Review.

User interface changes

None.

API changes

None, just adding a new trait.

Data model changes

None.

CommentFileSizeAuthor
#19 add-2653346-followup-19.patch434 bytesjibran
#12 2653346-9.patch4.3 KBborisson_
#12 interdiff.txt658 bytesborisson_
#9 2653346-9.patch4.3 KBwim leers
#3 2653346-3.patch981 byteswim leers

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#2624472: Make sure that cache tags of index and server are added to search result pages

Realized this while working on #2624472: Make sure that cache tags of index and server are added to search result pages.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new 2653346-3.patch981 bytes
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +D8 cacheability
fabianx’s picture

fabianx commented
Status: Needs review » Reviewed & tested by the community

RTBC - a nice helper

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Seems this should be 8.1.x right?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Core classes that could be updated to use this:

  • \Drupal\early_rendering_controller_test\CacheableTestDomainObject
  • \Drupal\history\Plugin\views\filter\HistoryUserTimestamp
  • \Drupal\views\Plugin\views\sort\Random

(Perhaps more, this is what a quick search turned up.)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 8.0.x-dev » 8.1.x-dev
Status: Reviewed & tested by the community » Needs work

I think we should put this 8.1.x since it is not a bug and at least have those classes use it.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new 2653346-9.patch4.3 KB

Agreed.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented 
+++ b/core/modules/history/src/Plugin/views/filter/HistoryUserTimestamp.php
@@ -22,6 +23,8 @@
 class HistoryUserTimestamp extends FilterPluginBase {
 
+  use UncacheableDependencyTrait;
+

This means we're going to override the implementation of FilterPluginBase::getCacheContexts() - do we not care about cache contexts if the max age is 0?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Not in this case, because the max-age is unconditionally zero. If it were conditional, then we'd need/want the cache contexts.

+++ b/core/lib/Drupal/Core/Cache/UncacheableDependencyTrait.php
@@ -0,0 +1,40 @@
+ * Use this for objects that are never cacheable.

Hence this bit of documentation. I could make that more explicit.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
StatusFileSize
new interdiff.txt658 bytes
new 2653346-9.patch4.3 KB

Added the extra documentation Wim mentioned in #11.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Thanks @borisson_, that looks great :)

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

fabianx’s picture

fabianx commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC for 8.x-2.x

  • catch committed a05ca5d on 8.2.x 
    Issue #2653346 by Wim Leers, borisson_: Add UncacheableDependencyTrait:...
catch’s picture

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

Committed/pushed to 8.2.x, thanks!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Updated documentation: https://www.drupal.org/node/2541914/revisions/view/9158196/9566219.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Fixed » Reviewed & tested by the community
StatusFileSize
new add-2653346-followup-19.patch434 bytes

Quick follow up patch.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Oh, hah! YAY!

RTBC++

  • catch committed 9ecb172 on 8.2.x 
    Issue #2653346 by Wim Leers, borisson_, jibran: Add...
catch’s picture

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

Doh. Committed/pushed to 8.2.x, thanks!

Status: Fixed » Closed (fixed)

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