dotnet / runtime

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

[API Proposal]: Keyed services; alternative registration API for existing non-keyed services #109017

Open mgravell opened 1 week ago

mgravell commented 1 week ago

Background and motivation

A lot of services exist that do not support keyed DI - they use TryAddSingleton etc internally, without any notion of keyed DI. Migrating those services to keyed-DI often requires the library-author to add additional keyed DI APIs for the purpose. This can also be a problem when we have a need for 2 instances of the same service to interact, such as decorator APIs - as discussed in this "extensions" topic

This is a proposal for an alternative API that works around this problem.

API Proposal

Consider an existing API of the form:

services.AddStackExchangeRedisCache(...);

This adds SE.Redis as an IDistributedCache implementation on the default key. There is no current API to add SE.Redis as a keyed service, and the relevant type is not directly exposed, making it impossible to register as a keyed service manually.

Now consider something along the lines of:

services.WithServiceKey("some key").AddStackExchangeRedisCache(...);

or

services.WithServiceKey("some key", keyed => keyed.AddStackExchangeRedisCache(...));

The key point here is that a decorator implementation of IServiceCollection is created which changes the Add method to add the specified key on non-keyed services. The 2 alternative suggestions have different ways of expressing the lifetime of the decorator; arguably the second version is clearer as to "here's the bits that are keyed", but I don't care to die on any hills.

A rough proposed implementation of the idea is shown below and kind-of works. There is a complication, however: if we take the AddStackExchangeRedisCache as an example, this may use multiple other services, and if done naively, they'd all end up "keyed". I genuinely don't know whether keyed services work transitively, i.e. if a service Foo is keyed "some key", and requires sub-service Bar without specifying a key, does it look for a keyed-service ("some key") Bar and then fall back to the non-keyed service Bar, or does it only look for the non-keyed Bar? I wonder whether it might be necessary to say "I want to configure a specific type with a key", i.e.

- services.WithServiceKey("some key", keyed => keyed.AddStackExchangeRedisCache(...));
+ services.WithServiceKey<IDistributedCache>("some key", keyed => keyed.AddStackExchangeRedisCache(...));

In this case, the decorator would only magically add the key for matching service types (presumably with an API to allow multiple types to be specified if required)


Rough implementation, for reference only, doesn't consider the "which service" complication:

public static class ServiceCollectionExtensions
{
    public static IServiceCollection WithServiceKey(this IServiceCollection services, object key)
        => new KeyedServiceCollection(services, key);
}
internal sealed class KeyedServiceCollection(IServiceCollection services, object serviceKey) : IServiceCollection
{
    void ICollection<ServiceDescriptor>.Add(ServiceDescriptor item) => services.Add(WithKey(item));
    private ServiceDescriptor WithKey(ServiceDescriptor item)
    {
        if (!item.IsKeyedService)
        {
            if (item.ImplementationInstance is not null)
            {
                item = new(item.ServiceType, serviceKey, item.ImplementationInstance);
            }
            else if (item.ImplementationFactory is { } factory)
            {
                item = new(item.ServiceType, serviceKey, (provider, _) => factory(provider), item.Lifetime);
            }
            else if (item.ImplementationType is not null)
            {
                item = new(item.ServiceType, serviceKey, item.ImplementationType, item.Lifetime);
            }
            else
            {
                throw new NotSupportedException("Unable to map service to key");
            }
        }
        return item;
    }
    // other methods not shown, but simply forward via "services"
}

API Usage

(shown above)

Alternative Designs

The alternative is to lean on library authors to add keyed-DI methods to all of their registration methods.

Risks

No response

dotnet-policy-service[bot] commented 1 week ago

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

julealgon commented 6 days ago

I would support this if this was also employed for other "concerns", such as lifetime itself.

services.WithLifetime(Singleton).WithKey("some key").AddStackExchangeRedisCache(...);

If this is only implemented for changing the registration key, I believe it would just lead to a super inconsistent public API, where some things are done one way, while others are done a different way, even though they are similar concerns.

This raises the question on the standard AddX methods, which I would also switch to this new style which is vastly more composable:

services.WithLifetime(Singleton).Add<IMyService, MyService>();

Instead of

services.AddSingleton<IMyService, MyService>();

In this "new API", Add would use Transient by default if not overridden.

AddTransient, AddScoped and AddSingleton would then be marked as deprecated.

Yes, this is a bit outside the scope of this proposal here, but I strongly believe they are super correlated to a point where the team should either do it all, or not do it.

mgravell commented 6 days ago

@julealgon not sure I entirely agree on the correlation aspect; keyed-DI is a (relatively) new concept; adding the ability to migrate an existing pre-keyed-DI configuration API to operate against a key, seems (IMO) a desirable scenario; however, overriding the actual lifetime scopes of configured services feels much more dangerous. I also don't see a world where AddSingleton et al would be marked as deprecated - that would cause too much build noise? But: not my area; these aren't strong views - just my views as a DI consumer/publisher.

steveharter commented 6 days ago

cc @benjaminpetit @halter73. Alternative registration that can work with both keyed- and unkeyed- services.

Moving to future for now.

halter73 commented 4 days ago

I like this idea a lot. It seems immediately useful and would negate the need to duplicate a lot of IServiceCollection extension methods that don't really need to be explicitly aware of keyed services. I also agree that it's generally safer to override a service key than a service lifetime.

However, I think the fact that keyed services aren't viral (meaning service constructors have to use [FromKeyedService(...)] explicitly to get keyed service even if they are being resolved as a keyed service themselves) makes this a lot less useful than it could be, and a potential footgun.

If a key oblivious IServiceCollection extension methods adds multiple required services that are interdependent, service resolution might fail because a keyless dependency won't be found. Worst case is that you have a lot more shared state than you intended which could have disastrous security implications.

It's also unfortunate that there's no way that I'm aware of for a service that's aware it might be registered with a key to opt into resolving dependencies with the same key other than using [ServiceKey] with GetRequiredKeyedService. When looking around to see how common this pattern is, I found GetRequiredOrKeyedRequiredService. It'd be nice if there were a declarative way to ask for this.   If we could develop this idea to make resolving dependent keyed services viral, it would be more interesting. In the meantime, if you want that kind of functionality, you're probably best of creating a service provider per key with BuildServiceProvider().