From 15b28dd97999fa1dedd98a4a7c99428f57362aae Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 22 Apr 2022 10:23:41 -0700 Subject: [PATCH 01/63] Core funcionality --- .../src/ParametersAttribute.cs | 14 ++ .../src/PublicAPI.Unshipped.txt | 2 + .../src/RequestDelegateFactory.cs | 179 +++++++++++++++--- 3 files changed, 172 insertions(+), 23 deletions(-) create mode 100644 src/Http/Http.Extensions/src/ParametersAttribute.cs diff --git a/src/Http/Http.Extensions/src/ParametersAttribute.cs b/src/Http/Http.Extensions/src/ParametersAttribute.cs new file mode 100644 index 000000000000..9ae4ccf24899 --- /dev/null +++ b/src/Http/Http.Extensions/src/ParametersAttribute.cs @@ -0,0 +1,14 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Http; + +using System; + +/// +/// +/// +[AttributeUsage(AttributeTargets.Parameter, Inherited = false, AllowMultiple = false)] +public sealed class ParametersAttribute : Attribute +{ +} diff --git a/src/Http/Http.Extensions/src/PublicAPI.Unshipped.txt b/src/Http/Http.Extensions/src/PublicAPI.Unshipped.txt index 93e10e87b5a6..4d91a73a4d15 100644 --- a/src/Http/Http.Extensions/src/PublicAPI.Unshipped.txt +++ b/src/Http/Http.Extensions/src/PublicAPI.Unshipped.txt @@ -13,6 +13,8 @@ Microsoft.AspNetCore.Http.Metadata.IEndpointMetadataProvider Microsoft.AspNetCore.Http.Metadata.IEndpointMetadataProvider.PopulateMetadata(Microsoft.AspNetCore.Http.Metadata.EndpointMetadataContext! context) -> void Microsoft.AspNetCore.Http.Metadata.IEndpointParameterMetadataProvider Microsoft.AspNetCore.Http.Metadata.IEndpointParameterMetadataProvider.PopulateMetadata(Microsoft.AspNetCore.Http.Metadata.EndpointParameterMetadataContext! parameterContext) -> void +Microsoft.AspNetCore.Http.ParametersAttribute +Microsoft.AspNetCore.Http.ParametersAttribute.ParametersAttribute() -> void Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.InitialEndpointMetadata.get -> System.Collections.Generic.IEnumerable? Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.InitialEndpointMetadata.init -> void Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.RouteHandlerFilterFactories.get -> System.Collections.Generic.IReadOnlyList!>? diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 7b25889d0c0c..e53a4baaeb81 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -214,7 +214,10 @@ private static FactoryContext CreateFactoryContext(RequestDelegateFactoryOptions factoryContext.MethodCall = CreateMethodCall(methodInfo, targetExpression, arguments); // Add metadata provided by the delegate return type and parameter types next, this will be more specific than inferred metadata from above - AddTypeProvidedMetadata(methodInfo, factoryContext.Metadata, factoryContext.ServiceProvider); + AddTypeProvidedMetadata(methodInfo, + factoryContext.Metadata, + factoryContext.ServiceProvider, + factoryContext.SurrogatedParameters.ToArray()); // Add method attributes as metadata *after* any inferred metadata so that the attributes hava a higher specificity AddMethodAttributesAsMetadata(methodInfo, factoryContext.Metadata); @@ -283,33 +286,40 @@ private static RouteHandlerFilterDelegate CreateFilterPipeline(MethodInfo method return filteredInvocation; } - private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List metadata, IServiceProvider? services) + private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List metadata, IServiceProvider? services, ParameterInfo[] surrogatedParameters) { object?[]? invokeArgs = null; - // Get metadata from parameter types - var parameters = methodInfo.GetParameters(); - foreach (var parameter in parameters) + void AddMetadata(ParameterInfo[] parameters) { - if (typeof(IEndpointParameterMetadataProvider).IsAssignableFrom(parameter.ParameterType)) + foreach (var parameter in parameters) { - // Parameter type implements IEndpointParameterMetadataProvider - var parameterContext = new EndpointParameterMetadataContext(parameter, metadata, services); - invokeArgs ??= new object[1]; - invokeArgs[0] = parameterContext; - PopulateMetadataForParameterMethod.MakeGenericMethod(parameter.ParameterType).Invoke(null, invokeArgs); - } + if (typeof(IEndpointParameterMetadataProvider).IsAssignableFrom(parameter.ParameterType)) + { + // Parameter type implements IEndpointParameterMetadataProvider + var parameterContext = new EndpointParameterMetadataContext(parameter, metadata, services); + invokeArgs ??= new object[1]; + invokeArgs[0] = parameterContext; + PopulateMetadataForParameterMethod.MakeGenericMethod(parameter.ParameterType).Invoke(null, invokeArgs); + } - if (typeof(IEndpointMetadataProvider).IsAssignableFrom(parameter.ParameterType)) - { - // Parameter type implements IEndpointMetadataProvider - var context = new EndpointMetadataContext(methodInfo, metadata, services); - invokeArgs ??= new object[1]; - invokeArgs[0] = context; - PopulateMetadataForEndpointMethod.MakeGenericMethod(parameter.ParameterType).Invoke(null, invokeArgs); + if (typeof(IEndpointMetadataProvider).IsAssignableFrom(parameter.ParameterType)) + { + // Parameter type implements IEndpointMetadataProvider + var context = new EndpointMetadataContext(methodInfo, metadata, services); + invokeArgs ??= new object[1]; + invokeArgs[0] = context; + PopulateMetadataForEndpointMethod.MakeGenericMethod(parameter.ParameterType).Invoke(null, invokeArgs); + } } } + // Get metadata from parameter types + AddMetadata(methodInfo.GetParameters()); + + // Get metadata from surrogated parameter types + AddMetadata(surrogatedParameters); + // Get metadata from return type if (methodInfo.ReturnType is not null && typeof(IEndpointMetadataProvider).IsAssignableFrom(methodInfo.ReturnType)) { @@ -391,14 +401,28 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory private static Expression CreateArgument(ParameterInfo parameter, FactoryContext factoryContext) { + var parameterCustomAttributes = parameter.GetCustomAttributes(); + if (parameter.Name is null) { throw new InvalidOperationException($"Encountered a parameter of type '{parameter.ParameterType}' without a name. Parameters must have a name."); } - var parameterCustomAttributes = parameter.GetCustomAttributes(); + if (parameterCustomAttributes.OfType().FirstOrDefault() is { } || + parameter.ParameterType.GetCustomAttributes().OfType().FirstOrDefault() is { }) + { + factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.SurrogatedParameter); + + if (factoryContext.SurrogatedParameter is { }) + { + var errorMessage = BuildErrorMessageForMultipleSurrogatedParameters(factoryContext); + throw new InvalidOperationException(errorMessage); + } - if (parameterCustomAttributes.OfType().FirstOrDefault() is { } routeAttribute) + factoryContext.SurrogatedParameter = parameter; + return CreateSurrogatedArgument(parameter, factoryContext); + } + else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } routeAttribute) { var routeName = routeAttribute.Name ?? parameter.Name; factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.RouteAttribute); @@ -541,6 +565,47 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext } } + private static Expression CreateSurrogatedArgument(ParameterInfo parameter, FactoryContext factoryContext) + { + var properties = parameter.ParameterType.GetProperties(); + + // Only the parameterless constructor is supported + var constructor = parameter.ParameterType.GetConstructor(Array.Empty()); + if (constructor is null) + { + throw new InvalidOperationException("A parameter's type declared with [Parameters] attribute must have a parameterless constructor."); + } + + var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); + + // arg_local = new T(); + // arg_local.Property[0] = expression[0]; + // arg_local.Property[n] = expression[n]; + // arg_local + + var expressions = new Expression[properties.Length + 2]; + expressions[0] = Expression.Assign(argumentExpression, Expression.New(constructor)); + expressions[^1] = argumentExpression; + + for (var i = 0; i < properties.Length; i++) + { + expressions[i + 1] = Expression.Empty(); + + // we DON'T support read-only or init properties + if (properties[i].CanWrite) + { + var propertyExpression = Expression.Property(argumentExpression, properties[i].Name); + var parameterInfo = new SurrogatedParameterInfo(properties[i], factoryContext); + + expressions[i + 1] = Expression.Assign(propertyExpression, CreateArgument(parameterInfo, factoryContext)); + factoryContext.SurrogatedParameters.Add(parameterInfo); + } + } + + factoryContext.ExtraLocals.Add(argumentExpression); + return Expression.Block(expressions); + } + private static Expression CreateMethodCall(MethodInfo methodInfo, Expression? target, Expression[] arguments) => target is null ? Expression.Call(methodInfo, arguments) : @@ -1560,15 +1625,20 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al private static bool IsOptionalParameter(ParameterInfo parameter, FactoryContext factoryContext) { + if (parameter is SurrogatedParameterInfo argument) + { + return argument.IsOptional; + } + // - Parameters representing value or reference types with a default value // under any nullability context are treated as optional. // - Value type parameters without a default value in an oblivious // nullability context are required. // - Reference type parameters without a default value in an oblivious // nullability context are optional. - var nullability = factoryContext.NullabilityContext.Create(parameter); + var nullabilityInfo = factoryContext.NullabilityContext.Create(parameter); return parameter.HasDefaultValue - || nullability.ReadState != NullabilityState.NotNull; + || nullabilityInfo.ReadState != NullabilityState.NotNull; } private static MethodInfo GetMethodInfo(Expression expr) @@ -1792,6 +1862,9 @@ private class FactoryContext public Expression? MethodCall { get; set; } public List BoxedArgs { get; } = new(); public List>? Filters { get; init; } + + public ParameterInfo? SurrogatedParameter { get; set; } + public List SurrogatedParameters { get; } = new(); } private static class RequestDelegateFactoryConstants @@ -1808,6 +1881,52 @@ private static class RequestDelegateFactoryConstants public const string BodyParameter = "Body (Inferred)"; public const string RouteOrQueryStringParameter = "Route or Query String (Inferred)"; public const string FormFileParameter = "Form File (Inferred)"; + public const string SurrogatedParameter = "Surrogate (Attribute)"; + } + + private class SurrogatedParameterInfo : ParameterInfo + { + private readonly PropertyInfo _underlyingProperty; + private readonly NullabilityInfo _nullabilityInfo; + + public SurrogatedParameterInfo(PropertyInfo propertyInfo, FactoryContext factoryContext) + { + Debug.Assert(null != propertyInfo); + + AttrsImpl = (ParameterAttributes)propertyInfo.Attributes; + MemberImpl = propertyInfo; + NameImpl = propertyInfo.Name; + ClassImpl = propertyInfo.PropertyType; + PositionImpl = -1;//parameter.Position; + + _nullabilityInfo = factoryContext.NullabilityContext.Create(propertyInfo); + _underlyingProperty = propertyInfo; + } + + public override bool HasDefaultValue => false; + public override object? DefaultValue => null; + public override int MetadataToken => _underlyingProperty.MetadataToken; + public override object? RawDefaultValue => null; + + public override object[] GetCustomAttributes(Type attributeType, bool inherit) + => _underlyingProperty.GetCustomAttributes(attributeType, inherit); + + public override object[] GetCustomAttributes(bool inherit) + => _underlyingProperty.GetCustomAttributes(inherit); + + public override IList GetCustomAttributesData() + => _underlyingProperty.GetCustomAttributesData(); + + public override Type[] GetOptionalCustomModifiers() + => _underlyingProperty.GetOptionalCustomModifiers(); + + public override Type[] GetRequiredCustomModifiers() + => _underlyingProperty.GetRequiredCustomModifiers(); + + public override bool IsDefined(Type attributeType, bool inherit) + => _underlyingProperty.IsDefined(attributeType, inherit); + + public new bool IsOptional => _nullabilityInfo.ReadState != NullabilityState.NotNull; } private static partial class Log @@ -2025,6 +2144,20 @@ private static string BuildErrorMessageForFormAndJsonBodyParameters(FactoryConte return errorMessage.ToString(); } + private static string BuildErrorMessageForMultipleSurrogatedParameters(FactoryContext factoryContext) + { + var errorMessage = new StringBuilder(); + errorMessage.AppendLine("An action cannot use more than one parameter decorated with [ParametersAttribute]."); + errorMessage.AppendLine("Below is the list of parameters that we found: "); + errorMessage.AppendLine(); + errorMessage.AppendLine(FormattableString.Invariant($"{"Parameter",-20}| {"Source",-30}")); + errorMessage.AppendLine("---------------------------------------------------------------------------------"); + + FormatTrackedParameters(factoryContext, errorMessage); + + return errorMessage.ToString(); + } + private static void FormatTrackedParameters(FactoryContext factoryContext, StringBuilder errorMessage) { foreach (var kv in factoryContext.TrackedParameters) From dbf6e216bda3b948c0e3abadae6d17607f56341d Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 22 Apr 2022 10:25:38 -0700 Subject: [PATCH 02/63] Changing the order --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index e53a4baaeb81..13df6fe94f9f 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -401,13 +401,13 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory private static Expression CreateArgument(ParameterInfo parameter, FactoryContext factoryContext) { - var parameterCustomAttributes = parameter.GetCustomAttributes(); - if (parameter.Name is null) { throw new InvalidOperationException($"Encountered a parameter of type '{parameter.ParameterType}' without a name. Parameters must have a name."); } + var parameterCustomAttributes = parameter.GetCustomAttributes(); + if (parameterCustomAttributes.OfType().FirstOrDefault() is { } || parameter.ParameterType.GetCustomAttributes().OfType().FirstOrDefault() is { }) { From a9a724c586387bca7f80466329660122bdb28fb8 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 22 Apr 2022 10:26:41 -0700 Subject: [PATCH 03/63] Not checking attribute from type --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 13df6fe94f9f..b147d81fa871 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -408,8 +408,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext var parameterCustomAttributes = parameter.GetCustomAttributes(); - if (parameterCustomAttributes.OfType().FirstOrDefault() is { } || - parameter.ParameterType.GetCustomAttributes().OfType().FirstOrDefault() is { }) + if (parameterCustomAttributes.OfType().FirstOrDefault() is { }) { factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.SurrogatedParameter); From e768b4d62f7be30c48bd9d385374e1f149e92370 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 22 Apr 2022 10:32:55 -0700 Subject: [PATCH 04/63] Code cleanup --- .../src/RequestDelegateFactory.cs | 103 +++++++++--------- 1 file changed, 52 insertions(+), 51 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index b147d81fa871..2b5535941b11 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -410,16 +410,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext if (parameterCustomAttributes.OfType().FirstOrDefault() is { }) { - factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.SurrogatedParameter); - - if (factoryContext.SurrogatedParameter is { }) - { - var errorMessage = BuildErrorMessageForMultipleSurrogatedParameters(factoryContext); - throw new InvalidOperationException(errorMessage); - } - - factoryContext.SurrogatedParameter = parameter; - return CreateSurrogatedArgument(parameter, factoryContext); + return BindSurrogatedArgument(parameter, factoryContext); } else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } routeAttribute) { @@ -564,47 +555,6 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext } } - private static Expression CreateSurrogatedArgument(ParameterInfo parameter, FactoryContext factoryContext) - { - var properties = parameter.ParameterType.GetProperties(); - - // Only the parameterless constructor is supported - var constructor = parameter.ParameterType.GetConstructor(Array.Empty()); - if (constructor is null) - { - throw new InvalidOperationException("A parameter's type declared with [Parameters] attribute must have a parameterless constructor."); - } - - var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); - - // arg_local = new T(); - // arg_local.Property[0] = expression[0]; - // arg_local.Property[n] = expression[n]; - // arg_local - - var expressions = new Expression[properties.Length + 2]; - expressions[0] = Expression.Assign(argumentExpression, Expression.New(constructor)); - expressions[^1] = argumentExpression; - - for (var i = 0; i < properties.Length; i++) - { - expressions[i + 1] = Expression.Empty(); - - // we DON'T support read-only or init properties - if (properties[i].CanWrite) - { - var propertyExpression = Expression.Property(argumentExpression, properties[i].Name); - var parameterInfo = new SurrogatedParameterInfo(properties[i], factoryContext); - - expressions[i + 1] = Expression.Assign(propertyExpression, CreateArgument(parameterInfo, factoryContext)); - factoryContext.SurrogatedParameters.Add(parameterInfo); - } - } - - factoryContext.ExtraLocals.Add(argumentExpression); - return Expression.Block(expressions); - } - private static Expression CreateMethodCall(MethodInfo methodInfo, Expression? target, Expression[] arguments) => target is null ? Expression.Call(methodInfo, arguments) : @@ -1134,6 +1084,57 @@ private static Expression GetValueFromProperty(Expression sourceExpression, stri return Expression.Convert(indexExpression, returnType ?? typeof(string)); } + private static Expression BindSurrogatedArgument(ParameterInfo parameter, FactoryContext factoryContext) + { + if (factoryContext.SurrogatedParameter is { }) + { + var errorMessage = BuildErrorMessageForMultipleSurrogatedParameters(factoryContext); + throw new InvalidOperationException(errorMessage); + } + + factoryContext.SurrogatedParameter = parameter; + + var properties = parameter.ParameterType.GetProperties(); + + // Only the parameterless constructor is supported + var constructor = parameter.ParameterType.GetConstructor(Array.Empty()); + if (constructor is null) + { + throw new InvalidOperationException("A type declared with [Parameters] attribute must have a parameterless constructor."); + } + + var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); + + // arg_local = new T(); + // arg_local.Property0 = expression[0]; + // arg_local.PropertyN = expression[n]; + // arg_local + + var expressions = new Expression[properties.Length + 2]; + expressions[0] = Expression.Assign(argumentExpression, Expression.New(constructor)); + expressions[^1] = argumentExpression; + + for (var i = 0; i < properties.Length; i++) + { + expressions[i + 1] = Expression.Empty(); + + // we DON'T support read-only or init properties + if (properties[i].CanWrite) + { + var propertyExpression = Expression.Property(argumentExpression, properties[i].Name); + var parameterInfo = new SurrogatedParameterInfo(properties[i], factoryContext); + + expressions[i + 1] = Expression.Assign(propertyExpression, CreateArgument(parameterInfo, factoryContext)); + factoryContext.SurrogatedParameters.Add(parameterInfo); + } + } + + factoryContext.TrackedParameters.Add(parameter.Name!, RequestDelegateFactoryConstants.SurrogatedParameter); + factoryContext.ExtraLocals.Add(argumentExpression); + + return Expression.Block(expressions); + } + private static Expression BindParameterFromService(ParameterInfo parameter, FactoryContext factoryContext) { var isOptional = IsOptionalParameter(parameter, factoryContext); From 48222148b61d01f82a2d5e5751b05ebfd1ea6e2d Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 22 Apr 2022 11:02:56 -0700 Subject: [PATCH 05/63] Adding missing ParamCheckExpression --- .../src/RequestDelegateFactory.cs | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 2b5535941b11..1057b8dd8ff9 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -401,14 +401,15 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory private static Expression CreateArgument(ParameterInfo parameter, FactoryContext factoryContext) { + var parameterCustomAttributes = parameter.GetCustomAttributes(); + if (parameter.Name is null) { throw new InvalidOperationException($"Encountered a parameter of type '{parameter.ParameterType}' without a name. Parameters must have a name."); } - var parameterCustomAttributes = parameter.GetCustomAttributes(); - - if (parameterCustomAttributes.OfType().FirstOrDefault() is { }) + if (parameterCustomAttributes.OfType().FirstOrDefault() is { } || + parameter.ParameterType.GetCustomAttributes().OfType().FirstOrDefault() is { }) { return BindSurrogatedArgument(parameter, factoryContext); } @@ -1105,14 +1106,14 @@ private static Expression BindSurrogatedArgument(ParameterInfo parameter, Factor var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); - // arg_local = new T(); - // arg_local.Property0 = expression[0]; - // arg_local.PropertyN = expression[n]; - // arg_local + // { + // arg_local = new T(); + // arg_local.Property[0] = expression[0]; + // arg_local.Property[n] = expression[n]; + // } - var expressions = new Expression[properties.Length + 2]; + var expressions = new Expression[properties.Length + 1]; expressions[0] = Expression.Assign(argumentExpression, Expression.New(constructor)); - expressions[^1] = argumentExpression; for (var i = 0; i < properties.Length; i++) { @@ -1131,8 +1132,9 @@ private static Expression BindSurrogatedArgument(ParameterInfo parameter, Factor factoryContext.TrackedParameters.Add(parameter.Name!, RequestDelegateFactoryConstants.SurrogatedParameter); factoryContext.ExtraLocals.Add(argumentExpression); + factoryContext.ParamCheckExpressions.Add(Expression.Block(expressions)); - return Expression.Block(expressions); + return argumentExpression; } private static Expression BindParameterFromService(ParameterInfo parameter, FactoryContext factoryContext) From 74010a298e3356f67d9c9297cf5393f0957d486f Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 22 Apr 2022 14:57:18 -0700 Subject: [PATCH 06/63] Adding support for records and structs --- .../src/RequestDelegateFactory.cs | 106 ++++++++++-------- 1 file changed, 62 insertions(+), 44 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 1057b8dd8ff9..b55515cfcb35 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1087,52 +1087,85 @@ private static Expression GetValueFromProperty(Expression sourceExpression, stri private static Expression BindSurrogatedArgument(ParameterInfo parameter, FactoryContext factoryContext) { - if (factoryContext.SurrogatedParameter is { }) - { - var errorMessage = BuildErrorMessageForMultipleSurrogatedParameters(factoryContext); - throw new InvalidOperationException(errorMessage); - } - - factoryContext.SurrogatedParameter = parameter; - var properties = parameter.ParameterType.GetProperties(); + var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); - // Only the parameterless constructor is supported - var constructor = parameter.ParameterType.GetConstructor(Array.Empty()); - if (constructor is null) + ConstructorInfo? GetConstructor() { - throw new InvalidOperationException("A type declared with [Parameters] attribute must have a parameterless constructor."); - } + if (parameter.ParameterType.IsValueType) + { + // Value types always have a default constructor, we will use + // the parameter type during the NewExpression creation + return null; + } - var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); + // Try to find the parameterless constructor + var constructor = parameter.ParameterType.GetConstructor(Array.Empty()); + if (constructor is { }) + { + return constructor; + } - // { - // arg_local = new T(); - // arg_local.Property[0] = expression[0]; - // arg_local.Property[n] = expression[n]; - // } + // If a parameterless ctor is not defined + // we will try to find a ctor that includes all the property types. + var types = new Type[properties.Length]; + for (var i = 0; i < properties.Length; i++) + { + types[i] = properties[i].PropertyType; + } + constructor = parameter.ParameterType.GetConstructor(types); - var expressions = new Expression[properties.Length + 1]; - expressions[0] = Expression.Assign(argumentExpression, Expression.New(constructor)); + if (constructor is { }) + { + return constructor; + } - for (var i = 0; i < properties.Length; i++) + throw new InvalidOperationException($"No '{parameter.ParameterType}' public parameterless constructor found for {parameter.Name}."); + } + + var constructor = GetConstructor(); + if (constructor?.GetParameters() is { Length: > 0 } parameters) { - expressions[i + 1] = Expression.Empty(); + // arg_local = new T(....) + var constructorArguments = new Expression[properties.Length]; - // we DON'T support read-only or init properties - if (properties[i].CanWrite) + for (var i = 0; i < properties.Length; i++) { - var propertyExpression = Expression.Property(argumentExpression, properties[i].Name); var parameterInfo = new SurrogatedParameterInfo(properties[i], factoryContext); - - expressions[i + 1] = Expression.Assign(propertyExpression, CreateArgument(parameterInfo, factoryContext)); + constructorArguments[i] = CreateArgument(parameterInfo, factoryContext); factoryContext.SurrogatedParameters.Add(parameterInfo); } + + factoryContext.ParamCheckExpressions.Add(Expression.Assign(argumentExpression, Expression.New(constructor, constructorArguments))); + } + else + { + // arg_local = new T() + // { + // arg_local.Property[0] = expression[0], + // arg_local.Property[n] = expression[n], + // } + + var bindings = new List(properties.Length); + + for (var i = 0; i < properties.Length; i++) + { + // For parameterless ctor we will init only writable properties. + if (properties[i].CanWrite) + { + var parameterInfo = new SurrogatedParameterInfo(properties[i], factoryContext); + bindings.Add(Expression.Bind(properties[i], CreateArgument(parameterInfo, factoryContext))); + factoryContext.SurrogatedParameters.Add(parameterInfo); + + } + } + + var newExpression = constructor is null ? Expression.New(parameter.ParameterType) : Expression.New(constructor); + factoryContext.ParamCheckExpressions.Add(Expression.Assign(argumentExpression, Expression.MemberInit(newExpression, bindings))); } factoryContext.TrackedParameters.Add(parameter.Name!, RequestDelegateFactoryConstants.SurrogatedParameter); factoryContext.ExtraLocals.Add(argumentExpression); - factoryContext.ParamCheckExpressions.Add(Expression.Block(expressions)); return argumentExpression; } @@ -1865,7 +1898,6 @@ private class FactoryContext public List BoxedArgs { get; } = new(); public List>? Filters { get; init; } - public ParameterInfo? SurrogatedParameter { get; set; } public List SurrogatedParameters { get; } = new(); } @@ -2146,20 +2178,6 @@ private static string BuildErrorMessageForFormAndJsonBodyParameters(FactoryConte return errorMessage.ToString(); } - private static string BuildErrorMessageForMultipleSurrogatedParameters(FactoryContext factoryContext) - { - var errorMessage = new StringBuilder(); - errorMessage.AppendLine("An action cannot use more than one parameter decorated with [ParametersAttribute]."); - errorMessage.AppendLine("Below is the list of parameters that we found: "); - errorMessage.AppendLine(); - errorMessage.AppendLine(FormattableString.Invariant($"{"Parameter",-20}| {"Source",-30}")); - errorMessage.AppendLine("---------------------------------------------------------------------------------"); - - FormatTrackedParameters(factoryContext, errorMessage); - - return errorMessage.ToString(); - } - private static void FormatTrackedParameters(FactoryContext factoryContext, StringBuilder errorMessage) { foreach (var kv in factoryContext.TrackedParameters) From 8f5726c65cdb797e289cd25d9559c416998dc8ea Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 22 Apr 2022 15:08:19 -0700 Subject: [PATCH 07/63] change to a static local function --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index b55515cfcb35..c6a9e2926d22 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1090,7 +1090,7 @@ private static Expression BindSurrogatedArgument(ParameterInfo parameter, Factor var properties = parameter.ParameterType.GetProperties(); var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); - ConstructorInfo? GetConstructor() + static ConstructorInfo? GetConstructor(ParameterInfo parameter, PropertyInfo[] properties) { if (parameter.ParameterType.IsValueType) { @@ -1123,7 +1123,7 @@ private static Expression BindSurrogatedArgument(ParameterInfo parameter, Factor throw new InvalidOperationException($"No '{parameter.ParameterType}' public parameterless constructor found for {parameter.Name}."); } - var constructor = GetConstructor(); + var constructor = GetConstructor(parameter, properties); if (constructor?.GetParameters() is { Length: > 0 } parameters) { // arg_local = new T(....) From 09bd8726a9172a8ce3d52b26f1f31cdacf923594 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 22 Apr 2022 15:21:43 -0700 Subject: [PATCH 08/63] Remove empty line --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index c6a9e2926d22..e434c0ecd9ed 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1156,7 +1156,6 @@ private static Expression BindSurrogatedArgument(ParameterInfo parameter, Factor var parameterInfo = new SurrogatedParameterInfo(properties[i], factoryContext); bindings.Add(Expression.Bind(properties[i], CreateArgument(parameterInfo, factoryContext))); factoryContext.SurrogatedParameters.Add(parameterInfo); - } } From 173c269352e397ee8fb35b9af6c0041fecbff125 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 22 Apr 2022 16:41:36 -0700 Subject: [PATCH 09/63] Updating APIExplorer --- ...icrosoft.AspNetCore.Http.Extensions.csproj | 3 +- .../src/RequestDelegateFactory.cs | 49 +------------------ 2 files changed, 4 insertions(+), 48 deletions(-) diff --git a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj index 718f747582be..bf1812bf7dc9 100644 --- a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj +++ b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj @@ -1,4 +1,4 @@ - + ASP.NET Core common extension methods for HTTP abstractions, HTTP headers, HTTP request/response, and session state. @@ -13,6 +13,7 @@ + diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index e434c0ecd9ed..9dbda7599d49 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1131,7 +1131,7 @@ private static Expression BindSurrogatedArgument(ParameterInfo parameter, Factor for (var i = 0; i < properties.Length; i++) { - var parameterInfo = new SurrogatedParameterInfo(properties[i], factoryContext); + var parameterInfo = new SurrogatedParameterInfo(properties[i], factoryContext.NullabilityContext); constructorArguments[i] = CreateArgument(parameterInfo, factoryContext); factoryContext.SurrogatedParameters.Add(parameterInfo); } @@ -1153,7 +1153,7 @@ private static Expression BindSurrogatedArgument(ParameterInfo parameter, Factor // For parameterless ctor we will init only writable properties. if (properties[i].CanWrite) { - var parameterInfo = new SurrogatedParameterInfo(properties[i], factoryContext); + var parameterInfo = new SurrogatedParameterInfo(properties[i], factoryContext.NullabilityContext); bindings.Add(Expression.Bind(properties[i], CreateArgument(parameterInfo, factoryContext))); factoryContext.SurrogatedParameters.Add(parameterInfo); } @@ -1917,51 +1917,6 @@ private static class RequestDelegateFactoryConstants public const string SurrogatedParameter = "Surrogate (Attribute)"; } - private class SurrogatedParameterInfo : ParameterInfo - { - private readonly PropertyInfo _underlyingProperty; - private readonly NullabilityInfo _nullabilityInfo; - - public SurrogatedParameterInfo(PropertyInfo propertyInfo, FactoryContext factoryContext) - { - Debug.Assert(null != propertyInfo); - - AttrsImpl = (ParameterAttributes)propertyInfo.Attributes; - MemberImpl = propertyInfo; - NameImpl = propertyInfo.Name; - ClassImpl = propertyInfo.PropertyType; - PositionImpl = -1;//parameter.Position; - - _nullabilityInfo = factoryContext.NullabilityContext.Create(propertyInfo); - _underlyingProperty = propertyInfo; - } - - public override bool HasDefaultValue => false; - public override object? DefaultValue => null; - public override int MetadataToken => _underlyingProperty.MetadataToken; - public override object? RawDefaultValue => null; - - public override object[] GetCustomAttributes(Type attributeType, bool inherit) - => _underlyingProperty.GetCustomAttributes(attributeType, inherit); - - public override object[] GetCustomAttributes(bool inherit) - => _underlyingProperty.GetCustomAttributes(inherit); - - public override IList GetCustomAttributesData() - => _underlyingProperty.GetCustomAttributesData(); - - public override Type[] GetOptionalCustomModifiers() - => _underlyingProperty.GetOptionalCustomModifiers(); - - public override Type[] GetRequiredCustomModifiers() - => _underlyingProperty.GetRequiredCustomModifiers(); - - public override bool IsDefined(Type attributeType, bool inherit) - => _underlyingProperty.IsDefined(attributeType, inherit); - - public new bool IsOptional => _nullabilityInfo.ReadState != NullabilityState.NotNull; - } - private static partial class Log { private const string InvalidJsonRequestBodyMessage = @"Failed to read parameter ""{ParameterType} {ParameterName}"" from the request body as JSON."; From f018d10330e69dc055323ee55f1a5765ed75226c Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 22 Apr 2022 16:41:52 -0700 Subject: [PATCH 10/63] Updating APIExplorer --- .../EndpointMetadataApiDescriptionProvider.cs | 33 +++++++++--- ...icrosoft.AspNetCore.Mvc.ApiExplorer.csproj | 1 + src/Shared/SurrogatedParameterInfo.cs | 52 +++++++++++++++++++ 3 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 src/Shared/SurrogatedParameterInfo.cs diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index b7241a243633..bd3c7d99ed10 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -118,18 +118,35 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string foreach (var parameter in methodInfo.GetParameters()) { - var parameterDescription = CreateApiParameterDescription(parameter, routeEndpoint.RoutePattern, disableInferredBody); - - if (parameterDescription is null) + void DummyMethod(ParameterInfo parameterInfo) { - continue; + var parameterDescription = CreateApiParameterDescription(parameterInfo, routeEndpoint.RoutePattern, disableInferredBody); + + if (parameterDescription is { }) + { + apiDescription.ParameterDescriptions.Add(parameterDescription); + + hasBodyOrFormFileParameter |= + parameterDescription.Source == BindingSource.Body || + parameterDescription.Source == BindingSource.FormFile; + } } - apiDescription.ParameterDescriptions.Add(parameterDescription); + if (parameter.GetCustomAttributes().OfType() is { }) + { + var properties = parameter.ParameterType.GetProperties(); + var nullabilityContext = new NullabilityInfoContext(); - hasBodyOrFormFileParameter |= - parameterDescription.Source == BindingSource.Body || - parameterDescription.Source == BindingSource.FormFile; + for (var i = 0; i < properties.Length; i++) + { + var parameterInfo = new SurrogatedParameterInfo(properties[i], nullabilityContext); + DummyMethod(parameterInfo); + } + } + else + { + DummyMethod(parameter); + } } // Get IAcceptsMetadata. diff --git a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj index 4b0491eb95ef..165099dfdaf6 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj +++ b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj @@ -11,6 +11,7 @@ + diff --git a/src/Shared/SurrogatedParameterInfo.cs b/src/Shared/SurrogatedParameterInfo.cs new file mode 100644 index 000000000000..9d0b14d1d22e --- /dev/null +++ b/src/Shared/SurrogatedParameterInfo.cs @@ -0,0 +1,52 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.Reflection; + +namespace Microsoft.AspNetCore.Http; + +internal class SurrogatedParameterInfo : ParameterInfo +{ + private readonly PropertyInfo _underlyingProperty; + private readonly NullabilityInfo _nullabilityInfo; + + public SurrogatedParameterInfo(PropertyInfo propertyInfo, NullabilityInfoContext nullabilityContext) + { + Debug.Assert(null != propertyInfo); + + AttrsImpl = (ParameterAttributes)propertyInfo.Attributes; + MemberImpl = propertyInfo; + NameImpl = propertyInfo.Name; + ClassImpl = propertyInfo.PropertyType; + PositionImpl = -1;//parameter.Position; + + _nullabilityInfo = nullabilityContext.Create(propertyInfo); + _underlyingProperty = propertyInfo; + } + + public override bool HasDefaultValue => false; + public override object? DefaultValue => null; + public override int MetadataToken => _underlyingProperty.MetadataToken; + public override object? RawDefaultValue => null; + + public override object[] GetCustomAttributes(Type attributeType, bool inherit) + => _underlyingProperty.GetCustomAttributes(attributeType, inherit); + + public override object[] GetCustomAttributes(bool inherit) + => _underlyingProperty.GetCustomAttributes(inherit); + + public override IList GetCustomAttributesData() + => _underlyingProperty.GetCustomAttributesData(); + + public override Type[] GetOptionalCustomModifiers() + => _underlyingProperty.GetOptionalCustomModifiers(); + + public override Type[] GetRequiredCustomModifiers() + => _underlyingProperty.GetRequiredCustomModifiers(); + + public override bool IsDefined(Type attributeType, bool inherit) + => _underlyingProperty.IsDefined(attributeType, inherit); + + public new bool IsOptional => _nullabilityInfo.ReadState != NullabilityState.NotNull; +} From b52b113cf81a4f453ac974941d1c71244f306b7a Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 25 Apr 2022 10:21:12 -0700 Subject: [PATCH 11/63] Updating OpenAPI --- .../src/RequestDelegateFactory.cs | 9 ++-- .../EndpointMetadataApiDescriptionProvider.cs | 54 +++++++++++-------- .../src/Microsoft.AspNetCore.OpenApi.csproj | 1 + src/OpenApi/src/OpenApiGenerator.cs | 36 ++++++++++++- 4 files changed, 73 insertions(+), 27 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index f4f84cddd920..7304f0567771 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -420,7 +420,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext if (parameterCustomAttributes.OfType().FirstOrDefault() is { } || parameter.ParameterType.GetCustomAttributes().OfType().FirstOrDefault() is { }) { - return BindSurrogatedArgument(parameter, factoryContext); + return BindParameterFromSurrogatedProperties(parameter, factoryContext); } else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } routeAttribute) { @@ -1094,7 +1094,7 @@ private static Expression GetValueFromProperty(Expression sourceExpression, stri return Expression.Convert(indexExpression, returnType ?? typeof(string)); } - private static Expression BindSurrogatedArgument(ParameterInfo parameter, FactoryContext factoryContext) + private static Expression BindParameterFromSurrogatedProperties(ParameterInfo parameter, FactoryContext factoryContext) { var properties = parameter.ParameterType.GetProperties(); var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); @@ -1110,13 +1110,14 @@ private static Expression BindSurrogatedArgument(ParameterInfo parameter, Factor // Try to find the parameterless constructor var constructor = parameter.ParameterType.GetConstructor(Array.Empty()); - if (constructor is { }) + if (constructor is not null) { return constructor; } // If a parameterless ctor is not defined - // we will try to find a ctor that includes all the property types. + // we will try to find a ctor that includes all + // property types and the right order. var types = new Type[properties.Length]; for (var i = 0; i < properties.Length; i++) { diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index bd3c7d99ed10..7de78bbd9a11 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -116,36 +116,48 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string var hasBodyOrFormFileParameter = false; - foreach (var parameter in methodInfo.GetParameters()) + static List FlattenParameters(ParameterInfo[] parameters) { - void DummyMethod(ParameterInfo parameterInfo) + var flattenedParameters = new List(); + NullabilityInfoContext? nullabilityContext = null; + + foreach (var parameter in parameters) { - var parameterDescription = CreateApiParameterDescription(parameterInfo, routeEndpoint.RoutePattern, disableInferredBody); + var attributes = parameter.GetCustomAttributes(); - if (parameterDescription is { }) + if (attributes.OfType().Any()) { - apiDescription.ParameterDescriptions.Add(parameterDescription); - - hasBodyOrFormFileParameter |= - parameterDescription.Source == BindingSource.Body || - parameterDescription.Source == BindingSource.FormFile; + nullabilityContext ??= new(); + + var properties = parameter.ParameterType.GetProperties(); + for (var i = 0; i < properties.Length; i++) + { + if (properties[i].CanWrite) + { + flattenedParameters.Add(new SurrogatedParameterInfo(properties[i], nullabilityContext)); + } + } } - } - - if (parameter.GetCustomAttributes().OfType() is { }) - { - var properties = parameter.ParameterType.GetProperties(); - var nullabilityContext = new NullabilityInfoContext(); - - for (var i = 0; i < properties.Length; i++) + else { - var parameterInfo = new SurrogatedParameterInfo(properties[i], nullabilityContext); - DummyMethod(parameterInfo); + flattenedParameters.Add(parameter); } } - else + + return flattenedParameters; + } + + foreach (var parameter in FlattenParameters(methodInfo.GetParameters())) + { + var parameterDescription = CreateApiParameterDescription(parameter, routeEndpoint.RoutePattern, disableInferredBody); + + if (parameterDescription is { }) { - DummyMethod(parameter); + apiDescription.ParameterDescriptions.Add(parameterDescription); + + hasBodyOrFormFileParameter |= + parameterDescription.Source == BindingSource.Body || + parameterDescription.Source == BindingSource.FormFile; } } diff --git a/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj b/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj index 91cc8230727b..40d1ba980296 100644 --- a/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj +++ b/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj @@ -17,6 +17,7 @@ + diff --git a/src/OpenApi/src/OpenApiGenerator.cs b/src/OpenApi/src/OpenApiGenerator.cs index e8f2f59d5b11..cd3108043029 100644 --- a/src/OpenApi/src/OpenApiGenerator.cs +++ b/src/OpenApi/src/OpenApiGenerator.cs @@ -250,7 +250,8 @@ private static void GenerateDefaultResponses(Dictionary GetOperationTags(MethodInfo methodInfo, EndpointMetadat : new List() { new OpenApiTag() { Name = controllerName } }; } + private static List FlattenParameters(ParameterInfo[] parameters) + { + var flattenedParameters = new List(); + NullabilityInfoContext? nullabilityContext = null; + + foreach (var parameter in parameters) + { + var attributes = parameter.GetCustomAttributes(); + + if (attributes.OfType().Any()) + { + nullabilityContext ??= new(); + + var properties = parameter.ParameterType.GetProperties(); + for (var i = 0; i < properties.Length; i++) + { + if (properties[i].CanWrite) + { + flattenedParameters.Add(new SurrogatedParameterInfo(properties[i], nullabilityContext)); + } + } + } + else + { + flattenedParameters.Add(parameter); + } + } + + return flattenedParameters; + } + private List GetOpenApiParameters(MethodInfo methodInfo, EndpointMetadataCollection metadata, RoutePattern pattern, bool disableInferredBody) { - var parameters = methodInfo.GetParameters(); + var parameters = FlattenParameters(methodInfo.GetParameters()); var openApiParameters = new List(); foreach (var parameter in parameters) From b441fc4459afbd60552f2d312df42e7a558d8d62 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 25 Apr 2022 10:23:05 -0700 Subject: [PATCH 12/63] PR feedback --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 7304f0567771..6a9abcdd3b90 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1125,7 +1125,7 @@ private static Expression BindParameterFromSurrogatedProperties(ParameterInfo pa } constructor = parameter.ParameterType.GetConstructor(types); - if (constructor is { }) + if (constructor is not null) { return constructor; } From bb3a599b550cf92c109d2f8f92197a7ae384e72e Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 25 Apr 2022 10:26:00 -0700 Subject: [PATCH 13/63] Updating comment --- .../Http.Extensions/src/RequestDelegateFactory.cs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 6a9abcdd3b90..3f148bc63cec 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1117,7 +1117,19 @@ private static Expression BindParameterFromSurrogatedProperties(ParameterInfo pa // If a parameterless ctor is not defined // we will try to find a ctor that includes all - // property types and the right order. + // property types and in the right order. + // Eg.: + // public class Sample + // { + // // Valid + // public Sample(int id, string name){} + // // InValid + // public Sample(string name, int id){} + // + // public int Id { get; set; } + // public string Name { get; set; } + //} + var types = new Type[properties.Length]; for (var i = 0; i < properties.Length; i++) { From 0b9512589f91175d4360bae69a63e849dd452c69 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 25 Apr 2022 10:52:28 -0700 Subject: [PATCH 14/63] Allowing attribute on classes --- src/Http/Http.Extensions/src/ParametersAttribute.cs | 5 ++++- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Http/Http.Extensions/src/ParametersAttribute.cs b/src/Http/Http.Extensions/src/ParametersAttribute.cs index 9ae4ccf24899..645686eab711 100644 --- a/src/Http/Http.Extensions/src/ParametersAttribute.cs +++ b/src/Http/Http.Extensions/src/ParametersAttribute.cs @@ -8,7 +8,10 @@ namespace Microsoft.AspNetCore.Http; /// /// /// -[AttributeUsage(AttributeTargets.Parameter, Inherited = false, AllowMultiple = false)] +[AttributeUsage( + AttributeTargets.Parameter | AttributeTargets.Class, + Inherited = false, + AllowMultiple = false)] public sealed class ParametersAttribute : Attribute { } diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 3f148bc63cec..65112ad85cf1 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -417,8 +417,8 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext throw new InvalidOperationException($"Encountered a parameter of type '{parameter.ParameterType}' without a name. Parameters must have a name."); } - if (parameterCustomAttributes.OfType().FirstOrDefault() is { } || - parameter.ParameterType.GetCustomAttributes().OfType().FirstOrDefault() is { }) + if (parameter.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType)) || + parameter.ParameterType.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) { return BindParameterFromSurrogatedProperties(parameter, factoryContext); } From 14b3eb921d5b4aa6c7d3453a5779418baa96a2d3 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 25 Apr 2022 15:15:25 -0700 Subject: [PATCH 15/63] Reducing initial memory allocation --- .../EndpointMetadataApiDescriptionProvider.cs | 32 +++++++++++-------- src/OpenApi/src/OpenApiGenerator.cs | 32 +++++++++++-------- src/Shared/SurrogatedParameterInfo.cs | 9 ++++-- 3 files changed, 44 insertions(+), 29 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 7de78bbd9a11..fbbe3aeb1852 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -116,35 +116,41 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string var hasBodyOrFormFileParameter = false; - static List FlattenParameters(ParameterInfo[] parameters) + static IEnumerable FlattenParameters(ParameterInfo[] parameters) { - var flattenedParameters = new List(); + if (parameters.Length == 0) + { + return Array.Empty(); + } + + List? flattenedParameters = null; NullabilityInfoContext? nullabilityContext = null; - foreach (var parameter in parameters) + for (var i = 0; i < parameters.Length; i++) { - var attributes = parameter.GetCustomAttributes(); - - if (attributes.OfType().Any()) + if (parameters[i].CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) { + // Initialize the list with all parameter already processed + // to keep the same parameter ordering + flattenedParameters ??= new(parameters[0..i]); nullabilityContext ??= new(); - var properties = parameter.ParameterType.GetProperties(); - for (var i = 0; i < properties.Length; i++) + var properties = parameters[i].ParameterType.GetProperties(); + foreach (var property in properties) { - if (properties[i].CanWrite) + if (property.CanWrite) { - flattenedParameters.Add(new SurrogatedParameterInfo(properties[i], nullabilityContext)); + flattenedParameters.Add(new SurrogatedParameterInfo(property, nullabilityContext)); } } } - else + else if (flattenedParameters is not null) { - flattenedParameters.Add(parameter); + flattenedParameters.Add(parameters[i]); } } - return flattenedParameters; + return flattenedParameters is not null ? flattenedParameters : parameters; } foreach (var parameter in FlattenParameters(methodInfo.GetParameters())) diff --git a/src/OpenApi/src/OpenApiGenerator.cs b/src/OpenApi/src/OpenApiGenerator.cs index cd3108043029..4b556eb80a9f 100644 --- a/src/OpenApi/src/OpenApiGenerator.cs +++ b/src/OpenApi/src/OpenApiGenerator.cs @@ -356,35 +356,41 @@ private List GetOperationTags(MethodInfo methodInfo, EndpointMetadat : new List() { new OpenApiTag() { Name = controllerName } }; } - private static List FlattenParameters(ParameterInfo[] parameters) + private static IEnumerable FlattenParameters(ParameterInfo[] parameters) { - var flattenedParameters = new List(); + if (parameters.Length == 0) + { + return Array.Empty(); + } + + List? flattenedParameters = null; NullabilityInfoContext? nullabilityContext = null; - foreach (var parameter in parameters) + for (var i = 0; i < parameters.Length; i++) { - var attributes = parameter.GetCustomAttributes(); - - if (attributes.OfType().Any()) + if (parameters[i].CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) { + // Initialize the list with all parameter already processed + // to keep the same parameter ordering + flattenedParameters ??= new(parameters[0..i]); nullabilityContext ??= new(); - var properties = parameter.ParameterType.GetProperties(); - for (var i = 0; i < properties.Length; i++) + var properties = parameters[i].ParameterType.GetProperties(); + foreach (var property in properties) { - if (properties[i].CanWrite) + if (property.CanWrite) { - flattenedParameters.Add(new SurrogatedParameterInfo(properties[i], nullabilityContext)); + flattenedParameters.Add(new SurrogatedParameterInfo(property, nullabilityContext)); } } } - else + else if (flattenedParameters is not null) { - flattenedParameters.Add(parameter); + flattenedParameters.Add(parameters[i]); } } - return flattenedParameters; + return flattenedParameters is not null ? flattenedParameters : parameters; } private List GetOpenApiParameters(MethodInfo methodInfo, EndpointMetadataCollection metadata, RoutePattern pattern, bool disableInferredBody) diff --git a/src/Shared/SurrogatedParameterInfo.cs b/src/Shared/SurrogatedParameterInfo.cs index 9d0b14d1d22e..606e06435262 100644 --- a/src/Shared/SurrogatedParameterInfo.cs +++ b/src/Shared/SurrogatedParameterInfo.cs @@ -9,7 +9,8 @@ namespace Microsoft.AspNetCore.Http; internal class SurrogatedParameterInfo : ParameterInfo { private readonly PropertyInfo _underlyingProperty; - private readonly NullabilityInfo _nullabilityInfo; + private readonly NullabilityInfoContext _nullabilityContext; + private NullabilityInfo? _nullabilityInfo; public SurrogatedParameterInfo(PropertyInfo propertyInfo, NullabilityInfoContext nullabilityContext) { @@ -21,7 +22,7 @@ public SurrogatedParameterInfo(PropertyInfo propertyInfo, NullabilityInfoContext ClassImpl = propertyInfo.PropertyType; PositionImpl = -1;//parameter.Position; - _nullabilityInfo = nullabilityContext.Create(propertyInfo); + _nullabilityContext = nullabilityContext; _underlyingProperty = propertyInfo; } @@ -48,5 +49,7 @@ public override Type[] GetRequiredCustomModifiers() public override bool IsDefined(Type attributeType, bool inherit) => _underlyingProperty.IsDefined(attributeType, inherit); - public new bool IsOptional => _nullabilityInfo.ReadState != NullabilityState.NotNull; + public new bool IsOptional => NullabilityInfo.ReadState != NullabilityState.NotNull; + + public NullabilityInfo NullabilityInfo => _nullabilityInfo ??= _nullabilityContext.Create(_underlyingProperty); } From b025c9a3b97158077a252be183684a3c96efb46f Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Wed, 27 Apr 2022 11:15:14 -0700 Subject: [PATCH 16/63] Adding constructorinfo caching --- .../ParameterBindingConstructorCache.cs | 66 +++++++++++++++++++ src/Shared/SurrogatedParameterInfo.cs | 2 +- 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 src/Shared/ParameterBindingConstructorCache.cs diff --git a/src/Shared/ParameterBindingConstructorCache.cs b/src/Shared/ParameterBindingConstructorCache.cs new file mode 100644 index 000000000000..c4426c7eb437 --- /dev/null +++ b/src/Shared/ParameterBindingConstructorCache.cs @@ -0,0 +1,66 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Concurrent; +using System.Reflection; + +namespace Microsoft.AspNetCore.Http; + +internal sealed class ParameterBindingConstructorCache +{ + private readonly ConcurrentDictionary _constructorCache = new(); + + public ConstructorInfo? GetParameterConstructor(ParameterInfo parameter) + { + static ConstructorInfo? GetConstructor(Type parameterType) + { + // Try to find the parameterless constructor + var constructor = parameterType.GetConstructor(Array.Empty()); + if (constructor is not null) + { + return constructor; + } + + // If a parameterless ctor is not defined + // we will try to find a ctor that includes all + // property types and in the right order. + // Eg.: + // public class Sample + // { + // // Valid + // public Sample(int Id, string Name){} + // // Valid ??? + // public Sample(int id, string name){} + // // InValid + // public Sample(string name, int id){} + // + // public int Id { get; set; } + // public string Name { get; set; } + //} + + var properties = parameterType.GetProperties(); + var types = new Type[properties.Length]; + for (var i = 0; i < properties.Length; i++) + { + types[i] = properties[i].PropertyType; + } + constructor = parameterType.GetConstructor(types); + + if (constructor is not null) + { + return constructor; + } + + if (parameterType.IsValueType) + { + // Value types always have a default constructor, we will use + // the parameter type during the NewExpression creation + return null; + } + + throw new InvalidOperationException($"No '{parameterType}' public parameterless constructor found."); + } + + return _constructorCache.GetOrAdd(parameter.ParameterType, GetConstructor); + } +} diff --git a/src/Shared/SurrogatedParameterInfo.cs b/src/Shared/SurrogatedParameterInfo.cs index 606e06435262..cb6fcec4f1e2 100644 --- a/src/Shared/SurrogatedParameterInfo.cs +++ b/src/Shared/SurrogatedParameterInfo.cs @@ -17,8 +17,8 @@ public SurrogatedParameterInfo(PropertyInfo propertyInfo, NullabilityInfoContext Debug.Assert(null != propertyInfo); AttrsImpl = (ParameterAttributes)propertyInfo.Attributes; - MemberImpl = propertyInfo; NameImpl = propertyInfo.Name; + MemberImpl = propertyInfo; ClassImpl = propertyInfo.PropertyType; PositionImpl = -1;//parameter.Position; From 6f8973a918f1acec5e8e770b98b3da6eccb99071 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Wed, 27 Apr 2022 11:15:36 -0700 Subject: [PATCH 17/63] Updating OpenAPI generator --- .../src/Microsoft.AspNetCore.OpenApi.csproj | 1 + src/OpenApi/src/OpenApiGenerator.cs | 17 +++- src/OpenApi/test/OpenApiGeneratorTests.cs | 80 ++++++++++++++++++- 3 files changed, 93 insertions(+), 5 deletions(-) diff --git a/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj b/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj index 40d1ba980296..ba094ce23ffc 100644 --- a/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj +++ b/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj @@ -19,6 +19,7 @@ + diff --git a/src/OpenApi/src/OpenApiGenerator.cs b/src/OpenApi/src/OpenApiGenerator.cs index 4b556eb80a9f..31e805c04fd2 100644 --- a/src/OpenApi/src/OpenApiGenerator.cs +++ b/src/OpenApi/src/OpenApiGenerator.cs @@ -30,6 +30,7 @@ internal class OpenApiGenerator private readonly IServiceProviderIsService? _serviceProviderIsService; internal static readonly ParameterBindingMethodCache ParameterBindingMethodCache = new(); + internal static readonly ParameterBindingConstructorCache ParameterBindingConstructorCache = new(); /// /// Creates an instance given an @@ -375,12 +376,20 @@ private static IEnumerable FlattenParameters(ParameterInfo[] para flattenedParameters ??= new(parameters[0..i]); nullabilityContext ??= new(); - var properties = parameters[i].ParameterType.GetProperties(); - foreach (var property in properties) + var constructor = ParameterBindingConstructorCache.GetParameterConstructor(parameters[i]); + if (constructor?.GetParameters() is { Length: > 0 } constructorParameters) { - if (property.CanWrite) + flattenedParameters.AddRange(constructorParameters); + } + else + { + var properties = parameters[i].ParameterType.GetProperties(); + foreach (var property in properties) { - flattenedParameters.Add(new SurrogatedParameterInfo(property, nullabilityContext)); + if (property.CanWrite) + { + flattenedParameters.Add(new SurrogatedParameterInfo(property, nullabilityContext)); + } } } } diff --git a/src/OpenApi/test/OpenApiGeneratorTests.cs b/src/OpenApi/test/OpenApiGeneratorTests.cs index 459966c62f43..929386fdd171 100644 --- a/src/OpenApi/test/OpenApiGeneratorTests.cs +++ b/src/OpenApi/test/OpenApiGeneratorTests.cs @@ -352,9 +352,49 @@ public void AddsMultipleParameters() Assert.Equal("object", fromBodyParam.Content.First().Value.Schema.Type); Assert.True(fromBodyParam.Required); } - #nullable disable + [Fact] + public void AddsMultipleParametersFromParametersAttribute() + { + static void AssertParameters(OpenApiOperation operation) + { + Assert.Collection( + operation.Parameters, + param => + { + Assert.Equal("Foo", param.Name); + Assert.Equal("number", param.Schema.Type); + Assert.Equal(ParameterLocation.Path, param.In); + Assert.True(param.Required); + }, + param => + { + Assert.Equal("Bar", param.Name); + Assert.Equal("number", param.Schema.Type); + Assert.Equal(ParameterLocation.Query, param.In); + Assert.True(param.Required); + }, + param => + { + Assert.Equal("FromBody", param.Name); + var fromBodyParam = operation.RequestBody; + Assert.Equal("object", fromBodyParam.Content.First().Value.Schema.Type); + Assert.False(fromBodyParam.Required); + } + ); + } + + AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListClass req) => { })); + AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListClassWithReadOnlyProperties req) => { })); + AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListStruct req) => { })); + AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListRecord req) => { })); + AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListRecordStruct req) => { })); + AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListRecordWithoutPositionalParameters req) => { })); + AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListRecordWithoutAttributes req) => { }, "/{foo}")); + AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListRecordWithoutAttributes req) => { }, "/{Foo}")); + } + [Fact] public void TestParameterIsRequired() { @@ -801,4 +841,42 @@ private record BindAsyncRecord(int Value) public static bool TryParse(string value, out BindAsyncRecord result) => throw new NotImplementedException(); } + + private record ArgumentListRecord([FromRoute] int Foo, int Bar, InferredJsonClass FromBody, HttpContext context); + + private record struct ArgumentListRecordStruct([FromRoute] int Foo, int Bar, InferredJsonClass FromBody, HttpContext context); + + private record ArgumentListRecordWithoutAttributes(int Foo, int Bar, InferredJsonClass FromBody, HttpContext context); + + private record ArgumentListRecordWithoutPositionalParameters + { + [FromRoute] + public int Foo { get; set; } + public int Bar { get; set; } + public InferredJsonClass FromBody { get; set; } + public HttpContext Context { get; set; } + } + + private class ArgumentListClass + { + [FromRoute] + public int Foo { get; set; } + public int Bar { get; set; } + public InferredJsonClass FromBody { get; set; } + public HttpContext Context { get; set; } + } + + private class ArgumentListClassWithReadOnlyProperties : ArgumentListClass + { + public int ReadOnly { get; } + } + + private struct ArgumentListStruct + { + [FromRoute] + public int Foo { get; set; } + public int Bar { get; set; } + public InferredJsonClass FromBody { get; set; } + public HttpContext Context { get; set; } + } } From a8a2c61710c0561a60a34aaee7cbcb790df5a2f9 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Wed, 27 Apr 2022 11:15:53 -0700 Subject: [PATCH 18/63] Updating APIExplorer --- .../EndpointMetadataApiDescriptionProvider.cs | 21 +++-- ...icrosoft.AspNetCore.Mvc.ApiExplorer.csproj | 3 +- ...pointMetadataApiDescriptionProviderTest.cs | 80 +++++++++++++++++++ 3 files changed, 97 insertions(+), 7 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index fbbe3aeb1852..fcad4a079879 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -26,6 +26,7 @@ internal class EndpointMetadataApiDescriptionProvider : IApiDescriptionProvider private readonly IHostEnvironment _environment; private readonly IServiceProviderIsService? _serviceProviderIsService; private readonly ParameterBindingMethodCache ParameterBindingMethodCache = new(); + private readonly ParameterBindingConstructorCache _parameterBindingConstructorCache = new(); private readonly ParameterPolicyFactory _parameterPolicyFactory; // Executes before MVC's DefaultApiDescriptionProvider and GrpcJsonTranscodingDescriptionProvider for no particular reason. @@ -116,7 +117,7 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string var hasBodyOrFormFileParameter = false; - static IEnumerable FlattenParameters(ParameterInfo[] parameters) + static IEnumerable FlattenParameters(ParameterInfo[] parameters, ParameterBindingConstructorCache cache) { if (parameters.Length == 0) { @@ -135,12 +136,20 @@ static IEnumerable FlattenParameters(ParameterInfo[] parameters) flattenedParameters ??= new(parameters[0..i]); nullabilityContext ??= new(); - var properties = parameters[i].ParameterType.GetProperties(); - foreach (var property in properties) + var constructor = cache.GetParameterConstructor(parameters[i]); + if (constructor?.GetParameters() is { Length: > 0 } constructorParameters) { - if (property.CanWrite) + flattenedParameters.AddRange(constructorParameters); + } + else + { + var properties = parameters[i].ParameterType.GetProperties(); + foreach (var property in properties) { - flattenedParameters.Add(new SurrogatedParameterInfo(property, nullabilityContext)); + if (property.CanWrite) + { + flattenedParameters.Add(new SurrogatedParameterInfo(property, nullabilityContext)); + } } } } @@ -153,7 +162,7 @@ static IEnumerable FlattenParameters(ParameterInfo[] parameters) return flattenedParameters is not null ? flattenedParameters : parameters; } - foreach (var parameter in FlattenParameters(methodInfo.GetParameters())) + foreach (var parameter in FlattenParameters(methodInfo.GetParameters(), _parameterBindingConstructorCache)) { var parameterDescription = CreateApiParameterDescription(parameter, routeEndpoint.RoutePattern, disableInferredBody); diff --git a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj index 165099dfdaf6..4c41192830df 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj +++ b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj @@ -11,7 +11,8 @@ - + + diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 5a77ef5f9b55..99f22a67b16e 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -447,6 +447,48 @@ public void AddsMultipleParameters() #nullable disable + [Fact] + public void AddsMultipleParametersFromParametersAttribute() + { + static void AssertParameters(ApiDescription apiDescription) + { + Assert.Collection( + apiDescription.ParameterDescriptions, + param => + { + Assert.Equal("Foo", param.Name); + Assert.Equal(typeof(int), param.ModelMetadata.ModelType); + Assert.Equal(BindingSource.Path, param.Source); + Assert.True(param.IsRequired); + }, + param => + { + Assert.Equal("Bar", param.Name); + Assert.Equal(typeof(int), param.ModelMetadata.ModelType); + Assert.Equal(BindingSource.Query, param.Source); + Assert.True(param.IsRequired); + }, + param => + { + Assert.Equal("FromBody", param.Name); + Assert.Equal(typeof(InferredJsonClass), param.Type); + Assert.Equal(typeof(InferredJsonClass), param.ModelMetadata.ModelType); + Assert.Equal(BindingSource.Body, param.Source); + Assert.False(param.IsRequired); + } + ); + } + + AssertParameters(GetApiDescription(([Parameters] ArgumentListClass req) => { })); + AssertParameters(GetApiDescription(([Parameters] ArgumentListClassWithReadOnlyProperties req) => { })); + AssertParameters(GetApiDescription(([Parameters] ArgumentListStruct req) => { })); + AssertParameters(GetApiDescription(([Parameters] ArgumentListRecord req) => { })); + AssertParameters(GetApiDescription(([Parameters] ArgumentListRecordStruct req) => { })); + AssertParameters(GetApiDescription(([Parameters] ArgumentListRecordWithoutPositionalParameters req) => { })); + AssertParameters(GetApiDescription(([Parameters] ArgumentListRecordWithoutAttributes req) => { }, "/{foo}")); + AssertParameters(GetApiDescription(([Parameters] ArgumentListRecordWithoutAttributes req) => { }, "/{Foo}")); + } + [Fact] public void TestParameterIsRequired() { @@ -1323,6 +1365,44 @@ private record BindAsyncRecord(int Value) throw new NotImplementedException(); } + private record ArgumentListRecord([FromRoute] int Foo, int Bar, InferredJsonClass FromBody, HttpContext context); + + private record struct ArgumentListRecordStruct([FromRoute] int Foo, int Bar, InferredJsonClass FromBody, HttpContext context); + + private record ArgumentListRecordWithoutAttributes(int Foo, int Bar, InferredJsonClass FromBody, HttpContext context); + + private record ArgumentListRecordWithoutPositionalParameters + { + [FromRoute] + public int Foo { get; set; } + public int Bar { get; set; } + public InferredJsonClass FromBody { get; set; } + public HttpContext Context { get; set; } + } + + private class ArgumentListClass + { + [FromRoute] + public int Foo { get; set; } + public int Bar { get; set; } + public InferredJsonClass FromBody { get; set; } + public HttpContext Context { get; set; } + } + + private class ArgumentListClassWithReadOnlyProperties : ArgumentListClass + { + public int ReadOnly { get; } + } + + private struct ArgumentListStruct + { + [FromRoute] + public int Foo { get; set; } + public int Bar { get; set; } + public InferredJsonClass FromBody { get; set; } + public HttpContext Context { get; set; } + } + private class TestServiceProvider : IServiceProvider { public void Dispose() From 21f2fd20d893cb233ba591cc5f3adbe299a583e3 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 28 Apr 2022 01:22:49 -0700 Subject: [PATCH 19/63] Adding constructor cache --- .../ParameterBindingConstructorCache.cs | 66 --------- src/Shared/ParameterBindingMethodCache.cs | 134 ++++++++++++++++++ 2 files changed, 134 insertions(+), 66 deletions(-) delete mode 100644 src/Shared/ParameterBindingConstructorCache.cs diff --git a/src/Shared/ParameterBindingConstructorCache.cs b/src/Shared/ParameterBindingConstructorCache.cs deleted file mode 100644 index c4426c7eb437..000000000000 --- a/src/Shared/ParameterBindingConstructorCache.cs +++ /dev/null @@ -1,66 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Collections.Concurrent; -using System.Reflection; - -namespace Microsoft.AspNetCore.Http; - -internal sealed class ParameterBindingConstructorCache -{ - private readonly ConcurrentDictionary _constructorCache = new(); - - public ConstructorInfo? GetParameterConstructor(ParameterInfo parameter) - { - static ConstructorInfo? GetConstructor(Type parameterType) - { - // Try to find the parameterless constructor - var constructor = parameterType.GetConstructor(Array.Empty()); - if (constructor is not null) - { - return constructor; - } - - // If a parameterless ctor is not defined - // we will try to find a ctor that includes all - // property types and in the right order. - // Eg.: - // public class Sample - // { - // // Valid - // public Sample(int Id, string Name){} - // // Valid ??? - // public Sample(int id, string name){} - // // InValid - // public Sample(string name, int id){} - // - // public int Id { get; set; } - // public string Name { get; set; } - //} - - var properties = parameterType.GetProperties(); - var types = new Type[properties.Length]; - for (var i = 0; i < properties.Length; i++) - { - types[i] = properties[i].PropertyType; - } - constructor = parameterType.GetConstructor(types); - - if (constructor is not null) - { - return constructor; - } - - if (parameterType.IsValueType) - { - // Value types always have a default constructor, we will use - // the parameter type during the NewExpression creation - return null; - } - - throw new InvalidOperationException($"No '{parameterType}' public parameterless constructor found."); - } - - return _constructorCache.GetOrAdd(parameter.ParameterType, GetConstructor); - } -} diff --git a/src/Shared/ParameterBindingMethodCache.cs b/src/Shared/ParameterBindingMethodCache.cs index 01b5c0895e60..17f3b816ed0b 100644 --- a/src/Shared/ParameterBindingMethodCache.cs +++ b/src/Shared/ParameterBindingMethodCache.cs @@ -34,6 +34,7 @@ internal sealed class ParameterBindingMethodCache // Since this is shared source, the cache won't be shared between RequestDelegateFactory and the ApiDescriptionProvider sadly :( private readonly ConcurrentDictionary?> _stringMethodCallCache = new(); private readonly ConcurrentDictionary?, int)> _bindAsyncMethodCallCache = new(); + private readonly ConcurrentDictionary _constructorCache = new(); // If IsDynamicCodeSupported is false, we can't use the static Enum.TryParse since there's no easy way for // this code to generate the specific instantiation for any enums used @@ -272,6 +273,102 @@ static bool ValidateReturnType(MethodInfo methodInfo) } } + public (ConstructorInfo?, ConstructorParameter[]) FindConstructor(Type type) + { + static (ConstructorInfo? constructor, ConstructorParameter[] parameters) Finder(Type type) + { + var constructor = GetConstructor(type); + + if (constructor is null || constructor.GetParameters().Length == 0) + { + return (constructor, Array.Empty()); + } + + var properties = type.GetProperties(); + var lookupTable = new Dictionary(properties.Length); + for (var i = 0; i < properties.Length; i++) + { + lookupTable.Add(new ParameterLookupKey(properties[i].Name, properties[i].PropertyType), properties[i]); + } + + // This behavior diverge from the JSON serialization + // since we don't have an attribute, eg. JsonConstructor, + // we need to be very restrictive about the ctor + // and only accept if the parameterized ctor has + // only arguments that we can match (Type and Name) + // with a public property. + + var parameters = constructor.GetParameters(); + var parametersWithPropertyInfo = new ConstructorParameter[parameters.Length]; + + for (var i = 0; i < parameters.Length; i++) + { + var key = new ParameterLookupKey(parameters[i].Name!, parameters[i].ParameterType); + if (!lookupTable.TryGetValue(key, out var property)) + { + throw new InvalidOperationException( + $"The '{type}' public parameterized constructor must contains only parameters that match to the declared properties."); + } + + parametersWithPropertyInfo[i] = new ConstructorParameter(parameters[i], property); + } + + return (constructor, parametersWithPropertyInfo); + } + + return _constructorCache.GetOrAdd(type, Finder); + } + + private static ConstructorInfo? GetConstructor(Type type) + { + if (type.IsAbstract) + { + throw new InvalidOperationException($"The '{type}' abstract type is not supported."); + } + + var constructors = type.GetConstructors(BindingFlags.Public | BindingFlags.Instance); + + // if only one constructor is declared + // we will use it to try match the properties + if (constructors.Length == 1) + { + return constructors[0]; + } + + // We will try to get the parameterless ctor + // as priority before visit the others + var parameterlessConstructor = constructors.SingleOrDefault(c => c.GetParameters().Length == 0); + if (parameterlessConstructor is not null) + { + return parameterlessConstructor; + } + + // If a parameterized constructors is not found at this point + // we will use a default constructor that is always available + // for value types. + if (type.IsValueType) + { + return null; + } + + // We don't have an attribute, similar to JsonConstructor, to + // disambiguate ctors, so, we will throw if more than one + // ctor is defined without a parameterless constructor. + // Eg.: + // public class X + // { + // public X(int foo) + // public X(int foo, int bar) + // ... + // } + if (parameterlessConstructor is null && constructors.Length > 1) + { + throw new InvalidOperationException($"Only a single public parameterized constructor is allowed for '{type}'."); + } + + throw new InvalidOperationException($"No '{type}' public parameterless constructor found."); + } + private MethodInfo? GetStaticMethodFromHierarchy(Type type, string name, Type[] parameterTypes, Func validateReturnType) { bool IsMatch(MethodInfo? method) => method is not null && !method.IsAbstract && validateReturnType(method); @@ -535,4 +632,41 @@ private static bool TryGetNumberStylesTryGetMethod(Type type, [NotNullWhen(true) static async ValueTask ConvertAwaited(ValueTask> typedValueTask) => await typedValueTask; return ConvertAwaited(typedValueTask); } + + private class ParameterLookupKey + { + public ParameterLookupKey(string name, Type type) + { + Name = name; + Type = type; + } + + public string Name { get; } + public Type Type { get; } + + public override int GetHashCode() + { + return StringComparer.OrdinalIgnoreCase.GetHashCode(Name); + } + + public override bool Equals([NotNullWhen(true)] object? obj) + { + Debug.Assert(obj is ParameterLookupKey); + + var other = (ParameterLookupKey)obj; + return Type == other.Type && string.Equals(Name, other.Name, StringComparison.OrdinalIgnoreCase); + } + } + + internal class ConstructorParameter + { + public ConstructorParameter(ParameterInfo parameter, PropertyInfo propertyInfo) + { + ParameterInfo = parameter; + PropertyInfo = propertyInfo; + } + + public ParameterInfo ParameterInfo { get; } + public PropertyInfo PropertyInfo { get; } + } } From 8956ff470345f46002d632abfc63d5efa781dd66 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 28 Apr 2022 01:23:18 -0700 Subject: [PATCH 20/63] Renaming to SurrogateParameterInfo --- src/Shared/SurrogateParameterInfo.cs | 102 ++++++++++++++++++++++++++ src/Shared/SurrogatedParameterInfo.cs | 55 -------------- 2 files changed, 102 insertions(+), 55 deletions(-) create mode 100644 src/Shared/SurrogateParameterInfo.cs delete mode 100644 src/Shared/SurrogatedParameterInfo.cs diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/SurrogateParameterInfo.cs new file mode 100644 index 000000000000..685b7c7aac40 --- /dev/null +++ b/src/Shared/SurrogateParameterInfo.cs @@ -0,0 +1,102 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.Reflection; + +namespace Microsoft.AspNetCore.Http; + +internal class SurrogateParameterInfo : ParameterInfo +{ + private readonly PropertyInfo _underlyingProperty; + private readonly ParameterInfo? _constructionParameterInfo; + + private readonly NullabilityInfoContext _nullabilityContext; + private NullabilityInfo? _nullabilityInfo; + + public SurrogateParameterInfo(PropertyInfo propertyInfo, NullabilityInfoContext nullabilityContext) + { + Debug.Assert(null != propertyInfo); + + AttrsImpl = (ParameterAttributes)propertyInfo.Attributes; + NameImpl = propertyInfo.Name; + MemberImpl = propertyInfo; + ClassImpl = propertyInfo.PropertyType; + + // It is not a real parameter in the delegate, so, + // not defining a real position. + PositionImpl = -1; + + _nullabilityContext = nullabilityContext; + _underlyingProperty = propertyInfo; + } + + public SurrogateParameterInfo(PropertyInfo property, ParameterInfo parameterInfo, NullabilityInfoContext nullabilityContext) + : this(property, nullabilityContext) + { + _constructionParameterInfo = parameterInfo; + } + + public override bool HasDefaultValue + => _constructionParameterInfo is not null && _constructionParameterInfo.HasDefaultValue; + public override object? DefaultValue + => _constructionParameterInfo is not null ? _constructionParameterInfo.DefaultValue : null; + public override int MetadataToken => _underlyingProperty.MetadataToken; + public override object? RawDefaultValue + => _constructionParameterInfo is not null ? _constructionParameterInfo.RawDefaultValue : null; + + public override object[] GetCustomAttributes(Type attributeType, bool inherit) + { + var attributes = _constructionParameterInfo?.GetCustomAttributes(attributeType, inherit); + + if (attributes == null || attributes is { Length: 0 }) + { + attributes = _underlyingProperty.GetCustomAttributes(attributeType, inherit); + } + + return attributes; + } + + public override object[] GetCustomAttributes(bool inherit) + { + var attributes = _constructionParameterInfo?.GetCustomAttributes(inherit); + + if (attributes == null || attributes is { Length: 0 }) + { + attributes = _underlyingProperty.GetCustomAttributes(inherit); + } + + return attributes; + } + + public override IList GetCustomAttributesData() + { + var attributes = _constructionParameterInfo?.GetCustomAttributesData(); + + if (attributes == null || attributes is { Count: 0 }) + { + attributes = _underlyingProperty.GetCustomAttributesData(); + } + + return attributes; + } + + public override Type[] GetOptionalCustomModifiers() + => _underlyingProperty.GetOptionalCustomModifiers(); + + public override Type[] GetRequiredCustomModifiers() + => _underlyingProperty.GetRequiredCustomModifiers(); + + public override bool IsDefined(Type attributeType, bool inherit) + { + return (_constructionParameterInfo is not null && _constructionParameterInfo.IsDefined(attributeType, inherit)) || + _underlyingProperty.IsDefined(attributeType, inherit); + } + + public new bool IsOptional => NullabilityInfo.ReadState != NullabilityState.NotNull; + + public NullabilityInfo NullabilityInfo + => _nullabilityInfo ??= _constructionParameterInfo is not null ? + _nullabilityContext.Create(_constructionParameterInfo) : + _nullabilityContext.Create(_underlyingProperty); +} diff --git a/src/Shared/SurrogatedParameterInfo.cs b/src/Shared/SurrogatedParameterInfo.cs deleted file mode 100644 index cb6fcec4f1e2..000000000000 --- a/src/Shared/SurrogatedParameterInfo.cs +++ /dev/null @@ -1,55 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Diagnostics; -using System.Reflection; - -namespace Microsoft.AspNetCore.Http; - -internal class SurrogatedParameterInfo : ParameterInfo -{ - private readonly PropertyInfo _underlyingProperty; - private readonly NullabilityInfoContext _nullabilityContext; - private NullabilityInfo? _nullabilityInfo; - - public SurrogatedParameterInfo(PropertyInfo propertyInfo, NullabilityInfoContext nullabilityContext) - { - Debug.Assert(null != propertyInfo); - - AttrsImpl = (ParameterAttributes)propertyInfo.Attributes; - NameImpl = propertyInfo.Name; - MemberImpl = propertyInfo; - ClassImpl = propertyInfo.PropertyType; - PositionImpl = -1;//parameter.Position; - - _nullabilityContext = nullabilityContext; - _underlyingProperty = propertyInfo; - } - - public override bool HasDefaultValue => false; - public override object? DefaultValue => null; - public override int MetadataToken => _underlyingProperty.MetadataToken; - public override object? RawDefaultValue => null; - - public override object[] GetCustomAttributes(Type attributeType, bool inherit) - => _underlyingProperty.GetCustomAttributes(attributeType, inherit); - - public override object[] GetCustomAttributes(bool inherit) - => _underlyingProperty.GetCustomAttributes(inherit); - - public override IList GetCustomAttributesData() - => _underlyingProperty.GetCustomAttributesData(); - - public override Type[] GetOptionalCustomModifiers() - => _underlyingProperty.GetOptionalCustomModifiers(); - - public override Type[] GetRequiredCustomModifiers() - => _underlyingProperty.GetRequiredCustomModifiers(); - - public override bool IsDefined(Type attributeType, bool inherit) - => _underlyingProperty.IsDefined(attributeType, inherit); - - public new bool IsOptional => NullabilityInfo.ReadState != NullabilityState.NotNull; - - public NullabilityInfo NullabilityInfo => _nullabilityInfo ??= _nullabilityContext.Create(_underlyingProperty); -} From 1b8418ef08bb0ecf4eb56e7e9b1e9234667c791c Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 28 Apr 2022 01:23:54 -0700 Subject: [PATCH 21/63] Updating OpenAPI Generator --- .../src/Microsoft.AspNetCore.OpenApi.csproj | 3 +-- src/OpenApi/src/OpenApiGenerator.cs | 16 +++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj b/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj index ba094ce23ffc..f3bedb527dc5 100644 --- a/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj +++ b/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj @@ -17,9 +17,8 @@ - + - diff --git a/src/OpenApi/src/OpenApiGenerator.cs b/src/OpenApi/src/OpenApiGenerator.cs index 31e805c04fd2..910205731059 100644 --- a/src/OpenApi/src/OpenApiGenerator.cs +++ b/src/OpenApi/src/OpenApiGenerator.cs @@ -30,7 +30,6 @@ internal class OpenApiGenerator private readonly IServiceProviderIsService? _serviceProviderIsService; internal static readonly ParameterBindingMethodCache ParameterBindingMethodCache = new(); - internal static readonly ParameterBindingConstructorCache ParameterBindingConstructorCache = new(); /// /// Creates an instance given an @@ -376,10 +375,17 @@ private static IEnumerable FlattenParameters(ParameterInfo[] para flattenedParameters ??= new(parameters[0..i]); nullabilityContext ??= new(); - var constructor = ParameterBindingConstructorCache.GetParameterConstructor(parameters[i]); - if (constructor?.GetParameters() is { Length: > 0 } constructorParameters) + var (constructor, constructorParameters) = ParameterBindingMethodCache.FindConstructor(parameters[i].ParameterType); + if (constructor is not null && constructorParameters is { Length: > 0 }) { - flattenedParameters.AddRange(constructorParameters); + foreach (var constructorParameter in constructorParameters) + { + flattenedParameters.Add( + new SurrogateParameterInfo( + constructorParameter.PropertyInfo, + constructorParameter.ParameterInfo, + nullabilityContext)); + } } else { @@ -388,7 +394,7 @@ private static IEnumerable FlattenParameters(ParameterInfo[] para { if (property.CanWrite) { - flattenedParameters.Add(new SurrogatedParameterInfo(property, nullabilityContext)); + flattenedParameters.Add(new SurrogateParameterInfo(property, nullabilityContext)); } } } From ec3046ed0c40fe68495bed3db8ec3884c965377c Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 28 Apr 2022 01:24:13 -0700 Subject: [PATCH 22/63] Updating ApiExplorer --- .../EndpointMetadataApiDescriptionProvider.cs | 20 ++++++++++++------- ...icrosoft.AspNetCore.Mvc.ApiExplorer.csproj | 3 +-- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index fcad4a079879..639474abef92 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -26,7 +26,6 @@ internal class EndpointMetadataApiDescriptionProvider : IApiDescriptionProvider private readonly IHostEnvironment _environment; private readonly IServiceProviderIsService? _serviceProviderIsService; private readonly ParameterBindingMethodCache ParameterBindingMethodCache = new(); - private readonly ParameterBindingConstructorCache _parameterBindingConstructorCache = new(); private readonly ParameterPolicyFactory _parameterPolicyFactory; // Executes before MVC's DefaultApiDescriptionProvider and GrpcJsonTranscodingDescriptionProvider for no particular reason. @@ -117,7 +116,7 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string var hasBodyOrFormFileParameter = false; - static IEnumerable FlattenParameters(ParameterInfo[] parameters, ParameterBindingConstructorCache cache) + static IEnumerable FlattenParameters(ParameterInfo[] parameters, ParameterBindingMethodCache cache) { if (parameters.Length == 0) { @@ -136,10 +135,17 @@ static IEnumerable FlattenParameters(ParameterInfo[] parameters, flattenedParameters ??= new(parameters[0..i]); nullabilityContext ??= new(); - var constructor = cache.GetParameterConstructor(parameters[i]); - if (constructor?.GetParameters() is { Length: > 0 } constructorParameters) + var (constructor, constructorParameters) = cache.FindConstructor(parameters[i].ParameterType); + if (constructor is not null && constructorParameters is { Length: > 0 }) { - flattenedParameters.AddRange(constructorParameters); + foreach (var constructorParameter in constructorParameters) + { + flattenedParameters.Add( + new SurrogateParameterInfo( + constructorParameter.PropertyInfo, + constructorParameter.ParameterInfo, + nullabilityContext)); + } } else { @@ -148,7 +154,7 @@ static IEnumerable FlattenParameters(ParameterInfo[] parameters, { if (property.CanWrite) { - flattenedParameters.Add(new SurrogatedParameterInfo(property, nullabilityContext)); + flattenedParameters.Add(new SurrogateParameterInfo(property, nullabilityContext)); } } } @@ -162,7 +168,7 @@ static IEnumerable FlattenParameters(ParameterInfo[] parameters, return flattenedParameters is not null ? flattenedParameters : parameters; } - foreach (var parameter in FlattenParameters(methodInfo.GetParameters(), _parameterBindingConstructorCache)) + foreach (var parameter in FlattenParameters(methodInfo.GetParameters(), ParameterBindingMethodCache)) { var parameterDescription = CreateApiParameterDescription(parameter, routeEndpoint.RoutePattern, disableInferredBody); diff --git a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj index 4c41192830df..1fdce20a3d3b 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj +++ b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj @@ -11,8 +11,7 @@ - - + From 4dd3a3eb29d8ff3bf15a7141d8b9c41ca46dd6df Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 28 Apr 2022 01:24:50 -0700 Subject: [PATCH 23/63] Updating RequestDelegateFactory --- ...icrosoft.AspNetCore.Http.Extensions.csproj | 2 +- .../src/RequestDelegateFactory.cs | 92 ++++++------------- 2 files changed, 30 insertions(+), 64 deletions(-) diff --git a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj index bf1812bf7dc9..6b1483b12b24 100644 --- a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj +++ b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj @@ -13,7 +13,7 @@ - + diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 65112ad85cf1..c6fff5b00017 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -219,7 +219,7 @@ private static FactoryContext CreateFactoryContext(RequestDelegateFactoryOptions AddTypeProvidedMetadata(methodInfo, factoryContext.Metadata, factoryContext.ServiceProvider, - factoryContext.SurrogatedParameters.ToArray()); + factoryContext.SurrogateParameters.ToArray()); // Add method attributes as metadata *after* any inferred metadata so that the attributes hava a higher specificity AddMethodAttributesAsMetadata(methodInfo, factoryContext.Metadata); @@ -420,7 +420,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext if (parameter.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType)) || parameter.ParameterType.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) { - return BindParameterFromSurrogatedProperties(parameter, factoryContext); + return BindParameterFromSurrogateProperties(parameter, factoryContext); } else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } routeAttribute) { @@ -1093,72 +1093,31 @@ private static Expression GetValueFromProperty(Expression sourceExpression, stri var indexExpression = Expression.MakeIndex(sourceExpression, itemProperty, indexArguments); return Expression.Convert(indexExpression, returnType ?? typeof(string)); } + - private static Expression BindParameterFromSurrogatedProperties(ParameterInfo parameter, FactoryContext factoryContext) + private static Expression BindParameterFromSurrogateProperties(ParameterInfo parameter, FactoryContext factoryContext) { - var properties = parameter.ParameterType.GetProperties(); var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); + var (constructor, parameters) = ParameterBindingMethodCache.FindConstructor(parameter.ParameterType); - static ConstructorInfo? GetConstructor(ParameterInfo parameter, PropertyInfo[] properties) - { - if (parameter.ParameterType.IsValueType) - { - // Value types always have a default constructor, we will use - // the parameter type during the NewExpression creation - return null; - } - - // Try to find the parameterless constructor - var constructor = parameter.ParameterType.GetConstructor(Array.Empty()); - if (constructor is not null) - { - return constructor; - } - - // If a parameterless ctor is not defined - // we will try to find a ctor that includes all - // property types and in the right order. - // Eg.: - // public class Sample - // { - // // Valid - // public Sample(int id, string name){} - // // InValid - // public Sample(string name, int id){} - // - // public int Id { get; set; } - // public string Name { get; set; } - //} - - var types = new Type[properties.Length]; - for (var i = 0; i < properties.Length; i++) - { - types[i] = properties[i].PropertyType; - } - constructor = parameter.ParameterType.GetConstructor(types); - - if (constructor is not null) - { - return constructor; - } - - throw new InvalidOperationException($"No '{parameter.ParameterType}' public parameterless constructor found for {parameter.Name}."); - } - - var constructor = GetConstructor(parameter, properties); - if (constructor?.GetParameters() is { Length: > 0 } parameters) + if (constructor is not null && parameters is { Length: > 0 }) { // arg_local = new T(....) - var constructorArguments = new Expression[properties.Length]; - for (var i = 0; i < properties.Length; i++) + var constructorArguments = new Expression[parameters.Length]; + + for (var i = 0; i < parameters.Length; i++) { - var parameterInfo = new SurrogatedParameterInfo(properties[i], factoryContext.NullabilityContext); + var parameterInfo = + new SurrogateParameterInfo(parameters[i].PropertyInfo, parameters[i].ParameterInfo, factoryContext.NullabilityContext); constructorArguments[i] = CreateArgument(parameterInfo, factoryContext); - factoryContext.SurrogatedParameters.Add(parameterInfo); + factoryContext.SurrogateParameters.Add(parameterInfo); } - factoryContext.ParamCheckExpressions.Add(Expression.Assign(argumentExpression, Expression.New(constructor, constructorArguments))); + factoryContext.ParamCheckExpressions.Add( + Expression.Assign( + argumentExpression, + Expression.New(constructor, constructorArguments))); } else { @@ -1168,6 +1127,7 @@ private static Expression BindParameterFromSurrogatedProperties(ParameterInfo pa // arg_local.Property[n] = expression[n], // } + var properties = parameter.ParameterType.GetProperties(); var bindings = new List(properties.Length); for (var i = 0; i < properties.Length; i++) @@ -1175,14 +1135,20 @@ private static Expression BindParameterFromSurrogatedProperties(ParameterInfo pa // For parameterless ctor we will init only writable properties. if (properties[i].CanWrite) { - var parameterInfo = new SurrogatedParameterInfo(properties[i], factoryContext.NullabilityContext); + var parameterInfo = new SurrogateParameterInfo(properties[i], factoryContext.NullabilityContext); bindings.Add(Expression.Bind(properties[i], CreateArgument(parameterInfo, factoryContext))); - factoryContext.SurrogatedParameters.Add(parameterInfo); + factoryContext.SurrogateParameters.Add(parameterInfo); } } - var newExpression = constructor is null ? Expression.New(parameter.ParameterType) : Expression.New(constructor); - factoryContext.ParamCheckExpressions.Add(Expression.Assign(argumentExpression, Expression.MemberInit(newExpression, bindings))); + var newExpression = constructor is null ? + Expression.New(parameter.ParameterType) : + Expression.New(constructor); + + factoryContext.ParamCheckExpressions.Add( + Expression.Assign( + argumentExpression, + Expression.MemberInit(newExpression, bindings))); } factoryContext.TrackedParameters.Add(parameter.Name!, RequestDelegateFactoryConstants.SurrogatedParameter); @@ -1681,7 +1647,7 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al private static bool IsOptionalParameter(ParameterInfo parameter, FactoryContext factoryContext) { - if (parameter is SurrogatedParameterInfo argument) + if (parameter is SurrogateParameterInfo argument) { return argument.IsOptional; } @@ -1919,7 +1885,7 @@ private class FactoryContext public List BoxedArgs { get; } = new(); public List>? Filters { get; init; } - public List SurrogatedParameters { get; } = new(); + public List SurrogateParameters { get; } = new(); } private static class RequestDelegateFactoryConstants From 8cbbeef49a4af68242ebd3e370f9cd8588965a3b Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 28 Apr 2022 01:25:13 -0700 Subject: [PATCH 24/63] Adding initial test cases --- .../test/RequestDelegateFactoryTests.cs | 84 ++++++++++++++++++- 1 file changed, 82 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 727219b43b98..89c0ab3ae2e0 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -2055,6 +2055,69 @@ private class BadBindAsyncClass throw new NotImplementedException(); } + [Fact] + public void BuildRequestDelegateThrowsInvalidOperationExceptionForInvalidParameterListConstructor() + { + void TestParameterListRecord([Parameters] BadArgumentListRecord req) { } + void TestParameterListClass([Parameters] BadArgumentListClass req) { } + void TestParameterListClassWithMutipleConstructors([Parameters] BadArgumentListClassMultipleCtors req) { } + void TestParameterListAbstractClass([Parameters] BadAbstractArgumentListClass req) { } + void TestParameterListNoPulicConstructorClass([Parameters] BadNoPublicConstructorArgumentListClass req) { } + + Assert.Throws(() => RequestDelegateFactory.Create(TestParameterListRecord)); + Assert.Throws(() => RequestDelegateFactory.Create(TestParameterListClass)); + Assert.Throws(() => RequestDelegateFactory.Create(TestParameterListClassWithMutipleConstructors)); + Assert.Throws(() => RequestDelegateFactory.Create(TestParameterListAbstractClass)); + Assert.Throws(() => RequestDelegateFactory.Create(TestParameterListNoPulicConstructorClass)); + } + + private record BadArgumentListRecord(int Foo) + { + public BadArgumentListRecord(int foo, int bar) + : this(foo) + { + + } + + public int Bar { get; set; } + } + + private class BadNoPublicConstructorArgumentListClass + { + private BadNoPublicConstructorArgumentListClass() + { } + + public int Foo { get; set; } + } + + private abstract class BadAbstractArgumentListClass + { + public int Foo { get; set; } + } + + private class BadArgumentListClass + { + public BadArgumentListClass(int foo, string name) + { + } + + public int Foo { get; set; } + public int Bar { get; set; } + } + private class BadArgumentListClassMultipleCtors + { + public BadArgumentListClassMultipleCtors(int foo) + { + } + + public BadArgumentListClassMultipleCtors(int foo, int bar) + { + } + + public int Foo { get; set; } + public int Bar { get; set; } + } + public static object[][] ExplicitFromServiceActions { get @@ -2719,13 +2782,20 @@ public async Task RequestDelegateWritesNullReturnNullValue(Delegate @delegate) Assert.Equal("null", responseBody); } + record NullableParametersRecord(string? Name); + record DefaultValueParametersRecord(string? Name = "DefaultName"); + record RequiredParametersRecord(string Name); + public static IEnumerable QueryParamOptionalityData { get { string requiredQueryParam(string name) => $"Hello {name}!"; string defaultValueQueryParam(string name = "DefaultName") => $"Hello {name}!"; + string requiredQueryParamFromArgumentList([Parameters] RequiredParametersRecord req) => $"Hello {req.Name}!"; + string defaultValueQueryParamFromArgumentList([Parameters] DefaultValueParametersRecord req) => $"Hello {req.Name}!"; string nullableQueryParam(string? name) => $"Hello {name}!"; + string nullableQueryParamFromArgumentList([Parameters] NullableParametersRecord req) => $"Hello {req.Name}!"; string requiredParseableQueryParam(int age) => $"Age: {age}"; string defaultValueParseableQueryParam(int age = 12) => $"Age: {age}"; string nullableQueryParseableParam(int? age) => $"Age: {age}"; @@ -2733,11 +2803,17 @@ public async Task RequestDelegateWritesNullReturnNullValue(Delegate @delegate) return new List { new object?[] { (Func)requiredQueryParam, "name", null, true, null}, + new object?[] { (Func)requiredQueryParamFromArgumentList, "Name", null, true, null}, new object?[] { (Func)requiredQueryParam, "name", "TestName", false, "Hello TestName!" }, + new object?[] { (Func)requiredQueryParamFromArgumentList, "Name", "TestName", false, "Hello TestName!" }, new object?[] { (Func)defaultValueQueryParam, "name", null, false, "Hello DefaultName!" }, + new object?[] { (Func)defaultValueQueryParamFromArgumentList, "Name", null, false, "Hello DefaultName!" }, new object?[] { (Func)defaultValueQueryParam, "name", "TestName", false, "Hello TestName!" }, + new object?[] { (Func)defaultValueQueryParamFromArgumentList, "Name", "TestName", false, "Hello TestName!" }, new object?[] { (Func)nullableQueryParam, "name", null, false, "Hello !" }, + new object?[] { (Func)nullableQueryParamFromArgumentList, "Name", null, false, "Hello !" }, new object?[] { (Func)nullableQueryParam, "name", "TestName", false, "Hello TestName!"}, + new object?[] { (Func)nullableQueryParamFromArgumentList, "Name", "TestName", false, "Hello TestName!"}, new object?[] { (Func)requiredParseableQueryParam, "age", null, true, null}, new object?[] { (Func)requiredParseableQueryParam, "age", "42", false, "Age: 42" }, @@ -2778,7 +2854,7 @@ public async Task RequestDelegateHandlesQueryParamOptionality(Delegate @delegate var log = Assert.Single(logs); Assert.Equal(LogLevel.Debug, log.LogLevel); Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), log.EventId); - var expectedType = paramName == "age" ? "int age" : "string name"; + var expectedType = paramName == "age" ? "int age" : $"string {paramName}"; Assert.Equal($@"Required parameter ""{expectedType}"" was not provided from route or query string.", log.Message); } else @@ -2795,7 +2871,9 @@ public async Task RequestDelegateHandlesQueryParamOptionality(Delegate @delegate get { string requiredRouteParam(string name) => $"Hello {name}!"; + string requiredRouteParamFromArgumentList([Parameters] RequiredParametersRecord req) => $"Hello {req.Name}!"; string defaultValueRouteParam(string name = "DefaultName") => $"Hello {name}!"; + string defaultValueRouteParamFromArgumentList([Parameters] DefaultValueParametersRecord req) => $"Hello {req.Name}!"; string nullableRouteParam(string? name) => $"Hello {name}!"; string requiredParseableRouteParam(int age) => $"Age: {age}"; string defaultValueParseableRouteParam(int age = 12) => $"Age: {age}"; @@ -2804,7 +2882,9 @@ public async Task RequestDelegateHandlesQueryParamOptionality(Delegate @delegate return new List { new object?[] { (Func)requiredRouteParam, "name", null, true, null}, + new object?[] { (Func)requiredRouteParamFromArgumentList, "Name", null, true, null}, new object?[] { (Func)requiredRouteParam, "name", "TestName", false, "Hello TestName!" }, + new object?[] { (Func)requiredRouteParamFromArgumentList, "Name", "TestName", false, "Hello TestName!" }, new object?[] { (Func)defaultValueRouteParam, "name", null, false, "Hello DefaultName!" }, new object?[] { (Func)defaultValueRouteParam, "name", "TestName", false, "Hello TestName!" }, new object?[] { (Func)nullableRouteParam, "name", null, false, "Hello !" }, @@ -2850,7 +2930,7 @@ public async Task RequestDelegateHandlesRouteParamOptionality(Delegate @delegate var log = Assert.Single(logs); Assert.Equal(LogLevel.Debug, log.LogLevel); Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), log.EventId); - var expectedType = paramName == "age" ? "int age" : "string name"; + var expectedType = paramName == "age" ? "int age" : $"string {paramName}"; Assert.Equal($@"Required parameter ""{expectedType}"" was not provided from query string.", log.Message); } else From c2f9c710b78393469138f186c71884a8a6e5b827 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 28 Apr 2022 05:55:15 -0700 Subject: [PATCH 25/63] Rollback bad change --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 0b7fa0369000..69eb42578ebb 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -504,13 +504,13 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory private static Expression CreateArgument(ParameterInfo parameter, FactoryContext factoryContext) { - var parameterCustomAttributes = parameter.GetCustomAttributes(); - if (parameter.Name is null) { throw new InvalidOperationException($"Encountered a parameter of type '{parameter.ParameterType}' without a name. Parameters must have a name."); } + var parameterCustomAttributes = parameter.GetCustomAttributes(); + if (parameter.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType)) || parameter.ParameterType.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) { From 88a69adac2b681f12c731cf4c565b78b952bdd1b Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 28 Apr 2022 07:36:08 -0700 Subject: [PATCH 26/63] Fixing merge issues --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 69eb42578ebb..b6bb3ac1988f 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -383,7 +383,7 @@ private static Expression MapHandlerReturnTypeToValueTask(Expression methodCall, return ExecuteAwaited(task); } - private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List metadata, IServiceProvider? services) + private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List metadata, IServiceProvider? services, ParameterInfo[] surrogateParameters) { object?[]? invokeArgs = null; @@ -415,7 +415,7 @@ void AddMetadata(ParameterInfo[] parameters) AddMetadata(methodInfo.GetParameters()); // Get metadata from surrogated parameter types - AddMetadata(surrogatedParameters); + AddMetadata(surrogateParameters); // Get metadata from return type var returnType = methodInfo.ReturnType; From e21d54fea737205eb2b9f6cec70e58ce7e553dcc Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 28 Apr 2022 13:59:16 -0700 Subject: [PATCH 27/63] Initial SurrogateParameterInfo tests --- .../test/SurrogateParameterInfoTests.cs | 66 +++++++++++++++++++ src/Shared/SurrogateParameterInfo.cs | 6 +- 2 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs diff --git a/src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs b/src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs new file mode 100644 index 000000000000..3977217ac12b --- /dev/null +++ b/src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs @@ -0,0 +1,66 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Reflection; + +namespace Microsoft.AspNetCore.Http.Extensions.Tests; + +internal class SurrogateParameterInfoTests +{ + public void Initialization_SetsTypeAndNameFromPropertyInfo() + { + // Arrange & Act + var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.NoAttribute)); + var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo); + + // Assert + Assert.Equal(propertyInfo.Name, surrogateParameterInfo.Name); + Assert.Equal(propertyInfo.PropertyType, surrogateParameterInfo.ParameterType); + } + + public void SurrogateParameterInfo_ContainsPropertyCustomAttributes() + { + // Arrange & Act + var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.PropertyWithCustomAttribute)); + var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo); + + // Assert + Assert.Equal(propertyInfo.CustomAttributes, surrogateParameterInfo.CustomAttributes); + Assert.Single(surrogateParameterInfo.GetCustomAttributes(typeof(TestAttribute))); + } + + public void SurrogateParameterInfo_ContainsPropertyInheritedCustomAttributes() + { + // Arrange & Act + var propertyInfo = GetProperty(typeof(DerivedArgumentList), nameof(DerivedArgumentList.PropertyWithCustomAttribute)); + var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo); + + // Assert + Assert.Equal(propertyInfo.CustomAttributes, surrogateParameterInfo.CustomAttributes); + Assert.Single(surrogateParameterInfo.GetCustomAttributes(typeof(TestAttribute))); + } + + private static PropertyInfo GetProperty(Type containerType, string propertyName) + => containerType.GetProperty(propertyName); + + private class ArgumentList + { + public int NoAttribute { get; set; } + + [TestAttribute] + public virtual int PropertyWithCustomAttribute { get; set; } + } + + private class DerivedArgumentList : ArgumentList + { + public override int PropertyWithCustomAttribute + { + get => base.PropertyWithCustomAttribute; + set => base.PropertyWithCustomAttribute = value; + } + } + + [AttributeUsage(AttributeTargets.Property, Inherited = true)] + private class TestAttribute : Attribute + { } +} diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/SurrogateParameterInfo.cs index 685b7c7aac40..174179658013 100644 --- a/src/Shared/SurrogateParameterInfo.cs +++ b/src/Shared/SurrogateParameterInfo.cs @@ -14,10 +14,12 @@ internal class SurrogateParameterInfo : ParameterInfo private readonly NullabilityInfoContext _nullabilityContext; private NullabilityInfo? _nullabilityInfo; - public SurrogateParameterInfo(PropertyInfo propertyInfo, NullabilityInfoContext nullabilityContext) + public SurrogateParameterInfo(PropertyInfo propertyInfo, NullabilityInfoContext? nullabilityContext = null) { Debug.Assert(null != propertyInfo); + ArgumentNullException.ThrowIfNull(nullabilityContext); + AttrsImpl = (ParameterAttributes)propertyInfo.Attributes; NameImpl = propertyInfo.Name; MemberImpl = propertyInfo; @@ -27,7 +29,7 @@ public SurrogateParameterInfo(PropertyInfo propertyInfo, NullabilityInfoContext // not defining a real position. PositionImpl = -1; - _nullabilityContext = nullabilityContext; + _nullabilityContext = nullabilityContext ?? new NullabilityInfoContext(); _underlyingProperty = propertyInfo; } From 3ba8262395b6bdf9c6e226d4ca7f030a193cc790 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 29 Apr 2022 09:47:27 -0700 Subject: [PATCH 28/63] Adding surrogateparameterinfo tests --- .../src/RequestDelegateFactory.cs | 143 ++++++++-------- .../test/SurrogateParameterInfoTests.cs | 156 ++++++++++++++++-- src/Shared/SurrogateParameterInfo.cs | 16 +- 3 files changed, 223 insertions(+), 92 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index b6bb3ac1988f..58acd56b6402 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -468,7 +468,16 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory for (var i = 0; i < parameters.Length; i++) { - args[i] = CreateArgument(parameters[i], factoryContext); + if (parameters[i].CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType)) || + parameters[i].ParameterType.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) + { + args[i] = CreateSurrogateArgument(parameters[i], factoryContext); + } + else + { + args[i] = CreateArgument(parameters[i], factoryContext); + } + // Register expressions containing the boxed and unboxed variants // of the route handler's arguments for use in RouteHandlerInvocationContext // construction and route handler invocation. @@ -511,12 +520,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext var parameterCustomAttributes = parameter.GetCustomAttributes(); - if (parameter.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType)) || - parameter.ParameterType.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) - { - return BindParameterFromSurrogateProperties(parameter, factoryContext); - } - else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } routeAttribute) + if (parameterCustomAttributes.OfType().FirstOrDefault() is { } routeAttribute) { var routeName = routeAttribute.Name ?? parameter.Name; factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.RouteAttribute); @@ -659,6 +663,68 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext } } + private static Expression CreateSurrogateArgument(ParameterInfo parameter, FactoryContext factoryContext) + { + var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); + var (constructor, parameters) = ParameterBindingMethodCache.FindConstructor(parameter.ParameterType); + + if (constructor is not null && parameters is { Length: > 0 }) + { + // arg_local = new T(....) + + var constructorArguments = new Expression[parameters.Length]; + + for (var i = 0; i < parameters.Length; i++) + { + var parameterInfo = + new SurrogateParameterInfo(parameters[i].PropertyInfo, parameters[i].ParameterInfo, factoryContext.NullabilityContext); + constructorArguments[i] = CreateArgument(parameterInfo, factoryContext); + factoryContext.SurrogateParameters.Add(parameterInfo); + } + + factoryContext.ParamCheckExpressions.Add( + Expression.Assign( + argumentExpression, + Expression.New(constructor, constructorArguments))); + } + else + { + // arg_local = new T() + // { + // arg_local.Property[0] = expression[0], + // arg_local.Property[n] = expression[n], + // } + + var properties = parameter.ParameterType.GetProperties(); + var bindings = new List(properties.Length); + + for (var i = 0; i < properties.Length; i++) + { + // For parameterless ctor we will init only writable properties. + if (properties[i].CanWrite) + { + var parameterInfo = new SurrogateParameterInfo(properties[i], factoryContext.NullabilityContext); + bindings.Add(Expression.Bind(properties[i], CreateArgument(parameterInfo, factoryContext))); + factoryContext.SurrogateParameters.Add(parameterInfo); + } + } + + var newExpression = constructor is null ? + Expression.New(parameter.ParameterType) : + Expression.New(constructor); + + factoryContext.ParamCheckExpressions.Add( + Expression.Assign( + argumentExpression, + Expression.MemberInit(newExpression, bindings))); + } + + factoryContext.TrackedParameters.Add(parameter.Name!, RequestDelegateFactoryConstants.SurrogatedParameter); + factoryContext.ExtraLocals.Add(argumentExpression); + + return argumentExpression; + } + private static Expression CreateMethodCall(MethodInfo methodInfo, Expression? target, Expression[] arguments) => target is null ? Expression.Call(methodInfo, arguments) : @@ -1188,69 +1254,6 @@ private static Expression GetValueFromProperty(Expression sourceExpression, stri var indexExpression = Expression.MakeIndex(sourceExpression, itemProperty, indexArguments); return Expression.Convert(indexExpression, returnType ?? typeof(string)); } - - - private static Expression BindParameterFromSurrogateProperties(ParameterInfo parameter, FactoryContext factoryContext) - { - var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); - var (constructor, parameters) = ParameterBindingMethodCache.FindConstructor(parameter.ParameterType); - - if (constructor is not null && parameters is { Length: > 0 }) - { - // arg_local = new T(....) - - var constructorArguments = new Expression[parameters.Length]; - - for (var i = 0; i < parameters.Length; i++) - { - var parameterInfo = - new SurrogateParameterInfo(parameters[i].PropertyInfo, parameters[i].ParameterInfo, factoryContext.NullabilityContext); - constructorArguments[i] = CreateArgument(parameterInfo, factoryContext); - factoryContext.SurrogateParameters.Add(parameterInfo); - } - - factoryContext.ParamCheckExpressions.Add( - Expression.Assign( - argumentExpression, - Expression.New(constructor, constructorArguments))); - } - else - { - // arg_local = new T() - // { - // arg_local.Property[0] = expression[0], - // arg_local.Property[n] = expression[n], - // } - - var properties = parameter.ParameterType.GetProperties(); - var bindings = new List(properties.Length); - - for (var i = 0; i < properties.Length; i++) - { - // For parameterless ctor we will init only writable properties. - if (properties[i].CanWrite) - { - var parameterInfo = new SurrogateParameterInfo(properties[i], factoryContext.NullabilityContext); - bindings.Add(Expression.Bind(properties[i], CreateArgument(parameterInfo, factoryContext))); - factoryContext.SurrogateParameters.Add(parameterInfo); - } - } - - var newExpression = constructor is null ? - Expression.New(parameter.ParameterType) : - Expression.New(constructor); - - factoryContext.ParamCheckExpressions.Add( - Expression.Assign( - argumentExpression, - Expression.MemberInit(newExpression, bindings))); - } - - factoryContext.TrackedParameters.Add(parameter.Name!, RequestDelegateFactoryConstants.SurrogatedParameter); - factoryContext.ExtraLocals.Add(argumentExpression); - - return argumentExpression; - } private static Expression BindParameterFromService(ParameterInfo parameter, FactoryContext factoryContext) { diff --git a/src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs b/src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs index 3977217ac12b..6b143d4d3328 100644 --- a/src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs +++ b/src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs @@ -5,8 +5,9 @@ namespace Microsoft.AspNetCore.Http.Extensions.Tests; -internal class SurrogateParameterInfoTests +public class SurrogateParameterInfoTests { + [Fact] public void Initialization_SetsTypeAndNameFromPropertyInfo() { // Arrange & Act @@ -18,49 +19,180 @@ public void Initialization_SetsTypeAndNameFromPropertyInfo() Assert.Equal(propertyInfo.PropertyType, surrogateParameterInfo.ParameterType); } + [Fact] + public void Initialization_WithConstructorArgument_SetsTypeAndNameFromPropertyInfo() + { + // Arrange & Act + var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.NoAttribute)); + var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.NoAttribute)); + var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + + // Assert + Assert.Equal(propertyInfo.Name, surrogateParameterInfo.Name); + Assert.Equal(propertyInfo.PropertyType, surrogateParameterInfo.ParameterType); + } + + [Fact] public void SurrogateParameterInfo_ContainsPropertyCustomAttributes() { // Arrange & Act - var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.PropertyWithCustomAttribute)); + var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.WithTestAttribute)); var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo); // Assert - Assert.Equal(propertyInfo.CustomAttributes, surrogateParameterInfo.CustomAttributes); Assert.Single(surrogateParameterInfo.GetCustomAttributes(typeof(TestAttribute))); } + [Fact] + public void SurrogateParameterInfo_WithConstructorArgument_UsesParameterCustomAttributes() + { + // Arrange & Act + var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.NoAttribute)); + var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.WithTestAttribute)); + var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + + // Assert + Assert.Single(surrogateParameterInfo.GetCustomAttributes(typeof(TestAttribute))); + } + + [Fact] + public void SurrogateParameterInfo_WithConstructorArgument_FallbackToPropertyCustomAttributes() + { + // Arrange & Act + var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.WithTestAttribute)); + var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.NoAttribute)); + var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + + // Assert + Assert.Single(surrogateParameterInfo.GetCustomAttributes(typeof(TestAttribute))); + } + + [Fact] + public void SurrogateParameterInfo_ContainsPropertyCustomAttributesData() + { + // Arrange & Act + var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.WithTestAttribute)); + var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo); + + // Assert + Assert.Single( + surrogateParameterInfo.GetCustomAttributesData(), + a => typeof(TestAttribute).IsAssignableFrom(a.AttributeType)); + } + + [Fact] + public void SurrogateParameterInfo_WithConstructorArgument_MergePropertyAndParameterCustomAttributesData() + { + // Arrange & Act + var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.WithTestAttribute)); + var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.WithSampleAttribute)); + var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + + // Assert + Assert.Single( + surrogateParameterInfo.GetCustomAttributesData(), + a => typeof(TestAttribute).IsAssignableFrom(a.AttributeType)); + Assert.Single( + surrogateParameterInfo.GetCustomAttributesData(), + a => typeof(SampleAttribute).IsAssignableFrom(a.AttributeType)); + } + + [Fact] public void SurrogateParameterInfo_ContainsPropertyInheritedCustomAttributes() { // Arrange & Act - var propertyInfo = GetProperty(typeof(DerivedArgumentList), nameof(DerivedArgumentList.PropertyWithCustomAttribute)); + var propertyInfo = GetProperty(typeof(DerivedArgumentList), nameof(DerivedArgumentList.WithTestAttribute)); var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo); // Assert - Assert.Equal(propertyInfo.CustomAttributes, surrogateParameterInfo.CustomAttributes); - Assert.Single(surrogateParameterInfo.GetCustomAttributes(typeof(TestAttribute))); + Assert.Single(surrogateParameterInfo.GetCustomAttributes(typeof(TestAttribute), true)); + } + + [Fact] + public void SurrogateParameterInfo_DoesNotHaveDefaultValueFromProperty() + { + // Arrange & Act + var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.NoAttribute)); + var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo); + + // Assert + Assert.False(surrogateParameterInfo.HasDefaultValue); + } + + [Fact] + public void SurrogateParameterInfo_WithConstructorArgument_HasDefaultValue() + { + // Arrange & Act + var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.NoAttribute)); + var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), "withDefaultValue"); + var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + + // Assert + Assert.True(surrogateParameterInfo.HasDefaultValue); + Assert.NotNull(surrogateParameterInfo.DefaultValue); + Assert.IsType(surrogateParameterInfo.DefaultValue); + Assert.NotNull(surrogateParameterInfo.RawDefaultValue); + Assert.IsType(surrogateParameterInfo.RawDefaultValue); + } + + [Fact] + public void SurrogateParameterInfo_WithConstructorArgument_DoesNotHaveDefaultValue() + { + // Arrange & Act + var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.NoAttribute)); + var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.NoAttribute)); + var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + + // Assert + Assert.False(surrogateParameterInfo.HasDefaultValue); } private static PropertyInfo GetProperty(Type containerType, string propertyName) => containerType.GetProperty(propertyName); + private static ParameterInfo GetParameter(string methodName, string parameterName) + { + var methodInfo = typeof(ArgumentList).GetMethod(methodName); + var parameters = methodInfo.GetParameters(); + return parameters.Single(p => p.Name.Equals(parameterName, StringComparison.OrdinalIgnoreCase)); + } + private class ArgumentList { public int NoAttribute { get; set; } - [TestAttribute] - public virtual int PropertyWithCustomAttribute { get; set; } + [Test] + public virtual int WithTestAttribute { get; set; } + + [Sample] + public int WithSampleAttribute { get; set; } + + public void DefaultMethod( + int noAttribute, + [Test] int withTestAttribute, + [Sample] int withSampleAttribute, + int withDefaultValue = 10) + { } } private class DerivedArgumentList : ArgumentList { - public override int PropertyWithCustomAttribute + [DerivedTest] + public override int WithTestAttribute { - get => base.PropertyWithCustomAttribute; - set => base.PropertyWithCustomAttribute = value; + get => base.WithTestAttribute; + set => base.WithTestAttribute = value; } } - [AttributeUsage(AttributeTargets.Property, Inherited = true)] + [AttributeUsage(AttributeTargets.Property | AttributeTargets.Parameter, Inherited = true)] + private class SampleAttribute : Attribute + { } + + [AttributeUsage(AttributeTargets.Property | AttributeTargets.Parameter, Inherited = true)] private class TestAttribute : Attribute { } + + private class DerivedTestAttribute : TestAttribute + { } } diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/SurrogateParameterInfo.cs index 174179658013..5cb7b42cad40 100644 --- a/src/Shared/SurrogateParameterInfo.cs +++ b/src/Shared/SurrogateParameterInfo.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.ObjectModel; using System.Diagnostics; using System.Reflection; @@ -18,8 +19,6 @@ public SurrogateParameterInfo(PropertyInfo propertyInfo, NullabilityInfoContext? { Debug.Assert(null != propertyInfo); - ArgumentNullException.ThrowIfNull(nullabilityContext); - AttrsImpl = (ParameterAttributes)propertyInfo.Attributes; NameImpl = propertyInfo.Name; MemberImpl = propertyInfo; @@ -33,7 +32,7 @@ public SurrogateParameterInfo(PropertyInfo propertyInfo, NullabilityInfoContext? _underlyingProperty = propertyInfo; } - public SurrogateParameterInfo(PropertyInfo property, ParameterInfo parameterInfo, NullabilityInfoContext nullabilityContext) + public SurrogateParameterInfo(PropertyInfo property, ParameterInfo parameterInfo, NullabilityInfoContext? nullabilityContext = null) : this(property, nullabilityContext) { _constructionParameterInfo = parameterInfo; @@ -73,14 +72,11 @@ public override object[] GetCustomAttributes(bool inherit) public override IList GetCustomAttributesData() { - var attributes = _constructionParameterInfo?.GetCustomAttributesData(); + var attributes = new List( + _constructionParameterInfo?.GetCustomAttributesData() ?? Array.Empty()); + attributes.AddRange(_underlyingProperty.GetCustomAttributesData()); - if (attributes == null || attributes is { Count: 0 }) - { - attributes = _underlyingProperty.GetCustomAttributesData(); - } - - return attributes; + return attributes.AsReadOnly(); } public override Type[] GetOptionalCustomModifiers() From e50922c52c60b3c1315ee6416a9f62b24d824666 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 29 Apr 2022 09:58:45 -0700 Subject: [PATCH 29/63] Using Span --- .../src/EndpointMetadataApiDescriptionProvider.cs | 5 +++-- src/OpenApi/src/OpenApiGenerator.cs | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 639474abef92..6bbe70d791ec 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Security.Claims; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Metadata; @@ -116,7 +117,7 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string var hasBodyOrFormFileParameter = false; - static IEnumerable FlattenParameters(ParameterInfo[] parameters, ParameterBindingMethodCache cache) + static ReadOnlySpan FlattenParameters(ParameterInfo[] parameters, ParameterBindingMethodCache cache) { if (parameters.Length == 0) { @@ -165,7 +166,7 @@ static IEnumerable FlattenParameters(ParameterInfo[] parameters, } } - return flattenedParameters is not null ? flattenedParameters : parameters; + return flattenedParameters is not null ? CollectionsMarshal.AsSpan(flattenedParameters) : parameters.AsSpan(); } foreach (var parameter in FlattenParameters(methodInfo.GetParameters(), ParameterBindingMethodCache)) diff --git a/src/OpenApi/src/OpenApiGenerator.cs b/src/OpenApi/src/OpenApiGenerator.cs index 910205731059..43552dfa31df 100644 --- a/src/OpenApi/src/OpenApiGenerator.cs +++ b/src/OpenApi/src/OpenApiGenerator.cs @@ -5,6 +5,7 @@ using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Security.Claims; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Metadata; @@ -356,7 +357,7 @@ private List GetOperationTags(MethodInfo methodInfo, EndpointMetadat : new List() { new OpenApiTag() { Name = controllerName } }; } - private static IEnumerable FlattenParameters(ParameterInfo[] parameters) + private static ReadOnlySpan FlattenParameters(ParameterInfo[] parameters) { if (parameters.Length == 0) { @@ -405,7 +406,7 @@ private static IEnumerable FlattenParameters(ParameterInfo[] para } } - return flattenedParameters is not null ? flattenedParameters : parameters; + return flattenedParameters is not null ? CollectionsMarshal.AsSpan(flattenedParameters) : parameters.AsSpan(); } private List GetOpenApiParameters(MethodInfo methodInfo, EndpointMetadataCollection metadata, RoutePattern pattern, bool disableInferredBody) From 337debcd884b6ff6299bd01e217fba8cbc9309a2 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 29 Apr 2022 15:15:51 -0700 Subject: [PATCH 30/63] Adding FindConstructor unit tests --- .../test/ParameterBindingMethodCacheTests.cs | 266 ++++++++++++++++++ 1 file changed, 266 insertions(+) diff --git a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs index 6e8043b5c4f4..bc7834457fe8 100644 --- a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs +++ b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs @@ -359,6 +359,70 @@ public async Task FindBindAsyncMethod_FindsFallbackMethodFromInheritedWhenPrefer Assert.Null(await parseHttpContext(httpContext)); } + [Theory] + [InlineData(typeof(ClassWithParameterlessConstructor))] + [InlineData(typeof(RecordClassParameterlessConstructor))] + [InlineData(typeof(StructWithParameterlessConstructor))] + [InlineData(typeof(RecordStructWithParameterlessConstructor))] + public void FindConstructor_FindsParameterlessConstructor_WhenExplicitlyDeclared(Type type) + { + var cache = new ParameterBindingMethodCache(); + var (constructor, parameters) = cache.FindConstructor(type); + + Assert.NotNull(constructor); + Assert.True(parameters.Length == 0); + } + + [Theory] + [InlineData(typeof(ClassWithDefaultConstructor))] + [InlineData(typeof(RecordClassWithDefaultConstructor))] + public void FindConstructor_FindsDefaultConstructor_WhenNotExplictlyDeclared(Type type) + { + var cache = new ParameterBindingMethodCache(); + var (constructor, parameters) = cache.FindConstructor(type); + + Assert.NotNull(constructor); + Assert.True(parameters.Length == 0); + } + + [Theory] + [InlineData(typeof(ClassWithParameterizedConstructor))] + [InlineData(typeof(RecordClassParameterizedConstructor))] + [InlineData(typeof(StructWithParameterizedConstructor))] + [InlineData(typeof(RecordStructParameterizedConstructor))] + public void FindConstructor_FindsParameterizedConstructor_WhenExplictlyDeclared(Type type) + { + var cache = new ParameterBindingMethodCache(); + var (constructor, parameters) = cache.FindConstructor(type); + + Assert.NotNull(constructor); + Assert.True(parameters.Length == 1); + } + + [Theory] + [InlineData(typeof(StructWithDefaultConstructor))] + [InlineData(typeof(RecordStructWithDefaultConstructor))] + public void FindConstructor_ReturnNullForStruct_WhenNotExplictlyDeclared(Type type) + { + var cache = new ParameterBindingMethodCache(); + var (constructor, parameters) = cache.FindConstructor(type); + + Assert.Null(constructor); + Assert.True(parameters.Length == 0); + } + + [Theory] + [InlineData(typeof(StructWithMultipleConstructors))] + [InlineData(typeof(RecordStructWithMultipleConstructors))] + public void FindConstructor_ReturnNullForStruct_WhenMultipleParameterizedConstructorsDeclared(Type type) + { + var cache = new ParameterBindingMethodCache(); + var (constructor, parameters) = cache.FindConstructor(type); + + Assert.Null(constructor); + Assert.True(parameters.Length == 0); + } + public static TheoryData InvalidTryParseStringTypesData { get @@ -493,6 +557,60 @@ public void FindBindAsyncMethod_IgnoresInvalidBindAsyncIfGoodOneFound(Type type) Assert.NotNull(expression); } + private class ClassWithInternalConstructor + { + internal ClassWithInternalConstructor() + { } + } + private record RecordWithInternalConstructor + { + internal RecordWithInternalConstructor() + { } + } + + [Theory] + [InlineData(typeof(ClassWithInternalConstructor))] + [InlineData(typeof(RecordWithInternalConstructor))] + public void FindConstructor_ThrowsIfNoPublicConstructors(Type type) + { + var cache = new ParameterBindingMethodCache(); + var ex = Assert.Throws(() => cache.FindConstructor(type)); + Assert.Equal($"No {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} public parameterless constructor found.", ex.Message); + } + + [Theory] + [InlineData(typeof(AbstractClass))] + [InlineData(typeof(AbstractRecord))] + public void FindConstructor_ThrowsIfAbstract(Type type) + { + var cache = new ParameterBindingMethodCache(); + var ex = Assert.Throws(() => cache.FindConstructor(type)); + Assert.Equal($"The {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} abstract type is not supported.", ex.Message); + } + + [Theory] + [InlineData(typeof(ClassWithMultipleConstructors))] + [InlineData(typeof(RecordWithMultipleConstructors))] + public void FindConstructor_ThrowsIfMultipleParameterizedConstructors(Type type) + { + var cache = new ParameterBindingMethodCache(); + var ex = Assert.Throws(() => cache.FindConstructor(type)); + Assert.Equal($"Only a single public parameterized constructor is allowed for {TypeNameHelper.GetTypeDisplayName(type, fullName: false)}.", ex.Message); + } + + [Theory] + [InlineData(typeof(ClassWithInvalidConstructors))] + [InlineData(typeof(RecordClassWithInvalidConstructors))] + [InlineData(typeof(RecordStructWithInvalidConstructors))] + [InlineData(typeof(StructWithInvalidConstructors))] + public void FindConstructor_ThrowsIfParameterizedConstructorIncludeNoMatchingArguments(Type type) + { + var cache = new ParameterBindingMethodCache(); + var ex = Assert.Throws(() => cache.FindConstructor(type)); + Assert.Equal($"The {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} public parameterized constructor must contains only parameters that match to the declared public properties.", ex.Message); + + } + enum Choice { One, @@ -1069,6 +1187,154 @@ public static void BindAsync(HttpContext context) => throw new NotImplementedException(); } + + public class ClassWithParameterizedConstructor + { + public int Foo { get; set; } + + public ClassWithParameterizedConstructor(int foo) + { + + } + } + public record RecordClassParameterizedConstructor(int Foo); + public record struct RecordStructParameterizedConstructor(int Foo); + + public struct StructWithParameterizedConstructor + { + public int Foo { get; set; } + + public StructWithParameterizedConstructor(int foo) + { + } + } + + public class ClassWithParameterlessConstructor + { + public ClassWithParameterlessConstructor() + { + } + + public ClassWithParameterlessConstructor(int foo) + { + + } + } + public record RecordClassParameterlessConstructor + { + public RecordClassParameterlessConstructor() + { + } + + public RecordClassParameterlessConstructor(int foo) + { + + } + } + + public struct StructWithParameterlessConstructor + { + public StructWithParameterlessConstructor() + { + } + + public StructWithParameterlessConstructor(int foo) + { + } + } + + public record struct RecordStructWithParameterlessConstructor + { + public RecordStructWithParameterlessConstructor() + { + } + + public RecordStructWithParameterlessConstructor(int foo) + { + + } + } + + public class ClassWithDefaultConstructor + { } + public record RecordClassWithDefaultConstructor + { } + + public struct StructWithDefaultConstructor + { } + public record struct RecordStructWithDefaultConstructor + { } + public struct StructWithMultipleConstructors + { + public StructWithMultipleConstructors(int foo) + { + } + public StructWithMultipleConstructors(int foo, int bar) + { + } + } + + public record struct RecordStructWithMultipleConstructors(int Foo) + { + public RecordStructWithMultipleConstructors(int foo, int bar) + : this(foo) + { + + } + } + + private abstract class AbstractClass { } + private abstract record AbstractRecord(); + + private class ClassWithMultipleConstructors + { + public ClassWithMultipleConstructors(int foo) + { } + + public ClassWithMultipleConstructors(int foo, int bar) + { } + } + + private record RecordWithMultipleConstructors + { + public RecordWithMultipleConstructors(int foo) + { } + + public RecordWithMultipleConstructors(int foo, int bar) + { } + } + private class ClassWithInvalidConstructors + { + public int Foo { get; set; } + + public ClassWithInvalidConstructors(int foo, int bar) + { } + } + + private record RecordClassWithInvalidConstructors + { + public int Foo { get; set; } + + public RecordClassWithInvalidConstructors(int foo, int bar) + { } + } + + private struct StructWithInvalidConstructors + { + public int Foo { get; set; } + + public StructWithInvalidConstructors(int foo, int bar) + { } + } + + private record struct RecordStructWithInvalidConstructors + { + public int Foo { get; set; } + + public RecordStructWithInvalidConstructors(int foo, int bar) + { } + } + private class MockParameterInfo : ParameterInfo { public MockParameterInfo(Type type, string name) From c2ecb7141f6900ff2cd5fa6dfa8c1c8a0c2b8f22 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 29 Apr 2022 15:20:58 -0700 Subject: [PATCH 31/63] Using span --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 58acd56b6402..da7695b87266 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -9,6 +9,7 @@ using System.Linq.Expressions; using System.Reflection; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Security.Claims; using System.Text; using System.Text.Json; @@ -225,7 +226,7 @@ private static FactoryContext CreateFactoryContext(RequestDelegateFactoryOptions AddTypeProvidedMetadata(methodInfo, factoryContext.Metadata, factoryContext.ServiceProvider, - factoryContext.SurrogateParameters.ToArray()); + CollectionsMarshal.AsSpan(factoryContext.SurrogateParameters)); // Add method attributes as metadata *after* any inferred metadata so that the attributes hava a higher specificity AddMethodAttributesAsMetadata(methodInfo, factoryContext.Metadata); @@ -383,11 +384,11 @@ private static Expression MapHandlerReturnTypeToValueTask(Expression methodCall, return ExecuteAwaited(task); } - private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List metadata, IServiceProvider? services, ParameterInfo[] surrogateParameters) + private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List metadata, IServiceProvider? services, Span surrogateParameters) { object?[]? invokeArgs = null; - void AddMetadata(ParameterInfo[] parameters) + void AddMetadata(ReadOnlySpan parameters) { foreach (var parameter in parameters) { From 39fb2ec767a1fb3501ab6441b06e51ef4befb91c Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 29 Apr 2022 15:21:28 -0700 Subject: [PATCH 32/63] Updating error message --- src/Shared/ParameterBindingMethodCache.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Shared/ParameterBindingMethodCache.cs b/src/Shared/ParameterBindingMethodCache.cs index 17f3b816ed0b..c12768d1e86e 100644 --- a/src/Shared/ParameterBindingMethodCache.cs +++ b/src/Shared/ParameterBindingMethodCache.cs @@ -307,7 +307,7 @@ static bool ValidateReturnType(MethodInfo methodInfo) if (!lookupTable.TryGetValue(key, out var property)) { throw new InvalidOperationException( - $"The '{type}' public parameterized constructor must contains only parameters that match to the declared properties."); + $"The {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} public parameterized constructor must contains only parameters that match to the declared public properties."); } parametersWithPropertyInfo[i] = new ConstructorParameter(parameters[i], property); @@ -323,7 +323,7 @@ static bool ValidateReturnType(MethodInfo methodInfo) { if (type.IsAbstract) { - throw new InvalidOperationException($"The '{type}' abstract type is not supported."); + throw new InvalidOperationException($"The {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} abstract type is not supported."); } var constructors = type.GetConstructors(BindingFlags.Public | BindingFlags.Instance); @@ -363,10 +363,10 @@ static bool ValidateReturnType(MethodInfo methodInfo) // } if (parameterlessConstructor is null && constructors.Length > 1) { - throw new InvalidOperationException($"Only a single public parameterized constructor is allowed for '{type}'."); + throw new InvalidOperationException($"Only a single public parameterized constructor is allowed for {TypeNameHelper.GetTypeDisplayName(type, fullName: false)}."); } - throw new InvalidOperationException($"No '{type}' public parameterless constructor found."); + throw new InvalidOperationException($"No {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} public parameterless constructor found."); } private MethodInfo? GetStaticMethodFromHierarchy(Type type, string name, Type[] parameterTypes, Func validateReturnType) From ba0fd73404c9af25c64bada7c0be9bbaaf3d0d0d Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 2 May 2022 14:03:33 -0700 Subject: [PATCH 33/63] Updating surrogateParameteInfo and fix unit test --- .../src/RequestDelegateFactory.cs | 147 +++++++++--------- .../test/SurrogateParameterInfoTests.cs | 42 ++++- src/Shared/SurrogateParameterInfo.cs | 16 +- 3 files changed, 121 insertions(+), 84 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index da7695b87266..cbae17d82c4c 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -469,15 +469,7 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory for (var i = 0; i < parameters.Length; i++) { - if (parameters[i].CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType)) || - parameters[i].ParameterType.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) - { - args[i] = CreateSurrogateArgument(parameters[i], factoryContext); - } - else - { - args[i] = CreateArgument(parameters[i], factoryContext); - } + args[i] = CreateArgument(parameters[i], factoryContext); // Register expressions containing the boxed and unboxed variants // of the route handler's arguments for use in RouteHandlerInvocationContext @@ -521,7 +513,18 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext var parameterCustomAttributes = parameter.GetCustomAttributes(); - if (parameterCustomAttributes.OfType().FirstOrDefault() is { } routeAttribute) + if (parameter.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType)) || + parameter.ParameterType.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) + { + if (parameter is SurrogateParameterInfo) + { + throw new NotSupportedException( + $"Nested {nameof(ParametersAttribute)} is not supported and should be used only for handler parameters or parameter types."); + } + + return BindParameterFromParametersType(parameter, factoryContext); + } + else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } routeAttribute) { var routeName = routeAttribute.Name ?? parameter.Name; factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.RouteAttribute); @@ -664,68 +667,6 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext } } - private static Expression CreateSurrogateArgument(ParameterInfo parameter, FactoryContext factoryContext) - { - var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); - var (constructor, parameters) = ParameterBindingMethodCache.FindConstructor(parameter.ParameterType); - - if (constructor is not null && parameters is { Length: > 0 }) - { - // arg_local = new T(....) - - var constructorArguments = new Expression[parameters.Length]; - - for (var i = 0; i < parameters.Length; i++) - { - var parameterInfo = - new SurrogateParameterInfo(parameters[i].PropertyInfo, parameters[i].ParameterInfo, factoryContext.NullabilityContext); - constructorArguments[i] = CreateArgument(parameterInfo, factoryContext); - factoryContext.SurrogateParameters.Add(parameterInfo); - } - - factoryContext.ParamCheckExpressions.Add( - Expression.Assign( - argumentExpression, - Expression.New(constructor, constructorArguments))); - } - else - { - // arg_local = new T() - // { - // arg_local.Property[0] = expression[0], - // arg_local.Property[n] = expression[n], - // } - - var properties = parameter.ParameterType.GetProperties(); - var bindings = new List(properties.Length); - - for (var i = 0; i < properties.Length; i++) - { - // For parameterless ctor we will init only writable properties. - if (properties[i].CanWrite) - { - var parameterInfo = new SurrogateParameterInfo(properties[i], factoryContext.NullabilityContext); - bindings.Add(Expression.Bind(properties[i], CreateArgument(parameterInfo, factoryContext))); - factoryContext.SurrogateParameters.Add(parameterInfo); - } - } - - var newExpression = constructor is null ? - Expression.New(parameter.ParameterType) : - Expression.New(constructor); - - factoryContext.ParamCheckExpressions.Add( - Expression.Assign( - argumentExpression, - Expression.MemberInit(newExpression, bindings))); - } - - factoryContext.TrackedParameters.Add(parameter.Name!, RequestDelegateFactoryConstants.SurrogatedParameter); - factoryContext.ExtraLocals.Add(argumentExpression); - - return argumentExpression; - } - private static Expression CreateMethodCall(MethodInfo methodInfo, Expression? target, Expression[] arguments) => target is null ? Expression.Call(methodInfo, arguments) : @@ -1256,6 +1197,68 @@ private static Expression GetValueFromProperty(Expression sourceExpression, stri return Expression.Convert(indexExpression, returnType ?? typeof(string)); } + private static Expression BindParameterFromParametersType(ParameterInfo parameter, FactoryContext factoryContext) + { + var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); + var (constructor, parameters) = ParameterBindingMethodCache.FindConstructor(parameter.ParameterType); + + if (constructor is not null && parameters is { Length: > 0 }) + { + // arg_local = new T(....) + + var constructorArguments = new Expression[parameters.Length]; + + for (var i = 0; i < parameters.Length; i++) + { + var parameterInfo = + new SurrogateParameterInfo(parameters[i].PropertyInfo, parameters[i].ParameterInfo, factoryContext.NullabilityContext); + constructorArguments[i] = CreateArgument(parameterInfo, factoryContext); + factoryContext.SurrogateParameters.Add(parameterInfo); + } + + factoryContext.ParamCheckExpressions.Add( + Expression.Assign( + argumentExpression, + Expression.New(constructor, constructorArguments))); + } + else + { + // arg_local = new T() + // { + // arg_local.Property[0] = expression[0], + // arg_local.Property[n] = expression[n], + // } + + var properties = parameter.ParameterType.GetProperties(); + var bindings = new List(properties.Length); + + for (var i = 0; i < properties.Length; i++) + { + // For parameterless ctor we will init only writable properties. + if (properties[i].CanWrite) + { + var parameterInfo = new SurrogateParameterInfo(properties[i], factoryContext.NullabilityContext); + bindings.Add(Expression.Bind(properties[i], CreateArgument(parameterInfo, factoryContext))); + factoryContext.SurrogateParameters.Add(parameterInfo); + } + } + + var newExpression = constructor is null ? + Expression.New(parameter.ParameterType) : + Expression.New(constructor); + + factoryContext.ParamCheckExpressions.Add( + Expression.Assign( + argumentExpression, + Expression.MemberInit(newExpression, bindings))); + } + + factoryContext.TrackedParameters.Add(parameter.Name!, RequestDelegateFactoryConstants.SurrogatedParameter); + factoryContext.ExtraLocals.Add(argumentExpression); + + return argumentExpression; + } + private static Expression BindParameterFromService(ParameterInfo parameter, FactoryContext factoryContext) { var isOptional = IsOptionalParameter(parameter, factoryContext); diff --git a/src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs b/src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs index 6b143d4d3328..20c886b3a200 100644 --- a/src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs +++ b/src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs @@ -35,48 +35,51 @@ public void Initialization_WithConstructorArgument_SetsTypeAndNameFromPropertyIn [Fact] public void SurrogateParameterInfo_ContainsPropertyCustomAttributes() { - // Arrange & Act + // Arrange var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.WithTestAttribute)); var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo); - // Assert + // Act & Assert Assert.Single(surrogateParameterInfo.GetCustomAttributes(typeof(TestAttribute))); } [Fact] public void SurrogateParameterInfo_WithConstructorArgument_UsesParameterCustomAttributes() { - // Arrange & Act + // Arrange var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.NoAttribute)); var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.WithTestAttribute)); var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); - // Assert + // Act & Assert Assert.Single(surrogateParameterInfo.GetCustomAttributes(typeof(TestAttribute))); } [Fact] public void SurrogateParameterInfo_WithConstructorArgument_FallbackToPropertyCustomAttributes() { - // Arrange & Act + // Arrange var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.WithTestAttribute)); var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.NoAttribute)); var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); - // Assert + // Act & Assert Assert.Single(surrogateParameterInfo.GetCustomAttributes(typeof(TestAttribute))); } [Fact] public void SurrogateParameterInfo_ContainsPropertyCustomAttributesData() { - // Arrange & Act + // Arrange var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.WithTestAttribute)); var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo); + // Act + var attributes = surrogateParameterInfo.GetCustomAttributesData(); + // Assert Assert.Single( - surrogateParameterInfo.GetCustomAttributesData(), + attributes, a => typeof(TestAttribute).IsAssignableFrom(a.AttributeType)); } @@ -88,6 +91,9 @@ public void SurrogateParameterInfo_WithConstructorArgument_MergePropertyAndParam var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.WithSampleAttribute)); var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + // Act + var attributes = surrogateParameterInfo.GetCustomAttributesData(); + // Assert Assert.Single( surrogateParameterInfo.GetCustomAttributesData(), @@ -97,6 +103,26 @@ public void SurrogateParameterInfo_WithConstructorArgument_MergePropertyAndParam a => typeof(SampleAttribute).IsAssignableFrom(a.AttributeType)); } + [Fact] + public void SurrogateParameterInfo_WithConstructorArgument_MergePropertyAndParameterCustomAttributes() + { + // Arrange + var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.WithTestAttribute)); + var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.WithSampleAttribute)); + var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + + // Act + var attributes = surrogateParameterInfo.GetCustomAttributes(true); + + // Assert + Assert.Single( + attributes, + a => typeof(TestAttribute).IsAssignableFrom(a.GetType())); + Assert.Single( + attributes, + a => typeof(SampleAttribute).IsAssignableFrom(a.GetType())); + } + [Fact] public void SurrogateParameterInfo_ContainsPropertyInheritedCustomAttributes() { diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/SurrogateParameterInfo.cs index 5cb7b42cad40..5c8300060a90 100644 --- a/src/Shared/SurrogateParameterInfo.cs +++ b/src/Shared/SurrogateParameterInfo.cs @@ -60,14 +60,22 @@ public override object[] GetCustomAttributes(Type attributeType, bool inherit) public override object[] GetCustomAttributes(bool inherit) { - var attributes = _constructionParameterInfo?.GetCustomAttributes(inherit); + var constructorAttributes = _constructionParameterInfo?.GetCustomAttributes(inherit); - if (attributes == null || attributes is { Length: 0 }) + if (constructorAttributes == null || constructorAttributes is { Length: 0 }) { - attributes = _underlyingProperty.GetCustomAttributes(inherit); + return _underlyingProperty.GetCustomAttributes(inherit); } - return attributes; + var propertyAttributes = _underlyingProperty.GetCustomAttributes(inherit); + + // Since the constructors attributes should take priority we will add them first, + // as we usually call it as First() or FirstOrDefault() in the argument creation + var mergedAttributes = new object[constructorAttributes.Length + propertyAttributes.Length]; + Array.Copy(constructorAttributes, mergedAttributes, constructorAttributes.Length); + Array.Copy(propertyAttributes, 0, mergedAttributes, constructorAttributes.Length, propertyAttributes.Length); + + return mergedAttributes; } public override IList GetCustomAttributesData() From 2d0d8e26c17083b82a286f5b2410f2380dec189e Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 2 May 2022 16:16:52 -0700 Subject: [PATCH 34/63] Adding RequestDelegateFactory tests --- .../test/RequestDelegateFactoryTests.cs | 405 +++++++++++++++++- src/Shared/SurrogateParameterInfo.cs | 2 +- 2 files changed, 388 insertions(+), 19 deletions(-) diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 6d41c5298abc..3b0195b33f3a 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -216,6 +216,30 @@ static void TestAction(HttpContext httpContext, [FromRoute] int value) Assert.Equal(originalRouteParam, httpContext.Items["input"]); } + private record ParameterListFromRoute(HttpContext HttpContext, int Value); + + [Fact] + public async Task RequestDelegatePopulatesFromRouteParameterBased_FromParameterList() + { + const string paramName = "value"; + const int originalRouteParam = 42; + + static void TestAction([Parameters] ParameterListFromRoute args) + { + args.HttpContext.Items.Add("input", args.Value); + } + + var httpContext = CreateHttpContext(); + httpContext.Request.RouteValues[paramName] = originalRouteParam.ToString(NumberFormatInfo.InvariantInfo); + + var factoryResult = RequestDelegateFactory.Create(TestAction); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + Assert.Equal(originalRouteParam, httpContext.Items["input"]); + } + private static void TestOptional(HttpContext httpContext, [FromRoute] int value = 42) { httpContext.Items.Add("input", value); @@ -1491,6 +1515,40 @@ void TestAction([FromQuery] int value) Assert.Equal(originalQueryParam, deserializedRouteParam); } + private record ParameterListFromQuery([FromQuery] int Value); + + [Fact] + public async Task RequestDelegatePopulatesFromQueryParameter_FromParameterList() + { + // QueryCollection is case sensitve, since we now getting + // the parameter name from the Property/Record constructor + // we should match the case here + const string paramName = "Value"; + const int originalQueryParam = 42; + + int? deserializedRouteParam = null; + + void TestAction([Parameters] ParameterListFromQuery args) + { + deserializedRouteParam = args.Value; + } + + var query = new QueryCollection(new Dictionary() + { + [paramName] = originalQueryParam.ToString(NumberFormatInfo.InvariantInfo) + }); + + var httpContext = CreateHttpContext(); + httpContext.Request.Query = query; + + var factoryResult = RequestDelegateFactory.Create(TestAction); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + Assert.Equal(originalQueryParam, deserializedRouteParam); + } + [Fact] public async Task RequestDelegatePopulatesFromHeaderParameterBasedOnParameterName() { @@ -1515,6 +1573,35 @@ void TestAction([FromHeader(Name = customHeaderName)] int value) Assert.Equal(originalHeaderParam, deserializedRouteParam); } + private record ParameterListFromHeader([FromHeader(Name = "X-Custom-Header")] int Value); + + [Fact] + public async Task RequestDelegatePopulatesFromHeaderParameter_FromParameterList() + { + const string customHeaderName = "X-Custom-Header"; + const int originalHeaderParam = 42; + + int? deserializedRouteParam = null; + + void TestAction([Parameters] ParameterListFromHeader args) + { + deserializedRouteParam = args.Value; + } + + var httpContext = CreateHttpContext(); + httpContext.Request.Headers[customHeaderName] = originalHeaderParam.ToString(NumberFormatInfo.InvariantInfo); + + var factoryResult = RequestDelegateFactory.Create(TestAction); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + Assert.Equal(originalHeaderParam, deserializedRouteParam); + } + + private record ParametersListWithImplictFromBody(HttpContext HttpContext, TodoStruct Todo); + private record ParametersListWithExplictFromBody(HttpContext HttpContext, [FromBody] Todo Todo); + public static object[][] ImplicitFromBodyActions { get @@ -1534,11 +1621,17 @@ void TestImpliedFromBodyStruct(HttpContext httpContext, TodoStruct todo) httpContext.Items.Add("body", todo); } + void TestImpliedFromBodyStruct_ParameterList([Parameters] ParametersListWithImplictFromBody args) + { + args.HttpContext.Items.Add("body", args.Todo); + } + return new[] { new[] { (Action)TestImpliedFromBody }, new[] { (Action)TestImpliedFromBodyInterface }, new object[] { (Action)TestImpliedFromBodyStruct }, + new object[] { (Action)TestImpliedFromBodyStruct_ParameterList }, }; } } @@ -1552,10 +1645,16 @@ void TestExplicitFromBody(HttpContext httpContext, [FromBody] Todo todo) httpContext.Items.Add("body", todo); } + void TestExplicitFromBody_ParameterList([Parameters] ParametersListWithExplictFromBody args) + { + args.HttpContext.Items.Add("body", args.Todo); + } + return new[] { new[] { (Action)TestExplicitFromBody }, - }; + new object[] { (Action)TestExplicitFromBody_ParameterList }, + }; } } @@ -2118,6 +2217,24 @@ public BadArgumentListClassMultipleCtors(int foo, int bar) public int Bar { get; set; } } + [Parameters] + private record NestedArgumentListRecord([Parameters] object NestedParameterList); + + private record RecordWithParametersConstructor([Parameters] object NestedParameterList); + + [Fact] + public void BuildRequestDelegateThrowsNotSupportedExceptionForNestedParametersList() + { + void TestNestedParameterListRecordOnType(NestedArgumentListRecord req) { } + void TestNestedParameterListRecordOnArgument([Parameters] RecordWithParametersConstructor req) { } + + Assert.Throws(() => RequestDelegateFactory.Create(TestNestedParameterListRecordOnType)); + Assert.Throws(() => RequestDelegateFactory.Create(TestNestedParameterListRecordOnArgument)); + } + + private record ParametersListWithImplictFromService(HttpContext HttpContext, IMyService MyService); + private record ParametersListWithExplictFromService(HttpContext HttpContext, [FromService] MyService MyService); + public static object[][] ExplicitFromServiceActions { get @@ -2127,6 +2244,11 @@ void TestExplicitFromService(HttpContext httpContext, [FromService] MyService my httpContext.Items.Add("service", myService); } + void TestExplicitFromService_FromParameterList([Parameters] ParametersListWithExplictFromService args) + { + args.HttpContext.Items.Add("service", args.MyService); + } + void TestExplicitFromIEnumerableService(HttpContext httpContext, [FromService] IEnumerable myServices) { httpContext.Items.Add("service", myServices.Single()); @@ -2140,6 +2262,7 @@ void TestExplicitMultipleFromService(HttpContext httpContext, [FromService] MySe return new object[][] { new[] { (Action)TestExplicitFromService }, + new object[] { (Action)TestExplicitFromService_FromParameterList }, new[] { (Action>)TestExplicitFromIEnumerableService }, new[] { (Action>)TestExplicitMultipleFromService }, }; @@ -2155,6 +2278,11 @@ void TestImpliedFromService(HttpContext httpContext, IMyService myService) httpContext.Items.Add("service", myService); } + void TestImpliedFromService_FromParameterList([Parameters] ParametersListWithImplictFromService args) + { + args.HttpContext.Items.Add("service", args.MyService); + } + void TestImpliedIEnumerableFromService(HttpContext httpContext, IEnumerable myServices) { httpContext.Items.Add("service", myServices.Single()); @@ -2168,6 +2296,7 @@ void TestImpliedFromServiceBasedOnContainer(HttpContext httpContext, MyService m return new object[][] { new[] { (Action)TestImpliedFromService }, + new object[] { (Action)TestImpliedFromService_FromParameterList }, new[] { (Action>)TestImpliedIEnumerableFromService }, new[] { (Action)TestImpliedFromServiceBasedOnContainer }, }; @@ -2260,6 +2389,31 @@ void TestAction(HttpContext httpContext) Assert.Same(httpContext, httpContextArgument); } + private record ParametersListWithHttpContext( + HttpContext HttpContext, + ClaimsPrincipal User, + HttpRequest HttpRequest, + HttpResponse HttpResponse); + + [Fact] + public async Task RequestDelegatePopulatesHttpContextParameterWithoutAttribute_FromParameterList() + { + HttpContext? httpContextArgument = null; + + void TestAction([Parameters] ParametersListWithHttpContext args) + { + httpContextArgument = args.HttpContext; + } + + var httpContext = CreateHttpContext(); + + var factoryResult = RequestDelegateFactory.Create(TestAction); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + Assert.Same(httpContext, httpContextArgument); + } [Fact] public async Task RequestDelegatePassHttpContextRequestAbortedAsCancellationToken() @@ -2306,6 +2460,27 @@ void TestAction(ClaimsPrincipal user) Assert.Equal(httpContext.User, userArgument); } + [Fact] + public async Task RequestDelegatePassHttpContextUserAsClaimsPrincipal_FromParameterList() + { + ClaimsPrincipal? userArgument = null; + + void TestAction([Parameters] ParametersListWithHttpContext args) + { + userArgument = args.User; + } + + var httpContext = CreateHttpContext(); + httpContext.User = new ClaimsPrincipal(); + + var factoryResult = RequestDelegateFactory.Create(TestAction); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + Assert.Equal(httpContext.User, userArgument); + } + [Fact] public async Task RequestDelegatePassHttpContextRequestAsHttpRequest() { @@ -2326,6 +2501,26 @@ void TestAction(HttpRequest httpRequest) Assert.Equal(httpContext.Request, httpRequestArgument); } + [Fact] + public async Task RequestDelegatePassHttpContextRequestAsHttpRequest_FromParameterList() + { + HttpRequest? httpRequestArgument = null; + + void TestAction([Parameters] ParametersListWithHttpContext args) + { + httpRequestArgument = args.HttpRequest; + } + + var httpContext = CreateHttpContext(); + + var factoryResult = RequestDelegateFactory.Create(TestAction); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + Assert.Equal(httpContext.Request, httpRequestArgument); + } + [Fact] public async Task RequestDelegatePassesHttpContextRresponseAsHttpResponse() { @@ -2346,6 +2541,26 @@ void TestAction(HttpResponse httpResponse) Assert.Equal(httpContext.Response, httpResponseArgument); } + [Fact] + public async Task RequestDelegatePassesHttpContextRresponseAsHttpResponse_FromParameterList() + { + HttpResponse? httpResponseArgument = null; + + void TestAction([Parameters] ParametersListWithHttpContext args) + { + httpResponseArgument = args.HttpResponse; + } + + var httpContext = CreateHttpContext(); + + var factoryResult = RequestDelegateFactory.Create(TestAction); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + Assert.Equal(httpContext.Response, httpResponseArgument); + } + public static IEnumerable ComplexResult { get @@ -2782,20 +2997,13 @@ public async Task RequestDelegateWritesNullReturnNullValue(Delegate @delegate) Assert.Equal("null", responseBody); } - record NullableParametersRecord(string? Name); - record DefaultValueParametersRecord(string? Name = "DefaultName"); - record RequiredParametersRecord(string Name); - public static IEnumerable QueryParamOptionalityData { get { string requiredQueryParam(string name) => $"Hello {name}!"; string defaultValueQueryParam(string name = "DefaultName") => $"Hello {name}!"; - string requiredQueryParamFromArgumentList([Parameters] RequiredParametersRecord req) => $"Hello {req.Name}!"; - string defaultValueQueryParamFromArgumentList([Parameters] DefaultValueParametersRecord req) => $"Hello {req.Name}!"; string nullableQueryParam(string? name) => $"Hello {name}!"; - string nullableQueryParamFromArgumentList([Parameters] NullableParametersRecord req) => $"Hello {req.Name}!"; string requiredParseableQueryParam(int age) => $"Age: {age}"; string defaultValueParseableQueryParam(int age = 12) => $"Age: {age}"; string nullableQueryParseableParam(int? age) => $"Age: {age}"; @@ -2803,17 +3011,11 @@ record RequiredParametersRecord(string Name); return new List { new object?[] { (Func)requiredQueryParam, "name", null, true, null}, - new object?[] { (Func)requiredQueryParamFromArgumentList, "Name", null, true, null}, new object?[] { (Func)requiredQueryParam, "name", "TestName", false, "Hello TestName!" }, - new object?[] { (Func)requiredQueryParamFromArgumentList, "Name", "TestName", false, "Hello TestName!" }, new object?[] { (Func)defaultValueQueryParam, "name", null, false, "Hello DefaultName!" }, - new object?[] { (Func)defaultValueQueryParamFromArgumentList, "Name", null, false, "Hello DefaultName!" }, new object?[] { (Func)defaultValueQueryParam, "name", "TestName", false, "Hello TestName!" }, - new object?[] { (Func)defaultValueQueryParamFromArgumentList, "Name", "TestName", false, "Hello TestName!" }, new object?[] { (Func)nullableQueryParam, "name", null, false, "Hello !" }, - new object?[] { (Func)nullableQueryParamFromArgumentList, "Name", null, false, "Hello !" }, new object?[] { (Func)nullableQueryParam, "name", "TestName", false, "Hello TestName!"}, - new object?[] { (Func)nullableQueryParamFromArgumentList, "Name", "TestName", false, "Hello TestName!"}, new object?[] { (Func)requiredParseableQueryParam, "age", null, true, null}, new object?[] { (Func)requiredParseableQueryParam, "age", "42", false, "Age: 42" }, @@ -2871,9 +3073,7 @@ public async Task RequestDelegateHandlesQueryParamOptionality(Delegate @delegate get { string requiredRouteParam(string name) => $"Hello {name}!"; - string requiredRouteParamFromArgumentList([Parameters] RequiredParametersRecord req) => $"Hello {req.Name}!"; string defaultValueRouteParam(string name = "DefaultName") => $"Hello {name}!"; - string defaultValueRouteParamFromArgumentList([Parameters] DefaultValueParametersRecord req) => $"Hello {req.Name}!"; string nullableRouteParam(string? name) => $"Hello {name}!"; string requiredParseableRouteParam(int age) => $"Age: {age}"; string defaultValueParseableRouteParam(int age = 12) => $"Age: {age}"; @@ -2882,9 +3082,7 @@ public async Task RequestDelegateHandlesQueryParamOptionality(Delegate @delegate return new List { new object?[] { (Func)requiredRouteParam, "name", null, true, null}, - new object?[] { (Func)requiredRouteParamFromArgumentList, "Name", null, true, null}, new object?[] { (Func)requiredRouteParam, "name", "TestName", false, "Hello TestName!" }, - new object?[] { (Func)requiredRouteParamFromArgumentList, "Name", "TestName", false, "Hello TestName!" }, new object?[] { (Func)defaultValueRouteParam, "name", null, false, "Hello DefaultName!" }, new object?[] { (Func)defaultValueRouteParam, "name", "TestName", false, "Hello TestName!" }, new object?[] { (Func)nullableRouteParam, "name", null, false, "Hello !" }, @@ -4298,6 +4496,177 @@ void TestAction(IFormFile file) Assert.Equal(400, badHttpRequestException.StatusCode); } + private record struct ParameterListRecordStruct(HttpContext HttpContext, [FromRoute] int Value); + + private record ParameterListRecordClass(HttpContext HttpContext, [FromRoute] int Value); + + private record ParameterListRecordWithoutPositionalParameters + { + public HttpContext? HttpContext { get; set; } + + [FromRoute] + public int Value { get; set; } + } + + private struct ParameterListStruct + { + public HttpContext HttpContext { get; set; } + + [FromRoute] + public int Value { get; set; } + } + + private class ParameterListStructWithParameterizedContructor + { + public ParameterListStructWithParameterizedContructor(HttpContext httpContext) + { + HttpContext = httpContext; + Value = 42; + } + + public HttpContext HttpContext { get; set; } + + public int Value { get; set; } + } + + private struct ParameterListStructWithMultipleParameterizedContructor + { + public ParameterListStructWithMultipleParameterizedContructor(HttpContext httpContext) + { + HttpContext = httpContext; + Value = 10; + } + + public ParameterListStructWithMultipleParameterizedContructor(HttpContext httpContext, [FromHeader(Name ="Value")] int value) + { + HttpContext = httpContext; + Value = value; + } + + public HttpContext HttpContext { get; set; } + + [FromRoute] + public int Value { get; set; } + } + + private class ParameterListClass + { + public HttpContext? HttpContext { get; set; } + + [FromRoute] + public int Value { get; set; } + } + private class ParameterListClassWithParameterizedContructor + { + public ParameterListClassWithParameterizedContructor(HttpContext httpContext) + { + HttpContext = httpContext; + Value = 42; + } + + public HttpContext HttpContext { get; set; } + + public int Value { get; set; } + } + + public static object[][] FromParameterListActions + { + get + { + void TestParameterListRecordStruct([Parameters] ParameterListRecordStruct args) + { + args.HttpContext.Items.Add("input", args.Value); + } + + void TestParameterListRecordClass([Parameters] ParameterListRecordClass args) + { + args.HttpContext.Items.Add("input", args.Value); + } + + void TestParameterListRecordWithoutPositionalParameters([Parameters] ParameterListRecordWithoutPositionalParameters args) + { + args.HttpContext!.Items.Add("input", args.Value); + } + + void TestParameterListStruct([Parameters] ParameterListStruct args) + { + args.HttpContext.Items.Add("input", args.Value); + } + + void TestParameterListStructWithParameterizedContructor([Parameters] ParameterListStructWithParameterizedContructor args) + { + args.HttpContext.Items.Add("input", args.Value); + } + + void TestParameterListStructWithMultipleParameterizedContructor([Parameters] ParameterListStructWithMultipleParameterizedContructor args) + { + args.HttpContext.Items.Add("input", args.Value); + } + + void TestParameterListClass([Parameters] ParameterListClass args) + { + args.HttpContext!.Items.Add("input", args.Value); + } + + void TestParameterListClassWithParameterizedContructor([Parameters] ParameterListClassWithParameterizedContructor args) + { + args.HttpContext.Items.Add("input", args.Value); + } + + return new[] + { + new object[] { (Action)TestParameterListRecordStruct }, + new object[] { (Action)TestParameterListRecordClass }, + new object[] { (Action)TestParameterListRecordWithoutPositionalParameters }, + new object[] { (Action)TestParameterListStruct }, + new object[] { (Action)TestParameterListStructWithParameterizedContructor }, + new object[] { (Action)TestParameterListStructWithMultipleParameterizedContructor }, + new object[] { (Action)TestParameterListClass }, + new object[] { (Action)TestParameterListClassWithParameterizedContructor }, + }; + } + } + + [Theory] + [MemberData(nameof(FromParameterListActions))] + public async Task RequestDelegatePopulatesFromParameterList(Delegate action) + { + const string paramName = "value"; + const int originalRouteParam = 42; + + var httpContext = CreateHttpContext(); + httpContext.Request.RouteValues[paramName] = originalRouteParam.ToString(NumberFormatInfo.InvariantInfo); + + var factoryResult = RequestDelegateFactory.Create(action); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + Assert.Equal(originalRouteParam, httpContext.Items["input"]); + } + + private record ParameterListWitDefaultValue(HttpContext HttpContext, [FromRoute] int Value = 42); + + [Fact] + public async Task RequestDelegatePopulatesFromParameterListUsesDefaultValue() + { + const int expectedValue = 42; + + void TestAction([Parameters] ParameterListWitDefaultValue args) + { + args.HttpContext.Items.Add("input", args.Value); + } + + var httpContext = CreateHttpContext(); + + var factoryResult = RequestDelegateFactory.Create(TestAction); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + Assert.Equal(expectedValue, httpContext.Items["input"]); + } + [Fact] public async Task RequestDelegateFactory_InvokesFiltersButNotHandler_OnArgumentError() { diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/SurrogateParameterInfo.cs index 5c8300060a90..4f1a3b38cd65 100644 --- a/src/Shared/SurrogateParameterInfo.cs +++ b/src/Shared/SurrogateParameterInfo.cs @@ -99,7 +99,7 @@ public override bool IsDefined(Type attributeType, bool inherit) _underlyingProperty.IsDefined(attributeType, inherit); } - public new bool IsOptional => NullabilityInfo.ReadState != NullabilityState.NotNull; + public new bool IsOptional => HasDefaultValue || NullabilityInfo.ReadState != NullabilityState.NotNull; public NullabilityInfo NullabilityInfo => _nullabilityInfo ??= _constructionParameterInfo is not null ? From cbbad7c3896ddef3a862543222d1993fa3114424 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Tue, 3 May 2022 09:52:31 -0700 Subject: [PATCH 35/63] Code cleanup --- .../src/RequestDelegateFactory.cs | 4 +- .../test/ParameterBindingMethodCacheTests.cs | 13 +++- .../test/RequestDelegateFactoryTests.cs | 2 +- .../EndpointMetadataApiDescriptionProvider.cs | 54 +-------------- src/OpenApi/src/OpenApiGenerator.cs | 57 +--------------- src/Shared/SurrogateParameterInfo.cs | 66 +++++++++++++++++++ 6 files changed, 82 insertions(+), 114 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index cbae17d82c4c..6d842f1a992e 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -1253,7 +1253,7 @@ private static Expression BindParameterFromParametersType(ParameterInfo paramete Expression.MemberInit(newExpression, bindings))); } - factoryContext.TrackedParameters.Add(parameter.Name!, RequestDelegateFactoryConstants.SurrogatedParameter); + factoryContext.TrackedParameters.Add(parameter.Name!, RequestDelegateFactoryConstants.SurrogateParameter); factoryContext.ExtraLocals.Add(argumentExpression); return argumentExpression; @@ -2061,7 +2061,7 @@ private static class RequestDelegateFactoryConstants public const string BodyParameter = "Body (Inferred)"; public const string RouteOrQueryStringParameter = "Route or Query String (Inferred)"; public const string FormFileParameter = "Form File (Inferred)"; - public const string SurrogatedParameter = "Surrogate (Attribute)"; + public const string SurrogateParameter = "Surrogate (Attribute)"; } private static partial class Log diff --git a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs index bc7834457fe8..a62e6738fa83 100644 --- a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs +++ b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs @@ -607,8 +607,9 @@ public void FindConstructor_ThrowsIfParameterizedConstructorIncludeNoMatchingArg { var cache = new ParameterBindingMethodCache(); var ex = Assert.Throws(() => cache.FindConstructor(type)); - Assert.Equal($"The {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} public parameterized constructor must contains only parameters that match to the declared public properties.", ex.Message); - + Assert.Equal( + $"The {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} public parameterized constructor must contains only parameters that match to the declared public properties.", + ex.Message); } enum Choice @@ -1187,7 +1188,6 @@ public static void BindAsync(HttpContext context) => throw new NotImplementedException(); } - public class ClassWithParameterizedConstructor { public int Foo { get; set; } @@ -1197,7 +1197,9 @@ public ClassWithParameterizedConstructor(int foo) } } + public record RecordClassParameterizedConstructor(int Foo); + public record struct RecordStructParameterizedConstructor(int Foo); public struct StructWithParameterizedConstructor @@ -1220,6 +1222,7 @@ public ClassWithParameterlessConstructor(int foo) } } + public record RecordClassParameterlessConstructor { public RecordClassParameterlessConstructor() @@ -1262,8 +1265,10 @@ public record RecordClassWithDefaultConstructor public struct StructWithDefaultConstructor { } + public record struct RecordStructWithDefaultConstructor { } + public struct StructWithMultipleConstructors { public StructWithMultipleConstructors(int foo) @@ -1284,6 +1289,7 @@ public RecordStructWithMultipleConstructors(int foo, int bar) } private abstract class AbstractClass { } + private abstract record AbstractRecord(); private class ClassWithMultipleConstructors @@ -1303,6 +1309,7 @@ public RecordWithMultipleConstructors(int foo) public RecordWithMultipleConstructors(int foo, int bar) { } } + private class ClassWithInvalidConstructors { public int Foo { get; set; } diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 3b0195b33f3a..cb8d290e2866 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -1600,6 +1600,7 @@ void TestAction([Parameters] ParameterListFromHeader args) } private record ParametersListWithImplictFromBody(HttpContext HttpContext, TodoStruct Todo); + private record ParametersListWithExplictFromBody(HttpContext HttpContext, [FromBody] Todo Todo); public static object[][] ImplicitFromBodyActions @@ -2175,7 +2176,6 @@ private record BadArgumentListRecord(int Foo) public BadArgumentListRecord(int foo, int bar) : this(foo) { - } public int Bar { get; set; } diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 6bbe70d791ec..aa96479d8c12 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -117,59 +117,7 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string var hasBodyOrFormFileParameter = false; - static ReadOnlySpan FlattenParameters(ParameterInfo[] parameters, ParameterBindingMethodCache cache) - { - if (parameters.Length == 0) - { - return Array.Empty(); - } - - List? flattenedParameters = null; - NullabilityInfoContext? nullabilityContext = null; - - for (var i = 0; i < parameters.Length; i++) - { - if (parameters[i].CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) - { - // Initialize the list with all parameter already processed - // to keep the same parameter ordering - flattenedParameters ??= new(parameters[0..i]); - nullabilityContext ??= new(); - - var (constructor, constructorParameters) = cache.FindConstructor(parameters[i].ParameterType); - if (constructor is not null && constructorParameters is { Length: > 0 }) - { - foreach (var constructorParameter in constructorParameters) - { - flattenedParameters.Add( - new SurrogateParameterInfo( - constructorParameter.PropertyInfo, - constructorParameter.ParameterInfo, - nullabilityContext)); - } - } - else - { - var properties = parameters[i].ParameterType.GetProperties(); - foreach (var property in properties) - { - if (property.CanWrite) - { - flattenedParameters.Add(new SurrogateParameterInfo(property, nullabilityContext)); - } - } - } - } - else if (flattenedParameters is not null) - { - flattenedParameters.Add(parameters[i]); - } - } - - return flattenedParameters is not null ? CollectionsMarshal.AsSpan(flattenedParameters) : parameters.AsSpan(); - } - - foreach (var parameter in FlattenParameters(methodInfo.GetParameters(), ParameterBindingMethodCache)) + foreach (var parameter in SurrogateParameterInfo.Flatten(methodInfo.GetParameters(), ParameterBindingMethodCache)) { var parameterDescription = CreateApiParameterDescription(parameter, routeEndpoint.RoutePattern, disableInferredBody); diff --git a/src/OpenApi/src/OpenApiGenerator.cs b/src/OpenApi/src/OpenApiGenerator.cs index 43552dfa31df..fab60607a6d4 100644 --- a/src/OpenApi/src/OpenApiGenerator.cs +++ b/src/OpenApi/src/OpenApiGenerator.cs @@ -5,7 +5,6 @@ using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; using System.Security.Claims; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Metadata; @@ -251,7 +250,7 @@ private static void GenerateDefaultResponses(Dictionary GetOperationTags(MethodInfo methodInfo, EndpointMetadat : new List() { new OpenApiTag() { Name = controllerName } }; } - private static ReadOnlySpan FlattenParameters(ParameterInfo[] parameters) - { - if (parameters.Length == 0) - { - return Array.Empty(); - } - - List? flattenedParameters = null; - NullabilityInfoContext? nullabilityContext = null; - - for (var i = 0; i < parameters.Length; i++) - { - if (parameters[i].CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) - { - // Initialize the list with all parameter already processed - // to keep the same parameter ordering - flattenedParameters ??= new(parameters[0..i]); - nullabilityContext ??= new(); - - var (constructor, constructorParameters) = ParameterBindingMethodCache.FindConstructor(parameters[i].ParameterType); - if (constructor is not null && constructorParameters is { Length: > 0 }) - { - foreach (var constructorParameter in constructorParameters) - { - flattenedParameters.Add( - new SurrogateParameterInfo( - constructorParameter.PropertyInfo, - constructorParameter.ParameterInfo, - nullabilityContext)); - } - } - else - { - var properties = parameters[i].ParameterType.GetProperties(); - foreach (var property in properties) - { - if (property.CanWrite) - { - flattenedParameters.Add(new SurrogateParameterInfo(property, nullabilityContext)); - } - } - } - } - else if (flattenedParameters is not null) - { - flattenedParameters.Add(parameters[i]); - } - } - - return flattenedParameters is not null ? CollectionsMarshal.AsSpan(flattenedParameters) : parameters.AsSpan(); - } - private List GetOpenApiParameters(MethodInfo methodInfo, EndpointMetadataCollection metadata, RoutePattern pattern, bool disableInferredBody) { - var parameters = FlattenParameters(methodInfo.GetParameters()); + var parameters = SurrogateParameterInfo.Flatten(methodInfo.GetParameters(), ParameterBindingMethodCache); var openApiParameters = new List(); foreach (var parameter in parameters) diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/SurrogateParameterInfo.cs index 4f1a3b38cd65..47a9137808af 100644 --- a/src/Shared/SurrogateParameterInfo.cs +++ b/src/Shared/SurrogateParameterInfo.cs @@ -3,7 +3,9 @@ using System.Collections.ObjectModel; using System.Diagnostics; +using System.Linq; using System.Reflection; +using System.Runtime.InteropServices; namespace Microsoft.AspNetCore.Http; @@ -46,6 +48,70 @@ public override bool HasDefaultValue public override object? RawDefaultValue => _constructionParameterInfo is not null ? _constructionParameterInfo.RawDefaultValue : null; + /// + /// Unwraps all parameters that contains and + /// creates a flat list merging the current parameters, not including the + /// parametres that contain a , and all additional + /// parameters detected. + /// + /// List of parameters to be flattened. + /// An instance of the method cache class. + /// Flat list of parameters. + public static ReadOnlySpan Flatten(ParameterInfo[] parameters, ParameterBindingMethodCache cache) + { + ArgumentNullException.ThrowIfNull(nameof(parameters)); + ArgumentNullException.ThrowIfNull(nameof(cache)); + + if (parameters.Length == 0) + { + return Array.Empty(); + } + + List? flattenedParameters = null; + NullabilityInfoContext? nullabilityContext = null; + + for (var i = 0; i < parameters.Length; i++) + { + if (parameters[i].CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) + { + // Initialize the list with all parameter already processed + // to keep the same parameter ordering + flattenedParameters ??= new(parameters[0..i]); + nullabilityContext ??= new(); + + var (constructor, constructorParameters) = cache.FindConstructor(parameters[i].ParameterType); + if (constructor is not null && constructorParameters is { Length: > 0 }) + { + foreach (var constructorParameter in constructorParameters) + { + flattenedParameters.Add( + new SurrogateParameterInfo( + constructorParameter.PropertyInfo, + constructorParameter.ParameterInfo, + nullabilityContext)); + } + } + else + { + var properties = parameters[i].ParameterType.GetProperties(); + foreach (var property in properties) + { + if (property.CanWrite) + { + flattenedParameters.Add(new SurrogateParameterInfo(property, nullabilityContext)); + } + } + } + } + else if (flattenedParameters is not null) + { + flattenedParameters.Add(parameters[i]); + } + } + + return flattenedParameters is not null ? CollectionsMarshal.AsSpan(flattenedParameters) : parameters.AsSpan(); + } + public override object[] GetCustomAttributes(Type attributeType, bool inherit) { var attributes = _constructionParameterInfo?.GetCustomAttributes(attributeType, inherit); From a658532988015b230f0975477fbefafc82234d4f Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Tue, 3 May 2022 22:56:02 -0700 Subject: [PATCH 36/63] code clean up --- .../test/ParameterBindingMethodCacheTests.cs | 9 +++++++-- .../Http.Extensions/test/RequestDelegateFactoryTests.cs | 8 ++++++-- .../src/EndpointMetadataApiDescriptionProvider.cs | 1 - src/Shared/SurrogateParameterInfo.cs | 5 ++++- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs index a62e6738fa83..8d2e36ba9a1f 100644 --- a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs +++ b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs @@ -1208,6 +1208,7 @@ public struct StructWithParameterizedConstructor public StructWithParameterizedConstructor(int foo) { + Foo = foo; } } @@ -1331,7 +1332,9 @@ private struct StructWithInvalidConstructors public int Foo { get; set; } public StructWithInvalidConstructors(int foo, int bar) - { } + { + Foo = foo; + } } private record struct RecordStructWithInvalidConstructors @@ -1339,7 +1342,9 @@ private record struct RecordStructWithInvalidConstructors public int Foo { get; set; } public RecordStructWithInvalidConstructors(int foo, int bar) - { } + { + Foo = foo; + } } private class MockParameterInfo : ParameterInfo diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index cb8d290e2866..3ed57455a897 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -2203,6 +2203,7 @@ public BadArgumentListClass(int foo, string name) public int Foo { get; set; } public int Bar { get; set; } } + private class BadArgumentListClassMultipleCtors { public BadArgumentListClassMultipleCtors(int foo) @@ -2233,6 +2234,7 @@ public void BuildRequestDelegateThrowsNotSupportedExceptionForNestedParametersLi } private record ParametersListWithImplictFromService(HttpContext HttpContext, IMyService MyService); + private record ParametersListWithExplictFromService(HttpContext HttpContext, [FromService] MyService MyService); public static object[][] ExplicitFromServiceActions @@ -2389,6 +2391,7 @@ void TestAction(HttpContext httpContext) Assert.Same(httpContext, httpContextArgument); } + private record ParametersListWithHttpContext( HttpContext HttpContext, ClaimsPrincipal User, @@ -3056,7 +3059,7 @@ public async Task RequestDelegateHandlesQueryParamOptionality(Delegate @delegate var log = Assert.Single(logs); Assert.Equal(LogLevel.Debug, log.LogLevel); Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), log.EventId); - var expectedType = paramName == "age" ? "int age" : $"string {paramName}"; + var expectedType = paramName == "age" ? "int age" : $"string name"; Assert.Equal($@"Required parameter ""{expectedType}"" was not provided from route or query string.", log.Message); } else @@ -3128,7 +3131,7 @@ public async Task RequestDelegateHandlesRouteParamOptionality(Delegate @delegate var log = Assert.Single(logs); Assert.Equal(LogLevel.Debug, log.LogLevel); Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), log.EventId); - var expectedType = paramName == "age" ? "int age" : $"string {paramName}"; + var expectedType = paramName == "age" ? "int age" : $"string name"; Assert.Equal($@"Required parameter ""{expectedType}"" was not provided from query string.", log.Message); } else @@ -4556,6 +4559,7 @@ private class ParameterListClass [FromRoute] public int Value { get; set; } } + private class ParameterListClassWithParameterizedContructor { public ParameterListClassWithParameterizedContructor(HttpContext httpContext) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index aa96479d8c12..a81e8a446356 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -4,7 +4,6 @@ using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; using System.Security.Claims; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Metadata; diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/SurrogateParameterInfo.cs index 47a9137808af..1ca74b590bd3 100644 --- a/src/Shared/SurrogateParameterInfo.cs +++ b/src/Shared/SurrogateParameterInfo.cs @@ -3,6 +3,7 @@ using System.Collections.ObjectModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection; using System.Runtime.InteropServices; @@ -57,7 +58,9 @@ public override bool HasDefaultValue /// List of parameters to be flattened. /// An instance of the method cache class. /// Flat list of parameters. - public static ReadOnlySpan Flatten(ParameterInfo[] parameters, ParameterBindingMethodCache cache) + public static ReadOnlySpan Flatten( + ParameterInfo[] parameters, + ParameterBindingMethodCache cache) { ArgumentNullException.ThrowIfNull(nameof(parameters)); ArgumentNullException.ThrowIfNull(nameof(cache)); From abb59821dd0d9f2d44c8ba64765919a10bf81028 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Tue, 3 May 2022 22:57:32 -0700 Subject: [PATCH 37/63] code clean up --- src/Shared/SurrogateParameterInfo.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/SurrogateParameterInfo.cs index 1ca74b590bd3..6e8b8bfb71ca 100644 --- a/src/Shared/SurrogateParameterInfo.cs +++ b/src/Shared/SurrogateParameterInfo.cs @@ -3,7 +3,6 @@ using System.Collections.ObjectModel; using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection; using System.Runtime.InteropServices; From 603ea248ac1a6061c0bfadec2daab3023da4c602 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Wed, 4 May 2022 16:37:29 -0700 Subject: [PATCH 38/63] Adding suppress --- src/Shared/SurrogateParameterInfo.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/SurrogateParameterInfo.cs index 6e8b8bfb71ca..2b510e4688e4 100644 --- a/src/Shared/SurrogateParameterInfo.cs +++ b/src/Shared/SurrogateParameterInfo.cs @@ -3,6 +3,7 @@ using System.Collections.ObjectModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection; using System.Runtime.InteropServices; @@ -57,9 +58,8 @@ public override bool HasDefaultValue /// List of parameters to be flattened. /// An instance of the method cache class. /// Flat list of parameters. - public static ReadOnlySpan Flatten( - ParameterInfo[] parameters, - ParameterBindingMethodCache cache) + [UnconditionalSuppressMessage("Trimmer", "IL2075", Justification = ".")] + public static ReadOnlySpan Flatten(ParameterInfo[] parameters, ParameterBindingMethodCache cache) { ArgumentNullException.ThrowIfNull(nameof(parameters)); ArgumentNullException.ThrowIfNull(nameof(cache)); @@ -96,6 +96,7 @@ public override bool HasDefaultValue else { var properties = parameters[i].ParameterType.GetProperties(); + foreach (var property in properties) { if (property.CanWrite) From e49a28adb5ffc7902637899ea6ab82f896028802 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 5 May 2022 10:49:21 -0700 Subject: [PATCH 39/63] Adding trimming warning suppress --- src/Shared/SurrogateParameterInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/SurrogateParameterInfo.cs index 2b510e4688e4..daf55a86a91c 100644 --- a/src/Shared/SurrogateParameterInfo.cs +++ b/src/Shared/SurrogateParameterInfo.cs @@ -58,7 +58,7 @@ public override bool HasDefaultValue /// List of parameters to be flattened. /// An instance of the method cache class. /// Flat list of parameters. - [UnconditionalSuppressMessage("Trimmer", "IL2075", Justification = ".")] + [UnconditionalSuppressMessage("Trimmer", "IL2075", Justification = "SurrogateParameterInfo.Flatten requires unreferenced code.")] public static ReadOnlySpan Flatten(ParameterInfo[] parameters, ParameterBindingMethodCache cache) { ArgumentNullException.ThrowIfNull(nameof(parameters)); From 0591fa0901b9de3071cc124052e0ca62b8e83d1a Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 5 May 2022 22:48:15 -0700 Subject: [PATCH 40/63] PR feeback --- src/OpenApi/test/OpenApiGeneratorTests.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/OpenApi/test/OpenApiGeneratorTests.cs b/src/OpenApi/test/OpenApiGeneratorTests.cs index 929386fdd171..476bf62d8444 100644 --- a/src/OpenApi/test/OpenApiGeneratorTests.cs +++ b/src/OpenApi/test/OpenApiGeneratorTests.cs @@ -385,6 +385,7 @@ static void AssertParameters(OpenApiOperation operation) ); } + AssertParameters(GetOpenApiOperation((ExplicitArgumentListClass req) => { })); AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListClass req) => { })); AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListClassWithReadOnlyProperties req) => { })); AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListStruct req) => { })); @@ -879,4 +880,14 @@ private struct ArgumentListStruct public InferredJsonClass FromBody { get; set; } public HttpContext Context { get; set; } } + + [Parameters] + private class ExplicitArgumentListClass + { + [FromRoute] + public int Foo { get; set; } + public int Bar { get; set; } + public InferredJsonClass FromBody { get; set; } + public HttpContext Context { get; set; } + } } From cc001b59b7868cb67d420e328c960bcaca429a79 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 5 May 2022 22:51:29 -0700 Subject: [PATCH 41/63] PR feeback --- .../src/RequestDelegateFactory.cs | 24 ++-- .../test/RequestDelegateFactoryTests.cs | 130 ++++++++++++++++-- ...pointMetadataApiDescriptionProviderTest.cs | 11 ++ src/Shared/SurrogateParameterInfo.cs | 3 +- 4 files changed, 142 insertions(+), 26 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 6d842f1a992e..1a73ecd201c2 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -513,18 +513,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext var parameterCustomAttributes = parameter.GetCustomAttributes(); - if (parameter.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType)) || - parameter.ParameterType.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) - { - if (parameter is SurrogateParameterInfo) - { - throw new NotSupportedException( - $"Nested {nameof(ParametersAttribute)} is not supported and should be used only for handler parameters or parameter types."); - } - - return BindParameterFromParametersType(parameter, factoryContext); - } - else if (parameterCustomAttributes.OfType().FirstOrDefault() is { } routeAttribute) + if (parameterCustomAttributes.OfType().FirstOrDefault() is { } routeAttribute) { var routeName = routeAttribute.Name ?? parameter.Name; factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.RouteAttribute); @@ -611,6 +600,17 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext { return RequestPipeReaderExpr; } + else if (parameter.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType)) || + parameter.ParameterType.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) + { + if (parameter is SurrogateParameterInfo) + { + throw new NotSupportedException( + $"Nested {nameof(ParametersAttribute)} is not supported and should be used only for handler parameters or parameter types."); + } + + return BindParameterFromParametersType(parameter, factoryContext); + } else if (ParameterBindingMethodCache.HasBindAsyncMethod(parameter)) { return BindParameterFromBindAsync(parameter, factoryContext); diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 3ed57455a897..ce9b52b9b9b2 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -24,6 +24,7 @@ using Microsoft.AspNetCore.Http.Metadata; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Internal; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; @@ -1651,9 +1652,15 @@ void TestExplicitFromBody_ParameterList([Parameters] ParametersListWithExplictFr args.HttpContext.Items.Add("body", args.Todo); } + void TestExplicitFromBody_ParameterListAttribute(HttpContext httpContext, [FromBody] TodoArgumentList todo) + { + httpContext.Items.Add("body", todo); + } + return new[] { new[] { (Action)TestExplicitFromBody }, + new[] { (Action)TestExplicitFromBody_ParameterListAttribute }, new object[] { (Action)TestExplicitFromBody_ParameterList }, }; } @@ -2155,20 +2162,47 @@ private class BadBindAsyncClass throw new NotImplementedException(); } - [Fact] - public void BuildRequestDelegateThrowsInvalidOperationExceptionForInvalidParameterListConstructor() + public static object[][] BadArgumentListActions { - void TestParameterListRecord([Parameters] BadArgumentListRecord req) { } - void TestParameterListClass([Parameters] BadArgumentListClass req) { } - void TestParameterListClassWithMutipleConstructors([Parameters] BadArgumentListClassMultipleCtors req) { } - void TestParameterListAbstractClass([Parameters] BadAbstractArgumentListClass req) { } - void TestParameterListNoPulicConstructorClass([Parameters] BadNoPublicConstructorArgumentListClass req) { } + get + { + void TestParameterListRecord([Parameters] BadArgumentListRecord req) { } + void TestParameterListClass([Parameters] BadArgumentListClass req) { } + void TestParameterListClassWithMutipleConstructors([Parameters] BadArgumentListClassMultipleCtors req) { } + void TestParameterListAbstractClass([Parameters] BadAbstractArgumentListClass req) { } + void TestParameterListNoPulicConstructorClass([Parameters] BadNoPublicConstructorArgumentListClass req) { } + + static string GetMultipleContructorsError(Type type) + => $"Only a single public parameterized constructor is allowed for {TypeNameHelper.GetTypeDisplayName(type, fullName: false)}."; + + static string GetAbstractClassError(Type type) + => $"The {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} abstract type is not supported."; - Assert.Throws(() => RequestDelegateFactory.Create(TestParameterListRecord)); - Assert.Throws(() => RequestDelegateFactory.Create(TestParameterListClass)); - Assert.Throws(() => RequestDelegateFactory.Create(TestParameterListClassWithMutipleConstructors)); - Assert.Throws(() => RequestDelegateFactory.Create(TestParameterListAbstractClass)); - Assert.Throws(() => RequestDelegateFactory.Create(TestParameterListNoPulicConstructorClass)); + static string GetNoContructorsError(Type type) + => $"No {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} public parameterless constructor found."; + + static string GetInvalidConstructorError(Type type) + => $"The {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} public parameterized constructor must contains only parameters that match to the declared public properties."; + + return new object[][] + { + new object[] { (Action)TestParameterListRecord, GetMultipleContructorsError(typeof(BadArgumentListRecord)) }, + new object[] { (Action)TestParameterListClass, GetInvalidConstructorError(typeof(BadArgumentListClass)) }, + new object[] { (Action)TestParameterListClassWithMutipleConstructors, GetMultipleContructorsError(typeof(BadArgumentListClassMultipleCtors)) }, + new object[] { (Action)TestParameterListAbstractClass, GetAbstractClassError(typeof(BadAbstractArgumentListClass)) }, + new object[] { (Action)TestParameterListNoPulicConstructorClass, GetNoContructorsError(typeof(BadNoPublicConstructorArgumentListClass)) }, + }; + } + } + + [Theory] + [MemberData(nameof(BadArgumentListActions))] + public void BuildRequestDelegateThrowsInvalidOperationExceptionForInvalidParameterListConstructor( + Delegate @delegate, + string errorMessage) + { + var exception = Assert.Throws(() => RequestDelegateFactory.Create(@delegate)); + Assert.Equal(errorMessage, exception.Message); } private record BadArgumentListRecord(int Foo) @@ -4503,6 +4537,9 @@ private record struct ParameterListRecordStruct(HttpContext HttpContext, [FromRo private record ParameterListRecordClass(HttpContext HttpContext, [FromRoute] int Value); + [Parameters] + private record ExplicityArgumentListRecord(HttpContext HttpContext, [FromRoute] int Value); + private record ParameterListRecordWithoutPositionalParameters { public HttpContext? HttpContext { get; set; } @@ -4519,6 +4556,20 @@ private struct ParameterListStruct public int Value { get; set; } } + private struct ParameterListMutableStruct + { + public ParameterListMutableStruct() + { + Value = -1; + HttpContext = default!; + } + + public HttpContext HttpContext { get; set; } + + [FromRoute] + public int Value { get; set; } + } + private class ParameterListStructWithParameterizedContructor { public ParameterListStructWithParameterizedContructor(HttpContext httpContext) @@ -4577,6 +4628,11 @@ public static object[][] FromParameterListActions { get { + void TestParameterListAttributeOnType(ExplicityArgumentListRecord args) + { + args.HttpContext.Items.Add("input", args.Value); + } + void TestParameterListRecordStruct([Parameters] ParameterListRecordStruct args) { args.HttpContext.Items.Add("input", args.Value); @@ -4597,6 +4653,11 @@ void TestParameterListStruct([Parameters] ParameterListStruct args) args.HttpContext.Items.Add("input", args.Value); } + void TestParameterListMutableStruct([Parameters] ParameterListMutableStruct args) + { + args.HttpContext.Items.Add("input", args.Value); + } + void TestParameterListStructWithParameterizedContructor([Parameters] ParameterListStructWithParameterizedContructor args) { args.HttpContext.Items.Add("input", args.Value); @@ -4619,10 +4680,12 @@ void TestParameterListClassWithParameterizedContructor([Parameters] ParameterLis return new[] { + new object[] { (Action)TestParameterListAttributeOnType }, new object[] { (Action)TestParameterListRecordStruct }, new object[] { (Action)TestParameterListRecordClass }, new object[] { (Action)TestParameterListRecordWithoutPositionalParameters }, new object[] { (Action)TestParameterListStruct }, + new object[] { (Action)TestParameterListMutableStruct }, new object[] { (Action)TestParameterListStructWithParameterizedContructor }, new object[] { (Action)TestParameterListStructWithMultipleParameterizedContructor }, new object[] { (Action)TestParameterListClass }, @@ -4649,7 +4712,39 @@ public async Task RequestDelegatePopulatesFromParameterList(Delegate action) Assert.Equal(originalRouteParam, httpContext.Items["input"]); } - private record ParameterListWitDefaultValue(HttpContext HttpContext, [FromRoute] int Value = 42); + private record ParameterListRecordWitDefaultValue(HttpContext HttpContext, [FromRoute] int Value = 42); + + [Fact] + public async Task RequestDelegatePopulatesFromParameterListRecordUsesDefaultValue() + { + const int expectedValue = 42; + + void TestAction([Parameters] ParameterListRecordWitDefaultValue args) + { + args.HttpContext.Items.Add("input", args.Value); + } + + var httpContext = CreateHttpContext(); + + var factoryResult = RequestDelegateFactory.Create(TestAction); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + Assert.Equal(expectedValue, httpContext.Items["input"]); + } + + private class ParameterListWitDefaultValue + { + public ParameterListWitDefaultValue(HttpContext httpContext, [FromRoute]int value = 42) + { + HttpContext = httpContext; + Value = value; + } + + public HttpContext HttpContext { get; } + public int Value { get; } + } [Fact] public async Task RequestDelegatePopulatesFromParameterListUsesDefaultValue() @@ -5940,6 +6035,15 @@ private class Todo : ITodo public bool IsComplete { get; set; } } + [Parameters] + private class TodoArgumentList : Todo + { + public TodoArgumentList() + { + + } + } + private class TodoChild : Todo { public string? Child { get; set; } diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 99f22a67b16e..faa4588111f4 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -479,6 +479,7 @@ static void AssertParameters(ApiDescription apiDescription) ); } + AssertParameters(GetApiDescription((ExplicitArgumentListClass req) => { })); AssertParameters(GetApiDescription(([Parameters] ArgumentListClass req) => { })); AssertParameters(GetApiDescription(([Parameters] ArgumentListClassWithReadOnlyProperties req) => { })); AssertParameters(GetApiDescription(([Parameters] ArgumentListStruct req) => { })); @@ -1389,6 +1390,16 @@ private class ArgumentListClass public HttpContext Context { get; set; } } + [Parameters] + private class ExplicitArgumentListClass + { + [FromRoute] + public int Foo { get; set; } + public int Bar { get; set; } + public InferredJsonClass FromBody { get; set; } + public HttpContext Context { get; set; } + } + private class ArgumentListClassWithReadOnlyProperties : ArgumentListClass { public int ReadOnly { get; } diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/SurrogateParameterInfo.cs index daf55a86a91c..0282aa822f71 100644 --- a/src/Shared/SurrogateParameterInfo.cs +++ b/src/Shared/SurrogateParameterInfo.cs @@ -74,7 +74,8 @@ public static ReadOnlySpan Flatten(ParameterInfo[] parameters, Pa for (var i = 0; i < parameters.Length; i++) { - if (parameters[i].CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) + if (parameters[i].CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType)) || + parameters[i].ParameterType.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) { // Initialize the list with all parameter already processed // to keep the same parameter ordering From 68737ff8081fbd0aede15b05a62ffb144dabd966 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 6 May 2022 11:06:07 -0700 Subject: [PATCH 42/63] Mark types as sealed --- src/Shared/ParameterBindingMethodCache.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Shared/ParameterBindingMethodCache.cs b/src/Shared/ParameterBindingMethodCache.cs index c12768d1e86e..77f2f6851bff 100644 --- a/src/Shared/ParameterBindingMethodCache.cs +++ b/src/Shared/ParameterBindingMethodCache.cs @@ -633,7 +633,7 @@ private static bool TryGetNumberStylesTryGetMethod(Type type, [NotNullWhen(true) return ConvertAwaited(typedValueTask); } - private class ParameterLookupKey + private sealed class ParameterLookupKey { public ParameterLookupKey(string name, Type type) { @@ -658,7 +658,7 @@ public override bool Equals([NotNullWhen(true)] object? obj) } } - internal class ConstructorParameter + internal sealed class ConstructorParameter { public ConstructorParameter(ParameterInfo parameter, PropertyInfo propertyInfo) { From 8fce5b02561f067d81ddc1af37a10c62d0e51d4d Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 6 May 2022 11:28:44 -0700 Subject: [PATCH 43/63] Seal SurrogateParameterInfo --- src/Shared/SurrogateParameterInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/SurrogateParameterInfo.cs index 0282aa822f71..e130e41b2d21 100644 --- a/src/Shared/SurrogateParameterInfo.cs +++ b/src/Shared/SurrogateParameterInfo.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Http; -internal class SurrogateParameterInfo : ParameterInfo +internal sealed class SurrogateParameterInfo : ParameterInfo { private readonly PropertyInfo _underlyingProperty; private readonly ParameterInfo? _constructionParameterInfo; From 15921414a4585ee219c4e5601fc06e35c66abb6d Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 6 May 2022 15:15:57 -0700 Subject: [PATCH 44/63] Removing attribute from type --- .../src/ParametersAttribute.cs | 2 +- .../src/RequestDelegateFactory.cs | 3 +- .../test/RequestDelegateFactoryTests.cs | 39 ++++++------------- ...pointMetadataApiDescriptionProviderTest.cs | 11 ------ src/OpenApi/test/OpenApiGeneratorTests.cs | 11 ------ src/Shared/SurrogateParameterInfo.cs | 3 +- 6 files changed, 14 insertions(+), 55 deletions(-) diff --git a/src/Http/Http.Extensions/src/ParametersAttribute.cs b/src/Http/Http.Extensions/src/ParametersAttribute.cs index 645686eab711..515560c3a41c 100644 --- a/src/Http/Http.Extensions/src/ParametersAttribute.cs +++ b/src/Http/Http.Extensions/src/ParametersAttribute.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Http; /// /// [AttributeUsage( - AttributeTargets.Parameter | AttributeTargets.Class, + AttributeTargets.Parameter, Inherited = false, AllowMultiple = false)] public sealed class ParametersAttribute : Attribute diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 8b984c5b5c99..e0fcff26d848 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -647,8 +647,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext { return RequestPipeReaderExpr; } - else if (parameter.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType)) || - parameter.ParameterType.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) + else if (parameter.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) { if (parameter is SurrogateParameterInfo) { diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 55a83c0bb0f0..399fa39dc689 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -1652,15 +1652,9 @@ void TestExplicitFromBody_ParameterList([Parameters] ParametersListWithExplictFr args.HttpContext.Items.Add("body", args.Todo); } - void TestExplicitFromBody_ParameterListAttribute(HttpContext httpContext, [FromBody] TodoArgumentList todo) - { - httpContext.Items.Add("body", todo); - } - return new[] { new[] { (Action)TestExplicitFromBody }, - new[] { (Action)TestExplicitFromBody_ParameterListAttribute }, new object[] { (Action)TestExplicitFromBody_ParameterList }, }; } @@ -2252,16 +2246,23 @@ public BadArgumentListClassMultipleCtors(int foo, int bar) public int Bar { get; set; } } - [Parameters] private record NestedArgumentListRecord([Parameters] object NestedParameterList); - private record RecordWithParametersConstructor([Parameters] object NestedParameterList); + private class ClassWithParametersConstructor + { + public ClassWithParametersConstructor([Parameters] object nestedParameterList) + { + NestedParameterList = nestedParameterList; + } + + public object NestedParameterList { get; set; } + } [Fact] public void BuildRequestDelegateThrowsNotSupportedExceptionForNestedParametersList() { - void TestNestedParameterListRecordOnType(NestedArgumentListRecord req) { } - void TestNestedParameterListRecordOnArgument([Parameters] RecordWithParametersConstructor req) { } + void TestNestedParameterListRecordOnType([Parameters] NestedArgumentListRecord req) { } + void TestNestedParameterListRecordOnArgument([Parameters] ClassWithParametersConstructor req) { } Assert.Throws(() => RequestDelegateFactory.Create(TestNestedParameterListRecordOnType)); Assert.Throws(() => RequestDelegateFactory.Create(TestNestedParameterListRecordOnArgument)); @@ -4537,9 +4538,6 @@ private record struct ParameterListRecordStruct(HttpContext HttpContext, [FromRo private record ParameterListRecordClass(HttpContext HttpContext, [FromRoute] int Value); - [Parameters] - private record ExplicityArgumentListRecord(HttpContext HttpContext, [FromRoute] int Value); - private record ParameterListRecordWithoutPositionalParameters { public HttpContext? HttpContext { get; set; } @@ -4628,11 +4626,6 @@ public static object[][] FromParameterListActions { get { - void TestParameterListAttributeOnType(ExplicityArgumentListRecord args) - { - args.HttpContext.Items.Add("input", args.Value); - } - void TestParameterListRecordStruct([Parameters] ParameterListRecordStruct args) { args.HttpContext.Items.Add("input", args.Value); @@ -4680,7 +4673,6 @@ void TestParameterListClassWithParameterizedContructor([Parameters] ParameterLis return new[] { - new object[] { (Action)TestParameterListAttributeOnType }, new object[] { (Action)TestParameterListRecordStruct }, new object[] { (Action)TestParameterListRecordClass }, new object[] { (Action)TestParameterListRecordWithoutPositionalParameters }, @@ -6098,15 +6090,6 @@ private class Todo : ITodo public bool IsComplete { get; set; } } - [Parameters] - private class TodoArgumentList : Todo - { - public TodoArgumentList() - { - - } - } - private class TodoChild : Todo { public string? Child { get; set; } diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index faa4588111f4..99f22a67b16e 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -479,7 +479,6 @@ static void AssertParameters(ApiDescription apiDescription) ); } - AssertParameters(GetApiDescription((ExplicitArgumentListClass req) => { })); AssertParameters(GetApiDescription(([Parameters] ArgumentListClass req) => { })); AssertParameters(GetApiDescription(([Parameters] ArgumentListClassWithReadOnlyProperties req) => { })); AssertParameters(GetApiDescription(([Parameters] ArgumentListStruct req) => { })); @@ -1390,16 +1389,6 @@ private class ArgumentListClass public HttpContext Context { get; set; } } - [Parameters] - private class ExplicitArgumentListClass - { - [FromRoute] - public int Foo { get; set; } - public int Bar { get; set; } - public InferredJsonClass FromBody { get; set; } - public HttpContext Context { get; set; } - } - private class ArgumentListClassWithReadOnlyProperties : ArgumentListClass { public int ReadOnly { get; } diff --git a/src/OpenApi/test/OpenApiGeneratorTests.cs b/src/OpenApi/test/OpenApiGeneratorTests.cs index 476bf62d8444..929386fdd171 100644 --- a/src/OpenApi/test/OpenApiGeneratorTests.cs +++ b/src/OpenApi/test/OpenApiGeneratorTests.cs @@ -385,7 +385,6 @@ static void AssertParameters(OpenApiOperation operation) ); } - AssertParameters(GetOpenApiOperation((ExplicitArgumentListClass req) => { })); AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListClass req) => { })); AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListClassWithReadOnlyProperties req) => { })); AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListStruct req) => { })); @@ -880,14 +879,4 @@ private struct ArgumentListStruct public InferredJsonClass FromBody { get; set; } public HttpContext Context { get; set; } } - - [Parameters] - private class ExplicitArgumentListClass - { - [FromRoute] - public int Foo { get; set; } - public int Bar { get; set; } - public InferredJsonClass FromBody { get; set; } - public HttpContext Context { get; set; } - } } diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/SurrogateParameterInfo.cs index 0282aa822f71..daf55a86a91c 100644 --- a/src/Shared/SurrogateParameterInfo.cs +++ b/src/Shared/SurrogateParameterInfo.cs @@ -74,8 +74,7 @@ public static ReadOnlySpan Flatten(ParameterInfo[] parameters, Pa for (var i = 0; i < parameters.Length; i++) { - if (parameters[i].CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType)) || - parameters[i].ParameterType.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) + if (parameters[i].CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) { // Initialize the list with all parameter already processed // to keep the same parameter ordering From 9458d6013b9c256bd08bc4fe9f965e3f0be61050 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 9 May 2022 12:26:11 -0700 Subject: [PATCH 45/63] API Review changes --- .../src/AsParametersAttribute.cs} | 2 +- .../src/PublicAPI.Unshipped.txt | 2 + .../src/PublicAPI.Unshipped.txt | 2 - .../src/RequestDelegateFactory.cs | 4 +- .../test/RequestDelegateFactoryTests.cs | 62 +++++++++---------- ...pointMetadataApiDescriptionProviderTest.cs | 16 ++--- src/Mvc/Mvc.Core/src/FromServicesAttribute.cs | 2 +- src/OpenApi/test/OpenApiGeneratorTests.cs | 16 ++--- src/Shared/SurrogateParameterInfo.cs | 6 +- 9 files changed, 56 insertions(+), 56 deletions(-) rename src/Http/{Http.Extensions/src/ParametersAttribute.cs => Http.Abstractions/src/AsParametersAttribute.cs} (85%) diff --git a/src/Http/Http.Extensions/src/ParametersAttribute.cs b/src/Http/Http.Abstractions/src/AsParametersAttribute.cs similarity index 85% rename from src/Http/Http.Extensions/src/ParametersAttribute.cs rename to src/Http/Http.Abstractions/src/AsParametersAttribute.cs index 515560c3a41c..7595cc2be9c3 100644 --- a/src/Http/Http.Extensions/src/ParametersAttribute.cs +++ b/src/Http/Http.Abstractions/src/AsParametersAttribute.cs @@ -12,6 +12,6 @@ namespace Microsoft.AspNetCore.Http; AttributeTargets.Parameter, Inherited = false, AllowMultiple = false)] -public sealed class ParametersAttribute : Attribute +public sealed class AsParametersAttribute : Attribute { } diff --git a/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt b/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt index 54c1f4be51fc..ae9027335d66 100644 --- a/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt +++ b/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt @@ -3,6 +3,8 @@ *REMOVED*Microsoft.AspNetCore.Http.EndpointMetadataCollection.Enumerator.Current.get -> object? Microsoft.AspNetCore.Builder.EndpointBuilder.ServiceProvider.get -> System.IServiceProvider? Microsoft.AspNetCore.Builder.EndpointBuilder.ServiceProvider.set -> void +Microsoft.AspNetCore.Http.AsParametersAttribute +Microsoft.AspNetCore.Http.AsParametersAttribute.AsParametersAttribute() -> void Microsoft.AspNetCore.Http.DefaultRouteHandlerInvocationContext Microsoft.AspNetCore.Http.DefaultRouteHandlerInvocationContext.DefaultRouteHandlerInvocationContext(Microsoft.AspNetCore.Http.HttpContext! httpContext, params object![]! arguments) -> void Microsoft.AspNetCore.Http.EndpointMetadataCollection.Enumerator.Current.get -> object! diff --git a/src/Http/Http.Extensions/src/PublicAPI.Unshipped.txt b/src/Http/Http.Extensions/src/PublicAPI.Unshipped.txt index 4d91a73a4d15..93e10e87b5a6 100644 --- a/src/Http/Http.Extensions/src/PublicAPI.Unshipped.txt +++ b/src/Http/Http.Extensions/src/PublicAPI.Unshipped.txt @@ -13,8 +13,6 @@ Microsoft.AspNetCore.Http.Metadata.IEndpointMetadataProvider Microsoft.AspNetCore.Http.Metadata.IEndpointMetadataProvider.PopulateMetadata(Microsoft.AspNetCore.Http.Metadata.EndpointMetadataContext! context) -> void Microsoft.AspNetCore.Http.Metadata.IEndpointParameterMetadataProvider Microsoft.AspNetCore.Http.Metadata.IEndpointParameterMetadataProvider.PopulateMetadata(Microsoft.AspNetCore.Http.Metadata.EndpointParameterMetadataContext! parameterContext) -> void -Microsoft.AspNetCore.Http.ParametersAttribute -Microsoft.AspNetCore.Http.ParametersAttribute.ParametersAttribute() -> void Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.InitialEndpointMetadata.get -> System.Collections.Generic.IEnumerable? Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.InitialEndpointMetadata.init -> void Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.RouteHandlerFilterFactories.get -> System.Collections.Generic.IReadOnlyList!>? diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index e0fcff26d848..e340b4508601 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -647,12 +647,12 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext { return RequestPipeReaderExpr; } - else if (parameter.CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) + else if (parameter.CustomAttributes.Any(a => typeof(AsParametersAttribute).IsAssignableFrom(a.AttributeType))) { if (parameter is SurrogateParameterInfo) { throw new NotSupportedException( - $"Nested {nameof(ParametersAttribute)} is not supported and should be used only for handler parameters or parameter types."); + $"Nested {nameof(AsParametersAttribute)} is not supported and should be used only for handler parameters or parameter types."); } return BindParameterFromParametersType(parameter, factoryContext); diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 399fa39dc689..759681164b3f 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -225,7 +225,7 @@ public async Task RequestDelegatePopulatesFromRouteParameterBased_FromParameterL const string paramName = "value"; const int originalRouteParam = 42; - static void TestAction([Parameters] ParameterListFromRoute args) + static void TestAction([AsParameters] ParameterListFromRoute args) { args.HttpContext.Items.Add("input", args.Value); } @@ -1529,7 +1529,7 @@ public async Task RequestDelegatePopulatesFromQueryParameter_FromParameterList() int? deserializedRouteParam = null; - void TestAction([Parameters] ParameterListFromQuery args) + void TestAction([AsParameters] ParameterListFromQuery args) { deserializedRouteParam = args.Value; } @@ -1584,7 +1584,7 @@ public async Task RequestDelegatePopulatesFromHeaderParameter_FromParameterList( int? deserializedRouteParam = null; - void TestAction([Parameters] ParameterListFromHeader args) + void TestAction([AsParameters] ParameterListFromHeader args) { deserializedRouteParam = args.Value; } @@ -1623,7 +1623,7 @@ void TestImpliedFromBodyStruct(HttpContext httpContext, TodoStruct todo) httpContext.Items.Add("body", todo); } - void TestImpliedFromBodyStruct_ParameterList([Parameters] ParametersListWithImplictFromBody args) + void TestImpliedFromBodyStruct_ParameterList([AsParameters] ParametersListWithImplictFromBody args) { args.HttpContext.Items.Add("body", args.Todo); } @@ -1647,7 +1647,7 @@ void TestExplicitFromBody(HttpContext httpContext, [FromBody] Todo todo) httpContext.Items.Add("body", todo); } - void TestExplicitFromBody_ParameterList([Parameters] ParametersListWithExplictFromBody args) + void TestExplicitFromBody_ParameterList([AsParameters] ParametersListWithExplictFromBody args) { args.HttpContext.Items.Add("body", args.Todo); } @@ -2160,11 +2160,11 @@ public static object[][] BadArgumentListActions { get { - void TestParameterListRecord([Parameters] BadArgumentListRecord req) { } - void TestParameterListClass([Parameters] BadArgumentListClass req) { } - void TestParameterListClassWithMutipleConstructors([Parameters] BadArgumentListClassMultipleCtors req) { } - void TestParameterListAbstractClass([Parameters] BadAbstractArgumentListClass req) { } - void TestParameterListNoPulicConstructorClass([Parameters] BadNoPublicConstructorArgumentListClass req) { } + void TestParameterListRecord([AsParameters] BadArgumentListRecord req) { } + void TestParameterListClass([AsParameters] BadArgumentListClass req) { } + void TestParameterListClassWithMutipleConstructors([AsParameters] BadArgumentListClassMultipleCtors req) { } + void TestParameterListAbstractClass([AsParameters] BadAbstractArgumentListClass req) { } + void TestParameterListNoPulicConstructorClass([AsParameters] BadNoPublicConstructorArgumentListClass req) { } static string GetMultipleContructorsError(Type type) => $"Only a single public parameterized constructor is allowed for {TypeNameHelper.GetTypeDisplayName(type, fullName: false)}."; @@ -2246,11 +2246,11 @@ public BadArgumentListClassMultipleCtors(int foo, int bar) public int Bar { get; set; } } - private record NestedArgumentListRecord([Parameters] object NestedParameterList); + private record NestedArgumentListRecord([AsParameters] object NestedParameterList); private class ClassWithParametersConstructor { - public ClassWithParametersConstructor([Parameters] object nestedParameterList) + public ClassWithParametersConstructor([AsParameters] object nestedParameterList) { NestedParameterList = nestedParameterList; } @@ -2261,8 +2261,8 @@ public ClassWithParametersConstructor([Parameters] object nestedParameterList) [Fact] public void BuildRequestDelegateThrowsNotSupportedExceptionForNestedParametersList() { - void TestNestedParameterListRecordOnType([Parameters] NestedArgumentListRecord req) { } - void TestNestedParameterListRecordOnArgument([Parameters] ClassWithParametersConstructor req) { } + void TestNestedParameterListRecordOnType([AsParameters] NestedArgumentListRecord req) { } + void TestNestedParameterListRecordOnArgument([AsParameters] ClassWithParametersConstructor req) { } Assert.Throws(() => RequestDelegateFactory.Create(TestNestedParameterListRecordOnType)); Assert.Throws(() => RequestDelegateFactory.Create(TestNestedParameterListRecordOnArgument)); @@ -2281,7 +2281,7 @@ void TestExplicitFromService(HttpContext httpContext, [FromService] MyService my httpContext.Items.Add("service", myService); } - void TestExplicitFromService_FromParameterList([Parameters] ParametersListWithExplictFromService args) + void TestExplicitFromService_FromParameterList([AsParameters] ParametersListWithExplictFromService args) { args.HttpContext.Items.Add("service", args.MyService); } @@ -2315,7 +2315,7 @@ void TestImpliedFromService(HttpContext httpContext, IMyService myService) httpContext.Items.Add("service", myService); } - void TestImpliedFromService_FromParameterList([Parameters] ParametersListWithImplictFromService args) + void TestImpliedFromService_FromParameterList([AsParameters] ParametersListWithImplictFromService args) { args.HttpContext.Items.Add("service", args.MyService); } @@ -2438,7 +2438,7 @@ public async Task RequestDelegatePopulatesHttpContextParameterWithoutAttribute_F { HttpContext? httpContextArgument = null; - void TestAction([Parameters] ParametersListWithHttpContext args) + void TestAction([AsParameters] ParametersListWithHttpContext args) { httpContextArgument = args.HttpContext; } @@ -2503,7 +2503,7 @@ public async Task RequestDelegatePassHttpContextUserAsClaimsPrincipal_FromParame { ClaimsPrincipal? userArgument = null; - void TestAction([Parameters] ParametersListWithHttpContext args) + void TestAction([AsParameters] ParametersListWithHttpContext args) { userArgument = args.User; } @@ -2544,7 +2544,7 @@ public async Task RequestDelegatePassHttpContextRequestAsHttpRequest_FromParamet { HttpRequest? httpRequestArgument = null; - void TestAction([Parameters] ParametersListWithHttpContext args) + void TestAction([AsParameters] ParametersListWithHttpContext args) { httpRequestArgument = args.HttpRequest; } @@ -2584,7 +2584,7 @@ public async Task RequestDelegatePassesHttpContextRresponseAsHttpResponse_FromPa { HttpResponse? httpResponseArgument = null; - void TestAction([Parameters] ParametersListWithHttpContext args) + void TestAction([AsParameters] ParametersListWithHttpContext args) { httpResponseArgument = args.HttpResponse; } @@ -4626,47 +4626,47 @@ public static object[][] FromParameterListActions { get { - void TestParameterListRecordStruct([Parameters] ParameterListRecordStruct args) + void TestParameterListRecordStruct([AsParameters] ParameterListRecordStruct args) { args.HttpContext.Items.Add("input", args.Value); } - void TestParameterListRecordClass([Parameters] ParameterListRecordClass args) + void TestParameterListRecordClass([AsParameters] ParameterListRecordClass args) { args.HttpContext.Items.Add("input", args.Value); } - void TestParameterListRecordWithoutPositionalParameters([Parameters] ParameterListRecordWithoutPositionalParameters args) + void TestParameterListRecordWithoutPositionalParameters([AsParameters] ParameterListRecordWithoutPositionalParameters args) { args.HttpContext!.Items.Add("input", args.Value); } - void TestParameterListStruct([Parameters] ParameterListStruct args) + void TestParameterListStruct([AsParameters] ParameterListStruct args) { args.HttpContext.Items.Add("input", args.Value); } - void TestParameterListMutableStruct([Parameters] ParameterListMutableStruct args) + void TestParameterListMutableStruct([AsParameters] ParameterListMutableStruct args) { args.HttpContext.Items.Add("input", args.Value); } - void TestParameterListStructWithParameterizedContructor([Parameters] ParameterListStructWithParameterizedContructor args) + void TestParameterListStructWithParameterizedContructor([AsParameters] ParameterListStructWithParameterizedContructor args) { args.HttpContext.Items.Add("input", args.Value); } - void TestParameterListStructWithMultipleParameterizedContructor([Parameters] ParameterListStructWithMultipleParameterizedContructor args) + void TestParameterListStructWithMultipleParameterizedContructor([AsParameters] ParameterListStructWithMultipleParameterizedContructor args) { args.HttpContext.Items.Add("input", args.Value); } - void TestParameterListClass([Parameters] ParameterListClass args) + void TestParameterListClass([AsParameters] ParameterListClass args) { args.HttpContext!.Items.Add("input", args.Value); } - void TestParameterListClassWithParameterizedContructor([Parameters] ParameterListClassWithParameterizedContructor args) + void TestParameterListClassWithParameterizedContructor([AsParameters] ParameterListClassWithParameterizedContructor args) { args.HttpContext.Items.Add("input", args.Value); } @@ -4711,7 +4711,7 @@ public async Task RequestDelegatePopulatesFromParameterListRecordUsesDefaultValu { const int expectedValue = 42; - void TestAction([Parameters] ParameterListRecordWitDefaultValue args) + void TestAction([AsParameters] ParameterListRecordWitDefaultValue args) { args.HttpContext.Items.Add("input", args.Value); } @@ -4743,7 +4743,7 @@ public async Task RequestDelegatePopulatesFromParameterListUsesDefaultValue() { const int expectedValue = 42; - void TestAction([Parameters] ParameterListWitDefaultValue args) + void TestAction([AsParameters] ParameterListWitDefaultValue args) { args.HttpContext.Items.Add("input", args.Value); } diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 99f22a67b16e..88f4f06e74f2 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -479,14 +479,14 @@ static void AssertParameters(ApiDescription apiDescription) ); } - AssertParameters(GetApiDescription(([Parameters] ArgumentListClass req) => { })); - AssertParameters(GetApiDescription(([Parameters] ArgumentListClassWithReadOnlyProperties req) => { })); - AssertParameters(GetApiDescription(([Parameters] ArgumentListStruct req) => { })); - AssertParameters(GetApiDescription(([Parameters] ArgumentListRecord req) => { })); - AssertParameters(GetApiDescription(([Parameters] ArgumentListRecordStruct req) => { })); - AssertParameters(GetApiDescription(([Parameters] ArgumentListRecordWithoutPositionalParameters req) => { })); - AssertParameters(GetApiDescription(([Parameters] ArgumentListRecordWithoutAttributes req) => { }, "/{foo}")); - AssertParameters(GetApiDescription(([Parameters] ArgumentListRecordWithoutAttributes req) => { }, "/{Foo}")); + AssertParameters(GetApiDescription(([AsParameters] ArgumentListClass req) => { })); + AssertParameters(GetApiDescription(([AsParameters] ArgumentListClassWithReadOnlyProperties req) => { })); + AssertParameters(GetApiDescription(([AsParameters] ArgumentListStruct req) => { })); + AssertParameters(GetApiDescription(([AsParameters] ArgumentListRecord req) => { })); + AssertParameters(GetApiDescription(([AsParameters] ArgumentListRecordStruct req) => { })); + AssertParameters(GetApiDescription(([AsParameters] ArgumentListRecordWithoutPositionalParameters req) => { })); + AssertParameters(GetApiDescription(([AsParameters] ArgumentListRecordWithoutAttributes req) => { }, "/{foo}")); + AssertParameters(GetApiDescription(([AsParameters] ArgumentListRecordWithoutAttributes req) => { }, "/{Foo}")); } [Fact] diff --git a/src/Mvc/Mvc.Core/src/FromServicesAttribute.cs b/src/Mvc/Mvc.Core/src/FromServicesAttribute.cs index 128d4742a8f4..b26d47acd6b9 100644 --- a/src/Mvc/Mvc.Core/src/FromServicesAttribute.cs +++ b/src/Mvc/Mvc.Core/src/FromServicesAttribute.cs @@ -22,7 +22,7 @@ namespace Microsoft.AspNetCore.Mvc; /// } /// /// -[AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false, Inherited = true)] +[AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] public class FromServicesAttribute : Attribute, IBindingSourceMetadata, IFromServiceMetadata { /// diff --git a/src/OpenApi/test/OpenApiGeneratorTests.cs b/src/OpenApi/test/OpenApiGeneratorTests.cs index 929386fdd171..f30b1cc9e7d1 100644 --- a/src/OpenApi/test/OpenApiGeneratorTests.cs +++ b/src/OpenApi/test/OpenApiGeneratorTests.cs @@ -385,14 +385,14 @@ static void AssertParameters(OpenApiOperation operation) ); } - AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListClass req) => { })); - AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListClassWithReadOnlyProperties req) => { })); - AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListStruct req) => { })); - AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListRecord req) => { })); - AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListRecordStruct req) => { })); - AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListRecordWithoutPositionalParameters req) => { })); - AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListRecordWithoutAttributes req) => { }, "/{foo}")); - AssertParameters(GetOpenApiOperation(([Parameters] ArgumentListRecordWithoutAttributes req) => { }, "/{Foo}")); + AssertParameters(GetOpenApiOperation(([AsParameters] ArgumentListClass req) => { })); + AssertParameters(GetOpenApiOperation(([AsParameters] ArgumentListClassWithReadOnlyProperties req) => { })); + AssertParameters(GetOpenApiOperation(([AsParameters] ArgumentListStruct req) => { })); + AssertParameters(GetOpenApiOperation(([AsParameters] ArgumentListRecord req) => { })); + AssertParameters(GetOpenApiOperation(([AsParameters] ArgumentListRecordStruct req) => { })); + AssertParameters(GetOpenApiOperation(([AsParameters] ArgumentListRecordWithoutPositionalParameters req) => { })); + AssertParameters(GetOpenApiOperation(([AsParameters] ArgumentListRecordWithoutAttributes req) => { }, "/{foo}")); + AssertParameters(GetOpenApiOperation(([AsParameters] ArgumentListRecordWithoutAttributes req) => { }, "/{Foo}")); } [Fact] diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/SurrogateParameterInfo.cs index 23ad304682ce..1d5b0d02e7a7 100644 --- a/src/Shared/SurrogateParameterInfo.cs +++ b/src/Shared/SurrogateParameterInfo.cs @@ -50,9 +50,9 @@ public override bool HasDefaultValue => _constructionParameterInfo is not null ? _constructionParameterInfo.RawDefaultValue : null; /// - /// Unwraps all parameters that contains and + /// Unwraps all parameters that contains and /// creates a flat list merging the current parameters, not including the - /// parametres that contain a , and all additional + /// parametres that contain a , and all additional /// parameters detected. /// /// List of parameters to be flattened. @@ -74,7 +74,7 @@ public static ReadOnlySpan Flatten(ParameterInfo[] parameters, Pa for (var i = 0; i < parameters.Length; i++) { - if (parameters[i].CustomAttributes.Any(a => typeof(ParametersAttribute).IsAssignableFrom(a.AttributeType))) + if (parameters[i].CustomAttributes.Any(a => typeof(AsParametersAttribute).IsAssignableFrom(a.AttributeType))) { // Initialize the list with all parameter already processed // to keep the same parameter ordering From 358db2c0fee4fea88435f0666bd7951af3bc33fe Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 9 May 2022 12:32:18 -0700 Subject: [PATCH 46/63] Updating documentation --- src/Http/Http.Abstractions/src/AsParametersAttribute.cs | 2 +- src/Mvc/Mvc.Core/src/FromServicesAttribute.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Abstractions/src/AsParametersAttribute.cs b/src/Http/Http.Abstractions/src/AsParametersAttribute.cs index 7595cc2be9c3..50d88758fc6d 100644 --- a/src/Http/Http.Abstractions/src/AsParametersAttribute.cs +++ b/src/Http/Http.Abstractions/src/AsParametersAttribute.cs @@ -6,7 +6,7 @@ namespace Microsoft.AspNetCore.Http; using System; /// -/// +/// Specifies that a parameter represents a structure delegate's parameter list. /// [AttributeUsage( AttributeTargets.Parameter, diff --git a/src/Mvc/Mvc.Core/src/FromServicesAttribute.cs b/src/Mvc/Mvc.Core/src/FromServicesAttribute.cs index b26d47acd6b9..77ecb5b993d7 100644 --- a/src/Mvc/Mvc.Core/src/FromServicesAttribute.cs +++ b/src/Mvc/Mvc.Core/src/FromServicesAttribute.cs @@ -7,7 +7,7 @@ namespace Microsoft.AspNetCore.Mvc; /// -/// Specifies that an action parameter should be bound using the request services. +/// Specifies that a parameter or property should be bound using the request services. /// /// /// In this example an implementation of IProductModelRequestService is registered as a service. From 6965cbabfff34baff039f75a817696f6005889bc Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 9 May 2022 12:51:06 -0700 Subject: [PATCH 47/63] Renaming surrogateParameterInfo --- ...icrosoft.AspNetCore.Http.Extensions.csproj | 2 +- .../src/RequestDelegateFactory.cs | 22 ++--- ...sts.cs => PropertyAsParameterInfoTests.cs} | 86 +++++++++---------- .../EndpointMetadataApiDescriptionProvider.cs | 2 +- ...icrosoft.AspNetCore.Mvc.ApiExplorer.csproj | 2 +- .../src/Microsoft.AspNetCore.OpenApi.csproj | 2 +- src/OpenApi/src/OpenApiGenerator.cs | 4 +- ...eterInfo.cs => PropertyAsParameterInfo.cs} | 12 +-- 8 files changed, 66 insertions(+), 66 deletions(-) rename src/Http/Http.Extensions/test/{SurrogateParameterInfoTests.cs => PropertyAsParameterInfoTests.cs} (60%) rename src/Shared/{SurrogateParameterInfo.cs => PropertyAsParameterInfo.cs} (92%) diff --git a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj index 6b1483b12b24..ba4e362f4db3 100644 --- a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj +++ b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj @@ -13,7 +13,7 @@ - + diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index e340b4508601..8d1f14bc0876 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -226,7 +226,7 @@ private static FactoryContext CreateFactoryContext(RequestDelegateFactoryOptions AddTypeProvidedMetadata(methodInfo, factoryContext.Metadata, factoryContext.ServiceProvider, - CollectionsMarshal.AsSpan(factoryContext.SurrogateParameters)); + CollectionsMarshal.AsSpan(factoryContext.PropertiesAsParameter)); // Add method attributes as metadata *after* any inferred metadata so that the attributes hava a higher specificity AddMethodAttributesAsMetadata(methodInfo, factoryContext.Metadata); @@ -428,7 +428,7 @@ private static Expression CreateRouteHandlerInvocationContextBase(FactoryContext return fallbackConstruction; } - private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List metadata, IServiceProvider? services, Span surrogateParameters) + private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List metadata, IServiceProvider? services, Span propertiesAsParameter) { object?[]? invokeArgs = null; @@ -459,8 +459,8 @@ void AddMetadata(ReadOnlySpan parameters) // Get metadata from parameter types AddMetadata(methodInfo.GetParameters()); - // Get metadata from surrogated parameter types - AddMetadata(surrogateParameters); + // Get metadata from properties surrogate as parameter types + AddMetadata(propertiesAsParameter); // Get metadata from return type var returnType = methodInfo.ReturnType; @@ -649,7 +649,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext } else if (parameter.CustomAttributes.Any(a => typeof(AsParametersAttribute).IsAssignableFrom(a.AttributeType))) { - if (parameter is SurrogateParameterInfo) + if (parameter is PropertyAsParameterInfo) { throw new NotSupportedException( $"Nested {nameof(AsParametersAttribute)} is not supported and should be used only for handler parameters or parameter types."); @@ -1257,9 +1257,9 @@ private static Expression BindParameterFromParametersType(ParameterInfo paramete for (var i = 0; i < parameters.Length; i++) { var parameterInfo = - new SurrogateParameterInfo(parameters[i].PropertyInfo, parameters[i].ParameterInfo, factoryContext.NullabilityContext); + new PropertyAsParameterInfo(parameters[i].PropertyInfo, parameters[i].ParameterInfo, factoryContext.NullabilityContext); constructorArguments[i] = CreateArgument(parameterInfo, factoryContext); - factoryContext.SurrogateParameters.Add(parameterInfo); + factoryContext.PropertiesAsParameter.Add(parameterInfo); } factoryContext.ParamCheckExpressions.Add( @@ -1283,9 +1283,9 @@ private static Expression BindParameterFromParametersType(ParameterInfo paramete // For parameterless ctor we will init only writable properties. if (properties[i].CanWrite) { - var parameterInfo = new SurrogateParameterInfo(properties[i], factoryContext.NullabilityContext); + var parameterInfo = new PropertyAsParameterInfo(properties[i], factoryContext.NullabilityContext); bindings.Add(Expression.Bind(properties[i], CreateArgument(parameterInfo, factoryContext))); - factoryContext.SurrogateParameters.Add(parameterInfo); + factoryContext.PropertiesAsParameter.Add(parameterInfo); } } @@ -1795,7 +1795,7 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al private static bool IsOptionalParameter(ParameterInfo parameter, FactoryContext factoryContext) { - if (parameter is SurrogateParameterInfo argument) + if (parameter is PropertyAsParameterInfo argument) { return argument.IsOptional; } @@ -2093,7 +2093,7 @@ private class FactoryContext public Expression[] BoxedArgs { get; set; } = Array.Empty(); public List>? Filters { get; init; } - public List SurrogateParameters { get; } = new(); + public List PropertiesAsParameter { get; } = new(); } private static class RequestDelegateFactoryConstants diff --git a/src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs b/src/Http/Http.Extensions/test/PropertyAsParameterInfoTests.cs similarity index 60% rename from src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs rename to src/Http/Http.Extensions/test/PropertyAsParameterInfoTests.cs index 20c886b3a200..a4781dd48951 100644 --- a/src/Http/Http.Extensions/test/SurrogateParameterInfoTests.cs +++ b/src/Http/Http.Extensions/test/PropertyAsParameterInfoTests.cs @@ -5,18 +5,18 @@ namespace Microsoft.AspNetCore.Http.Extensions.Tests; -public class SurrogateParameterInfoTests +public class PropertyAsParameterInfoTests { [Fact] public void Initialization_SetsTypeAndNameFromPropertyInfo() { // Arrange & Act var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.NoAttribute)); - var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo); + var parameterInfo = new PropertyAsParameterInfo(propertyInfo); // Assert - Assert.Equal(propertyInfo.Name, surrogateParameterInfo.Name); - Assert.Equal(propertyInfo.PropertyType, surrogateParameterInfo.ParameterType); + Assert.Equal(propertyInfo.Name, parameterInfo.Name); + Assert.Equal(propertyInfo.PropertyType, parameterInfo.ParameterType); } [Fact] @@ -25,57 +25,57 @@ public void Initialization_WithConstructorArgument_SetsTypeAndNameFromPropertyIn // Arrange & Act var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.NoAttribute)); var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.NoAttribute)); - var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + var parameterInfo = new PropertyAsParameterInfo(propertyInfo, parameter); // Assert - Assert.Equal(propertyInfo.Name, surrogateParameterInfo.Name); - Assert.Equal(propertyInfo.PropertyType, surrogateParameterInfo.ParameterType); + Assert.Equal(propertyInfo.Name, parameterInfo.Name); + Assert.Equal(propertyInfo.PropertyType, parameterInfo.ParameterType); } [Fact] - public void SurrogateParameterInfo_ContainsPropertyCustomAttributes() + public void PropertyAsParameterInfoTests_ContainsPropertyCustomAttributes() { // Arrange var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.WithTestAttribute)); - var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo); + var parameterInfo = new PropertyAsParameterInfo(propertyInfo); // Act & Assert - Assert.Single(surrogateParameterInfo.GetCustomAttributes(typeof(TestAttribute))); + Assert.Single(parameterInfo.GetCustomAttributes(typeof(TestAttribute))); } [Fact] - public void SurrogateParameterInfo_WithConstructorArgument_UsesParameterCustomAttributes() + public void PropertyAsParameterInfoTests_WithConstructorArgument_UsesParameterCustomAttributes() { // Arrange var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.NoAttribute)); var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.WithTestAttribute)); - var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + var parameterInfo = new PropertyAsParameterInfo(propertyInfo, parameter); // Act & Assert - Assert.Single(surrogateParameterInfo.GetCustomAttributes(typeof(TestAttribute))); + Assert.Single(parameterInfo.GetCustomAttributes(typeof(TestAttribute))); } [Fact] - public void SurrogateParameterInfo_WithConstructorArgument_FallbackToPropertyCustomAttributes() + public void PropertyAsParameterInfoTests_WithConstructorArgument_FallbackToPropertyCustomAttributes() { // Arrange var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.WithTestAttribute)); var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.NoAttribute)); - var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + var parameterInfo = new PropertyAsParameterInfo(propertyInfo, parameter); // Act & Assert - Assert.Single(surrogateParameterInfo.GetCustomAttributes(typeof(TestAttribute))); + Assert.Single(parameterInfo.GetCustomAttributes(typeof(TestAttribute))); } [Fact] - public void SurrogateParameterInfo_ContainsPropertyCustomAttributesData() + public void PropertyAsParameterInfoTests_ContainsPropertyCustomAttributesData() { // Arrange var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.WithTestAttribute)); - var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo); + var parameterInfo = new PropertyAsParameterInfo(propertyInfo); // Act - var attributes = surrogateParameterInfo.GetCustomAttributesData(); + var attributes = parameterInfo.GetCustomAttributesData(); // Assert Assert.Single( @@ -84,35 +84,35 @@ public void SurrogateParameterInfo_ContainsPropertyCustomAttributesData() } [Fact] - public void SurrogateParameterInfo_WithConstructorArgument_MergePropertyAndParameterCustomAttributesData() + public void PropertyAsParameterInfoTests_WithConstructorArgument_MergePropertyAndParameterCustomAttributesData() { // Arrange & Act var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.WithTestAttribute)); var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.WithSampleAttribute)); - var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + var parameterInfo = new PropertyAsParameterInfo(propertyInfo, parameter); // Act - var attributes = surrogateParameterInfo.GetCustomAttributesData(); + var attributes = parameterInfo.GetCustomAttributesData(); // Assert Assert.Single( - surrogateParameterInfo.GetCustomAttributesData(), + parameterInfo.GetCustomAttributesData(), a => typeof(TestAttribute).IsAssignableFrom(a.AttributeType)); Assert.Single( - surrogateParameterInfo.GetCustomAttributesData(), + parameterInfo.GetCustomAttributesData(), a => typeof(SampleAttribute).IsAssignableFrom(a.AttributeType)); } [Fact] - public void SurrogateParameterInfo_WithConstructorArgument_MergePropertyAndParameterCustomAttributes() + public void PropertyAsParameterInfoTests_WithConstructorArgument_MergePropertyAndParameterCustomAttributes() { // Arrange var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.WithTestAttribute)); var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.WithSampleAttribute)); - var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + var parameterInfo = new PropertyAsParameterInfo(propertyInfo, parameter); // Act - var attributes = surrogateParameterInfo.GetCustomAttributes(true); + var attributes = parameterInfo.GetCustomAttributes(true); // Assert Assert.Single( @@ -124,53 +124,53 @@ public void SurrogateParameterInfo_WithConstructorArgument_MergePropertyAndParam } [Fact] - public void SurrogateParameterInfo_ContainsPropertyInheritedCustomAttributes() + public void PropertyAsParameterInfoTests_ContainsPropertyInheritedCustomAttributes() { // Arrange & Act var propertyInfo = GetProperty(typeof(DerivedArgumentList), nameof(DerivedArgumentList.WithTestAttribute)); - var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo); + var parameterInfo = new PropertyAsParameterInfo(propertyInfo); // Assert - Assert.Single(surrogateParameterInfo.GetCustomAttributes(typeof(TestAttribute), true)); + Assert.Single(parameterInfo.GetCustomAttributes(typeof(TestAttribute), true)); } [Fact] - public void SurrogateParameterInfo_DoesNotHaveDefaultValueFromProperty() + public void PropertyAsParameterInfoTests_DoesNotHaveDefaultValueFromProperty() { // Arrange & Act var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.NoAttribute)); - var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo); + var parameterInfo = new PropertyAsParameterInfo(propertyInfo); // Assert - Assert.False(surrogateParameterInfo.HasDefaultValue); + Assert.False(parameterInfo.HasDefaultValue); } [Fact] - public void SurrogateParameterInfo_WithConstructorArgument_HasDefaultValue() + public void PropertyAsParameterInfoTests_WithConstructorArgument_HasDefaultValue() { // Arrange & Act var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.NoAttribute)); var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), "withDefaultValue"); - var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + var parameterInfo = new PropertyAsParameterInfo(propertyInfo, parameter); // Assert - Assert.True(surrogateParameterInfo.HasDefaultValue); - Assert.NotNull(surrogateParameterInfo.DefaultValue); - Assert.IsType(surrogateParameterInfo.DefaultValue); - Assert.NotNull(surrogateParameterInfo.RawDefaultValue); - Assert.IsType(surrogateParameterInfo.RawDefaultValue); + Assert.True(parameterInfo.HasDefaultValue); + Assert.NotNull(parameterInfo.DefaultValue); + Assert.IsType(parameterInfo.DefaultValue); + Assert.NotNull(parameterInfo.RawDefaultValue); + Assert.IsType(parameterInfo.RawDefaultValue); } [Fact] - public void SurrogateParameterInfo_WithConstructorArgument_DoesNotHaveDefaultValue() + public void PropertyAsParameterInfoTests_WithConstructorArgument_DoesNotHaveDefaultValue() { // Arrange & Act var propertyInfo = GetProperty(typeof(ArgumentList), nameof(ArgumentList.NoAttribute)); var parameter = GetParameter(nameof(ArgumentList.DefaultMethod), nameof(ArgumentList.NoAttribute)); - var surrogateParameterInfo = new SurrogateParameterInfo(propertyInfo, parameter); + var parameterInfo = new PropertyAsParameterInfo(propertyInfo, parameter); // Assert - Assert.False(surrogateParameterInfo.HasDefaultValue); + Assert.False(parameterInfo.HasDefaultValue); } private static PropertyInfo GetProperty(Type containerType, string propertyName) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index a81e8a446356..adc1b96b4d94 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -116,7 +116,7 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string var hasBodyOrFormFileParameter = false; - foreach (var parameter in SurrogateParameterInfo.Flatten(methodInfo.GetParameters(), ParameterBindingMethodCache)) + foreach (var parameter in PropertyAsParameterInfo.Flatten(methodInfo.GetParameters(), ParameterBindingMethodCache)) { var parameterDescription = CreateApiParameterDescription(parameter, routeEndpoint.RoutePattern, disableInferredBody); diff --git a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj index 1fdce20a3d3b..b6bbde313c3d 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj +++ b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj @@ -11,7 +11,7 @@ - + diff --git a/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj b/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj index f3bedb527dc5..34a81cb70466 100644 --- a/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj +++ b/src/OpenApi/src/Microsoft.AspNetCore.OpenApi.csproj @@ -17,7 +17,7 @@ - + diff --git a/src/OpenApi/src/OpenApiGenerator.cs b/src/OpenApi/src/OpenApiGenerator.cs index fab60607a6d4..fee27c152048 100644 --- a/src/OpenApi/src/OpenApiGenerator.cs +++ b/src/OpenApi/src/OpenApiGenerator.cs @@ -250,7 +250,7 @@ private static void GenerateDefaultResponses(Dictionary GetOperationTags(MethodInfo methodInfo, EndpointMetadat private List GetOpenApiParameters(MethodInfo methodInfo, EndpointMetadataCollection metadata, RoutePattern pattern, bool disableInferredBody) { - var parameters = SurrogateParameterInfo.Flatten(methodInfo.GetParameters(), ParameterBindingMethodCache); + var parameters = PropertyAsParameterInfo.Flatten(methodInfo.GetParameters(), ParameterBindingMethodCache); var openApiParameters = new List(); foreach (var parameter in parameters) diff --git a/src/Shared/SurrogateParameterInfo.cs b/src/Shared/PropertyAsParameterInfo.cs similarity index 92% rename from src/Shared/SurrogateParameterInfo.cs rename to src/Shared/PropertyAsParameterInfo.cs index 1d5b0d02e7a7..275c86334b1b 100644 --- a/src/Shared/SurrogateParameterInfo.cs +++ b/src/Shared/PropertyAsParameterInfo.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Http; -internal sealed class SurrogateParameterInfo : ParameterInfo +internal sealed class PropertyAsParameterInfo : ParameterInfo { private readonly PropertyInfo _underlyingProperty; private readonly ParameterInfo? _constructionParameterInfo; @@ -18,7 +18,7 @@ internal sealed class SurrogateParameterInfo : ParameterInfo private readonly NullabilityInfoContext _nullabilityContext; private NullabilityInfo? _nullabilityInfo; - public SurrogateParameterInfo(PropertyInfo propertyInfo, NullabilityInfoContext? nullabilityContext = null) + public PropertyAsParameterInfo(PropertyInfo propertyInfo, NullabilityInfoContext? nullabilityContext = null) { Debug.Assert(null != propertyInfo); @@ -35,7 +35,7 @@ public SurrogateParameterInfo(PropertyInfo propertyInfo, NullabilityInfoContext? _underlyingProperty = propertyInfo; } - public SurrogateParameterInfo(PropertyInfo property, ParameterInfo parameterInfo, NullabilityInfoContext? nullabilityContext = null) + public PropertyAsParameterInfo(PropertyInfo property, ParameterInfo parameterInfo, NullabilityInfoContext? nullabilityContext = null) : this(property, nullabilityContext) { _constructionParameterInfo = parameterInfo; @@ -58,7 +58,7 @@ public override bool HasDefaultValue /// List of parameters to be flattened. /// An instance of the method cache class. /// Flat list of parameters. - [UnconditionalSuppressMessage("Trimmer", "IL2075", Justification = "SurrogateParameterInfo.Flatten requires unreferenced code.")] + [UnconditionalSuppressMessage("Trimmer", "IL2075", Justification = "PropertyAsParameterInfo.Flatten requires unreferenced code.")] public static ReadOnlySpan Flatten(ParameterInfo[] parameters, ParameterBindingMethodCache cache) { ArgumentNullException.ThrowIfNull(nameof(parameters)); @@ -87,7 +87,7 @@ public static ReadOnlySpan Flatten(ParameterInfo[] parameters, Pa foreach (var constructorParameter in constructorParameters) { flattenedParameters.Add( - new SurrogateParameterInfo( + new PropertyAsParameterInfo( constructorParameter.PropertyInfo, constructorParameter.ParameterInfo, nullabilityContext)); @@ -101,7 +101,7 @@ public static ReadOnlySpan Flatten(ParameterInfo[] parameters, Pa { if (property.CanWrite) { - flattenedParameters.Add(new SurrogateParameterInfo(property, nullabilityContext)); + flattenedParameters.Add(new PropertyAsParameterInfo(property, nullabilityContext)); } } } From 55b26f283994e83955a83f468639b3a8f85d3c7e Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 9 May 2022 12:52:30 -0700 Subject: [PATCH 48/63] Code cleanup --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 8d1f14bc0876..62152dbb36f5 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -459,7 +459,7 @@ void AddMetadata(ReadOnlySpan parameters) // Get metadata from parameter types AddMetadata(methodInfo.GetParameters()); - // Get metadata from properties surrogate as parameter types + // Get metadata from properties as parameter types AddMetadata(propertiesAsParameter); // Get metadata from return type @@ -1299,7 +1299,7 @@ private static Expression BindParameterFromParametersType(ParameterInfo paramete Expression.MemberInit(newExpression, bindings))); } - factoryContext.TrackedParameters.Add(parameter.Name!, RequestDelegateFactoryConstants.SurrogateParameter); + factoryContext.TrackedParameters.Add(parameter.Name!, RequestDelegateFactoryConstants.PropertyAsParameter); factoryContext.ExtraLocals.Add(argumentExpression); return argumentExpression; @@ -2110,7 +2110,7 @@ private static class RequestDelegateFactoryConstants public const string BodyParameter = "Body (Inferred)"; public const string RouteOrQueryStringParameter = "Route or Query String (Inferred)"; public const string FormFileParameter = "Form File (Inferred)"; - public const string SurrogateParameter = "Surrogate (Attribute)"; + public const string PropertyAsParameter = "AsParameter (Attribute)"; } private static partial class Log From d6c9e63f32a86aa4d1249156010db126d6a1fd32 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 9 May 2022 12:52:51 -0700 Subject: [PATCH 49/63] Code cleanup --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 62152dbb36f5..b5d77f701bca 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -2110,7 +2110,7 @@ private static class RequestDelegateFactoryConstants public const string BodyParameter = "Body (Inferred)"; public const string RouteOrQueryStringParameter = "Route or Query String (Inferred)"; public const string FormFileParameter = "Form File (Inferred)"; - public const string PropertyAsParameter = "AsParameter (Attribute)"; + public const string PropertyAsParameter = "As Parameter (Attribute)"; } private static partial class Log From 74df5713cc27a927e502207411e8f3e46b055f33 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 9 May 2022 12:54:24 -0700 Subject: [PATCH 50/63] Code cleanup --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index b5d77f701bca..785ea48d7751 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -2089,7 +2089,7 @@ private class FactoryContext public List ContextArgAccess { get; } = new(); public Expression? MethodCall { get; set; } public Type[] ArgumentTypes { get; set; } = Array.Empty(); - public Expression[] ArgumentExpressions { get; set; } = Array.Empty(); + public Expression[] ArgumentExpressions { get; set; } = Array.Empty(); public Expression[] BoxedArgs { get; set; } = Array.Empty(); public List>? Filters { get; init; } From a9cc1866d556a71e497f0c3e37f9a81803a63420 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 9 May 2022 12:55:16 -0700 Subject: [PATCH 51/63] Code cleanup --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 785ea48d7751..fa7b3bf73c45 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -652,7 +652,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext if (parameter is PropertyAsParameterInfo) { throw new NotSupportedException( - $"Nested {nameof(AsParametersAttribute)} is not supported and should be used only for handler parameters or parameter types."); + $"Nested {nameof(AsParametersAttribute)} is not supported and should be used only for handler parameters."); } return BindParameterFromParametersType(parameter, factoryContext); From 902b65c669e120594e96f0de21dd22d9c2cd30e5 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 9 May 2022 14:21:59 -0700 Subject: [PATCH 52/63] Updating tests to include FromService in properties --- .../test/DefaultApiDescriptionProviderTest.cs | 6 ++++++ .../DefaultPageApplicationModelProviderTest.cs | 16 +++++++++++++++- .../Infrastructure/PageBinderFactoryTest.cs | 18 +++++++++++++++++- .../RequestServicesTestBase.cs | 1 + .../RequestScopedServiceController.cs | 9 +++++++++ 5 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/test/DefaultApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/DefaultApiDescriptionProviderTest.cs index 4c474c953877..0490be15740a 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/DefaultApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/DefaultApiDescriptionProviderTest.cs @@ -2382,6 +2382,9 @@ private class TestController [FromHeader] public string UserId { get; set; } + [FromServices] + public ITestService TestService { get; set; } + [ModelBinder] public string Comments { get; set; } @@ -2475,6 +2478,9 @@ private class ProductChangeDTO [FromHeader] public string UserId { get; set; } + [FromServices] + public ITestService TestService { get; set; } + public string Comments { get; set; } } diff --git a/src/Mvc/Mvc.RazorPages/test/ApplicationModels/DefaultPageApplicationModelProviderTest.cs b/src/Mvc/Mvc.RazorPages/test/ApplicationModels/DefaultPageApplicationModelProviderTest.cs index 9a603ca1af2c..41c8e15ca11b 100644 --- a/src/Mvc/Mvc.RazorPages/test/ApplicationModels/DefaultPageApplicationModelProviderTest.cs +++ b/src/Mvc/Mvc.RazorPages/test/ApplicationModels/DefaultPageApplicationModelProviderTest.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Reflection; @@ -353,6 +353,14 @@ public void OnProvidersExecuting_DiscoversPropertiesFromModel() Assert.Equal(name, property.PropertyName); Assert.NotNull(property.BindingInfo); Assert.Equal(BindingSource.Query, property.BindingInfo.BindingSource); + }, + property => + { + var name = nameof(TestPageModel.TestService); + Assert.Equal(modelType.GetProperty(name), property.PropertyInfo); + Assert.Equal(name, property.PropertyName); + Assert.NotNull(property.BindingInfo); + Assert.Equal(BindingSource.Services, property.BindingInfo.BindingSource); }); } @@ -1058,6 +1066,9 @@ private class PageWithModel : Page public override Task ExecuteAsync() => throw new NotImplementedException(); } + public interface ITestService + { } + [PageModel] private class TestPageModel { @@ -1066,6 +1077,9 @@ private class TestPageModel [FromQuery] public string Property2 { get; set; } + [FromServices] + public ITestService TestService { get; set; } + public void OnGetUser() { } } diff --git a/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageBinderFactoryTest.cs b/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageBinderFactoryTest.cs index 9c67e6856cdb..44146dd56879 100644 --- a/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageBinderFactoryTest.cs +++ b/src/Mvc/Mvc.RazorPages/test/Infrastructure/PageBinderFactoryTest.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.ComponentModel.DataAnnotations; @@ -838,6 +838,9 @@ public TestParameterBinder(IDictionary args) } } + private interface ITestService + { } + private class PageModelWithNoBoundProperties : PageModel { } @@ -855,6 +858,9 @@ private class PageWithNoVisibleBoundProperties : Page [FromQuery] protected string FromQuery { get; set; } + [FromServices] + protected ITestService FromService { get; set; } + [FromRoute] public static int FromRoute { get; set; } @@ -869,6 +875,9 @@ private class PageModelWithNoVisibleBoundProperties : PageModel [FromQuery] protected string FromQuery { get; set; } + [FromServices] + protected ITestService FromService { get; set; } + [FromRoute] public static int FromRoute { get; set; } } @@ -898,6 +907,9 @@ private class PageWithProperty : Page [FromForm] public string PropertyWithNoValue { get; set; } + [FromServices] + public ITestService FromService { get; set; } + public override Task ExecuteAsync() => Task.FromResult(0); } @@ -911,6 +923,10 @@ private class PageModelWithProperty : PageModel [FromForm] public string PropertyWithNoValue { get; set; } + + [FromServices] + public ITestService FromService { get; set; } + } private class PageModelWithDefaultValue diff --git a/src/Mvc/test/Mvc.FunctionalTests/RequestServicesTestBase.cs b/src/Mvc/test/Mvc.FunctionalTests/RequestServicesTestBase.cs index 794fb8f94a8c..51dd3eee89e9 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/RequestServicesTestBase.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/RequestServicesTestBase.cs @@ -31,6 +31,7 @@ protected RequestServicesTestBase(MvcTestFixture fixture) [InlineData("http://localhost/RequestScopedService/FromView")] [InlineData("http://localhost/RequestScopedService/FromViewComponent")] [InlineData("http://localhost/RequestScopedService/FromActionArgument")] + [InlineData("http://localhost/RequestScopedService/FromProperty")] public async Task RequestServices(string url) { for (var i = 0; i < 2; i++) diff --git a/src/Mvc/test/WebSites/BasicWebSite/Controllers/RequestScopedServiceController.cs b/src/Mvc/test/WebSites/BasicWebSite/Controllers/RequestScopedServiceController.cs index e88bf983b948..6c70dad43b62 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/Controllers/RequestScopedServiceController.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/Controllers/RequestScopedServiceController.cs @@ -45,4 +45,13 @@ public string FromActionArgument([FromServices] RequestIdService requestIdServic { return requestIdService.RequestId; } + + [FromServices] + public RequestIdService RequestIdService { get; set; } + + [HttpGet] + public string FromProperty() + { + return RequestIdService.RequestId; + } } From f44773d9fe8cada2423c83157f7bb43897d3a952 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 9 May 2022 14:24:58 -0700 Subject: [PATCH 53/63] Renaming to BindPropertiesAsParameter --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index fa7b3bf73c45..60b144803eb0 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -655,7 +655,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext $"Nested {nameof(AsParametersAttribute)} is not supported and should be used only for handler parameters."); } - return BindParameterFromParametersType(parameter, factoryContext); + return BindPropertiesAsParameter(parameter, factoryContext); } else if (ParameterBindingMethodCache.HasBindAsyncMethod(parameter)) { @@ -1243,7 +1243,7 @@ private static Expression GetValueFromProperty(Expression sourceExpression, stri return Expression.Convert(indexExpression, returnType ?? typeof(string)); } - private static Expression BindParameterFromParametersType(ParameterInfo parameter, FactoryContext factoryContext) + private static Expression BindPropertiesAsParameter(ParameterInfo parameter, FactoryContext factoryContext) { var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); var (constructor, parameters) = ParameterBindingMethodCache.FindConstructor(parameter.ParameterType); From 9d5bb1ceec157fdd0c712042d67d62fa1060b0d1 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 9 May 2022 14:25:50 -0700 Subject: [PATCH 54/63] Renaming to BindParameterFromProperties --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 60b144803eb0..684c58c269ae 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -655,7 +655,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext $"Nested {nameof(AsParametersAttribute)} is not supported and should be used only for handler parameters."); } - return BindPropertiesAsParameter(parameter, factoryContext); + return BindParameterFromProperties(parameter, factoryContext); } else if (ParameterBindingMethodCache.HasBindAsyncMethod(parameter)) { @@ -1243,7 +1243,7 @@ private static Expression GetValueFromProperty(Expression sourceExpression, stri return Expression.Convert(indexExpression, returnType ?? typeof(string)); } - private static Expression BindPropertiesAsParameter(ParameterInfo parameter, FactoryContext factoryContext) + private static Expression BindParameterFromProperties(ParameterInfo parameter, FactoryContext factoryContext) { var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); var (constructor, parameters) = ParameterBindingMethodCache.FindConstructor(parameter.ParameterType); From 917a54040e0a69640f59a64d6d4dc5212f4415d1 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 9 May 2022 16:50:49 -0700 Subject: [PATCH 55/63] Adding more FromServices tests --- .../DefaultApplicationModelProviderTest.cs | 28 +++++++++++++++++-- .../ServicesModelBinderIntegrationTest.cs | 4 +-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/DefaultApplicationModelProviderTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/DefaultApplicationModelProviderTest.cs index e910a21f19e6..3912588804c9 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/DefaultApplicationModelProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/DefaultApplicationModelProviderTest.cs @@ -93,6 +93,15 @@ public void OnProvidersExecuting_AddsControllerProperties() Assert.Empty(property.Attributes); }, property => + { + Assert.Equal(nameof(ModelBinderController.Service), property.PropertyName); + Assert.Equal(BindingSource.Services, property.BindingInfo.BindingSource); + Assert.Same(controllerModel, property.Controller); + + var attribute = Assert.Single(property.Attributes); + Assert.IsType(attribute); ; + }, + property => { Assert.Equal(nameof(ModelBinderController.Unbound), property.PropertyName); Assert.Null(property.BindingInfo); @@ -104,7 +113,7 @@ public void OnProvidersExecuting_AddsControllerProperties() public void OnProvidersExecuting_ReadsBindingSourceForPropertiesFromModelMetadata() { // Arrange - var detailsProvider = new BindingSourceMetadataProvider(typeof(string), BindingSource.Services); + var detailsProvider = new BindingSourceMetadataProvider(typeof(string), BindingSource.Special); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(new[] { detailsProvider }); var typeInfo = typeof(ModelBinderController).GetTypeInfo(); var provider = new TestApplicationModelProvider(new MvcOptions(), modelMetadataProvider); @@ -137,9 +146,18 @@ public void OnProvidersExecuting_ReadsBindingSourceForPropertiesFromModelMetadat }, property => { - Assert.Equal(nameof(ModelBinderController.Unbound), property.PropertyName); + Assert.Equal(nameof(ModelBinderController.Service), property.PropertyName); Assert.Equal(BindingSource.Services, property.BindingInfo.BindingSource); Assert.Same(controllerModel, property.Controller); + + var attribute = Assert.Single(property.Attributes); + Assert.IsType(attribute); + }, + property => + { + Assert.Equal(nameof(ModelBinderController.Unbound), property.PropertyName); + Assert.Equal(BindingSource.Special, property.BindingInfo.BindingSource); + Assert.Same(controllerModel, property.Controller); }); } @@ -1743,6 +1761,9 @@ public class NoFiltersController { } + public interface ITestService + { } + public class ModelBinderController { [FromQuery] @@ -1750,6 +1771,9 @@ public class ModelBinderController public string Unbound { get; set; } + [FromServices] + public ITestService Service { get; set; } + public IFormFile FormFile { get; set; } public IActionResult PostAction([FromQuery] string fromQuery, IFormFileCollection formFileCollection, string unbound) => null; diff --git a/src/Mvc/test/Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs b/src/Mvc/test/Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs index 3dfb12f23e73..67dbcd358f89 100644 --- a/src/Mvc/test/Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs +++ b/src/Mvc/test/Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs @@ -332,6 +332,7 @@ public async Task BindParameterWithDefaultValueFromService_NoService_BindsToDefa private class Person { + [FromServices] public ITypeActivatorCache Service { get; set; } } @@ -348,8 +349,7 @@ public async Task FromServicesOnPropertyType_WithData_Succeeds(BindingInfo bindi // Similar to a custom IBindingSourceMetadata implementation or [ModelBinder] subclass on a custom service. var metadataProvider = new TestModelMetadataProvider(); metadataProvider - .ForProperty(nameof(Person.Service)) - .BindingDetails(binding => binding.BindingSource = BindingSource.Services); + .ForProperty(nameof(Person.Service)); var testContext = ModelBindingTestHelper.GetTestContext(metadataProvider: metadataProvider); var modelState = testContext.ModelState; From 8a67a742e331a311de21eae6899a7ab47f3ecc7a Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Wed, 11 May 2022 12:06:29 -0700 Subject: [PATCH 56/63] PR Feedback --- .../src/AsParametersAttribute.cs | 2 +- .../test/ParameterBindingMethodCacheTests.cs | 8 +-- .../test/RequestDelegateFactoryTests.cs | 49 +++++++++++++++++-- src/Shared/ParameterBindingMethodCache.cs | 8 +-- 4 files changed, 54 insertions(+), 13 deletions(-) diff --git a/src/Http/Http.Abstractions/src/AsParametersAttribute.cs b/src/Http/Http.Abstractions/src/AsParametersAttribute.cs index 50d88758fc6d..5ed7ac1b1aba 100644 --- a/src/Http/Http.Abstractions/src/AsParametersAttribute.cs +++ b/src/Http/Http.Abstractions/src/AsParametersAttribute.cs @@ -6,7 +6,7 @@ namespace Microsoft.AspNetCore.Http; using System; /// -/// Specifies that a parameter represents a structure delegate's parameter list. +/// Specifies that a route handler delegate's parameter represents a structured parameter list. /// [AttributeUsage( AttributeTargets.Parameter, diff --git a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs index 8d2e36ba9a1f..6135a42c52d4 100644 --- a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs +++ b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs @@ -575,7 +575,7 @@ public void FindConstructor_ThrowsIfNoPublicConstructors(Type type) { var cache = new ParameterBindingMethodCache(); var ex = Assert.Throws(() => cache.FindConstructor(type)); - Assert.Equal($"No {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} public parameterless constructor found.", ex.Message); + Assert.Equal($"No public parameterless constructor found for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'.", ex.Message); } [Theory] @@ -585,7 +585,7 @@ public void FindConstructor_ThrowsIfAbstract(Type type) { var cache = new ParameterBindingMethodCache(); var ex = Assert.Throws(() => cache.FindConstructor(type)); - Assert.Equal($"The {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} abstract type is not supported.", ex.Message); + Assert.Equal($"The abstract type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}' is not supported.", ex.Message); } [Theory] @@ -595,7 +595,7 @@ public void FindConstructor_ThrowsIfMultipleParameterizedConstructors(Type type) { var cache = new ParameterBindingMethodCache(); var ex = Assert.Throws(() => cache.FindConstructor(type)); - Assert.Equal($"Only a single public parameterized constructor is allowed for {TypeNameHelper.GetTypeDisplayName(type, fullName: false)}.", ex.Message); + Assert.Equal($"Only a single public parameterized constructor is allowed for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'.", ex.Message); } [Theory] @@ -608,7 +608,7 @@ public void FindConstructor_ThrowsIfParameterizedConstructorIncludeNoMatchingArg var cache = new ParameterBindingMethodCache(); var ex = Assert.Throws(() => cache.FindConstructor(type)); Assert.Equal( - $"The {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} public parameterized constructor must contains only parameters that match to the declared public properties.", + $"The public parameterized constructor must contains only parameters that match to the declared public properties for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'.", ex.Message); } diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 759681164b3f..60b232f0ef98 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -2167,16 +2167,16 @@ public static object[][] BadArgumentListActions void TestParameterListNoPulicConstructorClass([AsParameters] BadNoPublicConstructorArgumentListClass req) { } static string GetMultipleContructorsError(Type type) - => $"Only a single public parameterized constructor is allowed for {TypeNameHelper.GetTypeDisplayName(type, fullName: false)}."; + => $"Only a single public parameterized constructor is allowed for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'."; static string GetAbstractClassError(Type type) - => $"The {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} abstract type is not supported."; + => $"The abstract type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}' is not supported."; static string GetNoContructorsError(Type type) - => $"No {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} public parameterless constructor found."; + => $"No public parameterless constructor found for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'."; static string GetInvalidConstructorError(Type type) - => $"The {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} public parameterized constructor must contains only parameters that match to the declared public properties."; + => $"The public parameterized constructor must contains only parameters that match to the declared public properties for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'."; return new object[][] { @@ -4726,6 +4726,47 @@ void TestAction([AsParameters] ParameterListRecordWitDefaultValue args) Assert.Equal(expectedValue, httpContext.Items["input"]); } + private class ParameterListWithReadOnlyProperties + { + public ParameterListWithReadOnlyProperties() + { + ReadOnlyValue = 1; + } + + public int Value { get; set; } + + public int ConstantValue => 1; + + public int ReadOnlyValue { get; } + } + + [Fact] + public async Task RequestDelegatePopulatesFromParameterListAndSkipReadOnlyProperties() + { + const int routeParamValue = 42; + var expectedInput = new ParameterListWithReadOnlyProperties() { Value = routeParamValue }; + + void TestAction(HttpContext context, [AsParameters] ParameterListWithReadOnlyProperties args) + { + context.Items.Add("input", args); + } + + var httpContext = CreateHttpContext(); + httpContext.Request.RouteValues[nameof(ParameterListWithReadOnlyProperties.Value)] = routeParamValue.ToString(NumberFormatInfo.InvariantInfo); + httpContext.Request.RouteValues[nameof(ParameterListWithReadOnlyProperties.ConstantValue)] = routeParamValue.ToString(NumberFormatInfo.InvariantInfo); + httpContext.Request.RouteValues[nameof(ParameterListWithReadOnlyProperties.ReadOnlyValue)] = routeParamValue.ToString(NumberFormatInfo.InvariantInfo); + + var factoryResult = RequestDelegateFactory.Create(TestAction); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + var input = Assert.IsType(httpContext.Items["input"]); + Assert.Equal(expectedInput.Value, input.Value); + Assert.Equal(expectedInput.ConstantValue, input.ConstantValue); + Assert.Equal(expectedInput.ReadOnlyValue, input.ReadOnlyValue); + } + private class ParameterListWitDefaultValue { public ParameterListWitDefaultValue(HttpContext httpContext, [FromRoute]int value = 42) diff --git a/src/Shared/ParameterBindingMethodCache.cs b/src/Shared/ParameterBindingMethodCache.cs index 77f2f6851bff..c8cc9afaf422 100644 --- a/src/Shared/ParameterBindingMethodCache.cs +++ b/src/Shared/ParameterBindingMethodCache.cs @@ -307,7 +307,7 @@ static bool ValidateReturnType(MethodInfo methodInfo) if (!lookupTable.TryGetValue(key, out var property)) { throw new InvalidOperationException( - $"The {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} public parameterized constructor must contains only parameters that match to the declared public properties."); + $"The public parameterized constructor must contains only parameters that match to the declared public properties for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'."); } parametersWithPropertyInfo[i] = new ConstructorParameter(parameters[i], property); @@ -323,7 +323,7 @@ static bool ValidateReturnType(MethodInfo methodInfo) { if (type.IsAbstract) { - throw new InvalidOperationException($"The {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} abstract type is not supported."); + throw new InvalidOperationException($"The abstract type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}' is not supported."); } var constructors = type.GetConstructors(BindingFlags.Public | BindingFlags.Instance); @@ -363,10 +363,10 @@ static bool ValidateReturnType(MethodInfo methodInfo) // } if (parameterlessConstructor is null && constructors.Length > 1) { - throw new InvalidOperationException($"Only a single public parameterized constructor is allowed for {TypeNameHelper.GetTypeDisplayName(type, fullName: false)}."); + throw new InvalidOperationException($"Only a single public parameterized constructor is allowed for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'."); } - throw new InvalidOperationException($"No {TypeNameHelper.GetTypeDisplayName(type, fullName: false)} public parameterless constructor found."); + throw new InvalidOperationException($"No public parameterless constructor found for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'."); } private MethodInfo? GetStaticMethodFromHierarchy(Type type, string name, Type[] parameterTypes, Func validateReturnType) From d7fe6afd4d94598f719141075d7991ae444c01d3 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Wed, 11 May 2022 14:03:18 -0700 Subject: [PATCH 57/63] PR Feeback --- src/Http/Http.Extensions/src/RequestDelegateFactory.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 684c58c269ae..5b8efd8608a1 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -432,7 +432,7 @@ private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List { object?[]? invokeArgs = null; - void AddMetadata(ReadOnlySpan parameters) + static void AddMetadata(ReadOnlySpan parameters, MethodInfo methodInfo, List metadata, IServiceProvider? services, object?[]? invokeArgs) { foreach (var parameter in parameters) { @@ -457,10 +457,10 @@ void AddMetadata(ReadOnlySpan parameters) } // Get metadata from parameter types - AddMetadata(methodInfo.GetParameters()); + AddMetadata(methodInfo.GetParameters(), methodInfo, metadata, services, invokeArgs); // Get metadata from properties as parameter types - AddMetadata(propertiesAsParameter); + AddMetadata(propertiesAsParameter, methodInfo, metadata, services, invokeArgs); // Get metadata from return type var returnType = methodInfo.ReturnType; From 4086eff77b42df3e8e845773cb9be95e548cd1f2 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Wed, 11 May 2022 17:07:51 -0700 Subject: [PATCH 58/63] Merging with latest OpenAPI changes --- src/OpenApi/test/OpenApiGeneratorTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenApi/test/OpenApiGeneratorTests.cs b/src/OpenApi/test/OpenApiGeneratorTests.cs index eea36a65aab8..e77cc51ae01e 100644 --- a/src/OpenApi/test/OpenApiGeneratorTests.cs +++ b/src/OpenApi/test/OpenApiGeneratorTests.cs @@ -364,14 +364,14 @@ static void AssertParameters(OpenApiOperation operation) param => { Assert.Equal("Foo", param.Name); - Assert.Equal("number", param.Schema.Type); + Assert.Equal("integer", param.Schema.Type); Assert.Equal(ParameterLocation.Path, param.In); Assert.True(param.Required); }, param => { Assert.Equal("Bar", param.Name); - Assert.Equal("number", param.Schema.Type); + Assert.Equal("integer", param.Schema.Type); Assert.Equal(ParameterLocation.Query, param.In); Assert.True(param.Required); }, From fbeb3a727f1de20b0a005218f0c48c22513cdf6b Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Wed, 11 May 2022 17:27:29 -0700 Subject: [PATCH 59/63] adding more tests --- .../test/RequestDelegateFactoryTests.cs | 53 ++++++++++++++++--- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 60b232f0ef98..0c5ba3fd791d 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -4704,26 +4704,45 @@ public async Task RequestDelegatePopulatesFromParameterList(Delegate action) Assert.Equal(originalRouteParam, httpContext.Items["input"]); } - private record ParameterListRecordWitDefaultValue(HttpContext HttpContext, [FromRoute] int Value = 42); + private record struct SampleParameterList(int Foo); + private record struct AdditionalSampleParameterList(int Bar); [Fact] - public async Task RequestDelegatePopulatesFromParameterListRecordUsesDefaultValue() + public async Task RequestDelegatePopulatesFromMultipleParameterLists() { - const int expectedValue = 42; + const int foo = 1; + const int bar = 2; - void TestAction([AsParameters] ParameterListRecordWitDefaultValue args) + void TestAction(HttpContext context, [AsParameters] SampleParameterList args, [AsParameters] AdditionalSampleParameterList args2) { - args.HttpContext.Items.Add("input", args.Value); + context.Items.Add("foo", args.Foo); + context.Items.Add("bar", args2.Bar); } var httpContext = CreateHttpContext(); + httpContext.Request.RouteValues[nameof(SampleParameterList.Foo)] = foo.ToString(NumberFormatInfo.InvariantInfo); + httpContext.Request.RouteValues[nameof(AdditionalSampleParameterList.Bar)] = bar.ToString(NumberFormatInfo.InvariantInfo); var factoryResult = RequestDelegateFactory.Create(TestAction); var requestDelegate = factoryResult.RequestDelegate; await requestDelegate(httpContext); - Assert.Equal(expectedValue, httpContext.Items["input"]); + Assert.Equal(foo, httpContext.Items["foo"]); + Assert.Equal(bar, httpContext.Items["bar"]); + } + + [Fact] + public void RequestDelegateThrowsWhenParameterNameConflicts() + { + void TestAction(HttpContext context, [AsParameters] SampleParameterList args, [AsParameters] SampleParameterList args2) + { + context.Items.Add("foo", args.Foo); + } + var httpContext = CreateHttpContext(); + + var exception = Assert.Throws(() => RequestDelegateFactory.Create(TestAction)); + Assert.Contains("An item with the same key has already been added. Key: Foo", exception.Message); } private class ParameterListWithReadOnlyProperties @@ -4767,6 +4786,28 @@ void TestAction(HttpContext context, [AsParameters] ParameterListWithReadOnlyPro Assert.Equal(expectedInput.ReadOnlyValue, input.ReadOnlyValue); } + private record ParameterListRecordWitDefaultValue(HttpContext HttpContext, [FromRoute] int Value = 42); + + [Fact] + public async Task RequestDelegatePopulatesFromParameterListRecordUsesDefaultValue() + { + const int expectedValue = 42; + + void TestAction([AsParameters] ParameterListRecordWitDefaultValue args) + { + args.HttpContext.Items.Add("input", args.Value); + } + + var httpContext = CreateHttpContext(); + + var factoryResult = RequestDelegateFactory.Create(TestAction); + var requestDelegate = factoryResult.RequestDelegate; + + await requestDelegate(httpContext); + + Assert.Equal(expectedValue, httpContext.Items["input"]); + } + private class ParameterListWitDefaultValue { public ParameterListWitDefaultValue(HttpContext httpContext, [FromRoute]int value = 42) From 5bf792ce17043c0cd44449ef2c5aba755ddffa6f Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 11 May 2022 17:28:06 -0700 Subject: [PATCH 60/63] Update src/Shared/ParameterBindingMethodCache.cs Co-authored-by: Brennan --- src/Shared/ParameterBindingMethodCache.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Shared/ParameterBindingMethodCache.cs b/src/Shared/ParameterBindingMethodCache.cs index c8cc9afaf422..03cd38756a08 100644 --- a/src/Shared/ParameterBindingMethodCache.cs +++ b/src/Shared/ParameterBindingMethodCache.cs @@ -307,7 +307,7 @@ static bool ValidateReturnType(MethodInfo methodInfo) if (!lookupTable.TryGetValue(key, out var property)) { throw new InvalidOperationException( - $"The public parameterized constructor must contains only parameters that match to the declared public properties for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'."); + $"The public parameterized constructor must contain only parameters that match the declared public properties for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'."); } parametersWithPropertyInfo[i] = new ConstructorParameter(parameters[i], property); From de5901e3cb887f2ed0dbe72af5674951370e0c89 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Wed, 11 May 2022 21:54:42 -0700 Subject: [PATCH 61/63] Updating errormessag on unit tests --- .../Http.Extensions/test/ParameterBindingMethodCacheTests.cs | 2 +- src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs index 6135a42c52d4..64320730ea7e 100644 --- a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs +++ b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs @@ -608,7 +608,7 @@ public void FindConstructor_ThrowsIfParameterizedConstructorIncludeNoMatchingArg var cache = new ParameterBindingMethodCache(); var ex = Assert.Throws(() => cache.FindConstructor(type)); Assert.Equal( - $"The public parameterized constructor must contains only parameters that match to the declared public properties for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'.", + $"The public parameterized constructor must contain only parameters that match to the declared public properties for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'.", ex.Message); } diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 0c5ba3fd791d..c635d46305be 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -2176,7 +2176,7 @@ static string GetNoContructorsError(Type type) => $"No public parameterless constructor found for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'."; static string GetInvalidConstructorError(Type type) - => $"The public parameterized constructor must contains only parameters that match to the declared public properties for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'."; + => $"The public parameterized constructor must contain only parameters that match to the declared public properties for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'."; return new object[][] { From c7888a15a4903814fb7d8d716ea20a919be2ca02 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Wed, 11 May 2022 21:57:54 -0700 Subject: [PATCH 62/63] Updating errormessag on unit tests --- .../Http.Extensions/test/ParameterBindingMethodCacheTests.cs | 2 +- src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs index 64320730ea7e..6a1085ca2c4f 100644 --- a/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs +++ b/src/Http/Http.Extensions/test/ParameterBindingMethodCacheTests.cs @@ -608,7 +608,7 @@ public void FindConstructor_ThrowsIfParameterizedConstructorIncludeNoMatchingArg var cache = new ParameterBindingMethodCache(); var ex = Assert.Throws(() => cache.FindConstructor(type)); Assert.Equal( - $"The public parameterized constructor must contain only parameters that match to the declared public properties for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'.", + $"The public parameterized constructor must contain only parameters that match the declared public properties for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'.", ex.Message); } diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index c635d46305be..d3f1e190c602 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -2176,7 +2176,7 @@ static string GetNoContructorsError(Type type) => $"No public parameterless constructor found for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'."; static string GetInvalidConstructorError(Type type) - => $"The public parameterized constructor must contain only parameters that match to the declared public properties for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'."; + => $"The public parameterized constructor must contain only parameters that match the declared public properties for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'."; return new object[][] { From f4b9354db5c2391ffe227d2d651d8465d452ad5e Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 12 May 2022 13:48:34 -0700 Subject: [PATCH 63/63] PR Feedback --- .../src/RequestDelegateFactory.cs | 71 +++++++++---------- .../test/RequestDelegateFactoryTests.cs | 41 ++++++++++- src/Shared/PropertyAsParameterInfo.cs | 2 +- 3 files changed, 73 insertions(+), 41 deletions(-) diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 648e1aa24850..dfe6c5bef609 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -226,7 +226,7 @@ private static FactoryContext CreateFactoryContext(RequestDelegateFactoryOptions AddTypeProvidedMetadata(methodInfo, factoryContext.Metadata, factoryContext.ServiceProvider, - CollectionsMarshal.AsSpan(factoryContext.PropertiesAsParameter)); + CollectionsMarshal.AsSpan(factoryContext.Parameters)); // Add method attributes as metadata *after* any inferred metadata so that the attributes hava a higher specificity AddMethodAttributesAsMetadata(methodInfo, factoryContext.Metadata); @@ -428,40 +428,32 @@ private static Expression CreateRouteHandlerInvocationContextBase(FactoryContext return fallbackConstruction; } - private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List metadata, IServiceProvider? services, Span propertiesAsParameter) + private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List metadata, IServiceProvider? services, ReadOnlySpan parameters) { object?[]? invokeArgs = null; - static void AddMetadata(ReadOnlySpan parameters, MethodInfo methodInfo, List metadata, IServiceProvider? services, object?[]? invokeArgs) + // Get metadata from parameter types + foreach (var parameter in parameters) { - foreach (var parameter in parameters) + if (typeof(IEndpointParameterMetadataProvider).IsAssignableFrom(parameter.ParameterType)) { - if (typeof(IEndpointParameterMetadataProvider).IsAssignableFrom(parameter.ParameterType)) - { - // Parameter type implements IEndpointParameterMetadataProvider - var parameterContext = new EndpointParameterMetadataContext(parameter, metadata, services); - invokeArgs ??= new object[1]; - invokeArgs[0] = parameterContext; - PopulateMetadataForParameterMethod.MakeGenericMethod(parameter.ParameterType).Invoke(null, invokeArgs); - } + // Parameter type implements IEndpointParameterMetadataProvider + var parameterContext = new EndpointParameterMetadataContext(parameter, metadata, services); + invokeArgs ??= new object[1]; + invokeArgs[0] = parameterContext; + PopulateMetadataForParameterMethod.MakeGenericMethod(parameter.ParameterType).Invoke(null, invokeArgs); + } - if (typeof(IEndpointMetadataProvider).IsAssignableFrom(parameter.ParameterType)) - { - // Parameter type implements IEndpointMetadataProvider - var context = new EndpointMetadataContext(methodInfo, metadata, services); - invokeArgs ??= new object[1]; - invokeArgs[0] = context; - PopulateMetadataForEndpointMethod.MakeGenericMethod(parameter.ParameterType).Invoke(null, invokeArgs); - } + if (typeof(IEndpointMetadataProvider).IsAssignableFrom(parameter.ParameterType)) + { + // Parameter type implements IEndpointMetadataProvider + var context = new EndpointMetadataContext(methodInfo, metadata, services); + invokeArgs ??= new object[1]; + invokeArgs[0] = context; + PopulateMetadataForEndpointMethod.MakeGenericMethod(parameter.ParameterType).Invoke(null, invokeArgs); } } - // Get metadata from parameter types - AddMetadata(methodInfo.GetParameters(), methodInfo, metadata, services, invokeArgs); - - // Get metadata from properties as parameter types - AddMetadata(propertiesAsParameter, methodInfo, metadata, services, invokeArgs); - // Get metadata from return type var returnType = methodInfo.ReturnType; if (AwaitableInfo.IsTypeAwaitable(returnType, out var awaitableInfo)) @@ -514,6 +506,7 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory factoryContext.ArgumentTypes = new Type[parameters.Length]; factoryContext.ArgumentExpressions = new Expression[parameters.Length]; factoryContext.BoxedArgs = new Expression[parameters.Length]; + factoryContext.Parameters = new List(parameters); for (var i = 0; i < parameters.Length; i++) { @@ -611,6 +604,16 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.ServiceAttribute); return BindParameterFromService(parameter, factoryContext); } + else if (parameterCustomAttributes.OfType().Any()) + { + if (parameter is PropertyAsParameterInfo) + { + throw new NotSupportedException( + $"Nested {nameof(AsParametersAttribute)} is not supported and should be used only for handler parameters."); + } + + return BindParameterFromProperties(parameter, factoryContext); + } else if (parameter.ParameterType == typeof(HttpContext)) { return HttpContextExpr; @@ -647,16 +650,6 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext { return RequestPipeReaderExpr; } - else if (parameter.CustomAttributes.Any(a => typeof(AsParametersAttribute).IsAssignableFrom(a.AttributeType))) - { - if (parameter is PropertyAsParameterInfo) - { - throw new NotSupportedException( - $"Nested {nameof(AsParametersAttribute)} is not supported and should be used only for handler parameters."); - } - - return BindParameterFromProperties(parameter, factoryContext); - } else if (ParameterBindingMethodCache.HasBindAsyncMethod(parameter)) { return BindParameterFromBindAsync(parameter, factoryContext); @@ -1259,7 +1252,7 @@ private static Expression BindParameterFromProperties(ParameterInfo parameter, F var parameterInfo = new PropertyAsParameterInfo(parameters[i].PropertyInfo, parameters[i].ParameterInfo, factoryContext.NullabilityContext); constructorArguments[i] = CreateArgument(parameterInfo, factoryContext); - factoryContext.PropertiesAsParameter.Add(parameterInfo); + factoryContext.Parameters.Add(parameterInfo); } factoryContext.ParamCheckExpressions.Add( @@ -1285,7 +1278,7 @@ private static Expression BindParameterFromProperties(ParameterInfo parameter, F { var parameterInfo = new PropertyAsParameterInfo(properties[i], factoryContext.NullabilityContext); bindings.Add(Expression.Bind(properties[i], CreateArgument(parameterInfo, factoryContext))); - factoryContext.PropertiesAsParameter.Add(parameterInfo); + factoryContext.Parameters.Add(parameterInfo); } } @@ -2093,7 +2086,7 @@ private sealed class FactoryContext public Expression[] BoxedArgs { get; set; } = Array.Empty(); public List>? Filters { get; init; } - public List PropertiesAsParameter { get; } = new(); + public List Parameters { get; set; } = new(); } private static class RequestDelegateFactoryConstants diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index d3f1e190c602..19aed0340a49 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -5900,6 +5900,29 @@ public void Create_CombinesDefaultMetadata_AndMetadataFromParameterTypesImplemen Assert.Contains(result.EndpointMetadata, m => m is CustomEndpointMetadata { Source: MetadataSource.Parameter }); } + [Fact] + public void Create_CombinesPropertiesAsParameterMetadata_AndTopLevelParameter() + { + // Arrange + var @delegate = ([AsParameters] AddsCustomParameterMetadata param1) => new CountsDefaultEndpointMetadataResult(); + var options = new RequestDelegateFactoryOptions + { + InitialEndpointMetadata = new List + { + new CustomEndpointMetadata { Source = MetadataSource.Caller } + } + }; + + // Act + var result = RequestDelegateFactory.Create(@delegate, options); + + // Assert + Assert.Contains(result.EndpointMetadata, m => m is CustomEndpointMetadata { Source: MetadataSource.Parameter }); + Assert.Contains(result.EndpointMetadata, m => m is ParameterNameMetadata { Name: "param1" }); + Assert.Contains(result.EndpointMetadata, m => m is CustomEndpointMetadata { Source: MetadataSource.Property }); + Assert.Contains(result.EndpointMetadata, m => m is ParameterNameMetadata { Name: nameof(AddsCustomParameterMetadata.Data) }); + } + [Fact] public void Create_CombinesAllMetadata_InCorrectOrder() { @@ -6113,8 +6136,23 @@ public static void PopulateMetadata(EndpointMetadataContext context) public Task ExecuteAsync(HttpContext httpContext) => throw new NotImplementedException(); } + private class AddsCustomParameterMetadataAsProperty : IEndpointParameterMetadataProvider, IEndpointMetadataProvider + { + public static void PopulateMetadata(EndpointParameterMetadataContext parameterContext) + { + parameterContext.EndpointMetadata?.Add(new ParameterNameMetadata { Name = parameterContext.Parameter?.Name }); + } + + public static void PopulateMetadata(EndpointMetadataContext context) + { + context.EndpointMetadata?.Add(new CustomEndpointMetadata { Source = MetadataSource.Property }); + } + } + private class AddsCustomParameterMetadata : IEndpointParameterMetadataProvider, IEndpointMetadataProvider { + public AddsCustomParameterMetadataAsProperty? Data { get; set; } + public static void PopulateMetadata(EndpointParameterMetadataContext parameterContext) { parameterContext.EndpointMetadata?.Add(new ParameterNameMetadata { Name = parameterContext.Parameter?.Name }); @@ -6162,7 +6200,8 @@ private enum MetadataSource { Caller, Parameter, - ReturnType + ReturnType, + Property } private class Todo : ITodo diff --git a/src/Shared/PropertyAsParameterInfo.cs b/src/Shared/PropertyAsParameterInfo.cs index 275c86334b1b..31e96c3b8b7b 100644 --- a/src/Shared/PropertyAsParameterInfo.cs +++ b/src/Shared/PropertyAsParameterInfo.cs @@ -74,7 +74,7 @@ public static ReadOnlySpan Flatten(ParameterInfo[] parameters, Pa for (var i = 0; i < parameters.Length; i++) { - if (parameters[i].CustomAttributes.Any(a => typeof(AsParametersAttribute).IsAssignableFrom(a.AttributeType))) + if (parameters[i].CustomAttributes.Any(a => a.AttributeType == typeof(AsParametersAttribute))) { // Initialize the list with all parameter already processed // to keep the same parameter ordering