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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers’s picture

Status: Active » Needs review
FileSize
1.07 KB

Trivial :)

Gábor Hojtsy’s picture

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

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Berdir’s picture

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

#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

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

tim.plunkett’s picture

#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

Status: Active » Closed (won't fix)
Issue tags: -sprint

All right, its rolled back already :)

Wim Leers’s picture

"won't fix" indeed.

Sorry guys :(