diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 81516568c14e64..c08ff9eb5085ea 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -31,49 +31,66 @@ public static object CreateInstance( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, params object[] parameters) { - int bestLength = -1; - bool seenPreferred = false; + string? message = $"A suitable constructor for type '{instanceType}' could not be located. Ensure the type is concrete and all parameters of a public constructor are either registered as services or passed as arguments. Also ensure no extraneous arguments are provided."; + if (instanceType.IsAbstract) + { + throw new InvalidOperationException(message); + } - ConstructorMatcher bestMatcher = default; + ConstructorInfo[] constructors = instanceType.GetConstructors(); + if (constructors.Length == 0) + { + throw new InvalidOperationException(message); + } - if (!instanceType.IsAbstract) + ConstructorInfo? preferredConstructor = null; + foreach (ConstructorInfo constructor in constructors) { - foreach (ConstructorInfo? constructor in instanceType.GetConstructors()) + if (!constructor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute), false)) { - var matcher = new ConstructorMatcher(constructor); - bool isPreferred = constructor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute), false); - int length = matcher.Match(parameters); - - if (isPreferred) - { - if (seenPreferred) - { - ThrowMultipleCtorsMarkedWithAttributeException(); - } - - if (length == -1) - { - ThrowMarkedCtorDoesNotTakeAllProvidedArguments(); - } - } - - if (isPreferred || bestLength < length) - { - bestLength = length; - bestMatcher = matcher; - } - - seenPreferred |= isPreferred; + continue; } + if (preferredConstructor is not null) + { + ThrowMultipleCtorsMarkedWithAttributeException(); + } + preferredConstructor = constructor; } - - if (bestLength == -1) + if (preferredConstructor is not null) { - string? message = $"A suitable constructor for type '{instanceType}' could not be located. Ensure the type is concrete and all parameters of a public constructor are either registered as services or passed as arguments. Also ensure no extraneous arguments are provided."; - throw new InvalidOperationException(message); + var matcher = new ConstructorMatcher(preferredConstructor); + matcher.Match(parameters); + if (matcher.MatchedLength == -1) + { + ThrowMarkedCtorDoesNotTakeAllProvidedArguments(); + } + return matcher.CreateInstance(provider, throwIfFailed: true)!; + } + var matchers = new ConstructorMatcher[constructors.Length]; + for (int i = 0; i < constructors.Length; i++) + { + ConstructorInfo constructor = constructors[i]; + var matcher = new ConstructorMatcher(constructor); + matcher.Match(parameters); + matchers[i] = matcher; } - return bestMatcher.CreateInstance(provider); + Array.Sort(matchers, (a, b) => + { + return a.MatchedLength == b.MatchedLength + ? b.ParametersCount - a.ParametersCount : b.MatchedLength - a.MatchedLength; + }); + + object? instance = null; + for (int i = 0; i < matchers.Length; i++) + { + ConstructorMatcher matcher = matchers[i]; + if (matcher.MatchedLength == -1) break; + bool lastChance = i == matchers.Length - 1; + instance = matcher.CreateInstance(provider, throwIfFailed: lastChance); + if (instance is not null) return instance; + } + throw new InvalidOperationException(message); } /// @@ -334,12 +351,21 @@ public ConstructorMatcher(ConstructorInfo constructor) _constructor = constructor; _parameters = _constructor.GetParameters(); _parameterValues = new object?[_parameters.Length]; + ParametersCount = _parameters.Length; } - public int Match(object[] givenParameters) + public int MatchedLength { get; private set; } = -1; + public int ParametersCount { get; } + + public void Match(object[] givenParameters) { + if (givenParameters.Length > _parameters.Length) + { + MatchedLength = -1; + return; + } int applyIndexStart = 0; - int applyExactLength = 0; + MatchedLength = 0; for (int givenIndex = 0; givenIndex != givenParameters.Length; givenIndex++) { Type? givenType = givenParameters[givenIndex]?.GetType(); @@ -357,7 +383,7 @@ public int Match(object[] givenParameters) applyIndexStart++; if (applyIndex == givenIndex) { - applyExactLength = applyIndex; + MatchedLength = applyIndex; } } } @@ -365,13 +391,13 @@ public int Match(object[] givenParameters) if (givenMatched == false) { - return -1; + MatchedLength = -1; + return; } } - return applyExactLength; } - public object CreateInstance(IServiceProvider provider) + public object? CreateInstance(IServiceProvider provider, bool throwIfFailed = false) { for (int index = 0; index != _parameters.Length; index++) { @@ -382,7 +408,11 @@ public object CreateInstance(IServiceProvider provider) { if (!ParameterDefaultValue.TryGetDefaultValue(_parameters[index], out object? defaultValue)) { - throw new InvalidOperationException($"Unable to resolve service for type '{_parameters[index].ParameterType}' while attempting to activate '{_constructor.DeclaringType}'."); + if (throwIfFailed) + { + throw new InvalidOperationException($"Unable to resolve service for type '{_parameters[index].ParameterType}' while attempting to activate '{_constructor.DeclaringType}'."); + } + return null; } else { diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Specification.Tests/src/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Specification.Tests/src/ActivatorUtilitiesTests.cs index fd9d639d9d091c..c298e0e763c91d 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Specification.Tests/src/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Specification.Tests/src/ActivatorUtilitiesTests.cs @@ -214,9 +214,9 @@ public void TypeActivatorCreateFactoryDoesNotAllowForAmbiguousConstructorMatches } [Theory] - [InlineData("", "string")] + [InlineData("", "IFakeService, string")] [InlineData(5, "IFakeService, int")] - public void TypeActivatorCreateInstanceUsesFirstMathchedConstructor(object value, string ctor) + public void TypeActivatorCreateInstanceUsesLongestAvailableConstructor(object value, string ctor) { // Arrange var serviceCollection = new TestServiceCollection(); diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs new file mode 100644 index 00000000000000..3366f9574e42a6 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -0,0 +1,190 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using Xunit; + +namespace Microsoft.Extensions.DependencyInjection.Tests +{ + public class ActivatorUtilitiesTests + { + [Fact] + public void ShouldUseTheLongestAvailableConstructor() + { + var services = new ServiceCollection(); + services.AddScoped(); + services.AddScoped(); + using var provider = services.BuildServiceProvider(); + var a = new A(); + var c = new C(); + + var instance = ActivatorUtilities.CreateInstance(provider, c, a); + + Assert.NotNull(instance.B); + } + + [Fact] + public void ShouldUseTheLongestAvailableConstructorOnlyIfConstructorsHaveTheSamePriority() + { + var services = new ServiceCollection(); + services.AddScoped(); + services.AddScoped(); + using var provider = services.BuildServiceProvider(); + var a = new A(); + var c = new C(); + + var instance = ActivatorUtilities.CreateInstance(provider, a, c); + + Assert.Null(instance.B); + } + + [Theory] + [InlineData(typeof(DefaultConstructorFirst))] + [InlineData(typeof(DefaultConstructorLast))] + public void ChoosesDefaultConstructorNoMatterOrder(Type instanceType) + { + var services = new ServiceCollection(); + using var provider = services.BuildServiceProvider(); + + var instance = ActivatorUtilities.CreateInstance(provider, instanceType); + + Assert.NotNull(instance); + } + + [Fact] + public void ShouldTryToUseAllAvailableConstructorsBeforeThrowingActivationException() + { // https://github.com/dotnet/runtime/issues/46132 + var services = new ServiceCollection(); + services.AddScoped(); + using var provider = services.BuildServiceProvider(); + var a = new A(); + var c = new C(); + + var instance = ActivatorUtilities.CreateInstance( + provider, c, a); + + Assert.Same(a, instance.A); + Assert.Same(c, instance.C); + Assert.NotNull(instance.S); + Assert.Null(instance.B); + } + + [Theory] + [InlineData(typeof(IClassWithAttribute.FirstConstructorWithAttribute))] + [InlineData(typeof(IClassWithAttribute.LastConstructorWithAttribute))] + public void ConstructorWithAttributeShouldHaveTheHighestPriorityNoMatterDefinitionOrder(Type instanceType) + { // https://github.com/dotnet/runtime/issues/42339 + var services = new ServiceCollection(); + var a = new A(); + services.AddSingleton(a); + using var provider = services.BuildServiceProvider(); + + var instance = (IClassWithAttribute)ActivatorUtilities + .CreateInstance(provider, instanceType, new B(), new C()); + + Assert.Same(a, instance.A); + } + } + + internal class A { } + internal class B { } + internal class C { } + internal class S { } + + internal class Creatable + { + public A A { get; } + public B B { get; } + public C C { get; } + public S S { get; } + public Creatable(A a, B b, C c, S s) + { + A = a; + B = b; + C = c; + S = s; + } + + public Creatable(A a, C c, S s) : this (a, null, c, s) { } + } + + internal interface IClassWithAttribute + { + public A A { get; } + public B B { get; } + public C C { get; } + + public class FirstConstructorWithAttribute : IClassWithAttribute + { + public A A { get; } + public B B { get; } + public C C { get; } + + [ActivatorUtilitiesConstructor] + public FirstConstructorWithAttribute(A a, B b, C c) + { + A = a; + B = b; + C = c; + } + + public FirstConstructorWithAttribute(B b, C c) : this(null, b, c) { } + } + + public class LastConstructorWithAttribute : IClassWithAttribute + { + public A A { get; } + public B B { get; } + public C C { get; } + + public LastConstructorWithAttribute(B b, C c) : this(null, b, c) { } + + + [ActivatorUtilitiesConstructor] + public LastConstructorWithAttribute(A a, B b, C c) + { + A = a; + B = b; + C = c; + } + } + } + + internal class DefaultConstructorFirst + { + public A A { get; } + public B B { get; } + + public DefaultConstructorFirst() { } + + public DefaultConstructorFirst(A a) + { + A = a; + } + + public DefaultConstructorFirst(A a, B b) + { + A = a; + B = b; + } + } + + internal class DefaultConstructorLast + { + public A A { get; } + public B B { get; } + + public DefaultConstructorLast(A a, B b) + { + A = a; + B = b; + } + + public DefaultConstructorLast(A a) + { + A = a; + } + + public DefaultConstructorLast() { } + } +}