|Issue tags:||Plugin system|
Plugin discovery is completely untrustworthy. After a long drawn out conversation in #1851706: Make all core plugin managers be consistent as to whether $this or $this->discovery is passed to the factory we came to a couple of conclusions that were never formalized there.
1.) Unnecessary method calls should be avoided when possible.
2.) The CacheDecorator is almost always the last in the chain of discovery decorators.
3.) The Plugin manager is the final authority about plugin definitions, which adds unnecessary method calls and can break points 1 and 2.
To that end, these are not all solvable with separate classes representing all of this. A single plugin manager class that solves all these problems at once is what's actually needed. To that end I've build a new DrupalPluginManagerBase which should likely become the default plugin manager implementation for both Core and Contrib.
Features of DrupalPluginManagerBase:
- Extends CacheDecorator
- Extending the Cache Decorator allows us to do custom cache miss logic, and add additional static or dynamic plugin definitions which are not easily added in other ways.
- Restores trust in the PluginManager's methods instead of encouraging developers to bypass them.
- Sets up caching correctly and provides a central mechanism by which to change all plugin cache implementations simultaneously if that proves to be a false statement. (and it might be)
Stuff that's needed:
- A bit of debate about constructor arguments. We may want to expose some of the existing cache arguments in it, but they should all be protected variables of the manager now, so it's not like they're inaccessible. But I'm sure this needs some additional tweaking.
- Tests. We don't actually have tests for the existing PluginManagerBase, we should provide some for this one since it actually does stuff.
Hopefully this is a starting point to a better solution we can ALL get on board with.