-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Comments
Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection Issue DetailsBackground and motivationI'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 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 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 ProposalThe 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 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:
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 The following extension methods would be added to 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 To abstract the container and mapping from the implementation, public string? Key { get; set; } The aforementioned extension methods are static and cannot have their implementations changed in the future. To ensure that public interface IKeyedTypeFactory
{
Type Create(Type type, string key);
}
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 API UsageWhat 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 DesignsThe This proposal does not mandate that Risks
|
Is it worth considering a model where a keyed registration plays out as a registration of a keyed service resolver type (similar to In that world, a service keyed T could be retrieved by first resolving a 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 |
@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 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()}); |
There are alternatives, though. One is to use one of the func-based methods like Another approach is to inject a key-based service lookup type instead of injecting the type itself. For example:
Or even:
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. |
I 👀 now. Yes, an attribute could be restrictive in that way. Using a 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 I didn't mean to steal your thunder. I guess this is effectively the same thing as whatever |
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 Pros
Cons
|
@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 |
@madelson I think we are largely on the same page. I don't like the idea of magic strings either (at all).
I'm trying to think of a scenario where I completely agree that consumers would want to unwrap the dependency. Where and how is at their discretion. It is anologous to I decided I wanted to flush the idea of using
The revised contract would be: public interface IDependency<in TKey, out TService>
where TKey : new()
where TService : notnull
{
TService Value { get; }
} Enforcing the The extension methods now simply revolve around 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;
} ExamplesHere 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, 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 The attached Keyed Service - Iteration 2 contains an end-to-end working example using only |
I like this direction. One thought:
I'd vote for nixing the
|
@madelson I'm onboard with that. There's no strict reason why the 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 public interface IDependency<in TKey, out TService> : IDependency where TService : notnull
{
new TService Value { get; }
} I don't really like that the The only other open item I have (so far) is how
This behavior could be done with a single registration, but the container implementer would have to know and understand that The other question is whether 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? |
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 |
@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. |
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 😄 |
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());
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:
Here's what I mean about the 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. |
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 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? |
Yeah, I'll second this. The conforming container thing has been a sore spot since inception. |
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 |
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 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. 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 |
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 @tillig, in the revised proposal, Autofact just works without a single change 😉. To answer your specific questions:
@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 |
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 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 |
Actually, you do not end up with a 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);
}
I'm not entirely sure how The idea of 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 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. 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:
With that in mind, the LightInject adapter would define two native // 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
Even if that might be what happens behind the scenes. |
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 @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! |
@steveharter @benjaminpetit with #87183 merged, can this be closed? |
Yup, closing. |
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:
Comparison to the Options PatternWhen I read the docs about the Options Pattern, you can choose to use This proposal adds different methods ( New interfaceI see there is a new interface to retrieve keyed services from. I don't like this approach for the following reasons: Right now, In the proposal it's established that 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 :) |
@sander1095 Extending |
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 overloadInstead of adding a new overload, could we not insert these new 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 Better method namesOne last suggestion would be to rename the |
@sander1095 we didn't want to put the new method on |
@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; }
} |
I think that's a good idea in principle. |
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. |
@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 The counter-argument that I proposed requires using the 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 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. 😄 |
@commonsensesoftware I prefer your version insted of using |
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. 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? |
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? |
@xakep139 Unity used to have something where you could register a type But something like that is incredibly brittle. |
Unfortunately, we can't rely on Unity or any other 3P IoC container. |
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 |
@rjgotten why |
@ENikS
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. 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. |
I maybe too late, but can we have generic version of |
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 The updated |
Thanks @commonsensesoftware for the original proposal. I edited this post to show the current proposition.
Original proposal from @commonsensesoftware
### Background and MotivationI'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 (ex: via
Func<string, T>
). The following proposal would add support for keyed services to the existingMicrosoft.Extensions.DependencyInjection.*
libraries without breaking theIServiceProvider
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 usingType
as a composite key. This design provides the following advantages:ISupportRequiredService
,ISupportKeyedService
)Resolving Services
To resolve a keyed dependency we'll define the following contracts:
The following extension methods will be added to
ServiceProviderServiceExtensions
:Here is a partial example of how it would be implemented:
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 theType
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 thatType
will be used as a key for service resolution in all container implementations.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:
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:
There is a minor drawback of requiring two registrations per keyed service in the container, but resolution for consumers is succintly:
The following extension methods will be added to
ServiceCollectionDescriptorExtensions
to provide common registration throughIServiceCollection
for all container frameworks:API Usage
Putting it all together, here's how the API can be leveraged for any container framework that supports registration through
IServiceCollection
.Container Integration
The following is a summary of results from Keyed Service POC repo.
(Generic)
By Key
Key (Generic)
Generics
Instance
Factory
Works
Changes
Changes
[1]: Only Implementation Factory doesn't work out-of-the-box
IServiceCollection
Risks
Alternate Proposals (TL;DR)
The remaining sections outline variations alternate designs that were rejected, but were retained for historical purposes.
Previous Code Iterations
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 astring
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
:This attribute could be used in the following ways:
Using an attribute has to main advantages:
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 theType
, but the key maps to different implementations. This also means thatIServiceProvider.GetService(Type type)
can support a key without breaking its contract.The following extension methods would be added to
ServiceProviderServiceExtensions
:It is not required for this proposal to work, but as an optimization, it may be worth adding:
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: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 addedto
Microsoft.Extensions.DependencyInjection.Abstractions
:Microsoft.Extensions.DependencyInjection
will provide a default implementation that leveragesCustomReflectionContext
.The implementation might look like the following:
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:
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 toAttributeTargets.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 ofType
alone as input.Risks
Microsoft.Extensions.DependencyInjection
would require one of the following:System.Reflection.Context
(unless another solution is found)System.Reflection.Context
and adds the keyed service capabilityIKeyedServiceProvider
and/orIKeyedTypeFactory
intefacesAPI 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 thatEquals
andGetHashCode
have a proper implementation.Service registration
ServiceDescriptor
will be modified to include theServiceKey
.KeyedImplementationInstance
,KeyedImplementationType
andKeyedImplementationFactory
will be added, matching their non-keyed equivalent.When accessing a non-keyed property (like
ImplementationInstance
) on a keyedServiceDescriptor
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.ServiceKey
will staynull
in non-keyed services.Extension methods for
IServiceCollection
are added to support keyed services: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
:Resolving service
Basic keyed resolution
Two new optional interfaces will be introduced:
This new interface will be accessible via the following extension methods:
These methods will throw an
InvalidOperationException
if the provider doesn't supportISupportKeyedService
.Resolving services via attributes
We introduce two attributes:
ServiceKeyAttribute
andFromKeyedServicesAttribute
.ServiceKeyAttribute
ServiceKeyAttribute
is used to inject the key that was used for registration/resolution in the constructor: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:
Open generics
Open generics are supported:
Enumeration
This kind of enumeration is possible:
Note that enumeration will not mix keyed and non keyed registrations:
But we do not support:
The text was updated successfully, but these errors were encountered: