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 61 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
17 changes: 17 additions & 0 deletions src/Http/Http.Abstractions/src/AsParametersAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// 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>
/// Specifies that a route handler delegate's parameter represents a structured parameter list.
/// </summary>
[AttributeUsage(
AttributeTargets.Parameter,
Inherited = false,
AllowMultiple = false)]
public sealed class AsParametersAttribute : Attribute
{
}
2 changes: 2 additions & 0 deletions src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<Description>ASP.NET Core common extension methods for HTTP abstractions, HTTP headers, HTTP request/response, and session state.</Description>
Expand All @@ -13,6 +13,7 @@
<ItemGroup>
<Compile Include="$(SharedSourceRoot)ObjectMethodExecutor\**\*.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)ParameterBindingMethodCache.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)PropertyAsParameterInfo.cs" LinkBase="Shared"/>
<Compile Include="..\..\Shared\StreamCopyOperationInternal.cs" LinkBase="Shared" />
<Compile Include="$(SharedSourceRoot)ProblemDetailsJsonConverter.cs" LinkBase="Shared"/>
<Compile Include="$(SharedSourceRoot)HttpValidationProblemDetailsJsonConverter.cs" LinkBase="Shared" />
Expand Down
136 changes: 114 additions & 22 deletions src/Http/Http.Extensions/src/RequestDelegateFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -222,7 +223,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,
CollectionsMarshal.AsSpan(factoryContext.PropertiesAsParameter));
brunolins16 marked this conversation as resolved.
Show resolved Hide resolved

// 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 @@ -424,33 +428,40 @@ private static Expression CreateRouteHandlerInvocationContextBase(FactoryContext
return fallbackConstruction;
}

private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List<object> metadata, IServiceProvider? services)
private static void AddTypeProvidedMetadata(MethodInfo methodInfo, List<object> metadata, IServiceProvider? services, Span<ParameterInfo> propertiesAsParameter)
{
object?[]? invokeArgs = null;

// Get metadata from parameter types
var parameters = methodInfo.GetParameters();
foreach (var parameter in parameters)
static void AddMetadata(ReadOnlySpan<ParameterInfo> parameters, MethodInfo methodInfo, List<object> metadata, IServiceProvider? services, object?[]? invokeArgs)
{
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(), 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))
Expand Down Expand Up @@ -507,6 +518,7 @@ private static Expression[] CreateArguments(ParameterInfo[]? parameters, Factory
for (var i = 0; i < parameters.Length; i++)
{
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.
Expand Down Expand Up @@ -635,6 +647,16 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext
{
return RequestPipeReaderExpr;
}
else if (parameter.CustomAttributes.Any(a => typeof(AsParametersAttribute).IsAssignableFrom(a.AttributeType)))
brunolins16 marked this conversation as resolved.
Show resolved Hide resolved
{
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);
Expand Down Expand Up @@ -1221,6 +1243,68 @@ private static Expression GetValueFromProperty(Expression sourceExpression, stri
return Expression.Convert(indexExpression, returnType ?? typeof(string));
}

private static Expression BindParameterFromProperties(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 PropertyAsParameterInfo(parameters[i].PropertyInfo, parameters[i].ParameterInfo, factoryContext.NullabilityContext);
constructorArguments[i] = CreateArgument(parameterInfo, factoryContext);
factoryContext.PropertiesAsParameter.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<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 PropertyAsParameterInfo(properties[i], factoryContext.NullabilityContext);
bindings.Add(Expression.Bind(properties[i], CreateArgument(parameterInfo, factoryContext)));
factoryContext.PropertiesAsParameter.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.PropertyAsParameter);
factoryContext.ExtraLocals.Add(argumentExpression);

return argumentExpression;
}

private static Expression BindParameterFromService(ParameterInfo parameter, FactoryContext factoryContext)
{
var isOptional = IsOptionalParameter(parameter, factoryContext);
Expand Down Expand Up @@ -1711,15 +1795,20 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al

private static bool IsOptionalParameter(ParameterInfo parameter, FactoryContext factoryContext)
{
if (parameter is PropertyAsParameterInfo 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 @@ -2000,9 +2089,11 @@ private class FactoryContext
public List<Expression> ContextArgAccess { get; } = new();
public Expression? MethodCall { get; set; }
public Type[] ArgumentTypes { get; set; } = Array.Empty<Type>();
public Expression[] ArgumentExpressions { get; set; } = Array.Empty<Expression>();
public Expression[] ArgumentExpressions { get; set; } = Array.Empty<Expression>();
public Expression[] BoxedArgs { get; set; } = Array.Empty<Expression>();
public List<Func<RouteHandlerContext, RouteHandlerFilterDelegate, RouteHandlerFilterDelegate>>? Filters { get; init; }

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

private static class RequestDelegateFactoryConstants
Expand All @@ -2019,6 +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 = "As Parameter (Attribute)";
}

private static partial class Log
Expand Down
Loading