Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]: Add Keyed Services Support to Dependency Injection #64427

Closed
commonsensesoftware opened this issue Jan 28, 2022 · 95 comments
Closed
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-DependencyInjection blocking Marks issues that we want to fast track in order to unblock other important work
Milestone

Comments

@commonsensesoftware
Copy link

commonsensesoftware commented Jan 28, 2022

Thanks @commonsensesoftware for the original proposal. I edited this post to show the current proposition.

Original proposal from @commonsensesoftware ### Background and Motivation

I'm fairly certain this has been asked or proposed before. I did my due diligence, but I couldn't find an existing, similar issue. It may be lost to time from merging issues across repos over the years.

A similar question was asked in Issue 2937

The main reason this has not been supported is that IServiceProvider.GetService(Type type) does not afford a way to retrieve a service by key. IServiceProvider has been the staple interface for service location since .NET 1.0 and changing or ignoring its well-established place in history is a nonstarter. However... what if we could have our cake and eat it to? 🤔

A keyed service is a concept that comes up often in the IoC world. All, if not almost all, DI frameworks support registering and retrieving one or more services by a combination of type and key. There are ways to make keyed services work in the existing design, but they are clunky to use (ex: via Func<string, T>). The following proposal would add support for keyed services to the existing Microsoft.Extensions.DependencyInjection.* libraries without breaking the IServiceProvider contract nor requiring any container framework changes.

I currently have a small prototype that works with the default ServiceProvider, Autofac and Unity container.

Current proposal: https://gist.github.com/benjaminpetit/49a6b01692d0089b1d0d14558017efbc


Previous proposal

Overview

For completeness, a minimal, viable solution with E2E tests for the most common containers is available in the Keyed Service POC repo. It's probably incomplete from where the final solution would land, but it's enough to illustrate the feasibility of the approach.

API Proposal

The first requirement is to define a key for a service. Type is already a key. This proposal will use the novel idea of also using Type as a composite key. This design provides the following advantages:

  • No magic strings or objects
  • No attributes or other required metadata
  • No hidden service location lookups (e.g. a la magic string)
  • No name collisions (types are unique)
  • No additional interfaces required for resolution (ex: ISupportRequiredService, ISupportKeyedService)
  • No implementation changes to the existing containers
  • No additional library references (from the FCL or otherwise)
  • Resolution intuitively fails if a key and service combination does not exist in the container

The type names that follow are for illustration and might change if the proposal is accepted.

Resolving Services

To resolve a keyed dependency we'll define the following contracts:

// required to 'access' a keyed service via typeof(T)
public interface IDependency
{
    object Value { get; }
}

public interface IDependency<in TKey, out TService> : IDependency
    where TService : notnull
{
    new TService Value { get; }
}

The following extension methods will be added to ServiceProviderServiceExtensions:

public static class ServiceProviderServiceExtensions
{
    public static object? GetService(this IServiceProvider serviceProvider, Type serviceType, Type key);

    public static object GetRequiredService(this IServiceProvider serviceProvider, Type serviceType, Type key);

    public static IEnumerable<object> GetServices(this IServiceProvider serviceProvider, Type serviceType, Type key);

    public static T? GetService<T>(this IServiceProvider serviceProvider, Type key) where T : notnull;

    public static T GetRequiredService<T>(this IServiceProvider serviceProvider, string key) where T : notnull;

    public static IEnumerable<T> GetServices<T>(this IServiceProvider serviceProvider, string key) where T : notnull;
}

Here is a partial example of how it would be implemented:

public static class ServiceProviderExtensions
{
    public static object? GetService(this IServiceProvider serviceProvider, Type serviceType, Type key)
    {
        var keyedType = typeof(IDependency<,>).MakeGenericType(key, serviceType);
        var dependency = (IDependency?)serviceProvider.GetService(keyedType);
        return dependency?.Value;
    }

    public static TService? GetService<TKey, TService>(this IServiceProvider serviceProvider)
        where TService : notnull
    {
        var dependency = serviceProvider.GetService<IDependency<TKey, TService>>();
        return dependency is null ? default : dependency.Value;
    }

    public static IEnumerable<TService> GetServices<TKey, TService>(this IServiceProvider serviceProvider)
        where TService : notnull
    {
        foreach (var dependency in serviceProvider.GetServices<IDependency<TKey, TService>>())
        {
            yield return dependency.Value;
        }
    }
}

Registering Services

Now that we have a way to resolve a keyed service, how do we register one? Type is already used as a key, but we need a way to create an arbitrary composite key. To achieve this, we'll perform a little trickery on the Type which only affects how it is mapped in a container; thus making it a composite key. It does not change the runtime behavior nor require special Reflection magic. We are effectively taking advantage of the knowledge that Type will be used as a key for service resolution in all container implementations.

public static class KeyedType
{
    public static Type Create(Type key, Type type) =>
        new TypeWithKey(key,type);
    
    public static Type Create<TKey, TType>() where TType : notnull =>
        new TypeWithKey(typeof(TKey), typeof(TType));

    private sealed class TypeWithKey : TypeDelegator
    {
        private readonly int hashCode;

        public TypeWithKey(Type keyType, Type customType)
            : base(customType) => hashCode = HashCode.Combine(typeImpl, keyType);

        public override int GetHashCode() => hashCode;

        // remainder is minimal, but ommitted for brevity
    }
}

This might look magical, but it's not. Type is already being used as a key when it's mapped in a container. TypeWithKey has all the appearance of the original type, but produces a different hash code when combined with another type. This affords for determinate, discrete unions of type registrations, which allows mapping the intended service multiple times.

Container implementers are free to perform the registration however they like, but the generic, out-of-the-box implementation would look like:

public sealed class Dependency<TKey, TService> : IDependency<TKey, TService>
    where TService : notnull
{
    private readonly IServiceProvider serviceProvider;

    public Dependency(IServiceProvider serviceProvider) => this.serviceProvider = serviceProvider;

    public TService Value => (TService)serviceProvider.GetRequiredService(KeyedType.Create<TKey, TService>());

    object IDependency.Value => Value;
}

Container implementers might provide their own extension methods to make registration more succinct, but it is not required. The following registrations would work today without any container implementation changes:

public void ConfigureServices(IServiceCollection services)
{
    services.AddTransient(KeyedType.Create<Key.Thing1, IThing>(), typeof(Thing1));
    services.AddTransient<IDependency<Key.Thing1, IThing>, Dependency<Key.Thing1, IThing>>();
}

public void ConfigureUnity(IUnityContainer container)
{
    container.RegisterType(KeyedType.Create<Key.Thing1, IThing>(), typeof(Thing1));
    container.RegisterType<IDependency<Key.Thing1, IThing>, Dependency<Key.Thing1, IThing>>();
}

public void ConfigureAutofac(ContainerBuilder builder)
{
    builder.RegisterType(typeof(Thing1)).As(KeyedType.Create<Key.Thing1, IThing>());
    builder.RegisterType<Dependency<Key.Thing1, IThing>>().As<IDependency<Key.Thing1, IThing>>();
}

There is a minor drawback of requiring two registrations per keyed service in the container, but resolution for consumers is succintly:

var longForm = serviceProvider.GetRequiredService<IDependency<Key.Thing1, IThing>>().Value;
var shortForm = serviceProvider.GetRequiredService<Key.Thing1, IThing>();

The following extension methods will be added to ServiceCollectionDescriptorExtensions to provide common registration through IServiceCollection for all container frameworks:

public static class ServiceCollectionExtensions
{
    public static IServiceCollection AddSingleton<TKey, TService, TImplementation>(this IServiceCollection services)
        where TService : class
        where TImplementation : class, TService;

    public static IServiceCollection AddSingleton(
        this IServiceCollection services,
        Type keyType,
        Type serviceType,
        Type implementationType);

    public static IServiceCollection TryAddSingleton<TKey, TService, TImplementation>(this IServiceCollection services)
        where TService : class
        where TImplementation : class, TService;

    public static IServiceCollection TryAddSingleton(
        this IServiceCollection services,
        Type keyType,
        Type serviceType,
        Type implementationType);

    public static IServiceCollection AddTransient<TKey, TService, TImplementation>(this IServiceCollection services)
        where TService : class
        where TImplementation : class, TService;

    public static IServiceCollection AddTransient(
        this IServiceCollection services,
        Type keyType,
        Type serviceType,
        Type implementationType);

    public static IServiceCollection TryAddTransient<TKey, TService, TImplementation>(this IServiceCollection services)
        where TService : class
        where TImplementation : class, TService;

    public static IServiceCollection TryAddTransient(
        this IServiceCollection services,
        Type keyType,
        Type serviceType,
        Type implementationType);

    public static IServiceCollection AddScoped<TKey, TService, TImplementation>(this IServiceCollection services)
        where TService : class
        where TImplementation : class, TService;

    public static IServiceCollection AddScoped(
        this IServiceCollection services,
        Type keyType,
        Type serviceType,
        Type implementationType);

    public static IServiceCollection TryAddScoped<TKey, TService, TImplementation>(this IServiceCollection services)
        where TService : class
        where TImplementation : class, TService;

    public static IServiceCollection TryAddScoped(
        this IServiceCollection services,
        Type keyType,
        Type serviceType,
        Type implementationType);

    public static IServiceCollection TryAddEnumerable<TKey, TService, TImplementation>(
        this IServiceCollection services,
        ServiceLifetime lifetime)
        where TService : class
        where TImplementation : class, TService;

    public static IServiceCollection TryAddEnumerable(
        this IServiceCollection services,
        Type keyType,
        Type serviceType,
        Type implementationType,
        ServiceLifetime lifetime);
}

API Usage

Putting it all together, here's how the API can be leveraged for any container framework that supports registration through IServiceCollection.

public interface IThing
{
    string ToString();
}

public abstract class ThingBase : IThing
{
    protected ThingBase() { }
    public override string ToString() => GetType().Name;
}

public sealed class Thing : ThingBase { }

public sealed class KeyedThing : ThingBase { }

public sealed class Thing1 : ThingBase { }

public sealed class Thing2 : ThingBase { }

public sealed class Thing3 : ThingBase { }

public static class Key
{
    public sealed class Thingies { }
    public sealed class Thing1 { }
    public sealed class Thing2 { }
}

public class CatInTheHat
{
    private readonly IDependency<Key.Thing1, IThing> thing1;
    private readonly IDependency<Key.Thing2, IThing> thing2;

    public CatInTheHat(
        IDependency<Key.Thing1, IThing> thing1,
        IDependency<Key.Thing2, IThing> thing2)
    {
        this.thing1 = thing1;
        this.thing2 = thing2;
    }

    public IThing Thing1 => thing1.Value;
    public IThing Thing2 => thing2.Value;
}

public void ConfigureServices(IServiceCollection collection)
{
    // keyed types
    services.AddSingleton<Key.Thing1, IThing, Thing1>();
    services.AddTransient<Key.Thing2, IThing, Thing2>();

    // non-keyed type with keyed type dependencies
    services.AddSingleton<CatInTheHat>();

    // keyed open generics
    services.AddTransient(typeof(IGeneric<>), typeof(Generic<>));
    services.AddSingleton(typeof(IDependency<,>), typeof(GenericDependency<,>));

    // keyed IEnumerable<T>
    services.TryAddEnumerable<Key.Thingies, IThing, Thing1>(ServiceLifetime.Transient);
    services.TryAddEnumerable<Key.Thingies, IThing, Thing2>(ServiceLifetime.Transient);
    services.TryAddEnumerable<Key.Thingies, IThing, Thing3>(ServiceLifetime.Transient);

    var provider = services.BuildServiceProvider();

    // resolve non-keyed type with keyed type dependencies
    var catInTheHat = provider.GetRequiredService<CatInTheHat>();

    // resolve keyed, open generic
    var openGeneric = provider.GetRequiredService<Key.Thingy, IGeneric<object>>();

    // resolve keyed IEnumerable<T>
    var thingies = provider.GetServices<Key.Thingies, IThing>();

    // related services such as IServiceProviderIsService
    // new extension methods could be added to make this more succinct
    var query = provider.GetRequiredService<IServiceProviderIsService>();
    var thing1Registered = query.IsService(typeof(IDependency<Key.Thing1, IThing>));
    var thing2Registered = query.IsService(typeof(IDependency<Key.Thing2, IThing>));
}

Container Integration

The following is a summary of results from Keyed Service POC repo.

Container By Key By Key
(Generic)
Many
By Key
Many By
Key (Generic)
Open
Generics
Existing
Instance
Implementation
Factory
Default
Autofac
DryIoc
Grace
Lamar
LightInject
Stashbox
StructureMap
Unity
Container Just
Works
No Container
Changes
No Adapter
Changes
Default
Autofac
DryIoc
Grace 1 1
Lamar
LightInject
Stashbox
StructureMap
Unity

[1]: Only Implementation Factory doesn't work out-of-the-box

  • Just Works: Works without any changes
  • No Container Changes: Works without requiring fundamental container changes
  • No Adapter Changes: Works without changing the way a container adapts to IServiceCollection

Risks

  • Container implementers may not be interested in adopting this approach
  • Suboptimal experience for developers using containers that need adapter changes
    • e.g. The feature doesn't work without a developer writing their own or relying on a 3rd party to bridge the gap

Alternate Proposals (TL;DR)

The remaining sections outline variations alternate designs that were rejected, but were retained for historical purposes.

Previous Code Iterations

  1. Thought experiment
  2. Initial proof of concept
  3. Practical API with a lot of ceremony removed

Proposal 1 (Rejected)

Proposal 1 revolved around using string as a key. While this approach is feasible, it requires a lot of magical ceremony under the hood. For this solution to be truly effective, container implementers would have to opt into the new design. The main limitation of this approach, however, is that a string key is another form of hidden dependency that cannot, or cannot easily, be expressed to consumers. Resolution of a keyed dependency in this proposal would require an attribute at the call site that specifies the key or some type of lookup that resolves, but hides, the key used in the injected constructor. The comments below describes and highlights many of the issues with this design.

Keyed Services Using a String (KeyedServiceV1.zip)

API Proposal

The first thing we need is a way to provide a key for a service. The simplest way to do that is to add a new attribute to Microsoft.Extensions.DependencyInjection.Abstractions:

using static System.AttributeTargets;

[AttributeUsage(Class | Interface | Parameter, AllowMultiple = false, Inherited = false)]
public sealed class ServiceKeyAttribute : Attribute
{
    public ServiceKeyAttribute(string key) => Key = key;
    public string Key { get; }
}

This attribute could be used in the following ways:

[ServiceKey("Bar")]
public interface IFoo { }
7
[ServiceKey("Foo")]
public class Foo { }

public class Bar
{
    public Bar([ServiceKey("Bar")] IFoo foo) { }
}

Using an attribute has to main advantages:

  1. There needs to be a way to specify the key at the call site when a dependency is injected
  2. An attribute can provide metadata (e.g. the key) to any type

What if we don't want to use an attribute on our class or interface? In fact, what if we can't apply an attribute to the target class or interface (because we don't control the source)? Using a little Bait & Switch, we can get around that limitation and achieve our goal using CustomReflectionContext. That will enable adding ServiceKeyAttribute to any arbitrary type. Moreover, the surrogate type doesn't change any runtime behavior; it is only used as a key in the container to lookup the corresponding resolver. This means that it's now possible to register a type more than once in combination with a key. The type is still the Type, but the key maps to different implementations. This also means that IServiceProvider.GetService(Type type) can support a key without breaking its contract.

The following extension methods would be added to ServiceProviderServiceExtensions:

public static class ServiceProviderServiceExtensions
{
    public static object? GetService(this IServiceProvider serviceProvider, Type serviceType, string key);

    public static IEnumerable<object> GetServices(this IServiceProvider serviceProvider, Type serviceType, string key);

    public static T? GetService<T>(this IServiceProvider serviceProvider, string key);

    public static object GetRequiredService(this IServiceProvider serviceProvider, Type serviceType, string key);

    public static T GetRequiredService<T>(this IServiceProvider serviceProvider, string key) where T : notnull;

    public static IEnumerable<T> GetServices<T>(this IServiceProvider serviceProvider, string key) where T : notnull;
}

It is not required for this proposal to work, but as an optimization, it may be worth adding:

public interface IKeyedServiceProvider : IServiceProvider
{
    object? GetService(Type serviceType, string key);
}

for implementers that know how to deal with Type and key separately.

To abstract the container and mapping from the implementation, ServiceDescriptor will need to add the property:

public string? Key { get; set; }

The aforementioned extension methods are static and cannot have their implementations changed in the future. To ensure that
container implementers have full control over how Type + key mappings are handled, I recommend the following be added
to Microsoft.Extensions.DependencyInjection.Abstractions:

public interface IKeyedTypeFactory
{
    Type Create(Type type, string key);
}

Microsoft.Extensions.DependencyInjection will provide a default implementation that leverages CustomReflectionContext.

The implementation might look like the following:

public static object? GetService(this IServiceProvider serviceProvider, Type serviceType, string key)
{
    var provider = serviceProvider as IKeyedServiceProvider ?? serviceProvider.GetService<IKeyServiceProvider>();
    
    if (provider != null)
    {
        return provider.GetService(serviceType, key);
    }

    var factory = serviceProvider.GetService<IKeyedTypeFactory>() ?? KeyedTypeFactory.Default;
    return serviceProvider.GetService(factory.Create(serviceType, key));
}

This approach would also work for new interfaces such as IServiceProviderIsService without requiring the
fundamental contract to change. It would make sense to add new extension methods for IServiceProviderIsService and potentially other interfaces as well.

API Usage

What we ultimately want to have is service registration that looks like:

class Team
{
    public Team([ServiceKey("A-Team")] IPityTheFoo foo) { } // ← MrT is injected
}

// ...

var services = new ServiceCollection();

// Microsoft.Extensions.DependencyInjection.Abstractions
services.AddSingleton<IPityTheFoo, MrT>("A-Team");
services.TryAddEnumerable(ServiceDescriptor.AddTransient<IThing, Thing1>("Thingies"));
services.TryAddEnumerable(ServiceDescriptor.AddTransient<IThing, Thing2>("Thingies"));
services.TryAddEnumerable(ServiceDescriptor.AddTransient<IThing, Thing3>("Thingies"));

var provider = services.BuildServiceProvider();
var foo = provider.GetRequiredService<IPityTheFoo>("A-Team");
var team = provider.GetRequiredService<Team>();
var thingies = provider.GetServices<IThing>("Thingies");

// related services such as IServiceProviderIsService
var query = provider.GetRequiredService<IServiceProviderIsService>();
var shorthand = query.IsService<IPityTheFoo>("A-Team");
var factory = provider.GetRequiredService<IKeyedTypeService>();
var longhand = query.IsService(factory.Create<IPityTheFoo>("A-Team"));

Alternative Designs

The ServiceKeyAttribute does not have to be applicable to classes or interfaces. That might make it easier to reason about without having to consider explicitly declared attributes and dynamically applied attributes. There still needs to be some attribute to apply to a parameter. Both scenarios can be achieved by restricting the value targets to AttributeTargets.Parameter. Dynamically adding the attribute does not have to abide by the same rules. A different attribute or method could also be used to map a key to the type.

This proposal does not mandate that CustomReflectionContext or even a custom attribute is the ideal solution. There may be other, more optimal ways to achieve it. IKeyedServiceProvider affords for optimization, while still ensuring that naive implementations will continue to work off of Type alone as input.

Risks

  • Microsoft.Extensions.DependencyInjection would require one of the following:
    1. A dependency on System.Reflection.Context (unless another solution is found)
    2. An new, separate library that that references System.Reflection.Context and adds the keyed service capability
  • There is a potential explosion of overloads and/or extension methods
    • The requirement that these exist can be mitigated via the IKeyedServiceProvider and/or IKeyedTypeFactory intefaces
      • The developer experience is less than ideal, but no functionality is lost

API Proposal

The API is optional

The API is optional, and will not break binary compatibility. If the service provider doesn't support the new methods, the user will get an exception at runtime.

The key type

The service key can be any object. It is important that Equals and GetHashCode have a proper implementation.

Service registration

ServiceDescriptor will be modified to include the ServiceKey. KeyedImplementationInstance, KeyedImplementationType and KeyedImplementationFactory will be added, matching their non-keyed equivalent.

When accessing a non-keyed property (like ImplementationInstance) on a keyed ServiceDescriptor will throw an exception: this way, if the developer added a keyed service and is using a non-compatible container, an error will be thrown during container build.

public class ServiceDescriptor
{
    [...]
    /// <summary>
    /// Get the key of the service, if applicable.
    /// </summary>
    public object? ServiceKey { get; }
    [...]
    /// <summary>
    /// Gets the instance that implements the service.
    /// </summary>
    public object? KeyedImplementationInstance { get; }
    /// <summary>
    /// Gets the <see cref="Type"/> that implements the service.
    /// </summary>
    public System.Type? KeyedImplementationType { get; }
    /// <summary>
    /// Gets the factory used for creating Keyed service instances.
    /// </summary>
    public Func<IServiceProvider, object, object>? KeyedImplementationFactory { get; }
    [...]
    /// <summary>
    /// Returns true if a ServiceKey was provided.
    /// </summary> 
    public bool IsKeyedService => ServiceKey != null;
}

ServiceKey will stay null in non-keyed services.

Extension methods for IServiceCollection are added to support keyed services:

public static IServiceCollection AddKeyedScoped(this IServiceCollection services, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] Type serviceType, object serviceKey);
public static IServiceCollection AddKeyedScoped(this IServiceCollection services, Type serviceType, object serviceKey, Func<IServiceProvider, object, object> implementationFactory);
public static IServiceCollection AddKeyedScoped(this IServiceCollection services, Type serviceType, object serviceKey, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] Type implementationType);
public static IServiceCollection AddKeyedScoped<[Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] TService>(this IServiceCollection services, object serviceKey) where TService : class;
public static IServiceCollection AddKeyedScoped<TService>(this IServiceCollection services, object serviceKey, Func<IServiceProvider, object, TService> implementationFactory) where TService : class;
public static IServiceCollection AddKeyedScoped<TService, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] TImplementation>(this IServiceCollection services, object serviceKey) where TService : class where TImplementation : class, TService;
public static IServiceCollection AddKeyedScoped<TService, TImplementation>(this IServiceCollection services, object serviceKey, Func<IServiceProvider, object, TImplementation> implementationFactory) where TService : class where TImplementation : class, TService;
public static IServiceCollection AddKeyedSingleton(this IServiceCollection services, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] Type serviceType, object serviceKey);
public static IServiceCollection AddKeyedSingleton(this IServiceCollection services, Type serviceType, object serviceKey, Func<IServiceProvider, object, object> implementationFactory);
public static IServiceCollection AddKeyedSingleton(this IServiceCollection services, Type serviceType, object serviceKey, object implementationInstance);
public static IServiceCollection AddKeyedSingleton(this IServiceCollection services, Type serviceType, object serviceKey, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] Type implementationType);
public static IServiceCollection AddKeyedSingleton<[Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] TService>(this IServiceCollection services, object serviceKey) where TService : class;
public static IServiceCollection AddKeyedSingleton<TService>(this IServiceCollection services, object serviceKey, Func<IServiceProvider, object, TService> implementationFactory) where TService : class;
public static IServiceCollection AddKeyedSingleton<TService>(this IServiceCollection services, object serviceKey, TService implementationInstance) where TService : class;
public static IServiceCollection AddKeyedSingleton<TService, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] TImplementation>(this IServiceCollection services, object serviceKey) where TService : class where TImplementation : class, TService;
public static IServiceCollection AddKeyedSingleton<TService, TImplementation>(this IServiceCollection services, object serviceKey, Func<IServiceProvider, object, TImplementation> implementationFactory) where TService : class where TImplementation : class, TService;
public static IServiceCollection AddKeyedTransient(this IServiceCollection services, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] Type serviceType, object serviceKey);
public static IServiceCollection AddKeyedTransient(this IServiceCollection services, Type serviceType, object serviceKey, Func<IServiceProvider, object, object> implementationFactory);
public static IServiceCollection AddKeyedTransient(this IServiceCollection services, Type serviceType, object serviceKey, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] Type implementationType);
public static IServiceCollection AddKeyedTransient<[Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] TService>(this IServiceCollection services, object serviceKey) where TService : class;
public static IServiceCollection AddKeyedTransient<TService>(this IServiceCollection services, object serviceKey, Func<IServiceProvider, object, TService> implementationFactory) where TService : class;
public static IServiceCollection AddKeyedTransient<TService, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] TImplementation>(this IServiceCollection services, object serviceKey) where TService : class where TImplementation : class, TService;
public static IServiceCollection AddKeyedTransient<TService, TImplementation>(this IServiceCollection services, object serviceKey, Func<IServiceProvider, object, TImplementation> implementationFactory) where TService : class where TImplementation : class, TService;

public static void TryAddKeyedScoped(this IServiceCollection collection, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] Type service, object serviceKey) { }
public static void TryAddKeyedScoped(this IServiceCollection collection, Type service, object serviceKey, Func<IServiceProvider, object, object> implementationFactory) { }
public static void TryAddKeyedScoped(this IServiceCollection collection, Type service, object serviceKey, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] Type implementationType) { }
public static void TryAddKeyedScoped<[Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] TService>(this IServiceCollection collection, object serviceKey) where TService : class { }
public static void TryAddKeyedScoped<TService>(this IServiceCollection services, object serviceKey, Func<IServiceProvider, object, TService> implementationFactory) where TService : class { }
public static void TryAddKeyedScoped<TService, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] TImplementation>(this IServiceCollection collection, object serviceKey) where TService : class where TImplementation : class, TService { }
public static void TryAddKeyedSingleton(this IServiceCollection collection, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] Type service, object serviceKey) { }
public static void TryAddKeyedSingleton(this IServiceCollection collection, Type service, object serviceKey, Func<IServiceProvider, object, object> implementationFactory) { }
public static void TryAddKeyedSingleton(this IServiceCollection collection, Type service, object serviceKey, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] Type implementationType) { }
public static void TryAddKeyedSingleton<[Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] TService>(this IServiceCollection collection, object serviceKey) where TService : class { }
public static void TryAddKeyedSingleton<TService>(this IServiceCollection services, object serviceKey, Func<IServiceProvider, object, TService> implementationFactory) where TService : class { }
public static void TryAddKeyedSingleton<TService>(this IServiceCollection collection, object serviceKey, TService instance) where TService : class { }
public static void TryAddKeyedSingleton<TService, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] TImplementation>(this IServiceCollection collection, object serviceKey) where TService : class where TImplementation : class, TService { }
public static void TryAddKeyedTransient(this IServiceCollection collection, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] Type service, object serviceKey) { }
public static void TryAddKeyedTransient(this IServiceCollection collection, Type service, object serviceKey, Func<IServiceProvider, object, object> implementationFactory) { }
public static void TryAddKeyedTransient(this IServiceCollection collection, Type service, object serviceKey, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] Type implementationType) { }
public static void TryAddKeyedTransient<[Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] TService>(this IServiceCollection collection, object serviceKey) where TService : class { }
public static void TryAddKeyedTransient<TService>(this IServiceCollection services, object serviceKey, Func<IServiceProvider, object, TService> implementationFactory) where TService : class { }
public static void TryAddKeyedTransient<TService, [Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] TImplementation>(this IServiceCollection collection, object serviceKey) where TService : class where TImplementation : class, TService { }

public static IServiceCollection RemoveAllKeyed(this IServiceCollection collection, Type serviceType, object serviceKey);
public static IServiceCollection RemoveAllKeyed<T>(this IServiceCollection collection, object serviceKey);

I think it's important that all new methods supporting Keyed service have a different name from the non-keyed equivalent, to avoid ambiguity.

"Any key" registration

It is possible to register a "catch all" key with KeyedService.AnyKey:

serviceCollection.AddKeyedSingleton<IService>(KeyedService.AnyKey, defaultService);
serviceCollection.AddKeyedSingleton<IService>("other-service", otherService);
[...] // build the provider
s1 = provider.GetKeyedService<IService>("other-service"); // returns otherService
s1 = provider.GetKeyedService<IService>("another-random-key"); // returns defaultService

Resolving service

Basic keyed resolution

Two new optional interfaces will be introduced:

namespace Microsoft.Extensions.DependencyInjection;

public interface ISupportKeyedService
{
    object? GetKeyedService(Type serviceType, object serviceKey);
    object GetRequiredKeyedService(Type serviceType, object serviceKey);
}

public interface IServiceProviderIsServiceKeyed
{
    bool IsService(Type serviceType, object serviceKey);
}

This new interface will be accessible via the following extension methods:

public static IEnumerable<object?> GetKeyedServices(this IServiceProvider provider, Type serviceType, object serviceKey);
public static IEnumerable<T> GetKeyedServices<T>(this IServiceProvider provider, object serviceKey);
public static T? GetKeyedService<T>(this IServiceProvider provider, object serviceKey);
public static object GetRequiredKeyedService(this IServiceProvider provider, Type serviceType, object serviceKey);
public static T GetRequiredKeyedService<T>(this IServiceProvider provider, object serviceKey) where T : notnull;
}

These methods will throw an InvalidOperationException if the provider doesn't support ISupportKeyedService.

Resolving services via attributes

We introduce two attributes: ServiceKeyAttribute and FromKeyedServicesAttribute.

ServiceKeyAttribute

ServiceKeyAttribute is used to inject the key that was used for registration/resolution in the constructor:

namespace Microsoft.Extensions.DependencyInjection;
[AttributeUsageAttribute(AttributeTargets.Parameter)]
public class ServiceKeyAttribute : Attribute
{
    public ServiceKeyAttribute() { }
}

class Service
{
    private readonly string _id;

    public Service([ServiceKey] string id) => _id = id;
}

serviceCollection.AddKeyedSingleton<Service>("some-service");
[...] // build the provider
var service = provider.GetKeyedService<Service>("some-service"); // service._id will be set to "some-service"

This attribute can be very useful when registering a service with KeyedService.AnyKey.

FromKeyedServicesAttribute

This attribute is used in a service constructor to mark parameters speficying which keyed service should be used:

namespace Microsoft.Extensions.DependencyInjection;
[AttributeUsageAttribute(AttributeTargets.Parameter)]
public class FromKeyedServicesAttribute : Attribute
{
    public FromKeyedServicesAttribute(object key) { }
    public object Key { get; }  
}

class OtherService
{
    public OtherService(
        [FromKeyedServices("service1")] IService service1,
        [FromKeyedServices("service2")] IService service2)
    {
        Service1 = service1;
        Service2 = service2;
    }
}

Open generics

Open generics are supported:

serviceCollection.AddTransient(typeof(IGenericInterface<>), "my-service", typeof(GenericService<>));
[...] // build the provider
var service = provider.GetKeyedService<IGenericInterface<SomeType>("my-service")

Enumeration

This kind of enumeration is possible:

serviceCollection.AddKeyedSingleton<IMyService, MyServiceA>("some-service");
serviceCollection.AddKeyedSingleton<IMyService, MyServiceB>("some-service");
serviceCollection.AddKeyedSingleton<IMyService, MyServiceC>("some-service");
[...] // build the provider
services = provider.GetKeyedServices<IMyService>("some-service"); // returns an instance of MyServiceA, MyServiceB and MyServiceC

Note that enumeration will not mix keyed and non keyed registrations:

serviceCollection.AddKeyedSingleton<IMyService, MyServiceA>("some-service");
serviceCollection.AddKeyedSingleton<IMyService, MyServiceB>("some-service");
serviceCollection.AddSingleton<IMyService, MyServiceC>();
[...] // build the provider
keyedServices = provider.GetKeyedServices<IMyService>("some-service"); // returns an instance of MyServiceA, MyServiceB but NOT MyServiceC
services = provider.GetServices<IMyService>(); // only returns MyServiceC

But we do not support:

serviceCollection.AddKeyedSingleton<MyServiceA>("some-service");
serviceCollection.AddKeyedSingleton<MyServiceB>("some-service");
serviceCollection.AddKeyedSingleton<MyServiceC>("some-service");
[...] // build the provider
services = provider.GetKeyedServices("some-service"); // Not supported
@commonsensesoftware commonsensesoftware added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 28, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added area-Extensions-DependencyInjection untriaged New issue has not been triaged by the area owner labels Jan 28, 2022
@ghost
Copy link

ghost commented Jan 28, 2022

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

Issue Details

Background and motivation

I'm fairly certain this has been asked or proposed before. I did my due diligence, but I couldn't find an existing, similar issue. It may be lost to time from merging issues across repos over the years.

The main reason this has not been supported is that IServiceProvider.GetService(Type type) does not afford a way to retrieve a service by key. IServiceProvider has been the staple interface for service location since .NET 1.0 and changing or ignoring its well-established place in history is a nonstarter. However... what if we could have our cake and eat it to? 🤔

A keyed service is a concept that comes up often in the IoC world. All, if not almost all, DI frameworks support registering and retrieving one or more services by a combination of type and key. There are ways to make keyed services work in the existing design, but they are clunky to use. The following proposal would add support for keyed services to the existing Microsoft.Extensions.DependencyInjection.* libraries without breaking the IServiceProvider contract. The clunkiness can be hidden behind extension methods and implementation details.

For completeness, I have attached a minimal, viable solution (KeyedService.zip). It's incomplete from where the final solution would land, but it's enough to illustrate the feasibility of the approach. If approved, I'm happy to start an formal pull request.

API Proposal

The first thing we need is a way to provide a key for a service. The simplest way to do that is to add a new attribute to Microsoft.Extensions.DependencyInjection.Abstractions:

using static System.AttributeTargets;

[AttributeUsage(Class | Interface | Parameter, AllowMultiple = false, Inherited = false)]
public sealed class ServiceKeyAttribute : Attribute
{
    public ServiceKeyAttribute(string key) => Key = key;
    public string Key { get; }
}

This attribute could be used in the following ways:

[ServiceKey("Bar")]
public interface IFoo { }

[ServiceKey("Foo")]
public class Foo { }

public class Bar
{
    public Bar([ServiceKey("Bar")] IFoo foo) { }
}

Using an attribute has to main advantages:

  1. There needs to be a way to specify the key at the call site when a dependency is injected
  2. An attribute can provide metadata (e.g. the key) to any type

What if we don't want to use an attribute on our class or interface? In fact, what if we can't apply an attribute to the target class or interface (because we don't control the source)? Using a little Bait & Switch, we can get around that limitation and achieve our goal using CustomReflectionContext. That will enable adding ServiceKeyAttribute to any arbitrary type. Moreover, the surrogate type doesn't change any runtime behavior; it is only used as a key in the container to lookup the corresponding resolver. This means that it's now possible to register a type more than once in combination with a key. The type is still the Type, but the key maps to different implementations. This also means that IServiceProvider.GetService(Type type) can support a key without breaking its contract.

The following extension methods would be added to ServiceProviderServiceExtensions:

public static class ServiceProviderServiceExtensions
{
    public static object? GetService(this IServiceProvider serviceProvider, Type serviceType, string key);

    public static IEnumerable<object> GetServices(this IServiceProvider serviceProvider, Type serviceType, string key);

    public static T? GetService<T>(this IServiceProvider serviceProvider, string key);

    public static object GetRequiredService(this IServiceProvider serviceProvider, Type serviceType, string key);

    public static T GetRequiredService<T>(this IServiceProvider serviceProvider, string key) where T : notnull;

    public static IEnumerable<T> GetServices<T>(this IServiceProvider serviceProvider, string key) where T : notnull;
}

It is not required for this proposal to work, but as an optimization, it may be worth adding:

public interface IKeyedServiceProvider : IServiceProvider
{
    object? GetService(Type serviceType, string key);
}

for implementers that know how to deal with Type and key separately.

To abstract the container and mapping from the implementation, ServiceDescriptor will need to add the property:

public string? Key { get; set; }

The aforementioned extension methods are static and cannot have their implementations changed in the future. To ensure that
container implementers have full control over how Type + key mappings are handled, I recommend the following be added
to Microsoft.Extensions.DependencyInjection.Abstractions:

public interface IKeyedTypeFactory
{
    Type Create(Type type, string key);
}

Microsoft.Extensions.DependencyInjection will provide a default implementation that leverages CustomReflectionContext.

The implementation might look like the following:

public static object? GetService(this IServiceProvider serviceProvider, Type serviceType, string key)
{
    var provider = serviceProvider as IKeyedServiceProvider ?? serviceProvider.GetService<IKeyServiceProvider>();
    
    if (provider != null)
    {
        return provider.GetService(serviceType, key);
    }

    var factory = serviceProvider.GetService<IKeyedTypeFactory>() ?? KeyedTypeFactory.Default;
    return serviceProvider.GetService(factory.Create(serviceType, key));
}

This approach would also work for new interfaces such as IServiceProviderIsService without requiring the
fundamental contract to change. It would make sense to add new extension methods for IServiceProviderIsService and potentially other interfaces as well.

API Usage

What we ultimately want to have is service registration that looks like:

class Team
{
    public Team([ServiceKey("A-Team")] IPityTheFoo foo) { } // ← MrT is injected
}

// ...

var services = new ServiceCollection();

// Microsoft.Extensions.DependencyInjection.Abstractions
services.AddSingleton<IPityTheFoo, MrT>("A-Team");
services.TryAddEnumerable(ServiceDescriptor.AddTransient<IThing, Thing1>("Thingies"));
services.TryAddEnumerable(ServiceDescriptor.AddTransient<IThing, Thing2>("Thingies"));
services.TryAddEnumerable(ServiceDescriptor.AddTransient<IThing, Thing3>("Thingies"));

var provider = services.BuildServiceProvider();
var foo = provider.GetRequiredService<IPityTheFoo>("A-Team");
var team = provider.GetRequiredService<Team>();
var thingies = provider.GetServices<IThing>("Thingies");

// related services such as IServiceProviderIsService
var query = provider.GetRequiredService<IServiceProviderIsService>();
var shorthand = query.IsService<IPityTheFoo>("A-Team");
var factory = provider.GetRequiredService<IKeyedTypeService>();
var longhand = query.IsService(factory.Create<IPityTheFoo>("A-Team"));

Alternative Designs

The ServiceKeyAttribute does not have to be applicable to classes or interfaces. That might make it easier to reason about without having to consider explicitly declared attributes and dynamically applied attributes. There still needs to be some attribute to apply to a parameter. Both scenarios can be achieved by restricting the value targets to AttributeTargets.Parameter. Dynamically adding the attribute does not have to abide by the same rules. A different attribute or method could also be used to map a key to the type.

This proposal does not mandate that CustomReflectionContext or even a custom attribute is the ideal solution. There may be other, more optimal ways to achieve it. IKeyedServiceProvider affords for optimization, while still ensuring that naive implementations will continue to work off of Type alone as input.

Risks

  • Microsoft.Extensions.DependencyInjection would require one of the following:
    1. A dependency on System.Reflection.Context (unless another solution is found)
    2. An new, separate library that that references System.Reflection.Context and adds the keyed service capability
  • There is a potential explosion of overloads and/or extension methods
    • The requirement that these exist can be mitigated via the IKeyedServiceProvider and/or IKeyedTypeFactory intefaces
      • The developer experience is less than ideal, but no functionality is lost
Author: commonsensesoftware
Assignees: -
Labels:

api-suggestion, untriaged, area-Extensions-DependencyInjection

Milestone: -

@madelson
Copy link
Contributor

madelson commented Jan 28, 2022

Is it worth considering a model where a keyed registration plays out as a registration of a keyed service resolver type (similar to IIndex in autofac)?

In that world, a service keyed T could be retrieved by first resolving a KeyedServiceResolver<T> and then calling it’s Resolve(object key) method. This indirection could be hidden by extension methods.

I think it would be nice to have a solution that isn’t based on attributes (attributes are a good option to have though).

EDIT: reading your post again, I see that you suggest that IKeyedServiceProvider can itself be a service retrieved from IServiceProvider, which is essentially the same thing I'm suggesting.

@commonsensesoftware
Copy link
Author

@madelson you are correct and I agree. Attributes are not a requirement. I'd be surprised if any form of a proposal would be accepted if the IServiceProvider.GetService(Type) contract is broken though. Ultimately, this means that there has to be a fallback that centers around Type. There are multiple ways that could be facilitated, but attributes provide a way to add the metadata as a runtime annotation. In the strictest of sense, the result from GetCustomAttibutes is just a collection Object anyway. An implementation could put any annotation/metadata object they want in there using this approach. It doesn't have to be a real Attribute.

If supported, I think you still need to have an attribute at the call site to specify the key that should be used. There might already be attribute that could facilitate that, but none come to mind. Perhaps there's an established convention that can be used, but at attribute seems to be the most obvious choice. As long as it's an option, then there is still a single attribute whether it is used for a single purpose or multiple.

ASP.NET Core already has its own attribute that could fill a similar role with a minor modification:

public IActionResult Get([FromServices("B-Team")] ISpeaker speaker) => Ok({text = speaker.Speak()});

@madelson
Copy link
Contributor

I think you still need to have an attribute at the call site to specify the key that should be used

There are alternatives, though. One is to use one of the func-based methods like TryAddTransient(IServiceCollection, Type, Func<IServiceProvider,Object>) for your registration of the service that consumes the keyed service rather than use reflection.

Another approach is to inject a key-based service lookup type instead of injecting the type itself. For example:

public MyConsumingService(IKeyedServiceLookup<MyKeyedService> keyedServiceLookup)
{
    this._keyedServiceLookup = keyedServiceLookup.Resolve("SomeKey");
}

Or even:

public MyConsumingService(Func<object, MyKeyedService> keyedServiceLookup)
{
    this._keyedServiceLookup = keyedServiceLookup("SomeKey");
}

While I see the appeal of the attribute, something I dislike about it is that (I think) it would force other IOC libraries that interop with IServiceProvider (e. g. Autofac) to know about and respect that attribute in their reflection-based systems, whereas injecting a specific service type is something any DI system should be able to do.

@commonsensesoftware
Copy link
Author

I 👀 now. Yes, an attribute could be restrictive in that way. Using a Func<string, T> is one of the most obvious, but somewhat clunky, ways to support it today.

I definitely see how some type of holder could work. Perhaps something like:

public interface IDependency<T>
{
   T? Get(string? key = default); 
}

This approach would be similar to how IOptionsMonitor<T> works. Different resolution methods (like required) can be achieved via extension methods (ex: dependency.GetRequired("key")). That means that an implementer would always inject an implementation of IDependency<T> using the Null Object pattern - say Dependency<T>.Unresolvable (or something like that).

I didn't mean to steal your thunder. I guess this is effectively the same thing as whatever IKeyedServiceLookup<T> would look like.

@commonsensesoftware
Copy link
Author

commonsensesoftware commented Jan 28, 2022

One thing I don't like about an attribute, function, or lookup interface is that it makes the key another form of hidden dependency to the caller.

It's a little far fetched, but I'm curious what people would think of a little abuse of the type system to raise that dependency out as a formal type.

Consider the following:

// intentional marker so that you can't just provide any old type as a key
public interface IKey { }

public interface IDependency<TKey, TDep>
    where TKey : IKey
    where TDep : notnull
{
    TDep Value { get; }
}

Now consider how it might be used:

public static class Key
{
    public sealed class ATeam : IKey { }
}

public interface IFoo { }
public interface IPityTheFoo : IFoo { } 
public sealed class MrT : IPityTheFoo { }

public class Bar
{
    private readonly IDependency<Key.ATeam, IPityTheFoo> foo;
    public Bar(IDependency<Key.ATeam, IPityTheFoo> foo) => this.foo = foo;
    public IFoo Foo => foo.Value;
}

This would expect that an implementer uses typeof(TKey).Name as the service key. An implementer would already have to understand IDependency - in any form.

Pros

  • The dependency key is conspicuously expressed as part of the dependency
  • The container knows the dependency doesn't exist before calling the constructor
  • Backing resolution, say through IKeyedServiceProvider.GetService(Type,string) still works
  • Largely removes magic strings, including at registration-time
    • ex: services.AddSingleton<Key.ATeam, IPityTheFoo, MrT>();
  • This could be a way to solve the IServiceProvider.GetService(Type) conundrum:
    • ex: sp.GetService<IDependency<Key.ATeam, IFoo>>()?.Value; // ← by 'key'

Cons

  • No guarantee an implementer consistently uses typeof(TKey).Name (caveat emptor?)
    • Maybe this is a non-issue; the Type implementing IKey is the key and string variants are simply unnecessary, except perhaps as a backing implementation detail
  • There are limitations on what the key can be since it must be a valid type name
    • IMO this is of little consequence given the tradeoff of expressing the dependency key

@madelson
Copy link
Contributor

@commonsensesoftware the idea of having keys be types rather than strings is a really interesting one. If the system went in that direction I wouldn't want it to be strings at all under the hood: just types all the way down. Otherwise it feels too magical.

The main weakness of types over arbitrary objects/strings is in more dynamic cases. For example you might want to have a set of services keyed to values of an enum and resolve the appropriate service at runtime based on the enum value. You can still do that with type keys, but you need an extra mapping layer to go enum value -> type.

One other minor thing to note: I don't think service implementers injecting IDependency<Key, Service> would save off the whole type. Instead, they'd just have private readonly Service _service; and do this._service = serviceDependency.Value; in the constructor.

@commonsensesoftware
Copy link
Author

commonsensesoftware commented Jan 30, 2022

@madelson I think we are largely on the same page. I don't like the idea of magic strings either (at all).

... you need an extra mapping layer to go enum value -> type

I'm trying to think of a scenario where Type wouldn't work. Yes, it's a little unnatural, but Type is effectively being used as an enumeration. I can't think of a scenario (off the top of my head) where Type couldn't be used this way. It should be able to be used in all the places a constant or enumeration could also be used. One counter argument that I like is that a Type cannot be abused in a way that an enumeration can. For example, var weekday = (DayOfWeek)8; is a lie, but happily compiles and executes at runtime.

I completely agree that consumers would want to unwrap the dependency. Where and how is at their discretion. It is anologous to IOptions<T>. You can unwrap it in the constructor, but consumers should be aware that the entire dependency graph may not be resolvable at that point in time. It doesn't happen often, but it is possible.

I decided I wanted to flush the idea of using Type as a key to see what it would look like in practice. Some interesting and positive results were revealed:

  • Using System.Reflection.Context is no longer required (e.g. no new assembly references)
  • No magic strings
  • No attributes (ServiceKeyAttribute was removed)
  • ServiceDescriptor.Key is unnecessary nor are any other changes to the existing type
  • IKeyedServiceProvider and IKeyedTypeFactory are unnecessary
  • IKey doesn't buy much value and was removed

The revised contract would be:

public interface IDependency<in TKey, out TService>
    where TKey : new()
    where TService : notnull
{
    TService Value { get; }
}

Enforcing the new() constraint on the key isn't strictly necessary. It's another variation of IKey, but with slightly more practical enforcement.

The extension methods now simply revolve around IDependency<TKey, TService>. For example:

public static TService? GetService<TKey, TService>(this IServiceProvider serviceProvider)
    where TKey : new()
    where TService : notnull
{
    var dependency = serviceProvider.GetService<IDependency<TKey, TService>>();
    return dependency is null ? default : dependency.Value;
}

Examples

Here are some revised examples:

[Fact]
public void get_service_generic_should_return_service_by_key()
{
    // arrange
    var services = new ServiceCollection();

    services.AddSingleton<Key.Thingy, IThing, Thing2>();
    services.AddSingleton<IThing, Thing>();

    var provider = services.BuildServiceProvider();

    // act
    var thing = provider.GetService<Key.Thingy, IThing>();

    // assert
    thing.Should().BeOfType<Thing2>();
}

[Fact]
public void get_services_generic_should_return_services_by_key()
{
    // arrange
    var services = new ServiceCollection();
    var expected = new[] { typeof(Thing1), typeof(Thing2), typeof(Thing3) };

    services.TryAddEnumerable<Key.Thingies, IThing, Thing1>(ServiceLifetime.Transient);
    services.TryAddEnumerable<Key.Thingies, IThing, Thing2>(ServiceLifetime.Transient);
    services.TryAddEnumerable<Key.Thingies, IThing, Thing3>(ServiceLifetime.Transient);

    var provider = services.BuildServiceProvider();

    // act
    var thingies = provider.GetServices<Key.Thingies, IThing>();

    // assert
    thingies.Select(t => t.GetType()).Should().BeEquivalentTo(expected);
}

[Fact]
public void get_required_service_should_inject_dependencies()
{
    // arrange
    var services = new ServiceCollection();

    services.AddSingleton<Key.Thing1, IThing, Thing1>();
    services.AddTransient<Key.Thing2, IThing, Thing2>();
    services.AddSingleton<CatInTheHat>();

    var provider = services.BuildServiceProvider();

    // act
    var catInTheHat = provider.GetRequiredService<CatInTheHat>();

    // assert
    catInTheHat.Thing1.Should().BeOfType<Thing1>();
    catInTheHat.Thing2.Should().BeOfType<Thing2>();
}

As previously mentioned, CatInTheHat.cs would look like:

public class CatInTheHat
{
    private readonly IDependency<Key.Thing1, IThing> thing1;
    private readonly IDependency<Key.Thing2, IThing> thing2;

    public CatInTheHat(
        IDependency<Key.Thing1, IThing> thing1,
        IDependency<Key.Thing2, IThing> thing2)
    {
        this.thing1 = thing1;
        this.thing2 = thing2;
    }

    public IThing Thing1 => thing1.Value;

    public IThing Thing2 => thing2.Value;
}

This second iteration doesn't require any fundamental changes to Microsoft.Extensions.DependencyInjection.*. All of the proposed changes are additive. While it is possible expose this functionality via an extension library, supporting keyed services as a first-class concept would be a nice edition.

The attached Keyed Service - Iteration 2 contains an end-to-end working example using only Type as a key.

@madelson
Copy link
Contributor

I like this direction. One thought:

Enforcing the new() constraint on the key isn't strictly necessary. It's another variation of IKey, but with slightly more practical enforcement.

I'd vote for nixing the new() constraint as well. For example, in some cases I could see wanting to use the target service type as the injected key:

public class MySpecialService
{
    // implication is that MySpecialService needs an implementation of ISomeService customized just for it
    public MySpecialService(IDependency<MySpecialService, ISomeService> someService) { ... }
}

@commonsensesoftware
Copy link
Author

@madelson I'm onboard with that. There's no strict reason why the new() constraint needs to exist. At the end of the day a Type is being used as a key. It doesn't really matter whether it makes sense to me. If it works and someone chooses to use it in an obscure way - so be it.

There are just a few other design points. There needs to be a non-generic way to get the value of a keyed dependency.

public interface IDependency
{
    object Value { get; }
}

This is already in the last iteration. A consumer still asks for typeof(IDependency<TKey,TService>), but there needs to be away to get IDependency<TKey,TService>.Value without resorting to Reflection or something. This potentially means the contract changes to:

public interface IDependency<in TKey, out TService> : IDependency where TService : notnull
{
    new TService Value { get; }
}

I don't really like that the Value property is shadowed this way, but it may be the best option. In the 2.0 version, I had them separate. The risk of them being separate is that an extender may not implement both interfaces, which breaks one path. The cast to IDependency would fail making it obvious, but that wouldn't happy until runtime. That feels like a bad experience. Since interfaces are not really inherited, shadowing the Value property with a more specific type should be fine. That would ensure any other provided implementation of IDependency<TKey,TService> provides a way to retrieve its Value without knowing/having the generic interface type. I validated this slight change and everything still works as expected.

The only other open item I have (so far) is how ServiceDescriptor is registered. Currently, this feature would require two registrations:

  1. Register the service with a keyed type
  2. Register IDependency<TKey,TService> which hides the keyed type mapping

This behavior could be done with a single registration, but the container implementer would have to know and understand that IDependency<TKey,TService> is special. Having two registrations may be a non-issue. There are truly two different, distinct service registrations that are ultimately both used.

The other question is whether IDependency<TKey,TService> should have the same lifetime as TService. The registration of IDependency<TKey,TService> can always be transient because the implementation is expected to be a simple facade over IServiceProvider.GetService(Type), which will always resolve TService using the correct scope. IDependency<TKey,TService> effectively serves as a transient accessor to TService. I'm not sure there is an advantage to holding on to an instance of IDependency<TKey,TService> longer. If it's always transient, the memory allocated for it can be reclaimed sooner. Ideally, it could be struct, but since it will always be boxed, I'm not sure that helps.

Beyond those basic mechanics, I think we're down to just the final naming, which I have no real strong opinions about, and whether this proposal would/will be accepted. Maybe I need to revise the original proposal with all the changes that have been made? I think we've landed in a much better and more sensible approach to keyed services than where we started. Is more support/evidence required to justify these additions being rolled in rather than exposed via some 3rd party extension?

@davidfowl
Copy link
Member

davidfowl commented Feb 1, 2022

Like all DI features, they need to be supported by a majority of the existing containers to be considered. I'm not sure this feature meets the bar but we can investigate.

DI council: @alexmg @tillig @pakrym @ENikS @ipjohnson @dadhi @seesharper @jeremydmiller @alistairjevans

@dadhi
Copy link

dadhi commented Feb 1, 2022

@davidfowl @commonsensesoftware

I would love to see a minimal set of features agreed upon (maybe with voting). The container authors may label on what is available in their libs.

And then the table with solution, pros, cons and comments.

Otherwise we will lost in the options.

@davidfowl
Copy link
Member

Named DI has come up several times before (e.g. dotnet/extensions#2937) and we've work around not having the feature in many systems (named options, named http clients etc). While I think the feature is useful, I am hesitant to add anything that couldn't be implemented super efficiently in the core container and that other containers don't support. The implementation shouldn't need to mess with the reflection context either. This feels very core to the DI system so the implementation cost would be large (we'd need changes to service descriptors etc).

The attribute seems like a reasonable approach if its supported everywhere. If the set of keys is known at compile time, then open generics can be used to solve this problem today.

Those are some late night thoughts before I go to bed 😄

@tillig
Copy link
Contributor

tillig commented Feb 1, 2022

I think Autofac could cover this without too much effort in adapting the service registrations from MS format to Autofac. Some specific behavior would need to be determined, though, before I could say more conclusively. Here is how Autofac handles some of the stuff I haven't seen talked about above.

First, simple non-generic types:

var builder = new ContainerBuilder();

// Keyed
builder.RegisterType<Thing1>().Keyed<IThing>("key");
builder.RegisterType<Thing2>().Keyed<IThing>("key");

// Not keyed
builder.RegisterType<Thing3>().As<IThing>();

// BOTH keyed AND not keyed
builder.RegisterType<Thing4>().As<IThing>().Keyed<IThing>("key");

var container = builder.Build();

// Resolving the set of keyed items will have
// Thing1, Thing2, Thing4.
var allKeyed = container.ResolveKeyed<IEnumerable<IThing>>("key");
Assert.Equal(3, allKeyed.Count());

// Resolving the non-keyed items only returns
// things that were explicitly registered without a key.
// Thing3, Thing4
var notKeyed = container.Resolve<IEnumerable<IThing>>();
Assert.Equal(2, notKeyed.Count());
  • What's the expected behavior when an IEnumerable<T> is resolved without a key? Should it include both things that have keys and don't have keys? Or just the explicitly non-keyed items? Autofac won't easily be able to implement "return everything" in any sort of performant way. It's been "only keyed" or "only non-keyed" for a very long time so it's kinda baked into the core of the thing.
  • Can you register the same component both as keyed and not keyed? This can be important if you want to have a "default instance" and have that same instance be specifically named. Especially if it's not a transient lifestyle, that also means it's literally the same instance instead of two instances due to two different registrations.

Open generics add a bit of an interesting mix:

var builder = new ContainerBuilder();

// You can specify the same key for all the things.
builder.RegisterGeneric(typeof(GenericThing<>))
       .Keyed("key", typeof(IGeneric<>));

// This all works.
_ = container.ResolveKeyed<IGeneric<string>>("key");
_ = container.ResolveKeyed<IGeneric<object>>("key");

// ...but it's not registered as non-keyed.
Assert.Throws<DependencyResolutionException>(() => container.Resolve<IGeneric<string>>());

Do all the open generics in a single registration get the same key? As you can see above, that's how Autofac works. The alternative might be to have some sort of function provided during registration where the key can be generated based on the type being resolved, but Autofac doesn't support that.

Other thoughts that may get the juices flowing:

  • Attributes should always be an optional thing. There's a fairly vocal minority of folks who are very interested in not letting DI-related code intermingle with their other code. It ties the DI system specifically to the implementation and that's not super cool. Some folks go to a lot of work writing their own wrappers around DI systems to make sure they're totally decoupled. It's fine if it's an added feature, but it shouldn't be the primary/only way things work. Autofac does support resolving based on filter attributes (like adding an attribute to a constructor parameter) but doesn't have an attribute to allow you to register and have a name automatically attached. There is definitely a perf hit when someone uses the keyed attribute filter because now you have to look for that attribute on every constructor and obey it on every resolution. Autofac makes this explicitly opt-in.
  • Registration metadata may be more interesting. As long as you're looking things up about a registration, it may be interesting to allow each registration to support a general dictionary of metadata so you could filter by different things. Here are some Autofac examples.
  • Lazy<T> support gets used a lot with metadata registrations. That is, it seems pretty common that folks will want to register like 10 things with different keys/metadata, resolve all of them, then say "I want to use abc and def but not ghi" so there's a sort of LINQ over the set of items before resolving. Which isn't to say "keyed registrations are a slippery slope," but just be aware some of these patterns go together somewhat naturally.

Here's what I mean about the Lazy<T> thing:

var builder = new ContainerBuilder();
builder.RegisterType<Thing1>().As<IThing>().WithMetadata("name", "rest-1");
builder.RegisterType<Thing2>().As<IThing>().WithMetadata("name", "rest-2");
builder.RegisterType<Thing3>().As<IThing>().WithMetadata("name", "soap-1");
var container = builder.Build();

// Get a list of all the registered `IThings`
// with all their metadata
// but don't resolve them quite yet
var restThings = container.Resolve<IEnumerable<Meta<Lazy<IThing>>>()
  // then filter to just the REST things
  .Where(meta => meta.Metadata["name"].ToString().StartsWith("rest"))
  // Get the Lazy<IThing> for each of those
  .Select(meta => meta.Value)
  // Now do the resolve operation
  .Select(lazy => lazy.Value);

I recognize we're just talking about keyed/named things, so I don't want to confuse the issue by bringing registration metadata into the mix, just that the two concepts are somewhat near to each other and I thought it might open some ideas or thoughts here that might not otherwise have come up. Sorry if the mention of it derails things. I hope it doesn't.

@jeremydmiller
Copy link

Lamar already has support for named registrations, and I'd guess that many of the mainstream IoC containers do as well as that was very common before the ASP.Net team decided retroactively how everything was meant to behave. Heck, StructureMap had that in 2004. That being said, I'm basically against any extension to the core, conforming container behavior unless there's some really compelling reason to do so. That being said, what's the use case here that folks want? Especially knowing that they can always go down to the inner container at any time and generally have quite a bit more functionality than what's exposed through IServiceProvider.

At a minimum, I'd eliminate any kind of fancy behavior about "do named vs not named instances get returned from GetServices(Type)" as much as possible. And be very clear about how you'll handle naming collisions. First one wins? Last?

@tillig
Copy link
Contributor

tillig commented Feb 1, 2022

I'm basically against any extension to the core, conforming container behavior unless there's some really compelling reason to do so.

Yeah, I'll second this. The conforming container thing has been a sore spot since inception.

@seesharper
Copy link

LightInject has always supported named registrations. Was it a good idea? Well, it has gotten me out of trouble on a number of occasions. In fact, it is used in the Ms.Ex.DI adapter to work around some of the differences between LightInject and Ms.Ex.DI. 😀. When it comes to adding this feature to to the specifications I'm not so sure if that is a good idea. It significantly raises the bar for conforming containers which might or not might support this out of the box.

@alistairjevans
Copy link
Contributor

Named/keyed registrations often have implications for how the service is consumed, rather than just how it is registered. I.e. do you use attributes, an IIndex style dependency, a direct call to a method on an injected scope of some kind, etc.

I believe there's a fair amount of strong opinion in the wild on how best to consume a keyed service (most of which have already been discussed here). I'm a little concerned that going down a specific path in the conforming container for how to consume a keyed service will lead to reduced choice in the ecosystem on this particular point, where there already exist plenty of community libraries people could use that allows them to take the approach they prefer.
As a general note, I'd probably lean towards not adding features to the conforming container that add constraints on how user components can/should specify their dependencies.

On a specific technical note re:attributes, I believe Autofac would require a chunk of additional work to inspect the constructor of components for non-Autofac attributes, without the explicit opt-in we currently require on the component that says "please consider the KeyFilter attribute on this component". Not having that explicit opt-in may have knock-on consequences in core Autofac which I'd like to avoid thinking about if possible, and I don't believe conforming container registration currently has the concept of "opt-in some behaviour on this component when resolving dependencies".

@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Feb 1, 2022
@maryamariyan maryamariyan added this to the Future milestone Feb 1, 2022
@commonsensesoftware
Copy link
Author

Apologies, but I had the assumption that the discussion would pick up and continue where it left off (with @madelson). After further thought and consideration, I've decided to revise the API proposal. Based on the additional comments, I feel that the revision aligns better with several of the callouts.

@davidfowl, I 💯 agree. If this API proposal cannot work with the existing container implementations, then it should be DOA 💀. I don't know that you're sold yet, but I think you might be surprised or pleased that the latest revision eliminates magic strings, unnecessary attributes, Reflection (context) hackery, no change to ServiceDescriptor, works with open generics, and still works through the super efficient mechanisms offered by existing containers. I've added integration tests for each of the existing container implementations to the example code and summarized the results in the proposal.

@tillig, in the revised proposal, Autofact just works without a single change 😉. To answer your specific questions:

  1. IEnumerable<T> and IEnumerable<Key+T> are distinctly different registrations and never shall the two meet. IMHO that is clean and straight forward to understand. ServiceDescriptor and/or native container implementations provide ways to use multiple registrations for a particular instance, but that's in the hands of a developer.
  2. Yes - a component can be keyed and non-key. A use case I previously had a was a scoped DbContext<T> in the request pipeline, but a separate, transient DbContext<T> of the same type used in the background.
  3. A keyed and non-key type should be distinct IMO; therefore, when registering an open generic, IGeneric<> and Key+IGeneric<>, should be separate without any possibility of resolution in each other's context.
  4. The revised proposal ditches attributes, so hopefully this is a non-issue now.
  5. I think metadata may be out of scope here, but - I admit - I'm a fan. 😄

@jeremymiller, you make an excellent point about the possible collisions of magic strings. In many cases, it's also a hidden dependency that a caller cannot know. Whether you agree with my revised proposal remains to be seen, but it would eliminate both of these issues without fancy ceremony for a consumer. In particular, by having to express key and type together, it's possible to know and validate whether a particular combination has been registered (which is difficult or impossible with magic strings).

@seesharper, agreed. In the revised proposal, LightInject just works out-of-the-box and without having to do any magic under the hood to use its native named registrations. 😉.

@alistairjevans, these are fair points and, at least on some level, give credence to my revised proposal; specifically with regard to removing the use of attributes. All implementations already agree on consuming a service by asking for its Type (which is the key). This proposal takes it a step further with a thinly-veiled abstraction, where registering a more specific type in a container may not be achievable through other means such as inheritance. Whether that maps as a plain 'ol type or a type with a key/name in the container is an implementation detail.

@tillig
Copy link
Contributor

tillig commented Feb 3, 2022

ServiceDescriptor, I assume, would end up with some Key property that adapter libraries would need to use and, if not null, register a keyed entity.

I'm personally less concerned about magic strings because people do some interesting stuff out in the wild, assembly scanning to find their own custom attributes and add string-based keys due to the needs of their own homegrown "plugin" systems. I think folks could find the requirement of a key being a type to be too restrictive and not dynamic enough, but maybe in this lowest-common-denominator conforming container case that's OK.

@alistairjevans makes a good point about having a specific consumption mechanism, but from a devil's advocate standpoint, without some way to consume this more formally (e.g., the Meta<T> decorator in that Autofac metadata example), folks who need keyed services are going to have to start injecting IServiceProvider directly and doing service location in the constructor to get what they need. I guess maybe that's OK, but it kinda defeats the purpose of the DI in the first place and makes testing a pain.

Which isn't to say I think the attribute consumption model is the way to go, just trying to walk through what it means on both sides of this construct.

I suppose a workaround for that consumption model might be a class that abstracts away the service location...

public class NamedServiceProvider<T>
{
  private readonly IServiceProvider _provider;
  public NamedServiceProvider(IServiceProvider provider)
  {
    this._provider = provider;
  }

  public T GetService(Type key)
  {
    return (T)this._provider.GetService(key, typeof(T));
  }
}

...and have that registered/injected into the consumer.

public MyConstructor(NamedServiceProvider<IThing> factory)
{
  this._instance = factory.GetService(Key.Thing1);
}

However, I'm still just sorta iffy on the whole concept. I'm not totally against it, but in general expanding the conforming container beyond the most amazingly simple, basic functionality has sort of a "smell" to it for me. It always seems like all this should "just work" and then Autofac will have some difference in how it functions from the base container; or Autofac will differ from SimpleInjector; or something like that, and way, way downstream consumers will get bent out of shape that they chose different backing containers but it isn't all behaving literally identically to the M.E.DI container.

My understanding of the intent of the original conforming container was to be just enough to provide an abstraction for the base ASP.NET Core framework to function. It's definitely taken off with the community, which is great, but I don't think keyed registrations is required for the framework to function. Again, not saying I'm 100% against it, just thinking back to reasons to expand on the conforming container and what the original intent (my understanding) is.

This same functionality could be implemented already by just using the backing container of your choice and registering the keyed stuff in ConfigureContainer. I get that the target here is library owners wanting to register keyed stuff (if it's not library owners, then the use of ConfigureContainer or similar is a pretty clear solution) but I also wonder if that means the library owners might need to look at different ways to handle this stuff. Like, maybe you don't have five IRepository implementations and instead have IUserRepository, IRoleRepository, and so on, and instead differentiate by actual type.

@commonsensesoftware
Copy link
Author

@tillig,

Actually, you do not end up with a Key property; it's not necessary. Type is the key. Consider this:

void Demo(IServiceProvider provider)
{
    // 'keyed' service location in the traditional way since day 1
    var key = typeof(IThing);
    var thing = (IThing) provider.GetService(key);

    // requiring 2 types forms a 'composite key'
    var compositeKey = typeof(IDependency<Key.Thingy, IThing>);
    var thingy = (IThing) provider.GetService(compositeKey);

    // this is technically the same thing, but is less 'natural' and
    // can't be used in all call sites where IThing could be requested
    var compositeKey = KeyedType.Create(typeof(Key.Thingy), typeof(IThing));
    var thingy = (IThing) provider.GetService(compositeKey);
}

IServiceProvider is consistently supported for all containers. I totally agree that eliminating magic strings is not what raises the bar here. That approach has been used for close to two decades. I have no intention of convincing people they have to ditch their strings if they want them. I am, however, suggesting that we already use Type as a key and I'm merely trying to expand upon that concept. If an implementer wants to use a string under the hood, they are in total control and free to do so.

I'm not entirely sure how Meta<T> would playout using this approach, but I'm willing to go the extra mile and think through what the impact would be or how it would work.

The idea of NamedServiceProvider<T> is exactly what IDependency<TKey,TService> is meant to provide. It's an interface so that a container can decide on its own how it should be satisfied. Having a dependency injected through this interface provides a way to define a composite key without relying on metadata (say from attributes) or manual lookup by a developer through IServiceProvider.GetService(Type). As you astutely pointed out, that would make testing more painful. Providing a mock or stub of IDependency`2 is very straight forward. More than anything, I think that using types this way addresses the issue of injecting keyed dependencies at the call site. The registration side is more straight forward; especially since a developer can drop down to the native container implementation without adding a lot of coupling elsewhere. The inverse is typically not true.

Consider injecting a keyed service into an ASP.NET Core action.

[HttpGet]
public IActionResult Get([FromServices] IDependency<Key.School, IGreeter> greeter) =>
    Ok(greeter.Value.Welcome());

This doesn't require anything special to be added to the core abstractions. Yes, obviously we need the interface or something that can be the placeholder. A container doesn't need to do any special to look this up. Furthermore, it is compatible with the established semantics of IServiceProvider for all containers. Resolution from a container has always been the how prerogative of the container.

I'm a visual person. I'm very much the "Show me the code" guy. I also realize that people are busy with their day jobs, side projects, family, etc. I've attached a working solution with test scenarios for all of the existing container support to enhance this conversation. Theory is great, but I'd rather see it in action. There are a lot more details and comments I've put in the code that are simply a lot to rehash in detail within the thread.

You'll be happy (maybe?) to know that Autofac works for the (now) complete set of scenarios (unless I missed something). The Autofact setup is still as simple as:

var builder = new ContainerBuilder();
builder.Populate(services);
return builder.Build().Resolve<IServiceProvider>();

👏 Nice work. Due to how Autofac works, it's actually unnecessary to go through the native key/name service resolution APIs. Everything works as is.

What about a container where that isn't the case? LightInject (@seesharper) worked for all scenarios, except for Implementation Factory (which was previously untested). This likely has to do with how LightInject registers the function. This is the main barrier for automatic support across containers. KeyedType.Create returns a fake type that is a TypeDelegator. This Type cannot be compiled or invoked. Clearly there's a way around this because it works for some frameworks. This could be a mistake or ommission on my part. Regardless, the examples demonstrate that if that were to be a problem, then there is an escape hatch.

It took a few iterations, but now that I've tested this out against all of the supported containers, I noticed a repeating pattern and removed as much of the ceremony as possible. The core abstractions will provide two things that will make adapting them to any other container simple:

  1. Remove and collate keyed services from IServiceCollection into IReadOnlyDictionary<Type, IServiceCollection>
    a. The Type key in the dictionary is the key generated by KeyedType.Create when the descriptor was registered
    b. The IServiceCollection contains all descriptors mapped to that key
  2. Provide a base visitor implementation to remap descriptors to their native container variants
    a. This process doesn't change how the developer registered the service, only how the container resolves it

With that in mind, the LightInject adapter would define two native IDependency`2 implementations:

// remaps the core Dependency<,> abstraction
internal sealed class LightInjectDependency<TKey, TService> :
    IDependency<TKey, TService>
    where TService : notnull
{
    // LightInject can do this however it wants, but the hash code will be unique
    // important: this does have to match how LightInject defines the key (see below)
    private readonly string key =
        KeyedType.Create<TKey, TService>().GetHashCode().ToString();
    private readonly IServiceContainer container;
    protected LightInjectDependency(IServiceContainer container) =>
        this.container = container;
    public TService Value => container.GetInstance<TService>(key);
    object IDependency.Value => Value;
}

// remaps the core Dependency<,,> abstraction for IEnumerable<T> support
internal sealed class LightInjectDependency<TKey, TService, TImplementation> :
    IDependency<TKey, TService>
    where TService : notnull
    where TImplementation : notnull, TService
{
    // note: that here we want to key on the concrete type
    private readonly string key =
        KeyedType.Create<TKey, TImplementation>().GetHashCode().ToString();
    private readonly IServiceContainer container;
    protected LightInjectDependency(IServiceContainer container) =>
        this.container = container;
    public TService Value => container.GetInstance<TImplementation>(key);
    object IDependency.Value => Value;
}

Now that we have LightInject-specific resolvable dependencies, we'll use a visitor to remap the existing keyed service descriptors. The base implementation does all the heavy lifting of enumerating service and determinging which descriptors need to be passed when and with what keys. The following highlights the relevant parts of the implementation:

internal sealed class LightInjectKeyedServiceVisitor : KeyedServiceDescriptorVisitor
{
    private readonly IServiceContainer container;
    private readonly Scope rootScope;

    public LightInjectKeyedServiceVisitor(IServiceContainer container)
        : base(
            /* remap Dependency<,>  → */ typeof(LightInjectDependency<,>),
            /* remap Dependency<,,> → */ typeof(LightInjectDependency<,,>))
    {
        var self = new ServiceRegistration()
        {
            ServiceType = typeof(IServiceContainer),
            Value = container,
        };
        this.container = container;
        this.container.Register(self);
        rootScope = container.BeginScope();
    }

    protected override void VisitDependency(ServiceDescriptor serviceDescriptor)
    {
        // ServiceDescriptor.ServiceType = IDependency<TKey,TService>
        // ServiceDescriptor.ImplementationType =
        //     LightInjectDependency<TKey,TService> ||
        //     LightInjectDependency<TKey,TService,TImplementation>
        //
        // lifetime probably doesn't matter here and can be transient, but the descriptor
        // will contain the same lifetime associated with the underlying resolved type
        container.Register(
            serviceDescriptor.ServiceType,
            serviceDescriptor.ImplementationType,
            ToLifetime(serviceDescriptor));
    }

    // no need to know how 'Key' is defined or how it's mapped to ServiceDescriptor
    protected override void VisitService(Type key, ServiceDescriptor serviceDescriptor)
    {
        // key = KeyedType<TKey, TService>
        // serviceDescriptor = ServiceDescriptor with real types
        var registration = new ServiceRegistration()
        {
            // LightInject is deciding how to use the key for lookup (see above)
            ServiceName = key.GetHashCode().ToString(),
            ServiceType = serviceDescriptor.ServiceType,
            Lifetime = ToLifetime(serviceDescriptor),
        };

        if (serviceDescriptor.ImplementationType != null)
        {
            registration.ImplementingType = serviceDescriptor.ImplementationType;
        }
        else if (serviceDescriptor.ImplementationFactory != null)
        {
            registration.FactoryExpression = CreateFactoryDelegate(serviceDescriptor);
        }
        else
        {
            registration.Value = serviceDescriptor.ImplementationInstance;
        }

        container.Register(registration);
    }
}

LightInject has the following setup today:

var builder = new ContainerBuilder();
builder.Populate(services);
return services.CreateLightInjectServiceProvider();

To make it work with a keyed Implementation Factory, the setup would change as follows:

static IServiceProvider BuildServiceProvider(IServiceCollection services)
{
    var options = new ContainerOptions().WithMicrosoftSettings();
    var container = new ServiceContainer(options);
    var original = new ServiceCollection();

    // make a shallow copy of the current collection
    for (var i = 0; i < services.Count; i++)
    {
        original.Insert(i, services[i]);
    }

    // new, built-in extension method in the core abstractions
    // this removes and maps keyed service descriptors
    var keyedServices = services.RemoveKeyedServices();

    if (keyedServices.Count == 0)
    {
        // perf: remapping can be skipped if the developer didn't
        // use ServiceDescriptor.ImplementationFactory for a keyed service
        var remapNotRequired = services.Values
                                       .SelectMany(v => v)
                                       .All(sd => sd.ImplementationFactory == null);

        if (remapNotRequired)
        {
            return container.CreateServiceProvider(original);
        }
    }

    var visitor = new LightInjectKeyedServiceVisitor(container);

    // visiting the keyed services will re-register them in the container
    // with container-natively understood semantics
    visitor.Visit(keyedServices);

    return container.CreateServiceProvider(services);
}

The goal is not get containers to change the way they use or define keyed/named services. This is only meant to provide support of a keyed type. As it relates to IServiceCollection and ServiceDescriptor, some containers may choose to facilitate that through their native implementations, while others - like Autofac and the default engine - work without using any native keying mechanisms whatsoever. The developer consumption model should not be:

IServiceProvider.GetService(typeof(IDependency<My.Key,IThing>)) →
    IContainer.GetService("key", typeof(IThing))

Even if that might be what happens behind the scenes.

@z4kn4fein
Copy link
Contributor

z4kn4fein commented May 3, 2022

Hello there! I think the idea of supporting keyed services is great, and thank you for involving Stashbox in the testing project. It revealed a major deficiency that you pointed out correctly, the resolution of many services by key was broken. I just wanted to let you know that this issue is fixed in Stashbox v5.4.3 and Stashbox.Extensions.DependencyInjection v4.2.3 packages. With the current state of the API proposal, Stashbox still needs the KeyedServiceVisitor that bypasses the registration of TypeDelegator types.
Thanks again!

@davidfowl I would be grateful if you could tag me also when you include the DI Council in a thread. This issue gave me useful information to improve my library, and I just found it accidentally. 😄 Thank you!

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jun 27, 2023
@ericstj
Copy link
Member

ericstj commented Jul 13, 2023

@steveharter @benjaminpetit with #87183 merged, can this be closed?

@adityamandaleeka
Copy link
Member

Yup, closing.

@sander1095
Copy link

Hey everyone :) I saw that this issue just got closed. However, I still want to raise some questions/concerns.

I like the idea of named services, but I dislike 2 things about this proposal:

  • Inconsistency with other features in .NET, like the Options Pattern with Named Options
  • The fact that there is a new interface to grab keyed items from.

Comparison to the Options Pattern

When I read the docs about the Options Pattern, you can choose to use services.Configure<TType>(config) OR services.Configure<TType>(string, config).

This proposal adds different methods (AddKeyed*) to do the same thing with service registrations, which I find inconsistent. I think this can cause a lot of confusion. I'd much prefer to get new overloads on existing AddSingleton/AddScoped/AddTransient methods so the approach is consistent with the Options Pattern.

New interface

I see there is a new interface to retrieve keyed services from. I don't like this approach for the following reasons:

Right now, IServiceProvider is the 1 interface you can use as a Service Locator to retrieve services from. But instead of just using IServiceProvider to get access to all the services you might need, you'll now also need to inject the IKeyedServiceProvider. So you need 2 interfaces. This doesn't feel very SOLID; what if in the future we get another way to register services, do we then also get a 3rd interface to retrieve items from?

In the proposal it's established that ServiceDescriptor gets extended with some new properties. So why can't IServiceProvider be extended with some new overloads on GetRequiredService() with a key parameter?

I think the new interface will make things much more complex than necessary. In .NET Framework, we used libraries like Autofac, Ninject, etc.. With .NET Core we got Microsoft's own DI framework and I found this very easy to work with. This new interface increases the complexity unnecessarily, in my opinion!

Curious for your thoughts :)

@deeprobin
Copy link
Contributor

@sander1095 Extending IServiceProvider with a new overload will create breaking changes.
You could watch the API Proposal video to understand the specific background.

Video

@sander1095
Copy link

Hi @deeprobin, I had a quick look at the video but couldn't find the source, but I'll believe you ;).

I'm not sure if these next thoughts are addressed in the video, so I'll go ahead and ask:

New methods instead of overload

Instead of adding a new overload, could we not insert these new Keyed methods on IServiceProvider like GetKeyedService() andGetRequiredKeyedService()? I think this makes the API a lot more friendly to use.

If it is part of another interface, it will be much more difficult to discover the existence of keyed services. What are your thoughts on this? I feel like that introducing a whole new interface is a pretty big deal considering everyone "knows" you only need IServiceProvider for all things DI, and now that won't be true anymore.

Better method names

One last suggestion would be to rename the AddKeyedTransient() (and others) to AddTransientKeyed(). This will benefit intellisense in IDE's and also make it easier to discover this new API.

@benjaminpetit
Copy link
Member

@sander1095 we didn't want to put the new method on IServiceProvider to not break backward compatibility. Note that all extensions methods are made with IServiceProvider, the user doesn't have to use IKeyedServiceProvider directly.

@deinok
Copy link

deinok commented Aug 2, 2023

@commonsensesoftware What about using the Generic Attributes ? I mean, the feature looks pretty coupled to the use of strings as keys.

As an example:

public static IServiceCollection AddKeyedScoped<TService, TKey>(this IServiceCollection services, TKey serviceKey, Func<IServiceProvider, TKey, TService> implementationFactory) where TService : class;

public class FromKeyedServicesAttribute<TKey> : Attribute
{
    public FromKeyedServicesAttribute(TKey key);
    public TKey Key { get; }
}

@deeprobin
Copy link
Contributor

@deinok

I think that's a good idea in principle.
It would probably make sense to create a new API proposal for it, since this one is already closed.

@davidfowl
Copy link
Member

The service key is an object, it can be anything. The generic overload is syntax sugar for keys of a certain type. As for the generic attribute, what would you imagine would go in there besides types and constant strings? C# doesn't support any values declared as attribute values.

@commonsensesoftware
Copy link
Author

@deinok While you can use an attribute, generic or otherwise, it requires inspecting the attribute and allows breaking the contract. For example:

public class Bar
{
    private readonly IFoo foo;
    public Bar([FromKeyedServices(typeof(IPityTheFoo))] IFoo foo) => this.foo = foo;
}

Allows passing any IFoo. There is also no guarantee that IPityTheFoo exists. It is possible for a DI framework to impose validation if it knows to look for it, but it is possible to construct Bar in a way not intended by the author; certainly not the way the self-describing way it was written.

The counter-argument that I proposed requires using the Type in code and allowing any implementor to choose to use the Type, string, enumeration, or any other choice for the actual key. For example:

public class Bar
{
    private readonly IFoo foo;
    public Bar(Keyed<IPityTheFoo, IFoo> foo) => this.foo = foo.value;
}

The main difference is formalizing the type such that you must pass a keyed type and it's explicitly expressed. No attribute required. It also meant no change to IServiceProvider or ServiceDescriptor. You'd ultimately have serviceProvider.GetService(typeof(Keyed<IPityTheFoo, IFoo>)), which can be further simplified and dressed up with extension methods.

Despite showing working implementations, my proposal was rejected. I guess people didn't like the way it looked 🤔. Using an attribute is just as verbose, if not more so, and lacks the benefits of explicitly specifying the parameter with a key.

Regardless, we're getting keyed service support - finally. It may not have been what I would have done, but we now have something to work with. 😄

@deinok
Copy link

deinok commented Aug 3, 2023

@commonsensesoftware I prefer your version insted of using object as a generic and attributes. I feel this API a little bit weird

@xakep139
Copy link
Contributor

xakep139 commented Aug 4, 2023

Maybe this is not the best place to ask, but we're trying to onboard to this new API in dotnet/extensions#4224 and didn't find a good way of having a chain of keyed dependencies to be easily registered/resolved.
Consider this hierarchy: MyRootType -> IDep1 -> IDep2 -> ..., so that each one resolves the next dependency using the same service key. We can't use [FromKeyedServices(...)] however, because serviceKey is unknown at compile-time.

We don't want to use service locator antipattern, i.e.:

public MyRootType(
    IServiceProvider serviceProvider,
    [ServiceKey] string? serviceKey = null)
{
  var myDep1 = serviceProvider.GetRequiredKeyedService<IDep1>(serviceKey);
  // ...
}

Are we missing something here and there's a better way of doing that?

cc @klauco @geeknoid

@vukovinski
Copy link

Just a suggestion, and it might not be best suited,

but, @xakep139 couldn't you subclass the FromKeyedServices attribute to somehow reflect on the service key of the root?

@rjgotten
Copy link

rjgotten commented Aug 4, 2023

@xakep139 Unity used to have something where you could register a type TService to be resolved and could instruct its resolvers that for one or more TDependency at any level in the hierarchy of dependencies down, it should substitute a preconfigured instance or type.

But something like that is incredibly brittle.
Modified Peter Parker Principle applies: "with great power; come great f--k ups."

@xakep139
Copy link
Contributor

xakep139 commented Aug 4, 2023

Unfortunately, we can't rely on Unity or any other 3P IoC container.
Hopefully this use-case have been discussed during design reviews and there's a clear guidance.

@halter73
Copy link
Member

halter73 commented Aug 4, 2023

We don't want to use service locator antipattern

Given your scenario, I think the service locator pattern your only real option. I don't think it's that bad. I do try to avoid it when possible for easier reviewing, testing and so forth, but sometimes there's no choice.

It might be interesting if someone made an API proposal for a [FromCurrentKeyedServices] or something like that for a future version of .NET. Of course there's a challenge on how you would deal with containers that support the base set of keyed service features in .NET 8, but not the new [FromCurrentKeyedServices] or similar attribute.

@ENikS
Copy link
Contributor

ENikS commented Aug 4, 2023

@xakep139 Unity used to have something where you could register a type TService to be resolved and could instruct its resolvers that for one or more TDependency at any level in the hierarchy of dependencies down, it should substitute a preconfigured instance or type.

But something like that is incredibly brittle. Modified Peter Parker Principle applies: "with great power; come great f--k ups."

@rjgotten why used to have?
This feature is still available and works for any TService dependency or only dependency on the specified type.
As for being incredibly brittle, I totally disagree, it works every single time, as designed.

@rjgotten
Copy link

rjgotten commented Aug 4, 2023

@ENikS
Because with all due respect to your tenure and stewardship on Unity - it's become outmoded and a has-been.

As for being incredibly brittle, I totally disagree, it works every single time, as designed.

If you are resolving a nested dependency through such a custom resolver which happens to be singleton, it can make the singleton take the override type rather than the normal type. It makes the result of type resolution non-deterministic and dependent on actual resolve order. First build-up chain wins.

I experienced that problem myself a few times trying to use it for registering decorator patterns.
Then I stopped using it and wrote a dedicated container-agnostic implementation for it at the IServiceCollection level with factory resolvers and ActivatorUtilities.CreateFactory. It's slower, but it's stable.

This is why I brought up Unity's thing in the first place. As a warning that this type of pattern where you attempt to push specialized context down the resolve chain, is generally a bad idea. Unless you make it 100% bulletproof and guarantee that everything resolved along that chain is isolated from the norm. Which may have yet again its own unforeseen consequences such as singletons suddenly becoming multiton.

@voroninp
Copy link

I maybe too late, but can we have generic version of IKeyedServiceProvider?

@commonsensesoftware
Copy link
Author

For anyone still following this thread and was interested in the original proposal, I've taken the liberty to fork the original POC repo and created an official Keyed Services repo. I've published a NuGet package for each of the 9 well-known containers that require (albeit minor) adapter changes. Since the proposal wasn't accepted, you'll still need one package for keyed services and possibly a second if your container also required adapter changes.

There's no need to wait for .NET 8. This will work today over the existing APIs and containers without IKeyedServiceProvider, FromKeyedServicesAttribute, or changes to ServiceDescriptor. I've published TFMs for net6.0 and net7.0 in case you're LTS bound. There's the added bonus that all DI registrations that go through the IServiceCollection are transparently portable to all other supported container implementations. This won't be true of IKeyedServiceProvider unfortunately. As an example, if your current container supports any object as a key, it may not work in a container that only accepts string key if you didn't happen to use a string. Perhaps there is a known or preconceived notion that people don't/won't switch container frameworks. I might have said the same thing about mocking frameworks - until this week 😬.

The updated README.md has links to all of the packages as well as links to example projects for each container. As you may notice, the setup for each container is virtually identical regardless of the backing type used by the container. I've released the first round under preview.1 in case there may be some additional thoughts or feedback. I'm curious what people think about it in practice - love it or hate it. Regardless, you now have something you can pull into your own experiments to play with, but without the need to clone, fork, or build. Enjoy!

@ghost ghost locked as resolved and limited conversation to collaborators Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-DependencyInjection blocking Marks issues that we want to fast track in order to unblock other important work
Projects
None yet
Development

No branches or pull requests