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.
Problem/Motivation
See #2241377: [meta] Profile/rationalise cache tags. Visiting the requirements page invalidates 3 entity cache tags, that do not only invalidate the entity type and field definitions caches, but also the complete entity storage cache.
That's not nice.
Proposed resolution
Add a way to entity manager to ignore persistent caches. Call that and then get the change list, instead of invalidating all caches.
Comments
Comment #1
BerdirComment #2
BerdirUntested, but something like this should do it?
Comment #3
Fabianx CreditAttribution: Fabianx commentedLooks great to me!
Do we need some tests for this?
Comment #4
BerdirReally, green, not even some unit test fails? How boring ;)
There are unfortunately no unit tests yet for EntityDefinitionUpdateManager, if so, then it would have been relatively easy to assert that no cache tags are invalidated, didn't find a good way to do something similar in the kernel tests. The only thing I can think of is that we'd have to register a cache tag invalidator that stores invalidations so that we can check which tags were invalidated.
What I did change is remove the explicit cache clears in the tests, I think those are wrong because then we're not testing that the entity definitions manager properly handles caches.
Also enabling caches again and explicitly invalidating them in applyChanges(), so if we actually change something, then I think it is correct to invalidate any changes.
Comment #5
plachNice solution, would it make sense to move it into the default plugin manager?
Comment #6
BerdirMaybe, could be in CachedDiscoveryInterface I guess? Then we wouldn't have to override those two methods in the entity manager.
Comment #7
plachSounds good :)
Comment #8
BerdirDid that, extended unit test for default plugin manager, also added the missing cacheSet()
Comment #9
Fabianx CreditAttribution: Fabianx commentedDoes that mean we can now globally in the installer / ModuleHandler::install instead of:
clearCachedDefinitions()
use the
->useCaches(FALSE)
And just invalidate all config caches maybe once at the end?
/me excited - Patch looks good.
Comment #10
plachLooks great aside from these dummy things below ;)
The PHP doc text is confusing :)
80 chars
Comment #11
Berdir@Fabianx: No, I don't think that is possible/correct, in that case, we have to clear caches, we can expect that they changed. Also, the flag would get lost during the container rebuild.
Fixed the comments.
Comment #12
plachLooks ready to me
Comment #14
catchCommitted/pushed to 8.0.x, thanks!