From f5e99565f0518d38694f90c6b03146b15e8fa08e Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Thu, 1 Aug 2024 15:44:53 -0500 Subject: [PATCH 1/2] Do not allow a non-keyed service to be injected into a keyed parameter --- ...edDependencyInjectionSpecificationTests.cs | 102 +++++++++++++++++- .../src/ServiceLookup/CallSiteFactory.cs | 7 +- .../DI.Tests/ServiceProviderContainerTests.cs | 21 ++++ 3 files changed, 125 insertions(+), 5 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Specification.Tests/src/KeyedDependencyInjectionSpecificationTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Specification.Tests/src/KeyedDependencyInjectionSpecificationTests.cs index 3664a3b81eafd4..8916a906d8572e 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Specification.Tests/src/KeyedDependencyInjectionSpecificationTests.cs +++ b/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("service1"); + // We are missing the registration for "service2" here and OtherService requires it. + + serviceCollection.AddSingleton(); + + var provider = CreateServiceProvider(serviceCollection); + + Assert.Null(provider.GetService()); + Assert.Throws(() => provider.GetService()); + } + + [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(); + + var provider = CreateServiceProvider(serviceCollection); + + Assert.Null(provider.GetService()); + Assert.Throws(() => provider.GetService()); + } + + [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(); + + var provider = CreateServiceProvider(serviceCollection); + + Assert.Null(provider.GetService()); + Assert.NotNull(provider.GetService()); + } + + [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(); + + serviceCollection.AddSingleton(); + + var provider = CreateServiceProvider(serviceCollection); + + Assert.NotNull(provider.GetService()); + Assert.Throws(() => provider.GetService()); + } + [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; diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteFactory.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteFactory.cs index f9b902dde19f3e..aec3f2c6745420 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteFactory.cs +++ b/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)) { diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs index 3e08a16db282e6..00becc9a282827 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs @@ -1290,6 +1290,27 @@ public void ScopedServiceResolvedFromSingletonAfterCompilation3() Assert.Same(sp.GetRequiredService>().Value.PropertyA, sp.GetRequiredService()); } + [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(); + + serviceCollection.AddSingleton(); + + AggregateException ex = Assert.Throws(() => serviceCollection.BuildServiceProvider(new ServiceProviderOptions + { + ValidateOnBuild = true + })); + + Assert.Equal(1, ex.InnerExceptions.Count); + Assert.Contains("ServiceType: Microsoft.Extensions.DependencyInjection.Specification.KeyedDependencyInjectionSpecificationTests+OtherService", ex.Message); + Assert.Contains("Microsoft.Extensions.DependencyInjection.Specification.KeyedDependencyInjectionSpecificationTests+IService", ex.InnerExceptions[0].Message); + } + private async Task ResolveUniqueServicesConcurrently() { var types = new Type[] From d87e6e47e72b0b24b9e072256fb6d1c9f8a3f876 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Wed, 7 Aug 2024 09:41:23 -0500 Subject: [PATCH 2/2] Fix test issue on NetFx --- .../tests/DI.Tests/ServiceProviderContainerTests.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs index 00becc9a282827..8bcaa9240f6399 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ServiceProviderContainerTests.cs @@ -1307,8 +1307,9 @@ public void ResolveKeyedServiceWithKeyedParameter_MissingRegistrationButWithUnke })); Assert.Equal(1, ex.InnerExceptions.Count); - Assert.Contains("ServiceType: Microsoft.Extensions.DependencyInjection.Specification.KeyedDependencyInjectionSpecificationTests+OtherService", ex.Message); - Assert.Contains("Microsoft.Extensions.DependencyInjection.Specification.KeyedDependencyInjectionSpecificationTests+IService", ex.InnerExceptions[0].Message); + 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 ResolveUniqueServicesConcurrently()