Problem/Motivation

The keyvalue service cannot be autowired since the Drupal\Core\KeyValueStore\KeyValueFactoryInterface service alias is missing. Same for keyvalue.expirable

Steps to reproduce

Proposed resolution

  • Add the Drupal\Core\KeyValueStore\KeyValueFactoryInterface alias to the container.
  • Add the Drupal\Core\KeyValueStore\KeyValueExpirableFactoryInterface alias to the container.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3534354

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:

About issue forks

Comments

znerol created an issue. See original summary.

znerol opened merge request !12640

znerol’s picture

znerol commented
Issue summary: View changes
Status: Active » Needs review
nicxvan’s picture

nicxvan commented

I don't see any reason not to do this, but two things I've seen.

You can still autowire using the attribute and service id.

Usually I see these conversions done in the issue that needs the autowiring.

If this is so contrib can do it a link to that issue would be helpful.

znerol’s picture

znerol commented

Came here from #3397420: Add a way to capture mails sent through the mailer transport service during tests. Autowiring is not a requirement over there, but I noticed that referencing KeyValueFactoryInterface::class did not work. I.e.:

  public function alter(ContainerBuilder $container): void {
    [...]
    $definition->addMethodCall('setKeyValueFactory', [new Reference(KeyValueFactoryInterface::class)]);
    [...]
  }

Regarding the process, we had a couple of these in the past (e.g., #3529592: Wrong alias for access_policy_processor service in core.services.yml and #3401331: Add Psr\EventDispatcher\EventDispatcherInterface alias to core services).

nicxvan’s picture

nicxvan commented

Oh are you in a service provider?

I was talking about this

public function __construct(
#[Autowire(service: 'monolog.logger.request')]
private LoggerInterface $logger,
) {

https://symfony.com/doc/current/service_container/autowiring.html

znerol’s picture

znerol commented
Title: The keyvalue service cannot be autowired » Add KeyValueFactoryInterface and KeyValueExpirableFactoryInterface alias to core services
Category: Bug report » Task

Retitled and made it into a task to match the previous issue.

nicxvan’s picture

nicxvan commented
Status: Needs review » Reviewed & tested by the community

Looks right to me!

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Needs review

When autowiring went in originally, we added core/tests/Drupal/KernelTests/Core/DependencyInjection/AutowireTest.php that is supposed to error if a services is added without a FQCN alias. Can we do some digging to work out why these services weren't causing that test to fail, so we can be sure that test is doing what we expect?

nicxvan’s picture

nicxvan commented

Is it just new services? I swear I have had to add them too.

znerol’s picture

znerol commented

Can we do some digging to work out why these services weren't causing that test to fail, so we can be sure that test is doing what we expect?

core.services.yml contains the following keyvalue related definitions:

  keyvalue:
    class: Drupal\Core\KeyValueStore\KeyValueFactory
    arguments: ['@service_container', '%factory.keyvalue%']
  keyvalue.database:
    class: Drupal\Core\KeyValueStore\KeyValueDatabaseFactory
    arguments: ['@serialization.phpserialize', '@database']
  keyvalue.expirable:
    class: Drupal\Core\KeyValueStore\KeyValueExpirableFactory
    arguments: ['@service_container', '%factory.keyvalue.expirable%']
  keyvalue.expirable.database:
    class: Drupal\Core\KeyValueStore\KeyValueDatabaseExpirableFactory
    arguments: ['@serialization.phpserialize', '@database', '@datetime.time']

The services keyvalue and keyvalue.database both implement KeyValueFactoryInterface. Likewise the services keyvalue.expirable and keyvalue.expirable.database both implement KeyValueExpirableFactoryInterface. The AutowireTest only requires an alias for classes that are the only implementations of an interface.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Reviewed & tested by the community

Thanks for explaining!

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

I deliberately did not add this at the time we added the rest of the aliases because of the possible confusion between keyvalue and keyvalue.database as they share the interface, but you will (almost) always want keyvalue to proxy to the selected implementation so adding the alias to that makes sense so it can be injected more easily. And exactly the same applies for the expirable services too.

  • catch committed ad3a2b45 on 10.6.x 
    Issue #3534354 by znerol, nicxvan, larowlan, longwave: Add...

  • catch committed 97b53f79 on 11.x 
    Issue #3534354 by znerol, nicxvan, larowlan, longwave: Add...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 10.6.x-dev
Status: Reviewed & tested by the community » Fixed

Thanks for #11 and #13 (and #9 which prompted the answers), this makes loads of sense with those explanations but a bit hard to get my head around otherwise.

Committed/pushed to 11.x, thanks! I think we want this in 10.6.x too so that contrib can use autowiring by requiring >= 10.6 | >= 11.3, so cherry-picked there too.

catch closed merge request !12640

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.