Dependency injection: FromKeyedServiceAttribute falls back to last defined unkeyed service of the type -- unexpected behavior?

[askogvold]

Description

I'm currently in a repo where we might need a mix of keyed and unkeyed services. For the specific case, various types of a service are configured, and injected through an IEnumerable. The keyed dependency would be used to retrieve a default one for some specific cases. We've been able to work around it, but I was surprised by the behavior, so I just want to check if this behavior is intended.

When configuring a service which has a constructor argument with the attribute [FromKeyedServices("myKey")] on an argument - if that service was not configured with the key, I will get injected the last one which was configured without a key.

This seems like a corner case, and in my mental model, I'd expect to get an exception if the argument is non-nullable, or null if it were. If the typing system makes that challenging, an option could be to add another attribute: FromRequiredKeyedService which would throw if the service is missing.

Reproduction Steps

The test called ShouldNotGetServiceIfKeyedIsNotAdded demonstrates the issue

namespace keys_repro;
using Microsoft.Extensions.DependencyInjection;

public class DemonstratingTheProblemTests
{
    [Fact]
    public void WorksIfKeyIsSpecified()
    {
        IServiceCollection services = new ServiceCollection();

        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceA"));
        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceB"));
        services.AddKeyedTransient<IService>("myKey", (_,_) => new ServiceImplementation("KeyedService"));
        services.AddKeyedTransient<IService>("someOtherKey", (_,_) => new ServiceImplementation("AnotherKeyedService"));
        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceC"));
        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceD"));
        services.AddTransient<ServiceWhichAcceptsKeyedService>();

        var serviceProvider = services.BuildServiceProvider();
        var myResolvedService = serviceProvider.GetRequiredService<ServiceWhichAcceptsKeyedService>();

        Assert.Equal("KeyedService", myResolvedService.ServiceIfDefinedByKey!.Name);
    }

    [Fact]
    public void ShouldNotGetServiceIfKeyedIsNotAdded()
    {
        IServiceCollection services = new ServiceCollection();

        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceA"));
        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceB"));
        // and no keyed service added - but continue with the rest as before
        services.AddKeyedTransient<IService>("someOtherKey", (_,_) => new ServiceImplementation("AnotherKeyedService"));
        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceC"));
        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceD"));

        services.AddTransient<ServiceWhichAcceptsKeyedService>();

        var serviceProvider = services.BuildServiceProvider();
        var myResolvedService = serviceProvider.GetRequiredService<ServiceWhichAcceptsKeyedService>();

        Assert.Null(myResolvedService.ServiceIfDefinedByKey); // This fails - ServiceD is resolved
    }
}

public record ServiceWhichAcceptsKeyedService([FromKeyedServices("myKey")] IService? ServiceIfDefinedByKey);

public interface IService
{
    string Name { get; }
}

public record ServiceImplementation(string Name) : IService;

Expected behavior

Test called ShouldNotGetServiceIfKeyedIsNotAdded should succeed - I would expect it not to resolve the service when I request a keyed service, and no keyed service is defined.

Actual behavior

The test called ShouldNotGetServiceIfKeyedIsNotAdded fails - I get the last configured unkeyed service of the type.

Regression?

No response

Known Workarounds

Explicitly taking a serviceProvider, and calling GetKeyedService or GetRequiredKeyedService works.

(for the types not defined in this example, see their implementation in the reproduction steps)

namespace keys_repro;

using Microsoft.Extensions.DependencyInjection;

public class WorkaroundTests
{
    [Fact]
    public void ShouldGetServiceWhenKeyedIsConfigured()
    {
        IServiceCollection services = new ServiceCollection();

        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceA"));
        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceB"));
        services.AddKeyedTransient<IService>("myKey", (_,_) => new ServiceImplementation("KeyedService"));
        services.AddKeyedTransient<IService>("someOtherKey", (_,_) => new ServiceImplementation("AnotherKeyedService"));
        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceC"));
        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceD"));

        services.AddTransient<ServiceWhichAcceptsKeyServiceExplicitWorkaround>();

        var serviceProvider = services.BuildServiceProvider();
        var myResolvedService = serviceProvider.GetRequiredService<ServiceWhichAcceptsKeyServiceExplicitWorkaround>();

        Assert.Equal("KeyedService", myResolvedService.ServiceIfDefinedByKey!.Name); // This workaround works
    }

    [Fact]
    public void ShouldNotGetServiceIfKeyedIsNotAddedWithExplicitWorkaround()
    {
        IServiceCollection services = new ServiceCollection();

        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceA"));
        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceB"));
        // and no keyed service added - but continue with the rest as before
        services.AddKeyedTransient<IService>("someOtherKey", (_,_) => new ServiceImplementation("AnotherKeyedService"));
        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceC"));
        services.AddTransient<IService>(_ => new ServiceImplementation("ServiceD"));

        services.AddTransient<ServiceWhichAcceptsKeyServiceExplicitWorkaround>();

        var serviceProvider = services.BuildServiceProvider();
        var myResolvedService = serviceProvider.GetRequiredService<ServiceWhichAcceptsKeyServiceExplicitWorkaround>();

        Assert.Null(myResolvedService.ServiceIfDefinedByKey); // This workaround works
    }
}

public class ServiceWhichAcceptsKeyServiceExplicitWorkaround
{
    public IService? ServiceIfDefinedByKey { get; }

    public ServiceWhichAcceptsKeyServiceExplicitWorkaround(IServiceProvider serviceProvider)
    {
        ServiceIfDefinedByKey = serviceProvider.GetKeyedService<IService>("myKey");
    }
}

Configuration

<TargetFramework>net8.0</TargetFramework>

<PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0" />

Other information

No response

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-serviceprocess
See info in area-owners.md if you want to be subscribed.

[askogvold]

@dotnet/area-extensions-dependencyinjection

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection
See info in area-owners.md if you want to be subscribed.

[steveharter]

cc @benjaminpetit

[julealgon]

Surprised by this behavior. I agree it looks weird and should not be like that: FromKeyedServices should have "required + keyed" semantics by default and throw if no keyed service with that key is found.

This "fallback" behavior, if intentional, should at least be behind a bool setting in the attribute, something like:

public record ServiceWhichAcceptsKeyedService(
    [FromKeyedServices("myKey", FallbackToNonKeyed = true)] IService service);

Where FallbackToNonKeyed would be false by default.

[CarnaViire]

It definitely looks more like a bug than a feature...

This was a nasty surprise that we've hit now while implementing #89755 (HttpClientFactory support for keyed DI) -- will the fix make it to 9.0? @steveharter @benjaminpetit

Come context:

HttpClientFactory is a singleton service that configures (services.AddHttpClient(...)) and creates (factory.CreateClient(...)) HttpClient instances by name. Historically, it registers a "default" (unconfigured) HttpClient (client with an empty name) as a plain-Transient HttpClient service

runtime/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientFactoryServiceCollectionExtensions.cs

Lines 65 to 69 in 8c95a64

	// Register default client as HttpClient
	services.TryAddTransient(s =>
	{
	return s.GetRequiredService<IHttpClientFactory>().CreateClient(string.Empty);
	});

#89755 is about allowing to use [FromKeyedServices] to inject the required configured Named HttpClients directly, so you won't have to use the factory to retrieve them manually.

Current behavior unfortunately means that every Named HttpClient, that was NOT opted-in to, or even opted-OUT from, the Keyed registration -- instead of throwing upon injection (expected) will end up substituted with an unconfigured HttpClient instead (super misleading!). This would be a hell to troubleshoot. 😞

cc @dotnet/ncl @halter73 @davidfowl

[halter73]

It definitely looks more like a bug than a feature...

Agreed. A parameter attributed with [FromKeyedServices("myKey")] should never get a service injected that GetKeyedService would not return.

I think we should patch .NET 8 to fix this. I cannot imagine on anyone relying on this behavior. And if anyone is relying on that behavior, we should nip that in the bud.

[paulomorgado]

I think we should patch .NET 8 to fix this. I cannot imagine on anyone relying on this behavior. And if anyone is relying on that behavior, we should nip that in the bud.

If it's there, it's being relied on by someone.

If you are patching .NET 8, consider putting it behind an app setting.

[steveharter]

This appears to be a duplicate of #102204 which was recently fixed.