From 9e048a707ff025ad8d15dd1a05b749a344a77e3c Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Fri, 27 Jan 2023 18:29:45 +0800 Subject: [PATCH 01/22] Add reflection path for ActivatorUtilities.CreateFactory --- .../src/ActivatorUtilities.cs | 82 +++++++++++++++++++ .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 48 +++++++---- 2 files changed, 113 insertions(+), 17 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 45b1cdc56ca5f1..30075798c29d1e 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -2,10 +2,12 @@ // 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; using System.Reflection; +using System.Runtime.CompilerServices; using System.Runtime.ExceptionServices; using Microsoft.Extensions.Internal; @@ -127,6 +129,16 @@ public static ObjectFactory CreateFactory( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, Type[] argumentTypes) { +#if NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER + //if (!RuntimeFeature.IsDynamicCodeSupported) + { + FindApplicableConstructor(instanceType, argumentTypes, out ConstructorInfo constructor, out int?[] parameterMap); + + return CreateFactory(constructor, parameterMap); + } +#endif + +#if !(NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER) CreateFactoryInternal(instanceType, argumentTypes, out ParameterExpression provider, out ParameterExpression argumentArray, out Expression factoryExpressionBody); var factoryLambda = Expression.Lambda>( @@ -134,6 +146,7 @@ public static ObjectFactory CreateFactory( Func? result = factoryLambda.Compile(); return result.Invoke; +#endif } /// @@ -152,6 +165,16 @@ public static ObjectFactory CreateFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>( Type[] argumentTypes) { +#if NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER + //if (!RuntimeFeature.IsDynamicCodeSupported) + { + FindApplicableConstructor(typeof(T), argumentTypes, out ConstructorInfo constructor, out int?[] parameterMap); + var factory = CreateFactory(constructor, parameterMap); + return (IServiceProvider serviceProvider, object?[]? arguments) => (T)factory(serviceProvider, arguments); + } +#endif + +#if !(NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER) CreateFactoryInternal(typeof(T), argumentTypes, out ParameterExpression provider, out ParameterExpression argumentArray, out Expression factoryExpressionBody); var factoryLambda = Expression.Lambda>( @@ -159,6 +182,7 @@ public static ObjectFactory Func? result = factoryLambda.Compile(); return result.Invoke; +#endif } private static void CreateFactoryInternal([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, Type[] argumentTypes, out ParameterExpression provider, out ParameterExpression argumentArray, out Expression factoryExpressionBody) @@ -264,6 +288,64 @@ private static NewExpression BuildFactoryExpression( return Expression.New(constructor, constructorArguments); } +#if NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER + private static ObjectFactory CreateFactory( + ConstructorInfo constructor, + int?[] parameterMap) + { + ParameterInfo[]? constructorParameters = constructor.GetParameters(); + List parameters = new List(); + for (int i = 0; i < constructorParameters.Length; i++) + { + ParameterInfo constructorParameter = constructorParameters[i]; + bool hasDefaultValue = ParameterDefaultValue.TryGetDefaultValue(constructorParameter, out object? defaultValue); + + parameters.Add(new FactoryParameterContext(constructorParameter, hasDefaultValue, defaultValue)); + } + + return (IServiceProvider serviceProvider, object?[]? arguments) => + { + var constructorArguments = new object?[parameters.Count]; + for (int i = 0; i < parameters.Count; i++) + { + var parameter = parameters[i]; + if (parameterMap[i] is { } index) + { + constructorArguments[i] = arguments?[index]; + } + else + { + constructorArguments[i] = GetService( + serviceProvider, + parameter.ParameterInfo.ParameterType, + constructor.DeclaringType!, + parameter.HasDefaultValue); + } + if (parameter.HasDefaultValue && constructorArguments[i] == null) + { + constructorArguments[i] = parameter.DefaultValue; + } + } + + return constructor.Invoke(BindingFlags.DoNotWrapExceptions, binder: null, constructorArguments, culture: null); + }; + } +#endif + + private class FactoryParameterContext + { + public FactoryParameterContext(ParameterInfo parameterInfo, bool hasDefaultValue, object? defaultValue) + { + ParameterInfo = parameterInfo; + HasDefaultValue = hasDefaultValue; + DefaultValue = defaultValue; + } + + public ParameterInfo ParameterInfo { get; } + public bool HasDefaultValue { get; } + public object? DefaultValue { get; } + } + private static void FindApplicableConstructor( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, Type?[] argumentTypes, 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 47af6f2558de37..ea54bc67e2a57c 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using Microsoft.DotNet.RemoteExecutor; using Xunit; using static Microsoft.Extensions.DependencyInjection.Tests.AsyncServiceScopeTests; @@ -171,25 +172,38 @@ public void CreateInstance_ClassWithABC_MultipleCtorsWithSameLength_ThrowsAmbigu Assert.Equal(message, exception.Message); } - [Fact] - public void CreateFactory_CreatesFactoryMethod() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] +#if NET6_0_OR_GREATER + [InlineData(false)] +#endif + public void CreateFactory_CreatesFactoryMethod(bool isDynamicCodeSupported) { - var factory1 = ActivatorUtilities.CreateFactory(typeof(ClassWithABCS), new Type[] { typeof(B) }); - var factory2 = ActivatorUtilities.CreateFactory(new Type[] { typeof(B) }); - - var services = new ServiceCollection(); - services.AddSingleton(new A()); - services.AddSingleton(new C()); - services.AddSingleton(new S()); - using var provider = services.BuildServiceProvider(); - object item1 = factory1(provider, new[] { new B() }); - var item2 = factory2(provider, new[] { new B() }); - - Assert.IsType(factory1); - Assert.IsType(item1); + var options = new RemoteInvokeOptions(); + if (!isDynamicCodeSupported) + { + options.RuntimeConfigurationOptions.Add("System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", isDynamicCodeSupported.ToString()); + } - Assert.IsType>(factory2); - Assert.IsType(item2); + using var remoteHandle = RemoteExecutor.Invoke(static () => + { + var factory1 = ActivatorUtilities.CreateFactory(typeof(ClassWithABCS), new Type[] { typeof(B) }); + var factory2 = ActivatorUtilities.CreateFactory(new Type[] { typeof(B) }); + + var services = new ServiceCollection(); + services.AddSingleton(new A()); + services.AddSingleton(new C()); + services.AddSingleton(new S()); + using var provider = services.BuildServiceProvider(); + object item1 = factory1(provider, new[] { new B() }); + var item2 = factory2(provider, new[] { new B() }); + + Assert.IsType(factory1); + Assert.IsType(item1); + + Assert.IsType>(factory2); + Assert.IsType(item2); + }, options); } } From 3938d5d744ac6ce400edf40d0a1447529c42d513 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Fri, 27 Jan 2023 18:35:22 +0800 Subject: [PATCH 02/22] Clean up --- .../src/ActivatorUtilities.cs | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 30075798c29d1e..1f2500bc00b28b 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -130,15 +130,12 @@ public static ObjectFactory CreateFactory( Type[] argumentTypes) { #if NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER - //if (!RuntimeFeature.IsDynamicCodeSupported) + if (!RuntimeFeature.IsDynamicCodeSupported) { - FindApplicableConstructor(instanceType, argumentTypes, out ConstructorInfo constructor, out int?[] parameterMap); - - return CreateFactory(constructor, parameterMap); + return CreateFactoryReflection(instanceType, argumentTypes); } #endif -#if !(NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER) CreateFactoryInternal(instanceType, argumentTypes, out ParameterExpression provider, out ParameterExpression argumentArray, out Expression factoryExpressionBody); var factoryLambda = Expression.Lambda>( @@ -146,7 +143,6 @@ public static ObjectFactory CreateFactory( Func? result = factoryLambda.Compile(); return result.Invoke; -#endif } /// @@ -166,15 +162,14 @@ public static ObjectFactory Type[] argumentTypes) { #if NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER - //if (!RuntimeFeature.IsDynamicCodeSupported) + if (!RuntimeFeature.IsDynamicCodeSupported) { - FindApplicableConstructor(typeof(T), argumentTypes, out ConstructorInfo constructor, out int?[] parameterMap); - var factory = CreateFactory(constructor, parameterMap); - return (IServiceProvider serviceProvider, object?[]? arguments) => (T)factory(serviceProvider, arguments); + var factory = CreateFactoryReflection(typeof(T), argumentTypes); + + return (serviceProvider, arguments) => (T)factory(serviceProvider, arguments); } #endif -#if !(NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER) CreateFactoryInternal(typeof(T), argumentTypes, out ParameterExpression provider, out ParameterExpression argumentArray, out Expression factoryExpressionBody); var factoryLambda = Expression.Lambda>( @@ -182,7 +177,6 @@ public static ObjectFactory Func? result = factoryLambda.Compile(); return result.Invoke; -#endif } private static void CreateFactoryInternal([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, Type[] argumentTypes, out ParameterExpression provider, out ParameterExpression argumentArray, out Expression factoryExpressionBody) @@ -289,10 +283,12 @@ private static NewExpression BuildFactoryExpression( } #if NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER - private static ObjectFactory CreateFactory( - ConstructorInfo constructor, - int?[] parameterMap) + private static ObjectFactory CreateFactoryReflection( + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, + Type?[] argumentTypes) { + FindApplicableConstructor(instanceType, argumentTypes, out ConstructorInfo constructor, out int?[] parameterMap); + ParameterInfo[]? constructorParameters = constructor.GetParameters(); List parameters = new List(); for (int i = 0; i < constructorParameters.Length; i++) From c93f592f03f9cce44b38b207be7e691b2434fc96 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Fri, 27 Jan 2023 18:38:16 +0800 Subject: [PATCH 03/22] Clean up --- .../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 1f2500bc00b28b..61e6e80d129742 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -326,9 +326,8 @@ private static ObjectFactory CreateFactoryReflection( return constructor.Invoke(BindingFlags.DoNotWrapExceptions, binder: null, constructorArguments, culture: null); }; } -#endif - private class FactoryParameterContext + private sealed class FactoryParameterContext { public FactoryParameterContext(ParameterInfo parameterInfo, bool hasDefaultValue, object? defaultValue) { @@ -341,6 +340,7 @@ public FactoryParameterContext(ParameterInfo parameterInfo, bool hasDefaultValue public bool HasDefaultValue { get; } public object? DefaultValue { get; } } +#endif private static void FindApplicableConstructor( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, From f98638f00a3049b5ea5f9545e40dc9e654507cb3 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Fri, 27 Jan 2023 20:31:52 +0800 Subject: [PATCH 04/22] Add argument index to context --- .../src/ActivatorUtilities.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 61e6e80d129742..ace33c1d73af56 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -296,7 +296,7 @@ private static ObjectFactory CreateFactoryReflection( ParameterInfo constructorParameter = constructorParameters[i]; bool hasDefaultValue = ParameterDefaultValue.TryGetDefaultValue(constructorParameter, out object? defaultValue); - parameters.Add(new FactoryParameterContext(constructorParameter, hasDefaultValue, defaultValue)); + parameters.Add(new FactoryParameterContext(constructorParameter, hasDefaultValue, defaultValue, parameterMap[i])); } return (IServiceProvider serviceProvider, object?[]? arguments) => @@ -304,8 +304,8 @@ private static ObjectFactory CreateFactoryReflection( var constructorArguments = new object?[parameters.Count]; for (int i = 0; i < parameters.Count; i++) { - var parameter = parameters[i]; - if (parameterMap[i] is { } index) + FactoryParameterContext parameter = parameters[i]; + if (parameter.ArgumentIndex is { } index) { constructorArguments[i] = arguments?[index]; } @@ -329,16 +329,18 @@ private static ObjectFactory CreateFactoryReflection( private sealed class FactoryParameterContext { - public FactoryParameterContext(ParameterInfo parameterInfo, bool hasDefaultValue, object? defaultValue) + public FactoryParameterContext(ParameterInfo parameterInfo, bool hasDefaultValue, object? defaultValue, int? argumentIndex) { ParameterInfo = parameterInfo; HasDefaultValue = hasDefaultValue; DefaultValue = defaultValue; + ArgumentIndex = argumentIndex; } public ParameterInfo ParameterInfo { get; } public bool HasDefaultValue { get; } public object? DefaultValue { get; } + public int? ArgumentIndex { get; } } #endif From 07e78a50d4eb231e5d45ecf91a6fae2428122a55 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 05:27:53 +0800 Subject: [PATCH 05/22] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../src/ActivatorUtilities.cs | 10 +++++----- 1 file changed, 5 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 ace33c1d73af56..de71f5cfada7e8 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -289,20 +289,20 @@ private static ObjectFactory CreateFactoryReflection( { FindApplicableConstructor(instanceType, argumentTypes, out ConstructorInfo constructor, out int?[] parameterMap); - ParameterInfo[]? constructorParameters = constructor.GetParameters(); - List parameters = new List(); + ParameterInfo[] constructorParameters = constructor.GetParameters(); + FactoryParameterContext[] parameters = new FactoryParameterContext[constructorParameters.Length]; for (int i = 0; i < constructorParameters.Length; i++) { ParameterInfo constructorParameter = constructorParameters[i]; bool hasDefaultValue = ParameterDefaultValue.TryGetDefaultValue(constructorParameter, out object? defaultValue); - parameters.Add(new FactoryParameterContext(constructorParameter, hasDefaultValue, defaultValue, parameterMap[i])); + parameters[i] = new FactoryParameterContext(constructorParameter, hasDefaultValue, defaultValue, parameterMap[i]); } return (IServiceProvider serviceProvider, object?[]? arguments) => { - var constructorArguments = new object?[parameters.Count]; - for (int i = 0; i < parameters.Count; i++) + var constructorArguments = new object?[parameters.Length]; + for (int i = 0; i < parameters.Length; i++) { FactoryParameterContext parameter = parameters[i]; if (parameter.ArgumentIndex is { } index) From 395f4df9465a14f0a2d3661361dcb230efc441d4 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 05:37:14 +0800 Subject: [PATCH 06/22] PR feedback --- .../src/ActivatorUtilities.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index de71f5cfada7e8..4545a5d293b62c 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -129,7 +129,7 @@ public static ObjectFactory CreateFactory( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, Type[] argumentTypes) { -#if NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER +#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP if (!RuntimeFeature.IsDynamicCodeSupported) { return CreateFactoryReflection(instanceType, argumentTypes); @@ -161,7 +161,7 @@ public static ObjectFactory CreateFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>( Type[] argumentTypes) { -#if NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER +#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP if (!RuntimeFeature.IsDynamicCodeSupported) { var factory = CreateFactoryReflection(typeof(T), argumentTypes); @@ -282,7 +282,7 @@ private static NewExpression BuildFactoryExpression( return Expression.New(constructor, constructorArguments); } -#if NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER +#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP private static ObjectFactory CreateFactoryReflection( [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type instanceType, Type?[] argumentTypes) @@ -296,7 +296,7 @@ private static ObjectFactory CreateFactoryReflection( ParameterInfo constructorParameter = constructorParameters[i]; bool hasDefaultValue = ParameterDefaultValue.TryGetDefaultValue(constructorParameter, out object? defaultValue); - parameters[i] = new FactoryParameterContext(constructorParameter, hasDefaultValue, defaultValue, parameterMap[i]); + parameters[i] = new FactoryParameterContext(constructorParameter.ParameterType, hasDefaultValue, defaultValue, parameterMap[i] ?? -1); } return (IServiceProvider serviceProvider, object?[]? arguments) => @@ -305,15 +305,15 @@ private static ObjectFactory CreateFactoryReflection( for (int i = 0; i < parameters.Length; i++) { FactoryParameterContext parameter = parameters[i]; - if (parameter.ArgumentIndex is { } index) + if (parameter.ArgumentIndex != -1) { - constructorArguments[i] = arguments?[index]; + constructorArguments[i] = arguments?[parameter.ArgumentIndex]; } else { constructorArguments[i] = GetService( serviceProvider, - parameter.ParameterInfo.ParameterType, + parameter.ParameterType, constructor.DeclaringType!, parameter.HasDefaultValue); } @@ -327,20 +327,20 @@ private static ObjectFactory CreateFactoryReflection( }; } - private sealed class FactoryParameterContext + private readonly struct FactoryParameterContext { - public FactoryParameterContext(ParameterInfo parameterInfo, bool hasDefaultValue, object? defaultValue, int? argumentIndex) + public FactoryParameterContext(Type parameterType, bool hasDefaultValue, object? defaultValue, int argumentIndex) { - ParameterInfo = parameterInfo; + ParameterType = parameterType; HasDefaultValue = hasDefaultValue; DefaultValue = defaultValue; ArgumentIndex = argumentIndex; } - public ParameterInfo ParameterInfo { get; } + public Type ParameterType { get; } public bool HasDefaultValue { get; } public object? DefaultValue { get; } - public int? ArgumentIndex { get; } + public int ArgumentIndex { get; } } #endif From d78435109231207b7ba0f7ce20a0866e17bf5d9d Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 05:48:58 +0800 Subject: [PATCH 07/22] PR feedback --- .../src/ActivatorUtilities.cs | 43 +++++++++++-------- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 23 ++++++++++ 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 4545a5d293b62c..00afa0b97cf743 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -301,25 +301,34 @@ private static ObjectFactory CreateFactoryReflection( return (IServiceProvider serviceProvider, object?[]? arguments) => { - var constructorArguments = new object?[parameters.Length]; - for (int i = 0; i < parameters.Length; i++) + object?[] constructorArguments; + if (parameters.Length == 0) { - FactoryParameterContext parameter = parameters[i]; - if (parameter.ArgumentIndex != -1) - { - constructorArguments[i] = arguments?[parameter.ArgumentIndex]; - } - else - { - constructorArguments[i] = GetService( - serviceProvider, - parameter.ParameterType, - constructor.DeclaringType!, - parameter.HasDefaultValue); - } - if (parameter.HasDefaultValue && constructorArguments[i] == null) + constructorArguments = Array.Empty(); + } + else + { + constructorArguments = new object?[parameters.Length]; + for (int i = 0; i < parameters.Length; i++) { - constructorArguments[i] = parameter.DefaultValue; + FactoryParameterContext parameter = parameters[i]; + if (parameter.ArgumentIndex != -1) + { + // Throws an NullReferenceException if arguments is null. Consistent with expression-based factory. + constructorArguments[i] = arguments![parameter.ArgumentIndex]; + } + else + { + constructorArguments[i] = GetService( + serviceProvider, + parameter.ParameterType, + constructor.DeclaringType!, + parameter.HasDefaultValue); + } + if (parameter.HasDefaultValue && constructorArguments[i] == null) + { + constructorArguments[i] = parameter.DefaultValue; + } } } 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 ea54bc67e2a57c..7f729bebf9a07b 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -205,6 +205,29 @@ public void CreateFactory_CreatesFactoryMethod(bool isDynamicCodeSupported) Assert.IsType(item2); }, options); } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] +#if NET6_0_OR_GREATER + [InlineData(false)] +#endif + public void CreateFactory_NullArguments_Throws(bool isDynamicCodeSupported) + { + var options = new RemoteInvokeOptions(); + if (!isDynamicCodeSupported) + { + options.RuntimeConfigurationOptions.Add("System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", isDynamicCodeSupported.ToString()); + } + + using var remoteHandle = RemoteExecutor.Invoke(static () => + { + var factory1 = ActivatorUtilities.CreateFactory(typeof(ClassWithA), new Type[] { typeof(A) }); + + var services = new ServiceCollection(); + using var provider = services.BuildServiceProvider(); + Assert.Throws(() => factory1(provider, null)); + }, options); + } } internal class A { } From 130ed59b8e205f68d76b667c2eed85a9da46e592 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 06:01:20 +0800 Subject: [PATCH 08/22] PR feedback --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 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 7f729bebf9a07b..22fdc859dd9be8 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -172,12 +172,33 @@ public void CreateInstance_ClassWithABC_MultipleCtorsWithSameLength_ThrowsAmbigu Assert.Equal(message, exception.Message); } + [Fact] + public void CreateFactory_CreatesFactoryMethod() + { + var factory1 = ActivatorUtilities.CreateFactory(typeof(ClassWithABCS), new Type[] { typeof(B) }); + var factory2 = ActivatorUtilities.CreateFactory(new Type[] { typeof(B) }); + + var services = new ServiceCollection(); + services.AddSingleton(new A()); + services.AddSingleton(new C()); + services.AddSingleton(new S()); + using var provider = services.BuildServiceProvider(); + object item1 = factory1(provider, new[] { new B() }); + var item2 = factory2(provider, new[] { new B() }); + + Assert.IsType(factory1); + Assert.IsType(item1); + + Assert.IsType>(factory2); + Assert.IsType(item2); + } + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [InlineData(true)] -#if NET6_0_OR_GREATER +#if NETCOREAPP [InlineData(false)] #endif - public void CreateFactory_CreatesFactoryMethod(bool isDynamicCodeSupported) + public void CreateFactory_RemoteExecutor_CreatesFactoryMethod(bool isDynamicCodeSupported) { var options = new RemoteInvokeOptions(); if (!isDynamicCodeSupported) @@ -208,10 +229,10 @@ public void CreateFactory_CreatesFactoryMethod(bool isDynamicCodeSupported) [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [InlineData(true)] -#if NET6_0_OR_GREATER +#if NETCOREAPP [InlineData(false)] #endif - public void CreateFactory_NullArguments_Throws(bool isDynamicCodeSupported) + public void CreateFactory_RemoteExecutor_NullArguments_Throws(bool isDynamicCodeSupported) { var options = new RemoteInvokeOptions(); if (!isDynamicCodeSupported) From 550a65769d0c681599e86d5324107ea2ccadde88 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 06:23:12 +0800 Subject: [PATCH 09/22] More tests --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 57 +++++++++++++++++++ 1 file changed, 57 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 22fdc859dd9be8..8c39f811660cd5 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -249,6 +249,54 @@ public void CreateFactory_RemoteExecutor_NullArguments_Throws(bool isDynamicCode Assert.Throws(() => factory1(provider, null)); }, options); } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] +#if NETCOREAPP + [InlineData(false)] +#endif + public void CreateFactory_RemoteExecutor_NoArguments_UseDefaultValue(bool isDynamicCodeSupported) + { + var options = new RemoteInvokeOptions(); + if (!isDynamicCodeSupported) + { + options.RuntimeConfigurationOptions.Add("System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", isDynamicCodeSupported.ToString()); + } + + using var remoteHandle = RemoteExecutor.Invoke(static () => + { + var factory1 = ActivatorUtilities.CreateFactory(typeof(ClassWithADefaultValue), new Type[0]); + + var services = new ServiceCollection(); + using var provider = services.BuildServiceProvider(); + var item = (ClassWithADefaultValue)factory1(provider, null); + Assert.Null(item.A); + }, options); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] +#if NETCOREAPP + [InlineData(false)] +#endif + public void CreateFactory_RemoteExecutor_NoArguments_ThrowRequiredValue(bool isDynamicCodeSupported) + { + var options = new RemoteInvokeOptions(); + if (!isDynamicCodeSupported) + { + options.RuntimeConfigurationOptions.Add("System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", isDynamicCodeSupported.ToString()); + } + + using var remoteHandle = RemoteExecutor.Invoke(static () => + { + var factory1 = ActivatorUtilities.CreateFactory(typeof(ClassWithA), new Type[0]); + + var services = new ServiceCollection(); + using var provider = services.BuildServiceProvider(); + var ex = Assert.Throws(() => factory1(provider, null)); + Assert.Equal($"Unable to resolve service for type '{typeof(A).FullName}' while attempting to activate '{typeof(ClassWithA).FullName}'.", ex.Message); + }, options); + } } internal class A { } @@ -323,6 +371,15 @@ public ClassWithA(A a) } } + internal class ClassWithADefaultValue + { + public A A { get; } + public ClassWithADefaultValue(A a = null) + { + A = a; + } + } + internal class ABCS { public A A { get; } From 1a95d874fc57f0825adcb82c4457e7132dfd55f9 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 06:34:02 +0800 Subject: [PATCH 10/22] PR feedback --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 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 8c39f811660cd5..641883280ff0a7 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -203,7 +203,7 @@ public void CreateFactory_RemoteExecutor_CreatesFactoryMethod(bool isDynamicCode var options = new RemoteInvokeOptions(); if (!isDynamicCodeSupported) { - options.RuntimeConfigurationOptions.Add("System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", isDynamicCodeSupported.ToString()); + options.RuntimeConfigurationOptions.Add("System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", "false"); } using var remoteHandle = RemoteExecutor.Invoke(static () => @@ -237,7 +237,7 @@ public void CreateFactory_RemoteExecutor_NullArguments_Throws(bool isDynamicCode var options = new RemoteInvokeOptions(); if (!isDynamicCodeSupported) { - options.RuntimeConfigurationOptions.Add("System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", isDynamicCodeSupported.ToString()); + options.RuntimeConfigurationOptions.Add("System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", "false"); } using var remoteHandle = RemoteExecutor.Invoke(static () => @@ -260,7 +260,7 @@ public void CreateFactory_RemoteExecutor_NoArguments_UseDefaultValue(bool isDyna var options = new RemoteInvokeOptions(); if (!isDynamicCodeSupported) { - options.RuntimeConfigurationOptions.Add("System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", isDynamicCodeSupported.ToString()); + options.RuntimeConfigurationOptions.Add("System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", "false"); } using var remoteHandle = RemoteExecutor.Invoke(static () => @@ -284,7 +284,7 @@ public void CreateFactory_RemoteExecutor_NoArguments_ThrowRequiredValue(bool isD var options = new RemoteInvokeOptions(); if (!isDynamicCodeSupported) { - options.RuntimeConfigurationOptions.Add("System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", isDynamicCodeSupported.ToString()); + options.RuntimeConfigurationOptions.Add("System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", "false"); } using var remoteHandle = RemoteExecutor.Invoke(static () => From 274c35fd4db712bcd68fb220234020f83194299c Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 06:37:00 +0800 Subject: [PATCH 11/22] Update src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs Co-authored-by: Jan Kotas --- .../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 00afa0b97cf743..bc65913aaafe3b 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -311,7 +311,7 @@ private static ObjectFactory CreateFactoryReflection( constructorArguments = new object?[parameters.Length]; for (int i = 0; i < parameters.Length; i++) { - FactoryParameterContext parameter = parameters[i]; + ref FactoryParameterContext parameter = ref parameters[i]; if (parameter.ArgumentIndex != -1) { // Throws an NullReferenceException if arguments is null. Consistent with expression-based factory. From a69cc92f359e42791349ed41902023a3a3c4a2c0 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 06:45:08 +0800 Subject: [PATCH 12/22] PR feedback --- .../src/ActivatorUtilities.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index bc65913aaafe3b..16188b3a3a0adb 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -298,15 +298,12 @@ private static ObjectFactory CreateFactoryReflection( parameters[i] = new FactoryParameterContext(constructorParameter.ParameterType, hasDefaultValue, defaultValue, parameterMap[i] ?? -1); } + Type declaringType = constructor.DeclaringType!; return (IServiceProvider serviceProvider, object?[]? arguments) => { - object?[] constructorArguments; - if (parameters.Length == 0) - { - constructorArguments = Array.Empty(); - } - else + object?[]? constructorArguments = null; + if (parameters.Length != 0) { constructorArguments = new object?[parameters.Length]; for (int i = 0; i < parameters.Length; i++) @@ -322,7 +319,7 @@ private static ObjectFactory CreateFactoryReflection( constructorArguments[i] = GetService( serviceProvider, parameter.ParameterType, - constructor.DeclaringType!, + declaringType, parameter.HasDefaultValue); } if (parameter.HasDefaultValue && constructorArguments[i] == null) From fa765c3f731dd85617ea34279edb7d7e3fd9d466 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 07:00:00 +0800 Subject: [PATCH 13/22] Update src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs Co-authored-by: Jan Kotas --- .../src/ActivatorUtilities.cs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index 16188b3a3a0adb..457fb25a2858c0 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -309,22 +309,15 @@ private static ObjectFactory CreateFactoryReflection( for (int i = 0; i < parameters.Length; i++) { ref FactoryParameterContext parameter = ref parameters[i]; - if (parameter.ArgumentIndex != -1) - { + constructorArguments[i] = ((parameter.ArgumentIndex != -1) ? // Throws an NullReferenceException if arguments is null. Consistent with expression-based factory. - constructorArguments[i] = arguments![parameter.ArgumentIndex]; - } - else - { - constructorArguments[i] = GetService( + arguments![parameter.ArgumentIndex] + : + GetService( serviceProvider, parameter.ParameterType, - declaringType, - parameter.HasDefaultValue); - } - if (parameter.HasDefaultValue && constructorArguments[i] == null) - { - constructorArguments[i] = parameter.DefaultValue; + constructor.DeclaringType!, + parameter.HasDefaultValue)) ?? parameter.DefaultValue; } } } From fdf1e4c37a19ec80c3a1ba0c4ca876c02455e87e Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 07:02:46 +0800 Subject: [PATCH 14/22] Remove extra end brace --- .../src/ActivatorUtilities.cs | 8 +++----- 1 file changed, 3 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 457fb25a2858c0..b467af02d46ca3 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -309,16 +309,14 @@ private static ObjectFactory CreateFactoryReflection( for (int i = 0; i < parameters.Length; i++) { ref FactoryParameterContext parameter = ref parameters[i]; - constructorArguments[i] = ((parameter.ArgumentIndex != -1) ? + constructorArguments[i] = ((parameter.ArgumentIndex != -1) // Throws an NullReferenceException if arguments is null. Consistent with expression-based factory. - arguments![parameter.ArgumentIndex] - : - GetService( + ? arguments![parameter.ArgumentIndex] + : GetService( serviceProvider, parameter.ParameterType, constructor.DeclaringType!, parameter.HasDefaultValue)) ?? parameter.DefaultValue; - } } } From 69dfa1080b24c2c5ab34fe697c38fcf13ca23900 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 27 Jan 2023 15:07:17 -0800 Subject: [PATCH 15/22] Update src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs --- .../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 b467af02d46ca3..0d83ae51774b8e 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -315,7 +315,7 @@ private static ObjectFactory CreateFactoryReflection( : GetService( serviceProvider, parameter.ParameterType, - constructor.DeclaringType!, + declaringType, parameter.HasDefaultValue)) ?? parameter.DefaultValue; } } From e99b5adea32312736c046ac4baf389363c9a59fb Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 07:13:05 +0800 Subject: [PATCH 16/22] Add test --- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 35 ++++++++++++++++++- 1 file changed, 34 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 641883280ff0a7..4523709ef657ad 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -255,7 +255,7 @@ public void CreateFactory_RemoteExecutor_NullArguments_Throws(bool isDynamicCode #if NETCOREAPP [InlineData(false)] #endif - public void CreateFactory_RemoteExecutor_NoArguments_UseDefaultValue(bool isDynamicCodeSupported) + public void CreateFactory_RemoteExecutor_NoArguments_UseNullDefaultValue(bool isDynamicCodeSupported) { var options = new RemoteInvokeOptions(); if (!isDynamicCodeSupported) @@ -297,6 +297,30 @@ public void CreateFactory_RemoteExecutor_NoArguments_ThrowRequiredValue(bool isD Assert.Equal($"Unable to resolve service for type '{typeof(A).FullName}' while attempting to activate '{typeof(ClassWithA).FullName}'.", ex.Message); }, options); } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] +#if NETCOREAPP + [InlineData(false)] +#endif + public void CreateFactory_RemoteExecutor_NoArguments_UseDefaultValue(bool isDynamicCodeSupported) + { + var options = new RemoteInvokeOptions(); + if (!isDynamicCodeSupported) + { + options.RuntimeConfigurationOptions.Add("System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", "false"); + } + + using var remoteHandle = RemoteExecutor.Invoke(static () => + { + var factory1 = ActivatorUtilities.CreateFactory(typeof(ClassWithStringDefaultValue), new[] { typeof(string) }); + + var services = new ServiceCollection(); + using var provider = services.BuildServiceProvider(); + var item = (ClassWithStringDefaultValue)factory1(provider, new object[] { null }); + Assert.Equal("DEFAULT", item.Text); + }, options); + } } internal class A { } @@ -469,4 +493,13 @@ public ClassWithABC_DefaultConstructorLast(A a, B b) : base (a, b) { } public ClassWithABC_DefaultConstructorLast(A a) : base(a) { } public ClassWithABC_DefaultConstructorLast() : base() { } } + + internal class ClassWithStringDefaultValue + { + public string Text { get; set; } + public ClassWithStringDefaultValue(string text = "DEFAULT") + { + Text = text; + } + } } From 26638d86eee628d294b701787a37786910a8b6ec Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 07:17:58 +0800 Subject: [PATCH 17/22] Fix test name --- .../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 4523709ef657ad..ec7939e13ef79d 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -303,7 +303,7 @@ public void CreateFactory_RemoteExecutor_NoArguments_ThrowRequiredValue(bool isD #if NETCOREAPP [InlineData(false)] #endif - public void CreateFactory_RemoteExecutor_NoArguments_UseDefaultValue(bool isDynamicCodeSupported) + public void CreateFactory_RemoteExecutor_NullArgument_UseDefaultValue(bool isDynamicCodeSupported) { var options = new RemoteInvokeOptions(); if (!isDynamicCodeSupported) From bd8af416530b901d62540be0f23d7152d7cbe293 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 11:22:55 +0800 Subject: [PATCH 18/22] Remove unused namespace --- .../src/ActivatorUtilities.cs | 1 - 1 file changed, 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 0d83ae51774b8e..b7957f48c55ea4 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; From ba02aa1ea6f07305cdc97b625faf0e576ffd47f7 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 12:22:32 +0800 Subject: [PATCH 19/22] Apply suggestions from code review Co-authored-by: Jan Kotas --- .../src/ActivatorUtilities.cs | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs index b7957f48c55ea4..c9f7c411302915 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -289,6 +289,11 @@ private static ObjectFactory CreateFactoryReflection( FindApplicableConstructor(instanceType, argumentTypes, out ConstructorInfo constructor, out int?[] parameterMap); ParameterInfo[] constructorParameters = constructor.GetParameters(); + if (constructorParameters.Length == 0) + { + return (IServiceProvider serviceProvider, object?[]? arguments) => + constructor.Invoke(BindingFlags.DoNotWrapExceptions, binder: null, parameters: null, culture: null); + } FactoryParameterContext[] parameters = new FactoryParameterContext[constructorParameters.Length]; for (int i = 0; i < constructorParameters.Length; i++) { @@ -301,23 +306,19 @@ private static ObjectFactory CreateFactoryReflection( return (IServiceProvider serviceProvider, object?[]? arguments) => { - object?[]? constructorArguments = null; - if (parameters.Length != 0) + object?[] constructorArguments = new object?[parameters.Length]; + for (int i = 0; i < parameters.Length; i++) { - constructorArguments = new object?[parameters.Length]; - for (int i = 0; i < parameters.Length; i++) - { - ref FactoryParameterContext parameter = ref parameters[i]; - constructorArguments[i] = ((parameter.ArgumentIndex != -1) - // Throws an NullReferenceException if arguments is null. Consistent with expression-based factory. - ? arguments![parameter.ArgumentIndex] - : GetService( - serviceProvider, - parameter.ParameterType, - declaringType, - parameter.HasDefaultValue)) ?? parameter.DefaultValue; + ref FactoryParameterContext parameter = ref parameters[i]; + constructorArguments[i] = ((parameter.ArgumentIndex != -1) + // Throws an NullReferenceException if arguments is null. Consistent with expression-based factory. + ? arguments![parameter.ArgumentIndex] + : GetService( + serviceProvider, + parameter.ParameterType, + declaringType, + parameter.HasDefaultValue)) ?? parameter.DefaultValue; } - } return constructor.Invoke(BindingFlags.DoNotWrapExceptions, binder: null, constructorArguments, culture: null); }; From 1bc528df807b3ed0c39143cbaab89b852a77b5a6 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 12:28:04 +0800 Subject: [PATCH 20/22] Indentation and test --- .../src/ActivatorUtilities.cs | 3 ++- .../tests/DI.Tests/ActivatorUtilitiesTests.cs | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+), 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 c9f7c411302915..652f2bf2c470e7 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -294,6 +294,7 @@ private static ObjectFactory CreateFactoryReflection( return (IServiceProvider serviceProvider, object?[]? arguments) => constructor.Invoke(BindingFlags.DoNotWrapExceptions, binder: null, parameters: null, culture: null); } + FactoryParameterContext[] parameters = new FactoryParameterContext[constructorParameters.Length]; for (int i = 0; i < constructorParameters.Length; i++) { @@ -318,7 +319,7 @@ private static ObjectFactory CreateFactoryReflection( parameter.ParameterType, declaringType, parameter.HasDefaultValue)) ?? parameter.DefaultValue; - } + } return constructor.Invoke(BindingFlags.DoNotWrapExceptions, binder: null, constructorArguments, culture: null); }; 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 ec7939e13ef79d..76c47d22b9f921 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -321,6 +321,30 @@ public void CreateFactory_RemoteExecutor_NullArgument_UseDefaultValue(bool isDyn Assert.Equal("DEFAULT", item.Text); }, options); } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] +#if NETCOREAPP + [InlineData(false)] +#endif + public void CreateFactory_RemoteExecutor_NoParameters_Success(bool isDynamicCodeSupported) + { + var options = new RemoteInvokeOptions(); + if (!isDynamicCodeSupported) + { + options.RuntimeConfigurationOptions.Add("System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported", "false"); + } + + using var remoteHandle = RemoteExecutor.Invoke(static () => + { + var factory1 = ActivatorUtilities.CreateFactory(typeof(A), new Type[0]); + + var services = new ServiceCollection(); + using var provider = services.BuildServiceProvider(); + var item = (A)factory1(provider, new object[] { null }); + Assert.NotNull(item); + }, options); + } } internal class A { } From 4fed4d3eb648c79bcbc9e7505a19f35ba89f9648 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 28 Jan 2023 12:33:08 +0800 Subject: [PATCH 21/22] Clean up --- .../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 76c47d22b9f921..f867f014906eef 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs @@ -341,7 +341,7 @@ public void CreateFactory_RemoteExecutor_NoParameters_Success(bool isDynamicCode var services = new ServiceCollection(); using var provider = services.BuildServiceProvider(); - var item = (A)factory1(provider, new object[] { null }); + var item = (A)factory1(provider, null); Assert.NotNull(item); }, options); } From 9c0a3dd712461bce8c269c2346268cc9c5cc1bbb Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Mon, 30 Jan 2023 10:50:20 +0800 Subject: [PATCH 22/22] Apply suggestions from code review --- .../src/ActivatorUtilities.cs | 5 ++++- 1 file changed, 4 insertions(+), 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 652f2bf2c470e7..5a42e1217f3832 100644 --- a/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs +++ b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs @@ -131,6 +131,8 @@ public static ObjectFactory CreateFactory( #if NETSTANDARD2_1_OR_GREATER || NETCOREAPP if (!RuntimeFeature.IsDynamicCodeSupported) { + // Create a reflection-based factory when dynamic code isn't supported, e.g. app is published with NativeAOT. + // Reflection-based factory is faster than interpreted expressions and doesn't pull in System.Linq.Expressions dependency. return CreateFactoryReflection(instanceType, argumentTypes); } #endif @@ -163,8 +165,9 @@ public static ObjectFactory #if NETSTANDARD2_1_OR_GREATER || NETCOREAPP if (!RuntimeFeature.IsDynamicCodeSupported) { + // Create a reflection-based factory when dynamic code isn't supported, e.g. app is published with NativeAOT. + // Reflection-based factory is faster than interpreted expressions and doesn't pull in System.Linq.Expressions dependency. var factory = CreateFactoryReflection(typeof(T), argumentTypes); - return (serviceProvider, arguments) => (T)factory(serviceProvider, arguments); } #endif