Problem/Motivation

We are using the Autowire attribute in a number of places (e.g. #[Autowire(service: 'foo')]) because we don't have a service alias for the interface/class name.

Let's add the aliases so the Autowire attribute isn't needed wherever we can.

From #6 the list is:

$ grep -RhoP '#\[Autowire\(\K[^)]+' core | grep -v 'param:' | grep -v 'logger\.' | grep -v 'cache\.' | grep -v 'router'| grep -v 'keyvalue' | grep -v 'lock' | sort -u
'@plugin.manager.config_action'
'@workspaces.negotiator.query_parameter'
service: 'access_check.theme'
service: 'asset.css.collection_optimizer'
service: 'entity.memory_cache'
service: 'file.mime_type.guesser'
service: 'file.recursive_validator'
service: 'kernel'
service: 'node.view_all_nodes_memory_cache'
service: 'plugin.manager.field.formatter'
service: 'plugin.manager.help_section'
service: 'plugin.manager.views.join'
service: 'transliteration'
service: 'validation.constraint'
service: TranslationInterface::class

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3559978

Command icon 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:

Comments

kim.pepper created an issue. See original summary.

kim.pepper’s picture

Title: Add service aliases for file module » Add service aliases for use in file module
mstrelan’s picture

I've had to manually autowire the lock service a few times, and there are several instances of it in core. I don't think it needed to be reverted, it looks like the failing tests are relying on it as an example of a service that's not autowireable. They should be updated to use a different service or even a dummy service in a test module.

kim.pepper’s picture

I guess I was trying to keep the scope of this issue to what is needed in the file module services. I bet there are a lot of other missing aliases. Do we want to try and find them and add them in this issue?

mstrelan’s picture

How about a compromise of anywhere that is currently using the Autowire attribute? Can find with this command below. Not sure all of these can be autowired.

$ grep -RhoP '#\[Autowire\(\K[^)]+' core | grep -v 'param:' | sort -u
'@cache.discovery'
'@cache.memory'
'@logger.channel.file'
'logger.dblog'
'@plugin.manager.config_action'
service: 'access_check.theme'
service: 'asset.css.collection_optimizer'
service: 'cache.bootstrap'
service: 'cache.memory'
service: 'entity.memory_cache'
service: 'kernel'
service: 'keyvalue'
service: 'lock'
service: 'lock.persistent'
service: 'logger.channel.default'
service: 'logger.channel.workspaces'
service: 'node.view_all_nodes_memory_cache'
service: 'plugin.manager.field.formatter'
service: 'plugin.manager.help_section'
service: 'plugin.manager.views.join'
service: 'router'
service: TranslationInterface::class
'@workspaces.negotiator.query_parameter'
kim.pepper’s picture

Title: Add service aliases for use in file module » Add service aliases for services that currently need the Autowire attribute
Issue summary: View changes

OK sounds good. I updated the title and IS.

From #6

kim.pepper’s picture

I think we can take logger.channel.foo off the list.

longwave’s picture

We can't autowire any of the cache or logger services, this is by design that you have to explicitly select which bin or channel that you want to use.

Similarly the reason I am not sure we can do lock or lock.persistent is because they both implement the same interface, and it felt better to force the developer to select which case. From what I remember it's the same with keyvalue and router, there are multiple implementations of the interface available. We could select a default implementation in these cases, but is that the right thing to do?

kim.pepper’s picture

Issue summary: View changes

Updated the list in the IS to exclude those services listed above.

kim.pepper’s picture

I had another review of this. I think we should avoid adding service aliases for classes and just focus on adding aliases for interfaces if they are missing. Adding aliases for plugin managers doesn't make sense to me. We might want to consider updating classes injecting plugin managers to use PluginManagerInterface instead?

I added a couple of things I think should be changed.

nicxvan’s picture

Should we somehow enforce or document the ones that are intentionally not aliased?

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.