Skip to content

Consistently serialize child members returned from route handlers #39858

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

Merged
merged 2 commits into from
Jan 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 14 additions & 6 deletions src/Http/Http.Extensions/src/RequestDelegateFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,12 @@ public static partial class RequestDelegateFactory
private static readonly MethodInfo GetServiceMethod = typeof(ServiceProviderServiceExtensions).GetMethod(nameof(ServiceProviderServiceExtensions.GetService), BindingFlags.Public | BindingFlags.Static, new Type[] { typeof(IServiceProvider) })!;
private static readonly MethodInfo ResultWriteResponseAsyncMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteResultWriteResponse), BindingFlags.NonPublic | BindingFlags.Static)!;
private static readonly MethodInfo StringResultWriteResponseAsyncMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteWriteStringResponseAsync), BindingFlags.NonPublic | BindingFlags.Static)!;
private static readonly MethodInfo JsonResultWriteResponseAsyncMethod = GetMethodInfo<Func<HttpResponse, object, Task>>((response, value) => HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, default));
private static readonly MethodInfo StringIsNullOrEmptyMethod = typeof(string).GetMethod(nameof(string.IsNullOrEmpty), BindingFlags.Static | BindingFlags.Public)!;

// Call WriteAsJsonAsync<object?>() to serialize the runtime return type rather than the declared return type.
// https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-polymorphism
private static readonly MethodInfo JsonResultWriteResponseAsyncMethod = GetMethodInfo<Func<HttpResponse, object?, Task>>((response, value) => HttpResponseJsonExtensions.WriteAsJsonAsync<object?>(response, value, default));

private static readonly MethodInfo LogParameterBindingFailedMethod = GetMethodInfo<Action<HttpContext, string, string, string, bool>>((httpContext, parameterType, parameterName, sourceValue, shouldThrow) =>
Log.ParameterBindingFailed(httpContext, parameterType, parameterName, sourceValue, shouldThrow));
private static readonly MethodInfo LogRequiredParameterNotProvidedMethod = GetMethodInfo<Action<HttpContext, string, string, string, bool>>((httpContext, parameterType, parameterName, source, shouldThrow) =>
Expand Down Expand Up @@ -1442,7 +1445,8 @@ private static async Task ExecuteObjectReturn(object? obj, HttpContext httpConte
else
{
// Otherwise, we JSON serialize when we reach the terminal state
await httpContext.Response.WriteAsJsonAsync(obj);
// Call WriteAsJsonAsync<object?>() to serialize the runtime return type rather than the declared return type.
await httpContext.Response.WriteAsJsonAsync<object?>(obj);
}
}

Expand All @@ -1452,12 +1456,14 @@ private static Task ExecuteTask<T>(Task<T> task, HttpContext httpContext)

static async Task ExecuteAwaited(Task<T> task, HttpContext httpContext)
{
await httpContext.Response.WriteAsJsonAsync(await task);
// Call WriteAsJsonAsync<object?>() to serialize the runtime return type rather than the declared return type.
await httpContext.Response.WriteAsJsonAsync<object?>(await task);
}

if (task.IsCompletedSuccessfully)
{
return httpContext.Response.WriteAsJsonAsync(task.GetAwaiter().GetResult());
// Call WriteAsJsonAsync<object?>() to serialize the runtime return type rather than the declared return type.
return httpContext.Response.WriteAsJsonAsync<object?>(task.GetAwaiter().GetResult());
}

return ExecuteAwaited(task, httpContext);
Expand Down Expand Up @@ -1506,12 +1512,14 @@ private static Task ExecuteValueTaskOfT<T>(ValueTask<T> task, HttpContext httpCo
{
static async Task ExecuteAwaited(ValueTask<T> task, HttpContext httpContext)
{
await httpContext.Response.WriteAsJsonAsync(await task);
// Call WriteAsJsonAsync<object?>() to serialize the runtime return type rather than the declared return type.
await httpContext.Response.WriteAsJsonAsync<object?>(await task);
}

if (task.IsCompletedSuccessfully)
{
return httpContext.Response.WriteAsJsonAsync(task.GetAwaiter().GetResult());
// Call WriteAsJsonAsync<object?>() to serialize the runtime return type rather than the declared return type.
return httpContext.Response.WriteAsJsonAsync<object?>(task.GetAwaiter().GetResult());
}

return ExecuteAwaited(task, httpContext);
Expand Down
67 changes: 65 additions & 2 deletions src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2309,15 +2309,73 @@ public async Task RequestDelegateWritesComplexReturnValueAsJsonResponseBody(Dele

var deserializedResponseBody = JsonSerializer.Deserialize<Todo>(responseBodyStream.ToArray(), new JsonSerializerOptions
{
// TODO: the output is "{\"id\":0,\"name\":\"Write even more tests!\",\"isComplete\":false}"
// Verify that the camelCased property names are consistent with MVC and if so whether we should keep the behavior.
PropertyNameCaseInsensitive = true
});

Assert.NotNull(deserializedResponseBody);
Assert.Equal("Write even more tests!", deserializedResponseBody!.Name);
}

public static IEnumerable<object[]> ChildResult
{
get
{
TodoChild originalTodo = new()
{
Name = "Write even more tests!",
Child = "With type hierarchies!",
};

Todo TestAction() => originalTodo;

Task<Todo> TaskTestAction() => Task.FromResult<Todo>(originalTodo);
async Task<Todo> TaskTestActionAwaited()
{
await Task.Yield();
return originalTodo;
}

ValueTask<Todo> ValueTaskTestAction() => ValueTask.FromResult<Todo>(originalTodo);
async ValueTask<Todo> ValueTaskTestActionAwaited()
{
await Task.Yield();
return originalTodo;
}

return new List<object[]>
{
new object[] { (Func<Todo>)TestAction },
new object[] { (Func<Task<Todo>>)TaskTestAction},
new object[] { (Func<Task<Todo>>)TaskTestActionAwaited},
new object[] { (Func<ValueTask<Todo>>)ValueTaskTestAction},
new object[] { (Func<ValueTask<Todo>>)ValueTaskTestActionAwaited},
};
}
}

[Theory]
[MemberData(nameof(ChildResult))]
public async Task RequestDelegateWritesMembersFromChildTypesToJsonResponseBody(Delegate @delegate)
{
var httpContext = CreateHttpContext();
var responseBodyStream = new MemoryStream();
httpContext.Response.Body = responseBodyStream;

var factoryResult = RequestDelegateFactory.Create(@delegate);
var requestDelegate = factoryResult.RequestDelegate;

await requestDelegate(httpContext);

var deserializedResponseBody = JsonSerializer.Deserialize<TodoChild>(responseBodyStream.ToArray(), new JsonSerializerOptions
{
PropertyNameCaseInsensitive = true
});

Assert.NotNull(deserializedResponseBody);
Assert.Equal("Write even more tests!", deserializedResponseBody!.Name);
Assert.Equal("With type hierarchies!", deserializedResponseBody!.Child);
}

public static IEnumerable<object[]> CustomResults
{
get
Expand Down Expand Up @@ -4165,6 +4223,11 @@ private class Todo : ITodo
public bool IsComplete { get; set; }
}

private class TodoChild : Todo
{
public string? Child { get; set; }
}

private class CustomTodo : Todo
{
public static async ValueTask<CustomTodo?> BindAsync(HttpContext context, ParameterInfo parameter)
Expand Down