Skip to content

Commit

Permalink
Do not allow a non-keyed service to be injected to a keyed parameter (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
steveharter authored Aug 7, 2024
1 parent 51582ae commit 208c40b
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
{
Expand Down Expand Up @@ -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;

Expand All @@ -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,
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand All @@ -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))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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[]
Expand Down

0 comments on commit 208c40b

Please sign in to comment.