From 6d88e34648389771937bef2ad4150669b05762fe Mon Sep 17 00:00:00 2001 From: mapogolions Date: Sun, 3 Apr 2022 02:21:44 +0500 Subject: [PATCH 01/29] Type activator depends on constructors definition order if preferred constructor is marked --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs 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..5e5e4cb35b4112 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -0,0 +1,100 @@ +// 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 System.Collections.Generic; +using Xunit; + +namespace Microsoft.Extensions.DependencyInjection.Tests +{ + public class ActivatorUtilitiesTests + { + [Theory] + [MemberData(nameof(ActivatorUtilitiesData))] + public void ActivatorUtilitiesShouldBeOrderAgnostic(Type instanceType) + { + var data = new Dictionary(); + var serviceProvider = new FakeServiceProvider(t => + { + if (t == typeof(FakeValidationStatus)) return FakeValidationStatus.Invalid; + throw new NotImplementedException(); + }); + + var instance = (IFakeValidationResult)ActivatorUtilities + .CreateInstance(serviceProvider, instanceType, "description", data); + + Assert.Equal(FakeValidationStatus.Invalid, instance.Status); + } + + public static TheoryData ActivatorUtilitiesData + { + get => new TheoryData + { + typeof(FakeValidationResult), + typeof(FakeValidationResultOps) + }; + } + } + + internal class FakeServiceProvider : IServiceProvider + { + private readonly Func _factory; + + public FakeServiceProvider(Func factory) + { + _factory = factory; + } + + public object? GetService(Type serviceType) => _factory(serviceType); + } + + internal interface IFakeValidationResult + { + FakeValidationStatus Status { get; } + string Description { get; } + IReadOnlyDictionary Data { get; } + } + + internal class FakeValidationResult : IFakeValidationResult + { + [ActivatorUtilitiesConstructor] + public FakeValidationResult(FakeValidationStatus status, string description, + IReadOnlyDictionary data) + { + Status = status; + Description = description; + Data = data; + } + + public FakeValidationResult(string description, IReadOnlyDictionary data) + : this(FakeValidationStatus.Valid, description, data) { } + + public FakeValidationStatus Status { get; } + public string Description { get; } + public IReadOnlyDictionary Data { get; } + } + internal class FakeValidationResultOps : IFakeValidationResult + { + public FakeValidationResultOps(string description, IReadOnlyDictionary data) + : this(FakeValidationStatus.Valid, description, data) { } + + [ActivatorUtilitiesConstructor] + public FakeValidationResultOps(FakeValidationStatus status, string description, + IReadOnlyDictionary data) + { + Status = status; + Description = description; + Data = data; + } + + public FakeValidationStatus Status { get; } + public string Description { get; } + public IReadOnlyDictionary Data { get; } + } + + internal enum FakeValidationStatus + { + Valid, + Invalid + } +} From 43818207be5ddd8d15bedc72a5ade2c0abceaa71 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Sun, 3 Apr 2022 02:33:04 +0500 Subject: [PATCH 02/29] constructor marked as preferred should be considered the first option --- .../src/ActivatorUtilities.cs | 47 ++++++++++--------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 81516568c14e64..3b4f3aea39a07e 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Linq.Expressions; using System.Reflection; using System.Runtime.ExceptionServices; @@ -32,38 +33,40 @@ public static object CreateInstance( params object[] parameters) { int bestLength = -1; - bool seenPreferred = false; - ConstructorMatcher bestMatcher = default; if (!instanceType.IsAbstract) { - foreach (ConstructorInfo? constructor in instanceType.GetConstructors()) + ConstructorInfo[] ctors = instanceType.GetConstructors(); + ConstructorInfo[] preferredCtors = ctors + .Where(x => x.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute))).ToArray(); + if (preferredCtors.Length > 1) { - var matcher = new ConstructorMatcher(constructor); - bool isPreferred = constructor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute), false); - int length = matcher.Match(parameters); + ThrowMultipleCtorsMarkedWithAttributeException(); + } - if (isPreferred) + if (preferredCtors.Length == 1) + { + bestMatcher = new ConstructorMatcher(preferredCtors[0]); + bestLength = bestMatcher.Match(parameters); + if (bestLength == -1) { - if (seenPreferred) - { - ThrowMultipleCtorsMarkedWithAttributeException(); - } - - if (length == -1) - { - ThrowMarkedCtorDoesNotTakeAllProvidedArguments(); - } + ThrowMarkedCtorDoesNotTakeAllProvidedArguments(); } - - if (isPreferred || bestLength < length) + } + else + { + foreach (ConstructorInfo? ctor in ctors) { - bestLength = length; - bestMatcher = matcher; + var matcher = new ConstructorMatcher(ctor); + bool isPreferred = ctor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute), false); + int length = matcher.Match(parameters); + if (bestLength < length) + { + bestLength = length; + bestMatcher = matcher; + } } - - seenPreferred |= isPreferred; } } From a859ea81cd289ed01502896a14dc41e98d05b140 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Sun, 3 Apr 2022 02:40:32 +0500 Subject: [PATCH 03/29] prevent parameter matching in case it is known that it will fail before it even starts --- .../src/ActivatorUtilities.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 3b4f3aea39a07e..28c6e76d2da14b 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -341,6 +341,10 @@ public ConstructorMatcher(ConstructorInfo constructor) public int Match(object[] givenParameters) { + if (givenParameters.Length > _parameters.Length) + { + return -1; + } int applyIndexStart = 0; int applyExactLength = 0; for (int givenIndex = 0; givenIndex != givenParameters.Length; givenIndex++) From 6f74f652e74a96ae6add5b316c6a238124e87cf1 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Sun, 3 Apr 2022 02:44:12 +0500 Subject: [PATCH 04/29] break loop if a perfect constructor is found --- .../src/ActivatorUtilities.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 28c6e76d2da14b..ddad7c1b69b173 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -66,6 +66,7 @@ public static object CreateInstance( bestLength = length; bestMatcher = matcher; } + if (bestLength == parameters.Length - 1) break; } } } From ed5655ffa06c1fd30efb7fc2d9b94bd183c1d9b0 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Sun, 3 Apr 2022 02:48:04 +0500 Subject: [PATCH 05/29] add empty line --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs index 5e5e4cb35b4112..7b019a4f0e5c72 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -73,6 +73,7 @@ public FakeValidationResult(string description, IReadOnlyDictionary Data { get; } } + internal class FakeValidationResultOps : IFakeValidationResult { public FakeValidationResultOps(string description, IReadOnlyDictionary data) From 2491f5083b6585a636695b2c8e8f9362b2e48ca1 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Sun, 3 Apr 2022 13:51:21 +0500 Subject: [PATCH 06/29] replace the wasteful array construction operation --- .../src/ActivatorUtilities.cs | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index ddad7c1b69b173..6e9c79cd7e2363 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -4,7 +4,6 @@ using System; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Linq; using System.Linq.Expressions; using System.Reflection; using System.Runtime.ExceptionServices; @@ -37,17 +36,22 @@ public static object CreateInstance( if (!instanceType.IsAbstract) { - ConstructorInfo[] ctors = instanceType.GetConstructors(); - ConstructorInfo[] preferredCtors = ctors - .Where(x => x.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute))).ToArray(); - if (preferredCtors.Length > 1) + ConstructorInfo[] constructors = instanceType.GetConstructors(); + ConstructorInfo? pereferredConstructor = null; + foreach (ConstructorInfo? constructor in constructors) { - ThrowMultipleCtorsMarkedWithAttributeException(); + if (constructor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute))) + { + if (pereferredConstructor is not null) + { + ThrowMultipleCtorsMarkedWithAttributeException(); + } + pereferredConstructor = constructor; + } } - - if (preferredCtors.Length == 1) + if (pereferredConstructor is not null) { - bestMatcher = new ConstructorMatcher(preferredCtors[0]); + bestMatcher = new ConstructorMatcher(pereferredConstructor); bestLength = bestMatcher.Match(parameters); if (bestLength == -1) { @@ -56,7 +60,7 @@ public static object CreateInstance( } else { - foreach (ConstructorInfo? ctor in ctors) + foreach (ConstructorInfo? ctor in constructors) { var matcher = new ConstructorMatcher(ctor); bool isPreferred = ctor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute), false); From 5dab06bc3ac630514d6300a56515dc8bfda4d429 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Sun, 3 Apr 2022 13:55:34 +0500 Subject: [PATCH 07/29] rename local variable --- .../src/ActivatorUtilities.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 6e9c79cd7e2363..b6c7e25937964a 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -60,10 +60,10 @@ public static object CreateInstance( } else { - foreach (ConstructorInfo? ctor in constructors) + foreach (ConstructorInfo? constructor in constructors) { - var matcher = new ConstructorMatcher(ctor); - bool isPreferred = ctor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute), false); + var matcher = new ConstructorMatcher(constructor); + bool isPreferred = constructor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute), false); int length = matcher.Match(parameters); if (bestLength < length) { From 9cdbf31c25af289c3a3edee0fc3e31c1c2a8e00c Mon Sep 17 00:00:00 2001 From: mapogolions Date: Sun, 3 Apr 2022 18:33:40 +0500 Subject: [PATCH 08/29] fix indent, rename unit test --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs index 7b019a4f0e5c72..7670a1ae5b6efb 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -11,7 +11,7 @@ public class ActivatorUtilitiesTests { [Theory] [MemberData(nameof(ActivatorUtilitiesData))] - public void ActivatorUtilitiesShouldBeOrderAgnostic(Type instanceType) + public void CreateInstanceShouldNotDependOnConstructorsDefinitionOrder(Type instanceType) { var data = new Dictionary(); var serviceProvider = new FakeServiceProvider(t => @@ -31,7 +31,7 @@ public static TheoryData ActivatorUtilitiesData get => new TheoryData { typeof(FakeValidationResult), - typeof(FakeValidationResultOps) + typeof(FakeValidationResultOps) }; } } From d9c5458ee925dbba3f44018abc3b0dd13609f537 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Sun, 3 Apr 2022 18:35:00 +0500 Subject: [PATCH 09/29] remove nullable operator --- .../src/ActivatorUtilities.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index b6c7e25937964a..41cc096739182f 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -38,7 +38,7 @@ public static object CreateInstance( { ConstructorInfo[] constructors = instanceType.GetConstructors(); ConstructorInfo? pereferredConstructor = null; - foreach (ConstructorInfo? constructor in constructors) + foreach (ConstructorInfo constructor in constructors) { if (constructor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute))) { @@ -60,7 +60,7 @@ public static object CreateInstance( } else { - foreach (ConstructorInfo? constructor in constructors) + foreach (ConstructorInfo constructor in constructors) { var matcher = new ConstructorMatcher(constructor); bool isPreferred = constructor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute), false); From d646ee1d575353fe427438c4ff1c0718597603ac Mon Sep 17 00:00:00 2001 From: mapogolions Date: Mon, 4 Apr 2022 02:20:17 +0500 Subject: [PATCH 10/29] reproduce issue #46132 --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs index 7670a1ae5b6efb..ac98e31930e996 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -9,6 +9,21 @@ namespace Microsoft.Extensions.DependencyInjection.Tests { public class ActivatorUtilitiesTests { + [Fact] + public void ShouldFixIssue_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, new C(), new A());; + + Assert.NotNull(instance); + } + [Theory] [MemberData(nameof(ActivatorUtilitiesData))] public void CreateInstanceShouldNotDependOnConstructorsDefinitionOrder(Type instanceType) @@ -36,6 +51,18 @@ public static TheoryData ActivatorUtilitiesData } } + internal class A { } + internal class B { } + internal class C { } + internal class S { } + + internal class Creatable + { + public Creatable(A a, B b, C c, S s) { } + + public Creatable(A a, C c, S s) { } + } + internal class FakeServiceProvider : IServiceProvider { private readonly Func _factory; From 92f188b29e83f95d22d53ac0d37abb954faada1f Mon Sep 17 00:00:00 2001 From: mapogolions Date: Mon, 4 Apr 2022 02:21:05 +0500 Subject: [PATCH 11/29] fix issue 46132 --- .../src/ActivatorUtilities.cs | 104 +++++++++++------- 1 file changed, 62 insertions(+), 42 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 41cc096739182f..7af96edc4586bc 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq.Expressions; @@ -31,57 +32,59 @@ public static object CreateInstance( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, params object[] parameters) { - int bestLength = -1; - ConstructorMatcher bestMatcher = default; + 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); + } + + ConstructorInfo[] constructors = instanceType.GetConstructors(); + if (constructors.Length == 0) + { + throw new InvalidOperationException(message); + } - if (!instanceType.IsAbstract) + ConstructorInfo? pereferredConstructor = null; + foreach (ConstructorInfo constructor in constructors) { - ConstructorInfo[] constructors = instanceType.GetConstructors(); - ConstructorInfo? pereferredConstructor = null; - foreach (ConstructorInfo constructor in constructors) + if (constructor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute))) { - if (constructor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute))) + if (pereferredConstructor is not null) { - if (pereferredConstructor is not null) - { - ThrowMultipleCtorsMarkedWithAttributeException(); - } - pereferredConstructor = constructor; + ThrowMultipleCtorsMarkedWithAttributeException(); } + pereferredConstructor = constructor; } - if (pereferredConstructor is not null) + } + if (pereferredConstructor is not null) + { + var matcher = new ConstructorMatcher(pereferredConstructor); + if (matcher.Match(parameters) == -1) { - bestMatcher = new ConstructorMatcher(pereferredConstructor); - bestLength = bestMatcher.Match(parameters); - if (bestLength == -1) - { - ThrowMarkedCtorDoesNotTakeAllProvidedArguments(); - } - } - else - { - foreach (ConstructorInfo constructor in constructors) - { - var matcher = new ConstructorMatcher(constructor); - bool isPreferred = constructor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute), false); - int length = matcher.Match(parameters); - if (bestLength < length) - { - bestLength = length; - bestMatcher = matcher; - } - if (bestLength == parameters.Length - 1) break; - } + ThrowMarkedCtorDoesNotTakeAllProvidedArguments(); } + return matcher.CreateInstance(provider, throwIfFailed: true)!; } - - if (bestLength == -1) + var priorityQueue = new PrioritizedConstructorMatcher[constructors.Length]; + for (int i = 0; i < constructors.Length; i++) { - 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); + ConstructorInfo constructor = constructors[i]; + var matcher = new ConstructorMatcher(constructor); + int length = matcher.Match(parameters); + priorityQueue[i] = new PrioritizedConstructorMatcher(length, matcher); } - - return bestMatcher.CreateInstance(provider); + Array.Sort(priorityQueue, (a, b) => a.Priority - b.Priority); + object? instance = null; + for (int i = 0; i < priorityQueue.Length; i++) + { + PrioritizedConstructorMatcher prioritizedMatcher = priorityQueue[i]; + if (prioritizedMatcher.Priority == -1) continue; + ConstructorMatcher matcher = priorityQueue[i].Matcher; + bool lastChance = i == priorityQueue.Length - 1; + instance = matcher.CreateInstance(provider, throwIfFailed: lastChance); + if (instance is not null) return instance; + } + throw new InvalidOperationException(message); } /// @@ -331,6 +334,19 @@ private static bool TryCreateParameterMap(ParameterInfo[] constructorParameters, return true; } + private struct PrioritizedConstructorMatcher + { + public PrioritizedConstructorMatcher(int priority, ConstructorMatcher matcher) + { + Priority = priority; + Matcher = matcher; + } + + public int Priority { get; } + + public ConstructorMatcher Matcher { get; } + } + private struct ConstructorMatcher { private readonly ConstructorInfo _constructor; @@ -383,7 +399,7 @@ public int Match(object[] givenParameters) return applyExactLength; } - public object CreateInstance(IServiceProvider provider) + public object? CreateInstance(IServiceProvider provider, bool throwIfFailed = false) { for (int index = 0; index != _parameters.Length; index++) { @@ -394,7 +410,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 { From 1012cf08953032fb591ef9aee7f50ccb09fa31e6 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Mon, 4 Apr 2022 02:29:34 +0500 Subject: [PATCH 12/29] remove prioritized constructor matcher --- .../src/ActivatorUtilities.cs | 37 +++++++------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 7af96edc4586bc..b934be4188abd8 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -65,21 +65,20 @@ public static object CreateInstance( } return matcher.CreateInstance(provider, throwIfFailed: true)!; } - var priorityQueue = new PrioritizedConstructorMatcher[constructors.Length]; + var priorityQueue = new ConstructorMatcher[constructors.Length]; for (int i = 0; i < constructors.Length; i++) { ConstructorInfo constructor = constructors[i]; var matcher = new ConstructorMatcher(constructor); int length = matcher.Match(parameters); - priorityQueue[i] = new PrioritizedConstructorMatcher(length, matcher); + priorityQueue[i] = matcher; } - Array.Sort(priorityQueue, (a, b) => a.Priority - b.Priority); + Array.Sort(priorityQueue, (a, b) => a.ApplyExectLength - b.ApplyExectLength); object? instance = null; for (int i = 0; i < priorityQueue.Length; i++) { - PrioritizedConstructorMatcher prioritizedMatcher = priorityQueue[i]; - if (prioritizedMatcher.Priority == -1) continue; - ConstructorMatcher matcher = priorityQueue[i].Matcher; + ConstructorMatcher matcher = priorityQueue[i]; + if (matcher.ApplyExectLength == -1) continue; bool lastChance = i == priorityQueue.Length - 1; instance = matcher.CreateInstance(provider, throwIfFailed: lastChance); if (instance is not null) return instance; @@ -334,19 +333,6 @@ private static bool TryCreateParameterMap(ParameterInfo[] constructorParameters, return true; } - private struct PrioritizedConstructorMatcher - { - public PrioritizedConstructorMatcher(int priority, ConstructorMatcher matcher) - { - Priority = priority; - Matcher = matcher; - } - - public int Priority { get; } - - public ConstructorMatcher Matcher { get; } - } - private struct ConstructorMatcher { private readonly ConstructorInfo _constructor; @@ -360,14 +346,16 @@ public ConstructorMatcher(ConstructorInfo constructor) _parameterValues = new object?[_parameters.Length]; } + public int ApplyExectLength { get; private set; } = -1; + public int Match(object[] givenParameters) { if (givenParameters.Length > _parameters.Length) { - return -1; + return ApplyExectLength; } int applyIndexStart = 0; - int applyExactLength = 0; + ApplyExectLength = 0; for (int givenIndex = 0; givenIndex != givenParameters.Length; givenIndex++) { Type? givenType = givenParameters[givenIndex]?.GetType(); @@ -385,7 +373,7 @@ public int Match(object[] givenParameters) applyIndexStart++; if (applyIndex == givenIndex) { - applyExactLength = applyIndex; + ApplyExectLength = applyIndex; } } } @@ -393,10 +381,11 @@ public int Match(object[] givenParameters) if (givenMatched == false) { - return -1; + ApplyExectLength = -1; + return ApplyExectLength; } } - return applyExactLength; + return ApplyExectLength; } public object? CreateInstance(IServiceProvider provider, bool throwIfFailed = false) From fb7264a095e1806c547b4a2a31d639b7a87ec019 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Mon, 4 Apr 2022 02:41:29 +0500 Subject: [PATCH 13/29] specify issue --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs index ac98e31930e996..0cd6ec0e432c26 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -26,7 +26,7 @@ public void ShouldFixIssue_46132() [Theory] [MemberData(nameof(ActivatorUtilitiesData))] - public void CreateInstanceShouldNotDependOnConstructorsDefinitionOrder(Type instanceType) + public void ShouldFixIssue_42339(Type instanceType) { var data = new Dictionary(); var serviceProvider = new FakeServiceProvider(t => From 9beb319eb17f9064c8dd7dc4871f9c11eccdc83b Mon Sep 17 00:00:00 2001 From: mapogolions Date: Mon, 4 Apr 2022 02:47:43 +0500 Subject: [PATCH 14/29] refactoring --- .../src/ActivatorUtilities.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index b934be4188abd8..e10d47abb3ce3d 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -65,21 +65,21 @@ public static object CreateInstance( } return matcher.CreateInstance(provider, throwIfFailed: true)!; } - var priorityQueue = new ConstructorMatcher[constructors.Length]; + var matchers = new ConstructorMatcher[constructors.Length]; for (int i = 0; i < constructors.Length; i++) { ConstructorInfo constructor = constructors[i]; var matcher = new ConstructorMatcher(constructor); int length = matcher.Match(parameters); - priorityQueue[i] = matcher; + matchers[i] = matcher; } - Array.Sort(priorityQueue, (a, b) => a.ApplyExectLength - b.ApplyExectLength); + Array.Sort(matchers, (a, b) => a.ApplyExectLength - b.ApplyExectLength); object? instance = null; - for (int i = 0; i < priorityQueue.Length; i++) + for (int i = 0; i < matchers.Length; i++) { - ConstructorMatcher matcher = priorityQueue[i]; + ConstructorMatcher matcher = matchers[i]; if (matcher.ApplyExectLength == -1) continue; - bool lastChance = i == priorityQueue.Length - 1; + bool lastChance = i == matchers.Length - 1; instance = matcher.CreateInstance(provider, throwIfFailed: lastChance); if (instance is not null) return instance; } From d93c5582d6442f50cab5ac83e527a25710a25d3c Mon Sep 17 00:00:00 2001 From: mapogolions Date: Mon, 4 Apr 2022 11:12:13 +0500 Subject: [PATCH 15/29] fix sorting --- .../src/ActivatorUtilities.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index e10d47abb3ce3d..79029da5610346 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -70,15 +70,15 @@ public static object CreateInstance( { ConstructorInfo constructor = constructors[i]; var matcher = new ConstructorMatcher(constructor); - int length = matcher.Match(parameters); + _ = matcher.Match(parameters); matchers[i] = matcher; } - Array.Sort(matchers, (a, b) => a.ApplyExectLength - b.ApplyExectLength); + Array.Sort(matchers, (a, b) => b.ApplyExectLength - a.ApplyExectLength); object? instance = null; for (int i = 0; i < matchers.Length; i++) { ConstructorMatcher matcher = matchers[i]; - if (matcher.ApplyExectLength == -1) continue; + if (matcher.ApplyExectLength == -1) break; bool lastChance = i == matchers.Length - 1; instance = matcher.CreateInstance(provider, throwIfFailed: lastChance); if (instance is not null) return instance; From a935ddca184ddcbb926482b2195f3bea1aa7e045 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Mon, 4 Apr 2022 11:27:59 +0500 Subject: [PATCH 16/29] add additional checks --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs index 0cd6ec0e432c26..d36277cb26580a 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -19,9 +19,13 @@ public void ShouldFixIssue_46132() var c = new C(); var instance = ActivatorUtilities.CreateInstance( - provider, new C(), new A());; + provider, c, a); Assert.NotNull(instance); + Assert.Same(a, instance.A); + Assert.Same(c, instance.C); + Assert.NotNull(instance.S); + Assert.Null(instance.B); } [Theory] @@ -58,9 +62,19 @@ internal class S { } internal class Creatable { - public Creatable(A a, B b, C c, S s) { } + 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) { } + public Creatable(A a, C c, S s) : this (a, null, c, s) { } } internal class FakeServiceProvider : IServiceProvider From 6d8fdc24b32afcbbd2dc72824bc8c6826c7d0cf7 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Fri, 29 Apr 2022 13:36:53 +0500 Subject: [PATCH 17/29] chooses default constructor no matter order --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs index d36277cb26580a..f610bfcf3c495f 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -9,6 +9,19 @@ namespace Microsoft.Extensions.DependencyInjection.Tests { public class ActivatorUtilitiesTests { + [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 ShouldFixIssue_46132() { @@ -139,4 +152,42 @@ internal enum FakeValidationStatus Valid, Invalid } + + 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() { } + } } From fe1c38ebdba5e9c054129cb0316d9bd3fafcb290 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Fri, 29 Apr 2022 14:16:56 +0500 Subject: [PATCH 18/29] type activator uses longest available constructor when multiple constructors are available --- .../src/ActivatorUtilities.cs | 4 +++- .../src/ActivatorUtilitiesTests.cs | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 79029da5610346..7b0a836503acb1 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -73,7 +73,7 @@ public static object CreateInstance( _ = matcher.Match(parameters); matchers[i] = matcher; } - Array.Sort(matchers, (a, b) => b.ApplyExectLength - a.ApplyExectLength); + Array.Sort(matchers, (a, b) => b.Priority - a.Priority); object? instance = null; for (int i = 0; i < matchers.Length; i++) { @@ -347,11 +347,13 @@ public ConstructorMatcher(ConstructorInfo constructor) } public int ApplyExectLength { get; private set; } = -1; + public int Priority => ApplyExectLength == -1 ? -1 : ApplyExectLength + _parameters.Length; public int Match(object[] givenParameters) { if (givenParameters.Length > _parameters.Length) { + ApplyExectLength = -1; return ApplyExectLength; } int applyIndexStart = 0; 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 92382c699d9edd..9aef6fa44a6473 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(); From 7e4a3637c9af106476c4099fecfcfa66d7a94a2c Mon Sep 17 00:00:00 2001 From: mapogolions Date: Fri, 29 Apr 2022 14:21:23 +0500 Subject: [PATCH 19/29] refactoring --- .../src/ActivatorUtilities.cs | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 7b0a836503acb1..588621668f31f2 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -47,14 +47,15 @@ public static object CreateInstance( ConstructorInfo? pereferredConstructor = null; foreach (ConstructorInfo constructor in constructors) { - if (constructor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute))) + if (!constructor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute), false)) { - if (pereferredConstructor is not null) - { - ThrowMultipleCtorsMarkedWithAttributeException(); - } - pereferredConstructor = constructor; + continue; + } + if (pereferredConstructor is not null) + { + ThrowMultipleCtorsMarkedWithAttributeException(); } + pereferredConstructor = constructor; } if (pereferredConstructor is not null) { @@ -78,7 +79,7 @@ public static object CreateInstance( for (int i = 0; i < matchers.Length; i++) { ConstructorMatcher matcher = matchers[i]; - if (matcher.ApplyExectLength == -1) break; + if (matcher.MatchedLength == -1) break; bool lastChance = i == matchers.Length - 1; instance = matcher.CreateInstance(provider, throwIfFailed: lastChance); if (instance is not null) return instance; @@ -346,18 +347,18 @@ public ConstructorMatcher(ConstructorInfo constructor) _parameterValues = new object?[_parameters.Length]; } - public int ApplyExectLength { get; private set; } = -1; - public int Priority => ApplyExectLength == -1 ? -1 : ApplyExectLength + _parameters.Length; + public int MatchedLength { get; private set; } = -1; + public int Priority => MatchedLength == -1 ? -1 : MatchedLength + _parameters.Length; public int Match(object[] givenParameters) { if (givenParameters.Length > _parameters.Length) { - ApplyExectLength = -1; - return ApplyExectLength; + MatchedLength = -1; + return MatchedLength; } int applyIndexStart = 0; - ApplyExectLength = 0; + MatchedLength = 0; for (int givenIndex = 0; givenIndex != givenParameters.Length; givenIndex++) { Type? givenType = givenParameters[givenIndex]?.GetType(); @@ -375,7 +376,7 @@ public int Match(object[] givenParameters) applyIndexStart++; if (applyIndex == givenIndex) { - ApplyExectLength = applyIndex; + MatchedLength = applyIndex; } } } @@ -383,11 +384,11 @@ public int Match(object[] givenParameters) if (givenMatched == false) { - ApplyExectLength = -1; - return ApplyExectLength; + MatchedLength = -1; + return MatchedLength; } } - return ApplyExectLength; + return MatchedLength; } public object? CreateInstance(IServiceProvider provider, bool throwIfFailed = false) From 7aabe2c7755b3ca66b1e49f5ff0683e9d3500330 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Fri, 29 Apr 2022 14:24:59 +0500 Subject: [PATCH 20/29] update method signature --- .../src/ActivatorUtilities.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 588621668f31f2..e93ffd7485cb31 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -60,7 +60,8 @@ public static object CreateInstance( if (pereferredConstructor is not null) { var matcher = new ConstructorMatcher(pereferredConstructor); - if (matcher.Match(parameters) == -1) + matcher.Match(parameters); + if (matcher.MatchedLength == -1) { ThrowMarkedCtorDoesNotTakeAllProvidedArguments(); } @@ -71,7 +72,7 @@ public static object CreateInstance( { ConstructorInfo constructor = constructors[i]; var matcher = new ConstructorMatcher(constructor); - _ = matcher.Match(parameters); + matcher.Match(parameters); matchers[i] = matcher; } Array.Sort(matchers, (a, b) => b.Priority - a.Priority); @@ -350,12 +351,12 @@ public ConstructorMatcher(ConstructorInfo constructor) public int MatchedLength { get; private set; } = -1; public int Priority => MatchedLength == -1 ? -1 : MatchedLength + _parameters.Length; - public int Match(object[] givenParameters) + public void Match(object[] givenParameters) { if (givenParameters.Length > _parameters.Length) { MatchedLength = -1; - return MatchedLength; + return; } int applyIndexStart = 0; MatchedLength = 0; @@ -385,10 +386,9 @@ public int Match(object[] givenParameters) if (givenMatched == false) { MatchedLength = -1; - return MatchedLength; + return; } } - return MatchedLength; } public object? CreateInstance(IServiceProvider provider, bool throwIfFailed = false) From 992751081a5e2e6dfe5d7cfef6e63863c011b36b Mon Sep 17 00:00:00 2001 From: mapogolions Date: Fri, 29 Apr 2022 15:17:30 +0500 Subject: [PATCH 21/29] should use the longest available constructor among constructors with the same priority --- .../src/ActivatorUtilities.cs | 11 +++++++++-- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index e93ffd7485cb31..e8d11764e32a16 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -75,7 +75,13 @@ public static object CreateInstance( matcher.Match(parameters); matchers[i] = matcher; } - Array.Sort(matchers, (a, b) => b.Priority - a.Priority); + + 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++) { @@ -346,10 +352,11 @@ public ConstructorMatcher(ConstructorInfo constructor) _constructor = constructor; _parameters = _constructor.GetParameters(); _parameterValues = new object?[_parameters.Length]; + ParametersCount = _parameters.Length; } public int MatchedLength { get; private set; } = -1; - public int Priority => MatchedLength == -1 ? -1 : MatchedLength + _parameters.Length; + public int ParametersCount { get; } public void Match(object[] givenParameters) { diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs index f610bfcf3c495f..02175c55f4f637 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -9,6 +9,21 @@ namespace Microsoft.Extensions.DependencyInjection.Tests { public class ActivatorUtilitiesTests { + [Fact] + public void ShouldUseLongestAvailableConstructorOnlyIfConstructorsHaveTheSamePriority() + { + 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))] From 68ff99926eb2d5f8428009b9a397c34411debbe2 Mon Sep 17 00:00:00 2001 From: Pavel Ivanov Date: Fri, 29 Apr 2022 21:43:52 +0500 Subject: [PATCH 22/29] Update src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs Co-authored-by: Eric Erhardt --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs index 02175c55f4f637..6d9cf7b8dde453 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -57,7 +57,8 @@ public void ShouldFixIssue_46132() } [Theory] - [MemberData(nameof(ActivatorUtilitiesData))] + [InlineData(typeof(FakeValidationResult))] + [InlineData(typeof(FakeValidationResultOps))] public void ShouldFixIssue_42339(Type instanceType) { var data = new Dictionary(); From 175c6372ae770750d7234a689a01c4d2bd4cf540 Mon Sep 17 00:00:00 2001 From: Pavel Ivanov Date: Fri, 29 Apr 2022 21:44:07 +0500 Subject: [PATCH 23/29] Update src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs Co-authored-by: Eric Erhardt --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs index 6d9cf7b8dde453..2ae4260bf88c53 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -92,7 +92,7 @@ internal class S { } internal class Creatable { public A A { get; } - public B? B { get; } + public B B { get; } public C C { get; } public S S { get; } public Creatable(A a, B b, C c, S s) From e03f7854aa5a75888ff0121ff7c349f23078c636 Mon Sep 17 00:00:00 2001 From: Pavel Ivanov Date: Fri, 29 Apr 2022 21:44:29 +0500 Subject: [PATCH 24/29] Update src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs Co-authored-by: Eric Erhardt --- .../src/ActivatorUtilities.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index e8d11764e32a16..9c3ac7673e080c 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -44,7 +44,7 @@ public static object CreateInstance( throw new InvalidOperationException(message); } - ConstructorInfo? pereferredConstructor = null; + ConstructorInfo? preferredConstructor = null; foreach (ConstructorInfo constructor in constructors) { if (!constructor.IsDefined(typeof(ActivatorUtilitiesConstructorAttribute), false)) From 35f0d212b950c79ffcaa689361e0bf3363ff5830 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Fri, 29 Apr 2022 22:12:22 +0500 Subject: [PATCH 25/29] remove dead code --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs index 2ae4260bf88c53..2114bf5b55db39 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -73,15 +73,6 @@ public void ShouldFixIssue_42339(Type instanceType) Assert.Equal(FakeValidationStatus.Invalid, instance.Status); } - - public static TheoryData ActivatorUtilitiesData - { - get => new TheoryData - { - typeof(FakeValidationResult), - typeof(FakeValidationResultOps) - }; - } } internal class A { } From d9052f1ce422b3bcabda2dde345f69134ba63ed1 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Fri, 29 Apr 2022 22:12:28 +0500 Subject: [PATCH 26/29] fix typo --- .../src/ActivatorUtilities.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 9c3ac7673e080c..c08ff9eb5085ea 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq.Expressions; @@ -51,15 +50,15 @@ public static object CreateInstance( { continue; } - if (pereferredConstructor is not null) + if (preferredConstructor is not null) { ThrowMultipleCtorsMarkedWithAttributeException(); } - pereferredConstructor = constructor; + preferredConstructor = constructor; } - if (pereferredConstructor is not null) + if (preferredConstructor is not null) { - var matcher = new ConstructorMatcher(pereferredConstructor); + var matcher = new ConstructorMatcher(preferredConstructor); matcher.Match(parameters); if (matcher.MatchedLength == -1) { From 219e21b768a5a63a4f6631d0c1d3f304e5eb4a6f Mon Sep 17 00:00:00 2001 From: mapogolions Date: Sat, 30 Apr 2022 15:14:02 +0500 Subject: [PATCH 27/29] rename test cases, remove useless code --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 107 +++++++----------- 1 file changed, 42 insertions(+), 65 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs index 2114bf5b55db39..9ef905acefee12 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -38,8 +38,8 @@ public void ChoosesDefaultConstructorNoMatterOrder(Type instanceType) } [Fact] - public void ShouldFixIssue_46132() - { + public void ShouldTryToUseAllAvailableConstructorsBeforeThrowingActivationException() + { // https://github.com/dotnet/runtime/issues/46132 var services = new ServiceCollection(); services.AddScoped(); using var provider = services.BuildServiceProvider(); @@ -57,21 +57,19 @@ public void ShouldFixIssue_46132() } [Theory] - [InlineData(typeof(FakeValidationResult))] - [InlineData(typeof(FakeValidationResultOps))] - public void ShouldFixIssue_42339(Type instanceType) - { - var data = new Dictionary(); - var serviceProvider = new FakeServiceProvider(t => - { - if (t == typeof(FakeValidationStatus)) return FakeValidationStatus.Invalid; - throw new NotImplementedException(); - }); + [InlineData(typeof(IClassWithAttribute.FirstConstructorWithAttribute))] + [InlineData(typeof(IClassWithAttribute.LastConstructorWithAttribute))] + public void ConstructorWithAttributeShouldHaveTheHighestPriorityNoMatterOrderDefinition(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 = (IFakeValidationResult)ActivatorUtilities - .CreateInstance(serviceProvider, instanceType, "description", data); + var instance = (IClassWithAttribute)ActivatorUtilities + .CreateInstance(provider, instanceType, new B(), new C()); - Assert.Equal(FakeValidationStatus.Invalid, instance.Status); + Assert.Same(a, instance.A); } } @@ -97,67 +95,46 @@ public Creatable(A a, B b, C c, S s) public Creatable(A a, C c, S s) : this (a, null, c, s) { } } - internal class FakeServiceProvider : IServiceProvider + internal interface IClassWithAttribute { - private readonly Func _factory; + public A A { get; } + public B B { get; } + public C C { get; } - public FakeServiceProvider(Func factory) + public class FirstConstructorWithAttribute : IClassWithAttribute { - _factory = factory; - } + public A A { get; } + public B B { get; } + public C C { get; } - public object? GetService(Type serviceType) => _factory(serviceType); - } - - internal interface IFakeValidationResult - { - FakeValidationStatus Status { get; } - string Description { get; } - IReadOnlyDictionary Data { get; } - } + [ActivatorUtilitiesConstructor] + public FirstConstructorWithAttribute(A a, B b, C c) + { + A = a; + B = b; + C = c; + } - internal class FakeValidationResult : IFakeValidationResult - { - [ActivatorUtilitiesConstructor] - public FakeValidationResult(FakeValidationStatus status, string description, - IReadOnlyDictionary data) - { - Status = status; - Description = description; - Data = data; + public FirstConstructorWithAttribute(B b, C c) : this(null, b, c) { } } - public FakeValidationResult(string description, IReadOnlyDictionary data) - : this(FakeValidationStatus.Valid, description, data) { } + public class LastConstructorWithAttribute : IClassWithAttribute + { + public A A { get; } + public B B { get; } + public C C { get; } - public FakeValidationStatus Status { get; } - public string Description { get; } - public IReadOnlyDictionary Data { get; } - } + public LastConstructorWithAttribute(B b, C c) : this(null, b, c) { } - internal class FakeValidationResultOps : IFakeValidationResult - { - public FakeValidationResultOps(string description, IReadOnlyDictionary data) - : this(FakeValidationStatus.Valid, description, data) { } - [ActivatorUtilitiesConstructor] - public FakeValidationResultOps(FakeValidationStatus status, string description, - IReadOnlyDictionary data) - { - Status = status; - Description = description; - Data = data; + [ActivatorUtilitiesConstructor] + public LastConstructorWithAttribute(A a, B b, C c) + { + A = a; + B = b; + C = c; + } } - - public FakeValidationStatus Status { get; } - public string Description { get; } - public IReadOnlyDictionary Data { get; } - } - - internal enum FakeValidationStatus - { - Valid, - Invalid } internal class DefaultConstructorFirst From 686d6605a1ed6bb007e48d1663914788f3339d83 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Sat, 30 Apr 2022 15:20:51 +0500 Subject: [PATCH 28/29] remove assert --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs index 9ef905acefee12..0af2c24f43c488 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -49,7 +49,6 @@ public void ShouldTryToUseAllAvailableConstructorsBeforeThrowingActivationExcept var instance = ActivatorUtilities.CreateInstance( provider, c, a); - Assert.NotNull(instance); Assert.Same(a, instance.A); Assert.Same(c, instance.C); Assert.NotNull(instance.S); @@ -59,7 +58,7 @@ public void ShouldTryToUseAllAvailableConstructorsBeforeThrowingActivationExcept [Theory] [InlineData(typeof(IClassWithAttribute.FirstConstructorWithAttribute))] [InlineData(typeof(IClassWithAttribute.LastConstructorWithAttribute))] - public void ConstructorWithAttributeShouldHaveTheHighestPriorityNoMatterOrderDefinition(Type instanceType) + public void ConstructorWithAttributeShouldHaveTheHighestPriorityNoMatterDefinitionOrder(Type instanceType) { // https://github.com/dotnet/runtime/issues/42339 var services = new ServiceCollection(); var a = new A(); From 3facdfe182fa8712ffd4c54477fe18dc3088ed10 Mon Sep 17 00:00:00 2001 From: mapogolions Date: Sat, 30 Apr 2022 15:27:59 +0500 Subject: [PATCH 29/29] add extra test for the longest available constructor rule --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs index 0af2c24f43c488..3366f9574e42a6 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Generic; using Xunit; namespace Microsoft.Extensions.DependencyInjection.Tests @@ -10,7 +9,22 @@ namespace Microsoft.Extensions.DependencyInjection.Tests public class ActivatorUtilitiesTests { [Fact] - public void ShouldUseLongestAvailableConstructorOnlyIfConstructorsHaveTheSamePriority() + 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();