Add support for IKeyedServiceProvider. #18

DominicUllmann · 2025-12-24T14:45:05Z

Tried to implement the keyed service provider support for >= .NET 8.0 without breaking < .NET 8.0

Sadly, there is one complication in this with resultion of IEnumerable. Ninject sadly doesn't consider metadata constraint when resolving an IEnumerable. This seems to be a potential bug in Ninject.

The following code snippet shows what Microsoft DI expects:

		var builder = WebApplication.CreateBuilder();
		builder.Services.AddKeyedTransient<IWarrior, Samurai>("Warrior");
		builder.Services.AddKeyedSingleton<IWarrior>("Warrior", new Ninja("test"));
		
		var app = builder.Build();
		var list = app.Services.GetKeyedServices(typeof(IWarrior),"Warrior") as IList<IWarrior>;
		list.Should().HaveCount(2);
		var list2 = app.Services.GetServices(typeof(IWarrior)) as IList<IWarrior>;
		var list3 = app.Services.GetRequiredService(typeof(IEnumerable<IWarrior>)) as IEnumerable<IWarrior>;
		list2.Should().HaveCount(0);
		list3.Should().HaveCount(0);

We can't resolve all the elements without considering the service key. The Microsoft DI always applies the constraint.

The PR should be extended with conversion to Array and List as well, that's not yet done. The current state is for disussion.

DominicUllmann · 2025-12-24T14:48:55Z

src/Ninject.Web.AspNetCore/NinjectServiceProvider.cs

 		{
-			var result = _resolutionRoot.Get(serviceType);
-			return result;
+			if (!IsListType(serviceType, out var elementType))


It is not nice that we have to distinguish here between request for IEnumerable and just T. Sadly, this is required due to a potential bug in Ninject.
I wonder if KernelBase.UpdateRequest should not pass the constraint instead of just ignoring it in case of requesting all of a certain type.

Fun fact: Explicitly binding an IList<T> and then requesting exactly that IList<T> does not actually resolve that binding.

var kernel = new StandardKernel(); kernel.Bind<IWarrior>().To<Ninja>().WithConstructorArgument("name", "Nick"); kernel.Bind<IList<IWarrior>>().ToConstant(new List<IWarrior> { new Samurai() }); var warriors = kernel.Get<IList<IWarrior>>(); warriors.Count().Should().Be(1); warriors.First().Should().BeOfType<Samurai>(); // FAIL: it's actually a Ninja

The code in Ninject.KernelBase.Resolve always treats collection type services as a special type of request. Constraints are not applied to collections, but they are also not applied to the collection items since as you mentioned the UpdateRequest just ignores them and passes null instead. If it were applied to collections, then I would call that a design decision, but since they are not applied to either of them, I'd call it a bug.

DominicUllmann · 2025-12-24T14:49:51Z

src/Ninject.Web.AspNetCore/NinjectServiceProvider.cs

+
+		private static object ConvertToTypedEnumerable(Type elementType, IEnumerable<object> objectList)
+		{
+			var castMethod = EnumerableCastMethod.MakeGenericMethod(elementType);


This is similar to how Ninject does it, but I can't reuse the Ninject version directly as it is internal. Here we should also handle the case if an array or list was requested similar to how Ninject does it.

DominicUllmann · 2025-12-24T14:52:36Z

src/Ninject.Web.AspNetCore/KeyedDescriptorAdapter.cs

+	/// <summary>
+	/// This ServiceDescriptorAdapter is used when ServiceDescriptor.IsKeyedService == true
+	/// </summary>
+	public class KeyedDescriptorAdapter : IDescriptorAdapter


Added this one to prevent code duplications in ServiceCollectionAdapter. MS decided to use Keyed* properties on the service descriptor for keyed services.
Note that the implementation factory receives the ServiceKey => that's maybe one reason for the different set of properties for keyed services.

…oject - This is currently failing 45 out of the 54 keyed service specification tests

lord-executor · 2025-12-26T09:58:31Z

Not sure if you saw that, but there is actually a separate set of "specification tests" in the "Microsoft.Extensions.DependencyInjection.Specification" namespace just for testing keyed services. And of course, a lot of them revolve around generics which is why currently 45 out of 54 of these services fail. I think if we're trying to support keyed services, the result should somehow get all of these tests to green. This is why I abandoned that side-project when keyed services first arrived on the scene. I could not figure out a way to satisfy all of those requirements without essentially building two different Kernels, one for keyed services and one for unkeyed services.

lord-executor · 2025-12-26T10:02:40Z

src/Ninject.Web.AspNetCore/NinjectServiceProvider.cs

+					return true;
+				}
+
+				if (genericTypeDefinition == typeof(IEnumerable<>))


Hmm... why is IEnumerable<> on its own while the other list-ish types are grouped together above?

…ither filtering for keyed or non-keyed

DominicUllmann · 2025-12-26T12:01:30Z

Not sure if you saw that, but there is actually a separate set of "specification tests" in the "Microsoft.Extensions.DependencyInjection.Specification" namespace just for testing keyed services. And of course, a lot of them revolve around generics which is why currently 45 out of 54 of these services fail. I think if we're trying to support keyed services, the result should somehow get all of these tests to green. This is why I abandoned that side-project when keyed services first arrived on the scene. I could not figure out a way to satisfy all of those requirements without essentially building two different Kernels, one for keyed services and one for unkeyed services.

Thanks a lot, I was wondering about that. Thanks for having added these. I have discovered a few features I have not implemented yet and also some implementation problems.
But there are really strange requirements, like e.g. ResolveKeyedServiceSingletonInstanceWithAnyKey or ResolveWithAnyKeyQuery_Constructor.
I will have to check if I can support these.

lord-executor · 2025-12-26T13:27:01Z

Thanks a lot, I was wondering about that. Thanks for having added these. I have discovered a few features I have not implemented yet and also some implementation problems. But there are really strange requirements, like e.g. ResolveKeyedServiceSingletonInstanceWithAnyKey or ResolveWithAnyKeyQuery_Constructor. I will have to check if I can support these.

I definitely didn't remember all of them, but I do remember that this was the best example I have ever seen that an interface on its own is just a horribly insufficient form of specification 😬. It appears that they simply made some design decisions for what would work best with their particular implementation and that then became the implicit specification.

lord-executor · 2025-12-26T13:30:41Z

This is also definitely interesting...

public class Service : IService
{
    private readonly string _id;

    public Service() => _id = Guid.NewGuid().ToString();

    public Service([ServiceKey] string id) => _id = id; // oh yeah, btw: the ServiceKey can be injected into the service

    public override string? ToString() => _id;
}

…binding happens; ensure that ConstructorScorer doesn't discard a constructor with ServiceKey attribute in favour of another constructor

DominicUllmann · 2025-12-28T15:00:36Z

src/Ninject.Web.AspNetCore/BindingIndex.cs

+		/// This ensures that a binding with a different servicekey
+		/// can't override a binding with a non-matching servicekey
+		/// </summary>
+		public class ServiceTypeKey : IEquatable<ServiceTypeKey>


We have to split the binidngindex by servicekey as otherwise the overriding doesn't work correctly.

DominicUllmann · 2025-12-28T15:01:16Z

src/Ninject.Web.AspNetCore/Components/ConstructorReflectionStrategyWithKeyedSupport.cs

+            {
+                var hasInjectAttribute = constructor.HasAttribute(Settings.InjectAttribute);
+                var hasObsoleteAttribute = constructor.HasAttribute(typeof(ObsoleteAttribute));
+                var directive = new ConstructorInjectionDirectiveWithKeyedSupport(constructor, InjectorFactory.Create(constructor))


We have to create a ConstructorInjectionDirectiveWithKeyedSupport instead of a ConstructorDirective. Therefore we have to use an own implementation here.

DominicUllmann · 2025-12-28T15:02:40Z

src/Ninject.Web.AspNetCore/Planning/ConstructorInjectionDirectiveWithKeyedSupport.cs

+		protected override ITarget[] CreateTargetsFromParameters(ConstructorInfo method)
+		{
+			return method.GetParameters().
+				Select((Func<ParameterInfo, ParameterTarget>) (parameter => new ParameterTargetWithKeyedSupport(method, parameter))).


Important here is to create a ParameterTargetWithKeyedSupport to support FromKeyedService as well as ServiceKey attribute.

DominicUllmann · 2025-12-28T15:09:54Z

src/Ninject.Web.AspNetCore/AspNetCoreKernel.cs

 			return binding => {
 				var latest = true;
-				if (request.IsUnique && request.Constraint == null)
+				if (request.IsUnique)


As we have now always constraints (either to exclude keyed bindings or only consider keyed bindings, we can't check for request.Constraint == null. But this should not be a big issue as Microsoft.Extension.DI bindings can't provide custom constraints. For normal ninject bindings we don't have BindingIndex, so we can always apply them here in my opinion. Do you see any reason why we have request.Constraint == null here in the first place @lord-executor ? All the existing tests still work when removing it.
Current state is that 20 of the new tests fail...

… instead of the servicekey bound

DominicUllmann added 6 commits December 24, 2025 12:57

initial implementation of optional keyed services

6ed4084

add support for keyed required service

439a94a

add test for keyedimplementationfactory as well

dd9919e

refactor to remove code duplication in ServiceCollectionAdapter

f30ada7

ensure that transient factory works as well correct with keyed service

6521615

align with ienumerable resolution in the Microsoft DI world

e9fadc6

DominicUllmann requested a review from lord-executor December 24, 2025 14:45

simplify code

85902e7

DominicUllmann commented Dec 24, 2025

View reviewed changes

DominicUllmann mentioned this pull request Dec 24, 2025

Implement new .NET 8 IKeyedServiceProvider interface #11

Open

Added the keyed service specification tests to the compliance test pr…

20ceda7

…oject - This is currently failing 45 out of the 54 keyed service specification tests

lord-executor reviewed Dec 26, 2025

View reviewed changes

DominicUllmann added 3 commits December 26, 2025 12:31

start to fix compliance tests

b03a743

add initial support for KeyedService.AnyKey

5919021

remove check for no contraint. We have always a constraint now with e…

a11c483

…ither filtering for keyed or non-keyed

DominicUllmann added 3 commits December 28, 2025 14:56

add support for ServiceKeyAttribute and FromKeyedService

09ef1f9

remove SelfBIndingResolver as one tests check that no automatic self …

baf79ef

…binding happens; ensure that ConstructorScorer doesn't discard a constructor with ServiceKey attribute in favour of another constructor

fix binding overriding with keyed services

13f268b

DominicUllmann commented Dec 28, 2025

View reviewed changes

DominicUllmann added 2 commits December 28, 2025 17:13

handle special cases with AnyKey as well as with null key

61c9c5a

ensure that we only match non-keyed if ServiceLookupMode is NullKey

cb44ae5

DominicUllmann added 3 commits December 28, 2025 19:27

introduce runtime parameter so that we can inject the servicekey used…

e6fe5d0

… instead of the servicekey bound

fix open generics resolution

32d066f

fix exception when trying unsupported case for servicekey

ae84882

Add support for IKeyedServiceProvider. #18

Are you sure you want to change the base?

Add support for IKeyedServiceProvider. #18

Uh oh!

Conversation

DominicUllmann commented Dec 24, 2025

Uh oh!

DominicUllmann Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

lord-executor Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

DominicUllmann Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DominicUllmann Dec 24, 2025

Choose a reason for hiding this comment

Uh oh!

lord-executor commented Dec 26, 2025

Uh oh!

lord-executor Dec 26, 2025

Choose a reason for hiding this comment

Uh oh!

DominicUllmann commented Dec 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lord-executor commented Dec 26, 2025

Uh oh!

lord-executor commented Dec 26, 2025

Uh oh!

DominicUllmann Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

DominicUllmann Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

DominicUllmann Dec 28, 2025

Choose a reason for hiding this comment

Uh oh!

DominicUllmann Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

DominicUllmann Dec 24, 2025 •

edited

Loading

DominicUllmann commented Dec 26, 2025 •

edited

Loading

DominicUllmann Dec 28, 2025 •

edited

Loading