Skip to content
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

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

Merged
merged 2 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above tests were not failing before this PR, but this test and the other new test in "ServiceProviderContainerTests.cs" were.

{
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
Loading