Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Surrogate argument list in Minimal APIs #41325

Merged
merged 68 commits into from
May 12, 2022
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
15b28dd
Core funcionality
brunolins16 Apr 22, 2022
dbf6e21
Changing the order
brunolins16 Apr 22, 2022
a9a724c
Not checking attribute from type
brunolins16 Apr 22, 2022
e768b4d
Code cleanup
brunolins16 Apr 22, 2022
4822214
Adding missing ParamCheckExpression
brunolins16 Apr 22, 2022
74010a2
Adding support for records and structs
brunolins16 Apr 22, 2022
8f5726c
change to a static local function
brunolins16 Apr 22, 2022
09bd872
Remove empty line
brunolins16 Apr 22, 2022
173c269
Updating APIExplorer
brunolins16 Apr 22, 2022
f018d10
Updating APIExplorer
brunolins16 Apr 22, 2022
0ee0a1c
Merge branch 'main' into brunolins16/issues/40712
brunolins16 Apr 22, 2022
b52b113
Updating OpenAPI
brunolins16 Apr 25, 2022
b441fc4
PR feedback
brunolins16 Apr 25, 2022
bb3a599
Updating comment
brunolins16 Apr 25, 2022
0b95125
Allowing attribute on classes
brunolins16 Apr 25, 2022
14b3eb9
Reducing initial memory allocation
brunolins16 Apr 25, 2022
b025c9a
Adding constructorinfo caching
brunolins16 Apr 27, 2022
6f8973a
Updating OpenAPI generator
brunolins16 Apr 27, 2022
a8a2c61
Updating APIExplorer
brunolins16 Apr 27, 2022
21f2fd2
Adding constructor cache
brunolins16 Apr 28, 2022
8956ff4
Renaming to SurrogateParameterInfo
brunolins16 Apr 28, 2022
1b8418e
Updating OpenAPI Generator
brunolins16 Apr 28, 2022
ec3046e
Updating ApiExplorer
brunolins16 Apr 28, 2022
4dd3a3e
Updating RequestDelegateFactory
brunolins16 Apr 28, 2022
8cbbeef
Adding initial test cases
brunolins16 Apr 28, 2022
423ff0d
Merge branch 'main' into brunolins16/issues/40712
brunolins16 Apr 28, 2022
c2f9c71
Rollback bad change
brunolins16 Apr 28, 2022
88a69ad
Fixing merge issues
brunolins16 Apr 28, 2022
e21d54f
Initial SurrogateParameterInfo tests
brunolins16 Apr 28, 2022
3ba8262
Adding surrogateparameterinfo tests
brunolins16 Apr 29, 2022
e50922c
Using Span
brunolins16 Apr 29, 2022
337debc
Adding FindConstructor unit tests
brunolins16 Apr 29, 2022
c2ecb71
Using span
brunolins16 Apr 29, 2022
39fb2ec
Updating error message
brunolins16 Apr 29, 2022
ba0fd73
Updating surrogateParameteInfo and fix unit test
brunolins16 May 2, 2022
2d0d8e2
Adding RequestDelegateFactory tests
brunolins16 May 2, 2022
cbbad7c
Code cleanup
brunolins16 May 3, 2022
a658532
code clean up
brunolins16 May 4, 2022
abb5982
code clean up
brunolins16 May 4, 2022
603ea24
Adding suppress
brunolins16 May 4, 2022
e49a28a
Adding trimming warning suppress
brunolins16 May 5, 2022
0591fa0
PR feeback
brunolins16 May 6, 2022
cc001b5
PR feeback
brunolins16 May 6, 2022
4a25037
Merge branch 'main' into brunolins16/issues/40712
brunolins16 May 6, 2022
68737ff
Mark types as sealed
brunolins16 May 6, 2022
8fce5b0
Seal SurrogateParameterInfo
brunolins16 May 6, 2022
1592141
Removing attribute from type
brunolins16 May 6, 2022
97ec99f
Merge branch 'brunolins16/issues/40712' of https://github.com/brunoli…
brunolins16 May 6, 2022
9458d60
API Review changes
brunolins16 May 9, 2022
358db2c
Updating documentation
brunolins16 May 9, 2022
6965cba
Renaming surrogateParameterInfo
brunolins16 May 9, 2022
55b26f2
Code cleanup
brunolins16 May 9, 2022
d6c9e63
Code cleanup
brunolins16 May 9, 2022
74df571
Code cleanup
brunolins16 May 9, 2022
a9cc186
Code cleanup
brunolins16 May 9, 2022
902b65c
Updating tests to include FromService in properties
brunolins16 May 9, 2022
f44773d
Renaming to BindPropertiesAsParameter
brunolins16 May 9, 2022
9d5bb1c
Renaming to BindParameterFromProperties
brunolins16 May 9, 2022
917a540
Adding more FromServices tests
brunolins16 May 9, 2022
8a67a74
PR Feedback
brunolins16 May 11, 2022
d7fe6af
PR Feeback
brunolins16 May 11, 2022
8a4ff7d
Merge branch 'main' into brunolins16/issues/40712
brunolins16 May 11, 2022
4086eff
Merging with latest OpenAPI changes
brunolins16 May 12, 2022
fbeb3a7
adding more tests
brunolins16 May 12, 2022
5bf792c
Update src/Shared/ParameterBindingMethodCache.cs
brunolins16 May 12, 2022
de5901e
Updating errormessag on unit tests
brunolins16 May 12, 2022
c7888a1
Updating errormessag on unit tests
brunolins16 May 12, 2022
f4b9354
PR Feedback
brunolins16 May 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/Http/Http.Extensions/src/ParametersAttribute.cs
Original file line number Diff line number Diff line change
@@ -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;

/// <summary>
///
/// </summary>
[AttributeUsage(AttributeTargets.Parameter, Inherited = false, AllowMultiple = false)]
public sealed class ParametersAttribute : Attribute
{
}
2 changes: 2 additions & 0 deletions src/Http/Http.Extensions/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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<object!>?
Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.InitialEndpointMetadata.init -> void
Microsoft.AspNetCore.Http.RequestDelegateFactoryOptions.RouteHandlerFilterFactories.get -> System.Collections.Generic.IReadOnlyList<System.Func<Microsoft.AspNetCore.Http.RouteHandlerContext!, Microsoft.AspNetCore.Http.RouteHandlerFilterDelegate!, Microsoft.AspNetCore.Http.RouteHandlerFilterDelegate!>!>?
Expand Down
200 changes: 176 additions & 24 deletions src/Http/Http.Extensions/src/RequestDelegateFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -283,33 +286,40 @@ private static RouteHandlerFilterDelegate CreateFilterPipeline(MethodInfo method
return filteredInvocation;
}

private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List<object> metadata, IServiceProvider? services)
private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List<object> 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))
{
Expand Down Expand Up @@ -391,14 +401,19 @@ 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<IFromRouteMetadata>().FirstOrDefault() is { } routeAttribute)
if (parameterCustomAttributes.OfType<ParametersAttribute>().FirstOrDefault() is { } ||
parameter.ParameterType.GetCustomAttributes().OfType<ParametersAttribute>().FirstOrDefault() is { })
{
return BindSurrogatedArgument(parameter, factoryContext);
}
else if (parameterCustomAttributes.OfType<IFromRouteMetadata>().FirstOrDefault() is { } routeAttribute)
{
var routeName = routeAttribute.Name ?? parameter.Name;
factoryContext.TrackedParameters.Add(parameter.Name, RequestDelegateFactoryConstants.RouteAttribute);
Expand Down Expand Up @@ -1070,6 +1085,90 @@ private static Expression GetValueFromProperty(Expression sourceExpression, stri
return Expression.Convert(indexExpression, returnType ?? typeof(string));
}

private static Expression BindSurrogatedArgument(ParameterInfo parameter, FactoryContext factoryContext)
{
var properties = parameter.ParameterType.GetProperties();
var argumentExpression = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local");

static ConstructorInfo? GetConstructor(ParameterInfo parameter, PropertyInfo[] properties)
brunolins16 marked this conversation as resolved.
Show resolved Hide resolved
{
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<Type>());
if (constructor is { })
brunolins16 marked this conversation as resolved.
Show resolved Hide resolved
{
return constructor;
}

// If a parameterless ctor is not defined
// we will try to find a ctor that includes all the property types.
brunolins16 marked this conversation as resolved.
Show resolved Hide resolved
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 { })
brunolins16 marked this conversation as resolved.
Show resolved Hide resolved
{
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)
{
// arg_local = new T(....)
var constructorArguments = new Expression[properties.Length];

for (var i = 0; i < properties.Length; i++)
{
var parameterInfo = new SurrogatedParameterInfo(properties[i], 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<MemberBinding>(properties.Length);

for (var i = 0; i < properties.Length; i++)
{
// For parameterless ctor we will init only writable properties.
if (properties[i].CanWrite)
brunolins16 marked this conversation as resolved.
Show resolved Hide resolved
{
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);

return argumentExpression;
}

private static Expression BindParameterFromService(ParameterInfo parameter, FactoryContext factoryContext)
{
var isOptional = IsOptionalParameter(parameter, factoryContext);
Expand Down Expand Up @@ -1560,15 +1659,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<T>(Expression<T> expr)
Expand Down Expand Up @@ -1792,6 +1896,8 @@ private class FactoryContext
public Expression? MethodCall { get; set; }
public List<Expression> BoxedArgs { get; } = new();
public List<Func<RouteHandlerContext, RouteHandlerFilterDelegate, RouteHandlerFilterDelegate>>? Filters { get; init; }

public List<ParameterInfo> SurrogatedParameters { get; } = new();
}

private static class RequestDelegateFactoryConstants
Expand All @@ -1808,6 +1914,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<CustomAttributeData> 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
Expand Down