Problem/Motivation
Symfony allows autoconfiguration - automatic tagging of services that implement a particular interface.
Drupal has lots of event subscribers, and they need to both implement EventSubscriberInterface and be tagged with event_subscriber.
Steps to reproduce
Proposed resolution
Implement autoconfiguration so services that implement EventSubscriberInterface are automatically tagged with event_subscriber.
Enable autoconfiguration in core.services.yml.
Remaining tasks
Open a followup to autoconfigure core modules.
Open followups to add more tags/interfaces that can be autoconfigured.
User interface changes
API changes
Services can now specify autoconfigure: true instead of manually tagging individual services with event_subscriber.
Data model changes
Release notes snippet
Original issue summary
For a variety of reasons, Drupal's implementation of the Symfony dependency injection container does not include features from upstream, either because Drupal 8's implementation lifecycle predated certain features being included in Symfony (D8 started requiring Symfony 3 and the upstream project is now on v5) or because certain Drupalisms prevent a Symfony-like implementation (e.g., our requirement the DI container be serialized/cached into the database and not onto disk, to enable runtime module enable/disable actions.)
The Symfony DI component allows service autowiring (always possible but not really utilized; core is likely to autowire, soon) and autoconfiguration (never implemented in Drupal.) Autoconfiguration means services may be auto-registered as event subscribers, controllers, and the like, based on the service's implementation of an interface.
Changes such as #3021898: Support _defaults key in service.yml files for public, tags and autowire settings get us closer to supporting autoconfiguration. Due to Drupal's model of services.yml files per module, and the Drupal-specific way in which module services are enabled in the container, this will require some domain-specific implementation. That said, with Drupal implementing more of the native Symfony pattern for services.yml _defaults syntax, many modules could combine autowiring and autoconfiguration to remove almost all the boilerplate now common in [module].services.yml.
Autoconfiguration is a bit of a special case as that option needs to be parsed in the Yaml services definition and added to the service definition...but the actual autoconfiguration occurs in ContainerBuilder::registerForAutoconfiguration() and ContainerBuilder::merge(). The former method we do not override and as such registration could happen rather easily; the latter method is not invoked by Drupal's implementation of the kernel. I think this could actually be implemented without a significant lift, if we stay true to the upstream container builder's architecture.
I would also add here that adding support for the Yaml parser's PHP constant interpolation (see https://symfony.com/blog/new-in-symfony-3-2-php-constants-in-yaml-files) would be nice/get us more inline with upstream, but I'd have to do some issue archaeology to determine if this feature flag is omitted purposefully.
| Comment | File | Size | Author |
|---|
Issue fork drupal-3221128
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3221128-enable-service-autoconfiguration
changes, plain diff MR !3899
Comments
Comment #2
bradjones1Comment #6
longwaveIt turns out this seems almost trivial to implement; the attached patch allows autoconfiguration and enables it for event subscribers and cache contexts, and then implements it in core.services.yml.
Comment #7
andypostNice idea but needs to fix cspell
Unknown word (autoconfigure)how that may affect contrib?
Comment #8
longwave_defaultsonly affects the file it is used in, so this makes no change for contrib or any core module, only core services.To improve DX further we could decide to enable autoconfiguration by default for all modules, as all it does is automatically tag services that implement a specific interface, but that change would probably need to be made in a major release.
Comment #9
longwaveThis has uncovered some bugs: a number of cache contexts do not implement CacheContextInterface or CalculatedCacheContextInterface. So, let's just implement this for event subscribers for now.
Another followup could be to automatically discover event subscribers as long as they live in a EventSubscriber namespace; once we have autowiring as well then we don't need to add them to .services.yml at all in many cases.
Comment #10
andypostcurious why that working, because tagged services has no check for interface?
Comment #11
longwaveYeah, there is no interface check - tagged services are collected and they work because the methods exist, but no type checking is done.
edit: see #2915594: SessionCacheContext class should implement CacheContextInterface
Comment #12
longwaveComment #14
longwaveComment #15
andypostAs it applies to only one file is there a way to add test to make sure that that subscribers has no such tag-name?
For example to prevent old patches to apply
Comment #17
longwaveAdded test coverage, which found a service in core.services.yml that I missed. However it doesn't really matter if a service is both autoconfigured and the tag is manually specified, it still works correctly in that case.
I think this is OK for now and we could autoconfigure core modules and enable autoconfiguration for other tags in a followup?
Comment #18
longwaveComment #19
longwaveChange record: https://www.drupal.org/node/3357408
Comment #20
longwaveComment #21
longwaveComment #22
smustgrave commentedOpened #3358641: Enable autoconfiguration for other service tags for the follow up.
Comment #24
larowlanNo longer applies cleanly
Fine to self RTBC after reroll
Comment #26
duadua commentedMR has been rebased.
Comment #27
duadua commentedComment #28
smustgrave commentedMoving back to RTBC
Comment #29
larowlanTagging reviewers, those who created follow-ups and the original reporter
Comment #31
larowlanCommitted to 11.x (nee 10.2.x) and published the change record, nice one!
Comment #34
claudiu.cristeaHow can we set _defaults for the whole project?