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.

Sign up for GitHub

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

ActivatorUtilities.CreateInstance() should respect [ActivatorUtilitiesConstructor] #99175

Merged
merged 4 commits into from
Mar 6, 2024
Merged

ActivatorUtilities.CreateInstance() should respect [ActivatorUtilitiesConstructor] #99175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
30be3c2
ActivatorUtilities.CreateInstance() should respect [ActivatorUtilitie…
steveharter Mar 1, 2024
1d3d47e
Have ctor with [ActivatorUtilitiesConstructor] always be selected
steveharter Mar 5, 2024
5b3d4a5
Remove comment
steveharter Mar 5, 2024
189ec99
Merge branch 'main' into DiCtor
jkotas Mar 5, 2024
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
19 changes: 12 additions & 7 deletions ...libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,20 +103,25 @@ public static object CreateInstance(
{
ThrowMarkedCtorDoesNotTakeAllProvidedArguments();
}
}

if (isPreferred || bestLength < length)
{
seenPreferred = true;
bestLength = length;
bestMatcher = matcher;
multipleBestLengthFound = false;
}
else if (bestLength == length)
else if (!seenPreferred)
{
multipleBestLengthFound = true;
if (bestLength < length)
{
bestLength = length;
bestMatcher = matcher;
multipleBestLengthFound = false;
}
else if (bestLength == length)
{
multipleBestLengthFound = true;
}
}

seenPreferred |= isPreferred;
}

if (bestLength != -1)
Expand Down
192 changes: 192 additions & 0 deletions ...raries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,96 @@ public void CreateInstance_ClassWithABC_ConstructorWithAttribute_PicksCtorWithAt
Assert.Same(a, instance.A);
}

[Fact]
public void CreateInstanceFailsWithAmbiguousConstructor()
{
var serviceCollection = new ServiceCollection();
serviceCollection.AddTransient<ClassWithA_And_B>();
serviceCollection.AddTransient<A>();
serviceCollection.AddTransient<B>();

var serviceProvider = serviceCollection.BuildServiceProvider();

// Neither ctor(A) nor ctor(B) have [ActivatorUtilitiesConstructor].
Assert.Throws<InvalidOperationException>(() => ActivatorUtilities.CreateInstance<ClassWithA_And_B>(serviceProvider));
}

[Fact]
public void CreateInstanceFailsWithAmbiguousConstructor_ReversedOrder()
{
var serviceCollection = new ServiceCollection();
serviceCollection.AddTransient<ClassWithB_And_A>();
serviceCollection.AddTransient<A>();
serviceCollection.AddTransient<B>();

var serviceProvider = serviceCollection.BuildServiceProvider();

// Neither ctor(A) nor ctor(B) have [ActivatorUtilitiesConstructor].
Assert.Throws<InvalidOperationException>(() => ActivatorUtilities.CreateInstance<ClassWithA_And_B>(serviceProvider));
}

[Fact]
public void CreateInstancePassesWithAmbiguousConstructor()
steveharter marked this conversation as resolved.
Show resolved Hide resolved
{
var serviceCollection = new ServiceCollection();
serviceCollection.AddTransient<ClassWithA_And_B_ActivatorUtilitiesConstructorAttribute>();
serviceCollection.AddTransient<A>();
serviceCollection.AddTransient<B>();

var serviceProvider = serviceCollection.BuildServiceProvider();
var service = ActivatorUtilities.CreateInstance<ClassWithA_And_B_ActivatorUtilitiesConstructorAttribute>(serviceProvider);

// Ensure ctor(A) was selected over ctor(B) since A has [ActivatorUtilitiesConstructor].
Assert.NotNull(service.A);
}

[Fact]
public void CreateInstancePassesWithAmbiguousConstructor_ReversedOrder()
{
var serviceCollection = new ServiceCollection();
serviceCollection.AddTransient<ClassWithB_And_A_ActivatorUtilitiesConstructorAttribute>();
serviceCollection.AddTransient<A>();
serviceCollection.AddTransient<B>();

var serviceProvider = serviceCollection.BuildServiceProvider();
var service = ActivatorUtilities.CreateInstance<ClassWithB_And_A_ActivatorUtilitiesConstructorAttribute>(serviceProvider);

// Ensure ctor(A) was selected over ctor(B) since A has [ActivatorUtilitiesConstructor].
Assert.NotNull(service.A);
}

[Fact]
public void CreateInstanceIgnoresActivatorUtilitiesConstructorAttribute()
{
var serviceCollection = new ServiceCollection();
serviceCollection.AddTransient<ClassWithA_And_AB_ActivatorUtilitiesConstructorAttribute>();
serviceCollection.AddTransient<A>();
serviceCollection.AddTransient<B>();

var serviceProvider = serviceCollection.BuildServiceProvider();
var service = ActivatorUtilities.CreateInstance<ClassWithA_And_AB_ActivatorUtilitiesConstructorAttribute>(serviceProvider);

// Ensure ctor(A) was selected since A has [ActivatorUtilitiesConstructor].
Assert.NotNull(service.A);
Assert.Null(service.B);
}

[Fact]
public void CreateInstanceIgnoresActivatorUtilitiesConstructorAttribute_ReversedOrder()
{
var serviceCollection = new ServiceCollection();
serviceCollection.AddTransient<ClassWithAB_And_A_ActivatorUtilitiesConstructorAttribute>();
serviceCollection.AddTransient<A>();
serviceCollection.AddTransient<B>();

var serviceProvider = serviceCollection.BuildServiceProvider();
var service = ActivatorUtilities.CreateInstance<ClassWithAB_And_A_ActivatorUtilitiesConstructorAttribute>(serviceProvider);

// Ensure ctor(A) was selected since it has [ActivatorUtilitiesConstructor].
Assert.NotNull(service.A);
Assert.Null(service.B);
}

[Fact]
public void CreateInstance_ClassWithABC_MultipleCtorsWithSameLength_ThrowsAmbiguous()
{
Expand Down Expand Up @@ -662,6 +752,108 @@ public ClassWithABC_LastConstructorWithAttribute(B b, C c) : this(null, b, c) {
public ClassWithABC_LastConstructorWithAttribute(A a, B b, C c) : base(a, b, c) { }
}

internal class ClassWithA_And_B
{
public ClassWithA_And_B(A a)
{
A = a;
}

public ClassWithA_And_B(B b)
{
B = b;
}

public A A { get; }
public B B { get; }
}

internal class ClassWithB_And_A
{
public ClassWithB_And_A(A a)
{
A = a;
}

public ClassWithB_And_A(B b)
{
B = b;
}

public A A { get; }
public B B { get; }
}

internal class ClassWithA_And_B_ActivatorUtilitiesConstructorAttribute
{
[ActivatorUtilitiesConstructor]
public ClassWithA_And_B_ActivatorUtilitiesConstructorAttribute(A a)
{
A = a;
}

public ClassWithA_And_B_ActivatorUtilitiesConstructorAttribute(B b)
{
B = b;
}

public A A { get; }
public B B { get; }
}

internal class ClassWithB_And_A_ActivatorUtilitiesConstructorAttribute
{
public ClassWithB_And_A_ActivatorUtilitiesConstructorAttribute(B b)
{
B = b;
}

[ActivatorUtilitiesConstructor]
public ClassWithB_And_A_ActivatorUtilitiesConstructorAttribute(A a)
{
A = a;
}

public A A { get; }
public B B { get; }
}

internal class ClassWithA_And_AB_ActivatorUtilitiesConstructorAttribute
{
[ActivatorUtilitiesConstructor]
public ClassWithA_And_AB_ActivatorUtilitiesConstructorAttribute(A a)
{
A = a;
}

public ClassWithA_And_AB_ActivatorUtilitiesConstructorAttribute(A a, B b)
{
A = a;
B = b;
}

public A A { get; }
public B B { get; }
}

internal class ClassWithAB_And_A_ActivatorUtilitiesConstructorAttribute
{
public ClassWithAB_And_A_ActivatorUtilitiesConstructorAttribute(A a, B b)
{
A = a;
B = b;
}

[ActivatorUtilitiesConstructor]
public ClassWithAB_And_A_ActivatorUtilitiesConstructorAttribute(A a)
{
A = a;
}

public A A { get; }
public B B { get; }
}

internal class FakeServiceProvider : IServiceProvider
{
private IServiceProvider _inner;
Expand Down