Do not allow a non-keyed service to be injected to a keyed parameter (#105839)

Author: steveharter

src/libraries/Microsoft.Extensions.DependencyInjection.Specification.Tests/src/KeyedDependencyInjectionSpecificationTests.cs

@@ -11,7 +11,7 @@ namespace Microsoft.Extensions.DependencyInjection.Specification
 {
     public abstract partial class KeyedDependencyInjectionSpecificationTests
     {
-        protected abstract  IServiceProvider CreateServiceProvider(IServiceCollection collection);
+        protected abstract IServiceProvider CreateServiceProvider(IServiceCollection collection);
 
         [Fact]
         public void ResolveKeyedService()
@@ -263,6 +263,70 @@ public void ResolveKeyedServiceSingletonInstanceWithKeyedParameter()
             Assert.Equal("service2", svc.Service2.ToString());
         }
 
+        [Fact]
+        public void ResolveKeyedServiceWithKeyedParameter_MissingRegistration_SecondParameter()
+        {
+            var serviceCollection = new ServiceCollection();
+
+            serviceCollection.AddKeyedSingleton<IService, Service>("service1");
+            // We are missing the registration for "service2" here and OtherService requires it.
+
+            serviceCollection.AddSingleton<OtherService>();
+
+            var provider = CreateServiceProvider(serviceCollection);
+
+            Assert.Null(provider.GetService<IService>());
+            Assert.Throws<InvalidOperationException>(() => provider.GetService<OtherService>());
+        }
+
+        [Fact]
+        public void ResolveKeyedServiceWithKeyedParameter_MissingRegistration_FirstParameter()
+        {
+            var serviceCollection = new ServiceCollection();
+
+            // We are not registering "service1" and "service1" keyed IService services and OtherService requires them.
+
+            serviceCollection.AddSingleton<OtherService>();
+
+            var provider = CreateServiceProvider(serviceCollection);
+
+            Assert.Null(provider.GetService<IService>());
+            Assert.Throws<InvalidOperationException>(() => provider.GetService<OtherService>());
+        }
+
+        [Fact]
+        public void ResolveKeyedServiceWithKeyedParameter_MissingRegistrationButWithDefaults()
+        {
+            var serviceCollection = new ServiceCollection();
+
+            // We are not registering "service1" and "service1" keyed IService services and OtherServiceWithDefaultCtorArgs
+            // specifies them but has argument defaults if missing.
+
+            serviceCollection.AddSingleton<OtherServiceWithDefaultCtorArgs>();
+
+            var provider = CreateServiceProvider(serviceCollection);
+
+            Assert.Null(provider.GetService<IService>());
+            Assert.NotNull(provider.GetService<OtherServiceWithDefaultCtorArgs>());
+        }
+
+        [Fact]
+        public void ResolveKeyedServiceWithKeyedParameter_MissingRegistrationButWithUnkeyedService()
+        {
+            var serviceCollection = new ServiceCollection();
+
+            // We are not registering "service1" and "service1" keyed IService services and OtherService requires them,
+            // but we are registering an unkeyed IService service which should not be injected into OtherService.
+            serviceCollection.AddSingleton<IService, Service>();
+
+            serviceCollection.AddSingleton<OtherService>();
+
+            var provider = CreateServiceProvider(serviceCollection);
+
+            Assert.NotNull(provider.GetService<IService>());
+            Assert.Throws<InvalidOperationException>(() => provider.GetService<OtherService>());
+        }
+
         [Fact]
         public void CreateServiceWithKeyedParameter()
         {
@@ -490,9 +554,9 @@ public void ResolveKeyedTransientFromScopeServiceProvider()
             Assert.NotSame(serviceA1, serviceB1);
         }
 
-        internal interface IService { }
+        public interface IService { }
 
-        internal class Service : IService
+        public class Service : IService
         {
             private readonly string _id;
 
@@ -503,7 +567,7 @@ internal class Service : IService
             public override string? ToString() => _id;
         }
 
-        internal class OtherService
+        public class OtherService
         {
             public OtherService(
                 [FromKeyedServices("service1")] IService service1,
@@ -518,6 +582,36 @@ public OtherService(
             public IService Service2 { get; }
         }
 
+        internal class OtherServiceWithDefaultCtorArgs
+        {
+            public OtherServiceWithDefaultCtorArgs(
+                [FromKeyedServices("service1")] IService service1 = null,
+                [FromKeyedServices("service2")] IService service2 = null)
+            {
+                Service1 = service1;
+                Service2 = service2;
+            }
+
+            public IService Service1 { get; }
+
+            public IService Service2 { get; }
+        }
+
+        internal class ServiceWithOtherService
+        {
+            public ServiceWithOtherService(
+                [FromKeyedServices("service1")] IService service1,
+                [FromKeyedServices("service2")] IService service2)
+            {
+                Service1 = service1;
+                Service2 = service2;
+            }
+
+            public IService Service1 { get; }
+
+            public IService Service2 { get; }
+        }
+
         internal class ServiceWithIntKey : IService
         {
             private readonly int _id;

src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteFactory.cs

@@ -574,6 +574,7 @@ private ConstructorCallSite CreateConstructorCallSite(
             for (int index = 0; index < parameters.Length; index++)
             {
                 ServiceCallSite? callSite = null;
+                bool isKeyedParameter = false;
                 Type parameterType = parameters[index].ParameterType;
                 foreach (var attribute in parameters[index].GetCustomAttributes(true))
                 {
@@ -591,11 +592,15 @@ private ConstructorCallSite CreateConstructorCallSite(
                     {
                         var parameterSvcId = new ServiceIdentifier(keyed.Key, parameterType);
                         callSite = GetCallSite(parameterSvcId, callSiteChain);
+                        isKeyedParameter = true;
                         break;
                     }
                 }
 
-                callSite ??= GetCallSite(ServiceIdentifier.FromServiceType(parameterType), callSiteChain);
+                if (!isKeyedParameter)
+                {
+                    callSite ??= GetCallSite(ServiceIdentifier.FromServiceType(parameterType), callSiteChain);
+                }
 
                 if (callSite == null && ParameterDefaultValue.TryGetDefaultValue(parameters[index], out object? defaultValue))
                 {

src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs

@@ -1290,6 +1290,28 @@ public void ScopedServiceResolvedFromSingletonAfterCompilation3()
             Assert.Same(sp.GetRequiredService<IFakeOpenGenericService<Aa>>().Value.PropertyA, sp.GetRequiredService<A>());
         }
 
+        [Fact]
+        public void ResolveKeyedServiceWithKeyedParameter_MissingRegistrationButWithUnkeyedService()
+        {
+            var serviceCollection = new ServiceCollection();
+
+            // We are not registering "service1" and "service1" keyed IService services and OtherService requires them,
+            // but we are registering an unkeyed IService service which should not be injected into OtherService.
+            serviceCollection.AddSingleton<KeyedDependencyInjectionSpecificationTests.IService, KeyedDependencyInjectionSpecificationTests.Service>();
+
+            serviceCollection.AddSingleton<KeyedDependencyInjectionSpecificationTests.OtherService>();
+
+            AggregateException ex = Assert.Throws<AggregateException>(() => serviceCollection.BuildServiceProvider(new ServiceProviderOptions
+            {
+                ValidateOnBuild = true
+            }));
+
+            Assert.Equal(1, ex.InnerExceptions.Count);
+            Assert.StartsWith("Some services are not able to be constructed", ex.Message);
+            Assert.Contains("ServiceType: Microsoft.Extensions.DependencyInjection.Specification.KeyedDependencyInjectionSpecificationTests+OtherService", ex.ToString());
+            Assert.Contains("Microsoft.Extensions.DependencyInjection.Specification.KeyedDependencyInjectionSpecificationTests+IService", ex.ToString());
+        }
+
         private async Task<bool> ResolveUniqueServicesConcurrently()
         {
             var types = new Type[]