Updated: Comment #0

Problem/Motivation

It should be possible to clear all the ckeditor_plugins:<langcode> cache entries at once. That's currently impossible because prefix cache clears no longer exist in favor of cache tags, yet cache tags are not yet being set on the above cache entries.

Proposed resolution

Set cache tags :)

Remaining tasks

None.

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#1 ckeditor_plugins_cache_tags-2109769-1.patch1.07 KBwim leers

Comments

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new ckeditor_plugins_cache_tags-2109769-1.patch1.07 KB

Trivial :)

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community

Discussed this with @Wim Leers:

Gábor Hojtsy> Wim Leers: so no use of that in core?!
Gábor Hojtsy> Wim Leers: looks like a trivial patch but no clearing to happen in core?
Wim Leers> Gábor Hojtsy: nope, no use of that in core, just like any other plugin manager, that cache is only cleared when people install additional modules

That makes sense. I don't think individual added cache tags need any test coverage, so this should be fine.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Fixed » Active

Uh, why?

There's an API to clear the cache, $plugin->clearCachedDefinitions(). Cache tags should only be used if this is supposed to be cleared together with other things.

It is by design that this doesn't use cache tags by default, as they are right now a considerable overhead (every tag adds another query)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

#4: then why do LocalActionManager and FilterPluginManager do the same? In fact, why is it part of the setCacheBackend() signature at all?

I do see how PluginManager::clearCachedDefinitions() can be used to achieve the same though. I feel silly now :( :P. But at the same time, if it shouldn't be done, then why is it part of the API at all?

And yes, clearCachedDefinitions() is more efficient because it doesn't need to add cache tags. I just always thought that clearCachedDefinitions() was a static cache, not a full-on cache, that's why I didn't think of looking at that.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Ok, reverted for now, since it looks like this needs more discussion.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

#5, quoting #4 again:

It is by design that this doesn't use cache tags by default, as they are right now a considerable overhead (every tag adds another query)

FilterPluginManager absolutely needs this because filter.module used cache tags previously, see filter_formats()

LocalActionManager likely doesn't need it, we should open a separate issue to consider removing it.

I think this is won't fix.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Active » Closed (won't fix)
Issue tags: -sprint

All right, its rolled back already :)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

"won't fix" indeed.

Sorry guys :(