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

Infer default sources for parameters of minimal actions #31603

Merged
merged 35 commits into from
Apr 9, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
9b1f55e
Move and update MapActionSample
halter73 Apr 2, 2021
dfc2ae4
Remove redundant test cases
halter73 Apr 2, 2021
e3f9632
TestImpliedFromService
halter73 Apr 2, 2021
f156d54
Add RequestDelegatePopulatesUnattributedTryParseableParametersFromRou…
halter73 Apr 6, 2021
552b2ab
Get one test passing
halter73 Apr 6, 2021
7d0c813
Add RequestDelegatePopulatesUnattributedTryParseableParametersFromQue…
halter73 Apr 6, 2021
edbeffa
refactor
halter73 Apr 6, 2021
9bfb50a
RequestDelegateFactory logger category
halter73 Apr 6, 2021
ae4b823
refactor parameter binding
halter73 Apr 6, 2021
44e854a
more refactoring
halter73 Apr 6, 2021
b67a233
Get RequestDelegatePopulatesFromServiceParameterBasedOnParameterType …
halter73 Apr 6, 2021
f590c89
cleanup
halter73 Apr 6, 2021
b113bc2
Add CreateArgumentExpression
halter73 Apr 6, 2021
962fa65
Add RequestDelegateRequiresServiceForAllFromServiceParameters
halter73 Apr 6, 2021
299985f
Add tests
halter73 Apr 6, 2021
46714f1
Start using TryParse
halter73 Apr 6, 2021
bd6087c
Log TryParse Failures
halter73 Apr 7, 2021
1a0609d
Add WasTryParseFailureVariable
halter73 Apr 7, 2021
6781c2e
Add comments
halter73 Apr 7, 2021
37823bd
Set 400 StatusCode for TryParse failure
halter73 Apr 7, 2021
828e4dc
WIP
halter73 Apr 7, 2021
1b33b3a
Fix nullable handling. Add test cases
halter73 Apr 7, 2021
a849e03
Add enum support
halter73 Apr 7, 2021
5d7f7f4
Less Info, More Expr
halter73 Apr 7, 2021
ff6b843
Add custom record and enum to TryParse tests
halter73 Apr 7, 2021
0b283d1
Add tests for DelegatesWithInvalidAttributes
halter73 Apr 7, 2021
f7601b9
Test cleanup
halter73 Apr 7, 2021
1aabf29
Add CreateThrowsInvalidOperationExceptionGivenUnnamedArgument
halter73 Apr 7, 2021
25e98ba
Add tempSourceString to avoid reevaluation
halter73 Apr 7, 2021
7b50d32
Add TryParseMethodCache
halter73 Apr 7, 2021
4060f1b
oops
halter73 Apr 8, 2021
3fc546e
cleanup
halter73 Apr 8, 2021
d3cf3a8
simplify test cases
halter73 Apr 8, 2021
1b2e1a2
wasTryParseFailureVariable -> wasTryParseFailure
halter73 Apr 8, 2021
8e53897
Use Uri in MyTryParsableRecord test case
halter73 Apr 9, 2021
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
Prev Previous commit
Next Next commit
Add enum support
  • Loading branch information
halter73 committed Apr 7, 2021
commit a849e03d8d820792a87eb1e5a1836ffaf7839b58
31 changes: 31 additions & 0 deletions src/Http/Http.Extensions/src/RequestDelegateFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ public static class RequestDelegateFactory
private static readonly MethodInfo ExecuteTaskResultOfTMethodInfo = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteTaskResult), BindingFlags.NonPublic | BindingFlags.Static)!;
private static readonly MethodInfo ExecuteValueResultTaskOfTMethodInfo = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteValueTaskResult), BindingFlags.NonPublic | BindingFlags.Static)!;
private static readonly MethodInfo GetRequiredServiceMethodInfo = typeof(ServiceProviderServiceExtensions).GetMethod(nameof(ServiceProviderServiceExtensions.GetRequiredService), BindingFlags.Public | BindingFlags.Static, new Type[] { typeof(IServiceProvider) })!;
private static readonly MethodInfo EnumTryParseMethodInfo = GetEnumTryParseMethod();

private static readonly MethodInfo ResultWriteResponseAsync = typeof(IResult).GetMethod(nameof(IResult.ExecuteAsync), BindingFlags.Public | BindingFlags.Instance)!;
private static readonly MethodInfo StringResultWriteResponseAsync = GetMethodInfo<Func<HttpResponse, string, Task>>((response, text) => HttpResponseWritingExtensions.WriteAsync(response, text, default));
private static readonly MethodInfo JsonResultWriteResponseAsync = GetMethodInfo<Func<HttpResponse, object, Task>>((response, value) => HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, default));
Expand Down Expand Up @@ -527,10 +529,39 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall,
}
}

private static MethodInfo GetEnumTryParseMethod()
{
var staticEnumMethods = typeof(Enum).GetMethods(BindingFlags.Public | BindingFlags.Static);

foreach (var method in staticEnumMethods)
{
if (!method.IsGenericMethod || method.Name != "TryParse" || method.ReturnType != typeof(bool))
{
continue;
}

var tryParseParameters = method.GetParameters();

if (tryParseParameters.Length == 2 &&
tryParseParameters[0].ParameterType == typeof(string) &&
tryParseParameters[1].IsOut)
{
return method;
}
}

throw new Exception("static bool System.Enum.TryParse<TEnum>(string? value, out TEnum result) does not exist!!?!?");
}

// TODO: Cache this.
// TODO: Use InvariantCulture where possible? Or is CurrentCulture fine because it's more flexible?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm interested in feedback on this. One wrinkle is there isn't a standard convention for TryParse methods taking IFormatProvider. For example, int.TryParse and similar want a NumberStyles argument as well for any overload taking IFormatProvider. We can special-case built-in numeric types of course.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a topic that's up for debate. I think MVC does different things for different parts of the request.

private static MethodInfo? FindTryParseMethod(Type type)
{
if (type.IsEnum)
{
return EnumTryParseMethodInfo.MakeGenericMethod(type);
}

var staticMethods = type.GetMethods(BindingFlags.Public | BindingFlags.Static);

foreach (var method in staticMethods)
Expand Down
90 changes: 55 additions & 35 deletions src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,18 @@
using System.Globalization;
using System.IO;
using System.Net;
using System.Net.Sockets;
using System.Numerics;
using System.Reflection;
using System.Reflection.Metadata;
using System.Text;
using System.Text.Json;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Http.Metadata;
using Microsoft.AspNetCore.Testing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Testing;
Expand All @@ -25,7 +28,7 @@

namespace Microsoft.AspNetCore.Routing.Internal
{
public class RequestDelegateFactoryTests
public class RequestDelegateFactoryTests : LoggedTest
{
public static IEnumerable<object[]> NoResult
{
Expand Down Expand Up @@ -289,74 +292,91 @@ void TestAction([FromRoute] int foo)
Assert.Equal(0, deserializedRouteParam);
}

public static object[][] FromTryParsableParameter
public static object?[][] TryParsableParameters
{
get
{
void StoreTryParsableParameter<T>(HttpContext httpContext, T tryParsable)
void Store<T>(HttpContext httpContext, T tryParsable)
{
httpContext.Items["tryParsable"] = tryParsable;
}

var today = DateTime.Now;
var todayOffset = DateTimeOffset.UtcNow;
var now = DateTime.Now;
var nowOffset = DateTimeOffset.UtcNow;
var guid = Guid.NewGuid();

return new[]
{
// User defined class!
// User define Enums
// String (technically not "TryParseable", but it's the the special case)
new object[] { (Action<HttpContext, string>)Store, "plain string", "plain string" },
// Int32
new object[] { (Action<HttpContext, int>)StoreTryParsableParameter, "-42", -42 },
new object[] { (Action<HttpContext, uint>)StoreTryParsableParameter, "42", 42U },
new object[] { (Action<HttpContext, int>)Store, "-42", -42 },
new object[] { (Action<HttpContext, uint>)Store, "42", 42U },
// Byte
new object[] { (Action<HttpContext, bool>)StoreTryParsableParameter, "true", true },
new object[] { (Action<HttpContext, bool>)Store, "true", true },
// Int16
new object[] { (Action<HttpContext, short>)StoreTryParsableParameter, "-42", (short)-42 },
new object[] { (Action<HttpContext, ushort>)StoreTryParsableParameter, "42", (ushort)42 },
new object[] { (Action<HttpContext, short>)Store, "-42", (short)-42 },
new object[] { (Action<HttpContext, ushort>)Store, "42", (ushort)42 },
// Int64
new object[] { (Action<HttpContext, long>)StoreTryParsableParameter, "-42", -42L },
new object[] { (Action<HttpContext, ulong>)StoreTryParsableParameter, "42", 42UL },
new object[] { (Action<HttpContext, long>)Store, "-42", -42L },
new object[] { (Action<HttpContext, ulong>)Store, "42", 42UL },
// IntPtr
new object[] { (Action<HttpContext, IntPtr>)StoreTryParsableParameter, "-42", new IntPtr(-42) },
new object[] { (Action<HttpContext, IntPtr>)Store, "-42", new IntPtr(-42) },
// Char
new object[] { (Action<HttpContext, char>)StoreTryParsableParameter, "A", 'A' },
new object[] { (Action<HttpContext, char>)Store, "A", 'A' },
// Double
new object[] { (Action<HttpContext, double>)StoreTryParsableParameter, "0.5", 0.5 },
new object[] { (Action<HttpContext, double>)Store, "0.5", 0.5 },
// Single
new object[] { (Action<HttpContext, float>)StoreTryParsableParameter, "0.5", 0.5f },
new object[] { (Action<HttpContext, float>)Store, "0.5", 0.5f },
// Half
new object[] { (Action<HttpContext, Half>)StoreTryParsableParameter, "0.5", (Half)0.5f },
new object[] { (Action<HttpContext, Half>)Store, "0.5", (Half)0.5f },
// Decimal
new object[] { (Action<HttpContext, decimal>)StoreTryParsableParameter, "0.5", 0.5m },
new object[] { (Action<HttpContext, decimal>)Store, "0.5", 0.5m },
// DateTime
new object[] { (Action<HttpContext, DateTime>)StoreTryParsableParameter, today.ToString("o"), today },
new object[] { (Action<HttpContext, DateTime>)Store, now.ToString("o"), now },
// DateTimeOffset
new object[] { (Action<HttpContext, DateTimeOffset>)StoreTryParsableParameter, todayOffset.ToString("o"), todayOffset },
new object[] { (Action<HttpContext, DateTimeOffset>)Store, nowOffset.ToString("o"), nowOffset },
// TimeSpan
new object[] { (Action<HttpContext, TimeSpan>)StoreTryParsableParameter, TimeSpan.FromSeconds(42).ToString(), TimeSpan.FromSeconds(42) },
new object[] { (Action<HttpContext, TimeSpan>)Store, TimeSpan.FromSeconds(42).ToString(), TimeSpan.FromSeconds(42) },
// Guid
new object[] { (Action<HttpContext, Guid>)StoreTryParsableParameter, guid.ToString(), guid },
new object[] { (Action<HttpContext, Guid>)Store, guid.ToString(), guid },
// Version
new object[] { (Action<HttpContext, Version>)StoreTryParsableParameter, "6.0.0.42", new Version("6.0.0.42") },
new object[] { (Action<HttpContext, Version>)Store, "6.0.0.42", new Version("6.0.0.42") },
// BigInteger
new object[] { (Action<HttpContext, BigInteger>)StoreTryParsableParameter, "-42", new BigInteger(-42) },
new object[] { (Action<HttpContext, BigInteger>)Store, "-42", new BigInteger(-42) },
// IPAddress
//new object[] { (Action<HttpContext, IPAddress>)StoreTryParsableParameter, "-42", new BigInteger(-42) },
// IPEndpoint
new object[] { (Action<HttpContext, IPAddress>)Store, "127.0.0.1", IPAddress.Loopback },
// IPEndPoint
new object[] { (Action<HttpContext, IPEndPoint>)Store, "127.0.0.1:80", new IPEndPoint(IPAddress.Loopback, 80) },
// System Enums
new object[] { (Action<HttpContext, AddressFamily>)Store, "Unix", AddressFamily.Unix },
new object[] { (Action<HttpContext, ILOpCode>)Store, "Nop", ILOpCode.Nop },
new object[] { (Action<HttpContext, AssemblyFlags>)Store, "PublicKey,Retargetable", AssemblyFlags.PublicKey | AssemblyFlags.Retargetable },
// Nullable<T>
new object[] { (Action<HttpContext, int?>)StoreTryParsableParameter, "42", 42 },
new object[] { (Action<HttpContext, int?>)Store, "42", 42 },

// Null route and/or query value gives default value.
new object?[] { (Action<HttpContext, int>)Store, null, 0 },
new object?[] { (Action<HttpContext, int?>)Store, null, null },

// User defined class!
// User defined Enums
};
}
}

[Theory]
[MemberData(nameof(FromTryParsableParameter))]
public async Task RequestDelegatePopulatesUnattributedTryParseableParametersFromRouteValue(Delegate action, string routeValue, object expectedParameterValue)
[MemberData(nameof(TryParsableParameters))]
public async Task RequestDelegatePopulatesUnattributedTryParseableParametersFromRouteValue(Delegate action, string? routeValue, object? expectedParameterValue)
{
var invalidDataException = new InvalidDataException();
var serviceCollection = new ServiceCollection();
serviceCollection.AddSingleton(LoggerFactory);

var httpContext = new DefaultHttpContext();
httpContext.Request.RouteValues["tryParsable"] = routeValue;
httpContext.Features.Set<IHttpRequestLifetimeFeature>(new TestHttpRequestLifetimeFeature());
httpContext.RequestServices = serviceCollection.BuildServiceProvider();

var requestDelegate = RequestDelegateFactory.Create(action);

Expand All @@ -366,8 +386,8 @@ public async Task RequestDelegatePopulatesUnattributedTryParseableParametersFrom
}

[Theory]
[MemberData(nameof(FromTryParsableParameter))]
public async Task RequestDelegatePopulatesUnattributedTryParseableParametersFromQueryString(Delegate action, string routeValue, object expectedParameterValue)
[MemberData(nameof(TryParsableParameters))]
public async Task RequestDelegatePopulatesUnattributedTryParseableParametersFromQueryString(Delegate action, string? routeValue, object? expectedParameterValue)
{
var httpContext = new DefaultHttpContext();
httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues>
Expand All @@ -383,8 +403,8 @@ public async Task RequestDelegatePopulatesUnattributedTryParseableParametersFrom
}

[Theory]
[MemberData(nameof(FromTryParsableParameter))]
public async Task RequestDelegatePopulatesUnattributedTryParseableParametersFromRouteValueBeforeQueryString(Delegate action, string routeValue, object expectedParameterValue)
[MemberData(nameof(TryParsableParameters))]
public async Task RequestDelegatePopulatesUnattributedTryParseableParametersFromRouteValueBeforeQueryString(Delegate action, string? routeValue, object? expectedParameterValue)
{
var httpContext = new DefaultHttpContext();
httpContext.Request.RouteValues["tryParsable"] = routeValue;
Expand Down