Problem/Motivation

Purge tag headers output all cache tags and can cause issues with response header sizes. Others have proposed cache tag minification to address this matter, and I propose that we add a tag blacklist option to omit tags entirely based on need.

The purge_queuer_coretags submodule provideds such a blacklist for limiting the cache tags being queued for purging.

Proposed resolution

Provide a purge setting form for configuring a purge tag header blacklist.

Remaining tasks

Implement config schema, settings form, testing scenarios, and blacklisting logic

User interface changes

Yes

API changes

No

Data model changes

Yes, config.

CommentFileSizeAuthor
#17 purge-tagsheaders-blacklist-2977563-16.patch13.01 KBshree.yesare
#13 purge-tagsheaders-blacklist-2977563-13.patch7.6 KBluksak
#12 purge-tagsheaders-blacklist-2977563-12.patch7.61 KBluksak
#9 purge-tagsheaders-blacklist-2977563-9.patch13.12 KBartem_sylchuk
#5 tags-headers-blacklist-2977563-5.png433.56 KBbighappyface
#5 tags-headers-dashboard-2977563-5.png502.2 KBbighappyface
#2 purge-tagsheaders-blacklist-2977563-2.patch13.78 KBbighappyface

Comments

bighappyface created an issue. See original summary.

bighappyface’s picture

bighappyface commented
StatusFileSize
new purge-tagsheaders-blacklist-2977563-2.patch13.78 KB

This patch:

  • Adds new purge.tagsheaders config to purge.schema.yml
  • Duplicates Drupal\purge_queuer_coretags\Form\ConfigurationForm into new form Drupal\purge_ui\Form\TagsHeadersConfigForm
  • Adds "Tags Headers" secton to admin (screenshot below)
  • Modifies Drupal\purge\EventSubscriber\CacheableResponseSubscriber to enforce blacklist for all tags headers
  • Add Drupal\purge\Tests\TagsHeader\CacheableResponseSubscriberTest::testTagsHeadersBlacklist test coverage
bighappyface’s picture

bighappyface commented
Status: Active » Needs review
WidgetsBurritos’s picture

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

+1 RTBC

Seems to be working for me

bighappyface’s picture

bighappyface commented
Issue summary: View changes
StatusFileSize
new tags-headers-dashboard-2977563-5.png502.2 KB
new tags-headers-blacklist-2977563-5.png433.56 KB

Screenshots:

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: purge-tagsheaders-blacklist-2977563-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

vegantriathlete’s picture

vegantriathlete
he/him/his
Primary language English
Location Lakewood, CO
commented

Would it be better to create a whitelist? The problem with a blacklist is that it's possible for things to slip through (unnoticed) in the future because somebody forgot to add them to the blacklist. With a whitelist people would need to take an affirmative action to get something new to happen.

azinck’s picture

azinck commented

I'd agree that a whitelist would be a really nice option, at least.

artem_sylchuk’s picture

artem_sylchuk
Primary language Ukrainian
Location Lutsk
commented
Status: Needs work » Needs review
StatusFileSize
new purge-tagsheaders-blacklist-2977563-9.patch13.12 KB

Here is the re-roll of the patch, it wasn't applying anymore

pbabin’s picture

pbabin commented

Disregard comment.

luksak’s picture

luksak
he/him
commented
Status: Needs review » Needs work

In my case I need exact matches. I want to blacklist node_list, but that would also match node_list:article, which is not what I want.

luksak’s picture

luksak
he/him
commented
StatusFileSize
new purge-tagsheaders-blacklist-2977563-12.patch7.61 KB

Attached you can find a re-roll.

luksak’s picture

luksak
he/him
commented
StatusFileSize
new purge-tagsheaders-blacklist-2977563-13.patch7.6 KB

This one only checks for exact matches. But I didn't fix anything else. So just ignore this patch in case you don't need that.

luksak’s picture

luksak
he/him
commented

Oh, I just realized that using prefixes is the way the blacklist of purge_queuer_coretags works already. I don't think that that is optimal, since that gives us less flexibility, but I guess that's is another issue. Please ignore the patch in #13.

luksak’s picture

luksak
he/him
commented
Status: Needs work » Needs review

The last submitted patch, 12: purge-tagsheaders-blacklist-2977563-12.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

shree.yesare’s picture

shree.yesare commented
StatusFileSize
new purge-tagsheaders-blacklist-2977563-16.patch13.01 KB

Updated patch #12 with missing TagsHeadersConfigForm.php

If using acquia_purge module, it will require below change in acquia_purge module.

diff --git a/acquia_purge.services.yml b/acquia_purge.services.yml
index c2a102f..01ce505 100644
--- a/acquia_purge.services.yml
+++ b/acquia_purge.services.yml
@@ -5,7 +5,7 @@ services:
     public: true
   acquia_purge.tagsheaders.cacheable_response_subscriber:
     class: Drupal\acquia_purge\EventSubscriber\CacheableResponseSubscriber
-    arguments: ['@purge.tagsheaders']
+    arguments: ['@purge.tagsheaders', '@config.factory']
     public: true
     tags:
       - { name: event_subscriber }

Status: Needs review » Needs work

The last submitted patch, 17: purge-tagsheaders-blacklist-2977563-16.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

firewaller’s picture

firewaller commented

This is great! Some suggestions to make it more viable:

  1. Add an "Invert" checkbox to deny/allow the listed cache tags dynamically
  2. Add URL groups to deny/allow cache tags for specific pages
  3. Use wildcard character instead of "contains" functionality for more granular control