Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.As part of #2124957: Replace 'content' cache tag with 'rendered' and use it sparingly, we added standardized cache tags to all entities in #2217749: Entity base class should provide standardized cache tags and built-in invalidation (with test coverage).
tour.module's Tour config entity type affects rendered content: that for users with the permission to use tours. Let's set the appropriate cache tags and add basic test coverage.
Since this typically only affects authenticated users, the importance of this issue is lower.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | tour_cache_tags-2241267-10.patch | 4.44 KB | Wim Leers |
Comments
Comment #1
Wim LeersComment #2
Wim LeersComment #3
Wim LeersOn second thought, I think that a similar approach as the one in #2241275: DateFormat cache tag: don't set the cache tag, but invalidate the entire render cache? is applicable here.
Just like Date Formats, Tours rarely change. When they do, they will usually arrive via a module update, and when a module is updated, all caches are cleared anyway. That makes the case for using the
array('rendered' => TRUE')cache tag.However, tours are only added at the page level, so tours setting cache tags will only affect page cache items, so not "almost every render cache item", as was the case for Date Formats.
Hence making tour entities set the
array('rendered' => TRUE)cache tag would invalidate all render cache entries, even though we'd really only need to invalidate page render cache items. That's unnecessary.Hence I think we might want to have a single cache tag for all tours (so
array('tour' => TRUE)), rather than one per tour. That:Comment #4
Wim LeersComment #5
moshe weitzman CreditAttribution: moshe weitzman commentedI don't really see the bloat problem with an additional tour-n tag indicating a given tour. I foresee tour changes that happen via a UI instead of code deploy and it would be a shame to flush all pages containing any tour when the tour that was edited has not even been added to a page yet.
Comment #6
nick_schuch CreditAttribution: nick_schuch commentedMoshe is right, we've got a basic prototype of a Tour UI in contrib that will be fleshed out once we hit a BETA. If we could keep the "tour-n" tag that would be awesome!
Comment #7
Wim LeersD'oh! Of course! I totally forgot about the "build tours in UI" use case.
I will update the patch.
Comment #8
Wim LeersEt voilà.
Comment #10
Wim LeersOops, bad patch. Better patch attached. Interdiff in #8 was correct.
Comment #11
moshe weitzman CreditAttribution: moshe weitzman commentedLooks good.
Comment #12
alexpottCommitted 650bc9f and pushed to 8.0.x. Thanks!
Comment #14
Wim Leers