dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.97k stars 4.66k forks source link

[Behaviour Proposal]: Allow resolving keyed dependencies without FromKeyedServicesAttribute #105913

Open wvpm opened 1 month ago

wvpm commented 1 month ago

Background and motivation

Currently (Microsoft.Extensions.DependencyInjection 8.0.0) you can apply the FromKeyedServicesAttribute to parameters in a constructor. This allows you to inject services that are registered with a key. Great.

Now what if I can't change the constructor? I'm forced to write an implementation factory that calls serviceProvider.GetRequiredKeyedService<T>(key) for every parameter. (If there is a better way and I missed it, please let me know.)

To me, it's clear that when I register a service as keyed, its dependencies should be resolved using that key (perhaps with a fallback to unkeyed). Example ServiceWithDependency requires ITestable:

object key = new();
ServiceCollection serviceCollection = new();
serviceCollection.AddKeyedTransient<ITestable, DummyService>(key);
serviceCollection.AddKeyedTransient<ServiceWithDependency>(key);
using ServiceProvider serviceProvider = serviceCollection.BuildServiceProvider(
    new ServiceProviderOptions() { ValidateOnBuild = true, ValidateScopes = true }
);
ServiceWithDependency instance = serviceProvider.GetRequiredKeyedService<ServiceWithDependency>(key);
  1. That example will throw on the BuildServiceProvider as ServiceWithDependency tries to resolve ITestable without a key.

API Proposal

When registering an implementation type as keyed, first try to resolve its dependencies using that key. If a service can't be resolved using the key, try resolving it without a key.

Code change: https://github.com/wvpm/runtime/commit/b20d2f38075296f8038e50d0e59b78b983daba75

API Usage

    object key = "key";
    ServiceCollection serviceCollection = new();
    serviceCollection.AddKeyedTransient<ITestable, DummyService>(key);
    serviceCollection.AddKeyedTransient<ServiceWithDependency>(key);
    using ServiceProvider serviceProvider = serviceCollection.BuildServiceProvider(
        new ServiceProviderOptions() { ValidateOnBuild = true, ValidateScopes = true }
    );
    ServiceWithDependency instance = serviceProvider.GetRequiredKeyedService<ServiceWithDependency>(key);

    class ServiceWithDependency(
-       [FromKeyedServices("key")]  //no longer required
        ITestable dependency
    ) {
        private readonly ITestable _dependency = dependency;
    }

ServiceWithDependency should first try to resolve ITestable using the key even if the FromKeyedServicesAttribute is removed.

Alternative Designs

No response

Risks

While the behaviour would change, I struggle to see how it would break existing usage. Existing usage has to rely on the FromKeyedServicesAttribute, otherwise the BuildServiceProvider throws. There might be more risks that I am not aware of. Feel free to post your thought.

pinkfloydx33 commented 1 month ago

I think this might be a duplicate of https://github.com/dotnet/runtime/issues/99084 or at least related/very similar

wvpm commented 1 month ago

@pinkfloydx33 thanks for linking. I don't think they overlap entirely. They are very similar though.

julealgon commented 1 month ago

Wouldn't the proposed implementation lead to a substantial performance impact? I would imagine for a deep graph this could end up being extremely costly as every single child dependency would need to be attempted twice.

wvpm commented 1 month ago

Wouldn't the proposed implementation lead to a substantial performance impact? I would imagine for a deep graph this could end up being extremely costly as every single child dependency would need to be attempted twice.

Honestly I can't say whether the performance impact would be substantial. The change impacts only the constructor arguments used to instantiate a keyed service. And you only attempt it twice if the dependency is registered as an unkeyed service.

steffan267 commented 1 month ago

I have sort of the same issue I think - or at least depending on the solution here could probably solve both needs.

Given the dependency hierarchy below: Class A -> Class B -> Class C -> Class D

I want to easily resolve only B and D to be from the key when I inject class A. I don't seem to think this is currently possible without boiler plate registration. I tried going down the rabbithole for how the KeyedServices and maybe a new attribute: "FromKeyedScope or FromKeyedContext" for injection would make sense?

The important part is then that there is a ConstructorResolver that does the "try get from key" and then "get from rootProvider" the same for all dependencies in the hierachy

And used like:


Controller Method injection:
    public async Task<IActionResult> [FromKeyedScope("Read")] IAmNotRegisteredAsKeyed s)
``