Skip to content

Commit 1735db4

Browse files
KahbaziKahbazi
authored
Support optional input for MapAction (#30434)
* Support optional input for MapAction * Revert passing HttpContext.RequestAborted * Fix tests
1 parent 1256a3b commit 1735db4

File tree

2 files changed

+218
-44
lines changed

2 files changed

+218
-44
lines changed

‎src/Http/Routing/src/Internal/MapActionExpressionTreeBuilder.cs

Lines changed: 60 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ internal static class MapActionExpressionTreeBuilder
2424
private static readonly MethodInfo ChangeTypeMethodInfo = GetMethodInfo<Func<object, Type, object>>((value, type) => Convert.ChangeType(value, type, CultureInfo.InvariantCulture));
2525
private static readonly MethodInfo ExecuteTaskOfTMethodInfo = typeof(MapActionExpressionTreeBuilder).GetMethod(nameof(ExecuteTask), BindingFlags.NonPublic | BindingFlags.Static)!;
2626
private static readonly MethodInfo ExecuteTaskOfStringMethodInfo = typeof(MapActionExpressionTreeBuilder).GetMethod(nameof(ExecuteTaskOfString), BindingFlags.NonPublic | BindingFlags.Static)!;
27-
private static readonly MethodInfo ExecuteValueTaskOfTMethodInfo = typeof(MapActionExpressionTreeBuilder).GetMethod(nameof(ExecuteValueTask), BindingFlags.NonPublic | BindingFlags.Static)!;
27+
private static readonly MethodInfo ExecuteValueTaskOfTMethodInfo = typeof(MapActionExpressionTreeBuilder).GetMethod(nameof(ExecuteValueTaskOfT), BindingFlags.NonPublic | BindingFlags.Static)!;
28+
private static readonly MethodInfo ExecuteValueTaskMethodInfo = typeof(MapActionExpressionTreeBuilder).GetMethod(nameof(ExecuteValueTask), BindingFlags.NonPublic | BindingFlags.Static)!;
2829
private static readonly MethodInfo ExecuteValueTaskOfStringMethodInfo = typeof(MapActionExpressionTreeBuilder).GetMethod(nameof(ExecuteValueTaskOfString), BindingFlags.NonPublic | BindingFlags.Static)!;
2930
private static readonly MethodInfo ExecuteTaskResultOfTMethodInfo = typeof(MapActionExpressionTreeBuilder).GetMethod(nameof(ExecuteTaskResult), BindingFlags.NonPublic | BindingFlags.Static)!;
3031
private static readonly MethodInfo ExecuteValueResultTaskOfTMethodInfo = typeof(MapActionExpressionTreeBuilder).GetMethod(nameof(ExecuteValueTaskResult), BindingFlags.NonPublic | BindingFlags.Static)!;
@@ -71,28 +72,31 @@ public static RequestDelegate BuildRequestDelegate(Delegate action)
7172
// This argument represents the deserialized body returned from IHttpRequestReader
7273
// when the method has a FromBody attribute declared
7374

74-
var args = new List<Expression>();
75+
var methodParameters = method.GetParameters();
76+
var args = new List<Expression>(methodParameters.Length);
7577

76-
foreach (var parameter in method.GetParameters())
78+
foreach (var parameter in methodParameters)
7779
{
7880
Expression paramterExpression = Expression.Default(parameter.ParameterType);
7981

80-
if (parameter.GetCustomAttributes().OfType<IFromRouteMetadata>().FirstOrDefault() is { } routeAttribute)
82+
var parameterCustomAttributes = parameter.GetCustomAttributes();
83+
84+
if (parameterCustomAttributes.OfType<IFromRouteMetadata>().FirstOrDefault() is { } routeAttribute)
8185
{
8286
var routeValuesProperty = Expression.Property(HttpRequestExpr, nameof(HttpRequest.RouteValues));
8387
paramterExpression = BindParamenter(routeValuesProperty, parameter, routeAttribute.Name);
8488
}
85-
else if (parameter.GetCustomAttributes().OfType<IFromQueryMetadata>().FirstOrDefault() is { } queryAttribute)
89+
else if (parameterCustomAttributes.OfType<IFromQueryMetadata>().FirstOrDefault() is { } queryAttribute)
8690
{
8791
var queryProperty = Expression.Property(HttpRequestExpr, nameof(HttpRequest.Query));
8892
paramterExpression = BindParamenter(queryProperty, parameter, queryAttribute.Name);
8993
}
90-
else if (parameter.GetCustomAttributes().OfType<IFromHeaderMetadata>().FirstOrDefault() is { } headerAttribute)
94+
else if (parameterCustomAttributes.OfType<IFromHeaderMetadata>().FirstOrDefault() is { } headerAttribute)
9195
{
9296
var headersProperty = Expression.Property(HttpRequestExpr, nameof(HttpRequest.Headers));
9397
paramterExpression = BindParamenter(headersProperty, parameter, headerAttribute.Name);
9498
}
95-
else if (parameter.GetCustomAttributes().OfType<IFromBodyMetadata>().FirstOrDefault() is { } bodyAttribute)
99+
else if (parameterCustomAttributes.OfType<IFromBodyMetadata>().FirstOrDefault() is { } bodyAttribute)
96100
{
97101
if (consumeBodyDirectly)
98102
{
@@ -109,7 +113,7 @@ public static RequestDelegate BuildRequestDelegate(Delegate action)
109113
bodyType = parameter.ParameterType;
110114
paramterExpression = Expression.Convert(DeserializedBodyArg, bodyType);
111115
}
112-
else if (parameter.GetCustomAttributes().OfType<IFromFormMetadata>().FirstOrDefault() is { } formAttribute)
116+
else if (parameterCustomAttributes.OfType<IFromFormMetadata>().FirstOrDefault() is { } formAttribute)
113117
{
114118
if (consumeBodyDirectly)
115119
{
@@ -125,27 +129,24 @@ public static RequestDelegate BuildRequestDelegate(Delegate action)
125129
{
126130
paramterExpression = Expression.Call(GetRequiredServiceMethodInfo.MakeGenericMethod(parameter.ParameterType), RequestServicesExpr);
127131
}
128-
else
132+
else if (parameter.ParameterType == typeof(IFormCollection))
129133
{
130-
if (parameter.ParameterType == typeof(IFormCollection))
134+
if (consumeBodyDirectly)
131135
{
132-
if (consumeBodyDirectly)
133-
{
134-
ThrowCannotReadBodyDirectlyAndAsForm();
135-
}
136+
ThrowCannotReadBodyDirectlyAndAsForm();
137+
}
136138

137-
consumeBodyAsForm = true;
139+
consumeBodyAsForm = true;
138140

139-
paramterExpression = Expression.Property(HttpRequestExpr, nameof(HttpRequest.Form));
140-
}
141-
else if (parameter.ParameterType == typeof(HttpContext))
142-
{
143-
paramterExpression = HttpContextParameter;
144-
}
145-
else if (parameter.ParameterType == typeof(CancellationToken))
146-
{
147-
paramterExpression = RequestAbortedExpr;
148-
}
141+
paramterExpression = Expression.Property(HttpRequestExpr, nameof(HttpRequest.Form));
142+
}
143+
else if (parameter.ParameterType == typeof(HttpContext))
144+
{
145+
paramterExpression = HttpContextParameter;
146+
}
147+
else if (parameter.ParameterType == typeof(CancellationToken))
148+
{
149+
paramterExpression = RequestAbortedExpr;
149150
}
150151

151152
args.Add(paramterExpression);
@@ -182,6 +183,12 @@ public static RequestDelegate BuildRequestDelegate(Delegate action)
182183
{
183184
body = methodCall;
184185
}
186+
else if (method.ReturnType == typeof(ValueTask))
187+
{
188+
body = Expression.Call(
189+
ExecuteValueTaskMethodInfo,
190+
methodCall);
191+
}
185192
else if (method.ReturnType.IsGenericType &&
186193
method.ReturnType.GetGenericTypeDefinition() == typeof(Task<>))
187194
{
@@ -263,7 +270,7 @@ public static RequestDelegate BuildRequestDelegate(Delegate action)
263270
var box = Expression.TypeAs(methodCall, typeof(object));
264271
body = Expression.Call(JsonResultWriteResponseAsync, HttpResponseExpr, box, Expression.Constant(CancellationToken.None));
265272
}
266-
else
273+
else
267274
{
268275
body = Expression.Call(JsonResultWriteResponseAsync, HttpResponseExpr, methodCall, Expression.Constant(CancellationToken.None));
269276
}
@@ -398,10 +405,20 @@ private static Expression BindParamenter(Expression sourceExpression, ParameterI
398405
expr = Expression.Convert(expr, parameter.ParameterType);
399406
}
400407

408+
Expression defaultExpression;
409+
if (parameter.HasDefaultValue)
410+
{
411+
defaultExpression = Expression.Constant(parameter.DefaultValue);
412+
}
413+
else
414+
{
415+
defaultExpression = Expression.Default(parameter.ParameterType);
416+
}
417+
401418
// property[key] == null ? default : (ParameterType){Type}.Parse(property[key]);
402419
expr = Expression.Condition(
403420
Expression.Equal(valueArg, Expression.Constant(null)),
404-
Expression.Default(parameter.ParameterType),
421+
defaultExpression,
405422
expr);
406423

407424
return expr;
@@ -449,7 +466,22 @@ static async Task ExecuteAwaited(Task<string> task, HttpContext httpContext)
449466
return ExecuteAwaited(task, httpContext);
450467
}
451468

452-
private static Task ExecuteValueTask<T>(ValueTask<T> task, HttpContext httpContext)
469+
private static Task ExecuteValueTask(ValueTask task)
470+
{
471+
static async Task ExecuteAwaited(ValueTask task)
472+
{
473+
await task;
474+
}
475+
476+
if (task.IsCompletedSuccessfully)
477+
{
478+
task.GetAwaiter().GetResult();
479+
}
480+
481+
return ExecuteAwaited(task);
482+
}
483+
484+
private static Task ExecuteValueTaskOfT<T>(ValueTask<T> task, HttpContext httpContext)
453485
{
454486
static async Task ExecuteAwaited(ValueTask<T> task, HttpContext httpContext)
455487
{

‎src/Http/Routing/test/UnitTests/Internal/MapActionExpressionTreeBuilderTest.cs

Lines changed: 158 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,44 +24,186 @@ namespace Microsoft.AspNetCore.Routing.Internal
2424
{
2525
public class MapActionExpressionTreeBuilderTest
2626
{
27-
[Fact]
28-
public async Task RequestDelegateInvokesAction()
27+
public static IEnumerable<object[]> NoResult
2928
{
30-
var invoked = false;
31-
32-
void TestAction()
29+
get
3330
{
34-
invoked = true;
31+
void TestAction(HttpContext httpContext)
32+
{
33+
MarkAsInvoked(httpContext);
34+
}
35+
36+
Task TaskTestAction(HttpContext httpContext)
37+
{
38+
MarkAsInvoked(httpContext);
39+
return Task.CompletedTask;
40+
}
41+
42+
ValueTask ValueTaskTestAction(HttpContext httpContext)
43+
{
44+
MarkAsInvoked(httpContext);
45+
return ValueTask.CompletedTask;
46+
}
47+
48+
void StaticTestAction(HttpContext httpContext)
49+
{
50+
MarkAsInvoked(httpContext);
51+
}
52+
53+
Task StaticTaskTestAction(HttpContext httpContext)
54+
{
55+
MarkAsInvoked(httpContext);
56+
return Task.CompletedTask;
57+
}
58+
59+
ValueTask StaticValueTaskTestAction(HttpContext httpContext)
60+
{
61+
MarkAsInvoked(httpContext);
62+
return ValueTask.CompletedTask;
63+
}
64+
65+
void MarkAsInvoked(HttpContext httpContext)
66+
{
67+
httpContext.Items.Add("invoked", true);
68+
}
69+
70+
return new List<object[]>
71+
{
72+
new object[] { (Action<HttpContext>)TestAction },
73+
new object[] { (Func<HttpContext, Task>)TaskTestAction },
74+
new object[] { (Func<HttpContext, ValueTask>)ValueTaskTestAction },
75+
new object[] { (Action<HttpContext>)StaticTestAction },
76+
new object[] { (Func<HttpContext, Task>)StaticTaskTestAction },
77+
new object[] { (Func<HttpContext, ValueTask>)StaticValueTaskTestAction },
78+
};
3579
}
80+
}
3681

37-
var requestDelegate = MapActionExpressionTreeBuilder.BuildRequestDelegate((Action)TestAction);
82+
[Theory]
83+
[MemberData(nameof(NoResult))]
84+
public async Task RequestDelegateInvokesAction(Delegate @delegate)
85+
{
86+
var httpContext = new DefaultHttpContext();
87+
88+
var requestDelegate = MapActionExpressionTreeBuilder.BuildRequestDelegate(@delegate);
3889

39-
await requestDelegate(null!);
90+
await requestDelegate(httpContext);
4091

41-
Assert.True(invoked);
92+
Assert.True(httpContext.Items["invoked"] as bool?);
4293
}
4394

44-
[Fact]
45-
public async Task RequestDelegatePopulatesFromRouteParameterBasedOnParameterName()
95+
public static IEnumerable<object[]> FromRouteResult
96+
{
97+
get
98+
{
99+
void TestAction(HttpContext httpContext, [FromRoute] int value)
100+
{
101+
StoreInput(httpContext, value);
102+
};
103+
104+
Task TaskTestAction(HttpContext httpContext, [FromRoute] int value)
105+
{
106+
StoreInput(httpContext, value);
107+
return Task.CompletedTask;
108+
}
109+
110+
ValueTask ValueTaskTestAction(HttpContext httpContext, [FromRoute] int value)
111+
{
112+
StoreInput(httpContext, value);
113+
return ValueTask.CompletedTask;
114+
}
115+
116+
117+
118+
return new List<object[]>
119+
{
120+
new object[] { (Action<HttpContext, int>)TestAction },
121+
new object[] { (Func<HttpContext, int, Task>)TaskTestAction },
122+
new object[] { (Func<HttpContext, int, ValueTask>)ValueTaskTestAction },
123+
};
124+
}
125+
}
126+
private static void StoreInput(HttpContext httpContext, object value)
127+
{
128+
httpContext.Items.Add("input", value);
129+
}
130+
131+
[Theory]
132+
[MemberData(nameof(FromRouteResult))]
133+
public async Task RequestDelegatePopulatesFromRouteParameterBasedOnParameterName(Delegate @delegate)
46134
{
47135
const string paramName = "value";
48136
const int originalRouteParam = 42;
49137

50-
int? deserializedRouteParam = null;
138+
var httpContext = new DefaultHttpContext();
139+
httpContext.Request.RouteValues[paramName] = originalRouteParam.ToString(NumberFormatInfo.InvariantInfo);
140+
141+
var requestDelegate = MapActionExpressionTreeBuilder.BuildRequestDelegate(@delegate);
142+
143+
await requestDelegate(httpContext);
144+
145+
Assert.Equal(originalRouteParam, httpContext.Items["input"] as int?);
146+
}
51147

52-
void TestAction([FromRoute] int value)
148+
public static IEnumerable<object[]> FromRouteOptionalResult
149+
{
150+
get
53151
{
54-
deserializedRouteParam = value;
152+
return new List<object[]>
153+
{
154+
new object[] { (Action<HttpContext, int>)TestAction },
155+
new object[] { (Func<HttpContext, int, Task>)TaskTestAction },
156+
new object[] { (Func<HttpContext, int, ValueTask>)ValueTaskTestAction }
157+
};
55158
}
159+
}
160+
161+
private static void TestAction(HttpContext httpContext, [FromRoute] int value = 42)
162+
{
163+
StoreInput(httpContext, value);
164+
}
165+
166+
private static Task TaskTestAction(HttpContext httpContext, [FromRoute] int value = 42)
167+
{
168+
StoreInput(httpContext, value);
169+
return Task.CompletedTask;
170+
}
171+
172+
private static ValueTask ValueTaskTestAction(HttpContext httpContext, [FromRoute] int value = 42)
173+
{
174+
StoreInput(httpContext, value);
175+
return ValueTask.CompletedTask;
176+
}
56177

178+
[Theory]
179+
[MemberData(nameof(FromRouteOptionalResult))]
180+
public async Task RequestDelegatePopulatesFromRouteOptionalParameter(Delegate @delegate)
181+
{
57182
var httpContext = new DefaultHttpContext();
183+
184+
var requestDelegate = MapActionExpressionTreeBuilder.BuildRequestDelegate(@delegate);
185+
186+
await requestDelegate(httpContext);
187+
188+
Assert.Equal(42, httpContext.Items["input"] as int?);
189+
}
190+
191+
[Theory]
192+
[MemberData(nameof(FromRouteOptionalResult))]
193+
public async Task RequestDelegatePopulatesFromRouteOptionalParameterBasedOnParameterName(Delegate @delegate)
194+
{
195+
const string paramName = "value";
196+
const int originalRouteParam = 47;
197+
198+
var httpContext = new DefaultHttpContext();
199+
58200
httpContext.Request.RouteValues[paramName] = originalRouteParam.ToString(NumberFormatInfo.InvariantInfo);
59201

60-
var requestDelegate = MapActionExpressionTreeBuilder.BuildRequestDelegate((Action<int>)TestAction);
202+
var requestDelegate = MapActionExpressionTreeBuilder.BuildRequestDelegate(@delegate);
61203

62204
await requestDelegate(httpContext);
63205

64-
Assert.Equal(originalRouteParam, deserializedRouteParam);
206+
Assert.Equal(47, httpContext.Items["input"] as int?);
65207
}
66208

67209
[Fact]

0 commit comments

Comments
 (0)